Re: [Xen-devel] [PATCH] qemu-xen-traditional: Enable MSI after host sleep (v2)

[Jean Guyader <jean.guyader@xxxxxxxxx>]

On 11 May 2012 17:06, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> wrote:

On Thu, 10 May 2012, Jean Guyader wrote:
> After a host sleep MSI will be off on the host but qemu still thinks
> it's on because of some state that have been set previously.
>
> If qemu thinks that the device has been configure already
> and the host MSI are disabled tell Xen to reconfigure the MSI.
>
> Signed-off-by: Jean Guyader <jean.guyader@xxxxxxxxx>
>

> diff --git a/hw/pass-through.c b/hw/pass-through.c
> index f832c5a..a6a9b7a 100644
> --- a/hw/pass-through.c
> +++ b/hw/pass-through.c
> @@ -3772,6 +3772,21 @@ static int pt_pmcsr_reg_write(struct pt_dev *ptdev,
>      return 0;
>  }
>
> +static int msi_is_enable(struct pt_dev *dev)
> +{
> +    uint16_t val = 0;
> +    uint32_t address = 0;
> +    if (!dev->msi)
> +        return 0;
> +
> +    address = dev->msi->ctrl_offset;
> +    if (!address)
> +        return 0;
> +
> +    val = pci_read_word(dev->pci_dev, address);
> +    return val & PCI_MSI_FLAGS_ENABLE;
> +}
> +
>  /* write Message Control register */
>  static int pt_msgctrl_reg_write(struct pt_dev *ptdev,
>      struct pt_reg_tbl *cfg_entry,
> @@ -3803,8 +3818,7 @@ static int pt_msgctrl_reg_write(struct pt_dev *ptdev,
>      /* update MSI */
>      if (val & PCI_MSI_FLAGS_ENABLE)
>      {
> -        /* setup MSI pirq for the first time */
> -        if (ptdev->msi->flags & MSI_FLAG_UNINIT)
> +        if (!msi_is_enable(ptdev))
>          {
>              if (ptdev->msi_trans_en) {
>                  PT_LOG("guest enabling MSI, disable MSI-INTx translation\n");
> diff --git a/hw/pt-msi.c b/hw/pt-msi.c
> index 70c4023..99f9afd 100644
> --- a/hw/pt-msi.c
> +++ b/hw/pt-msi.c
> @@ -67,12 +67,6 @@ int pt_msi_setup(struct pt_dev *dev)
>      int pirq = -1;
>      uint8_t gvec = 0;
>
> -    if ( !(dev->msi->flags & MSI_FLAG_UNINIT) )
> -    {
> -        PT_LOG("Error: setup physical after initialized?? \n");
> -        return -1;
> -    }
> -
>      gvec = dev->msi->data & 0xFF;
>      if (!gvec) {
>          /* if gvec is 0, the guest is asking for a particular pirq that

The patch makes sense.
Only one last comment, sorry for not having noticed this before, but
there might be another (flags & MSI_FLAG_UNINIT) check that should
probably be changed into msi_is_enable in pt_msi_disable.

Also I think that we should always call pt_msi_setup (the first time) even if the MSI are set in the config space.
It should maybe look like my first patch.

Jean

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel