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

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

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 306 bytes --]

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@gmail.com>

[-- Attachment #2: msi-after-sleep.patch --]
[-- Type: text/x-patch, Size: 1553 bytes --]

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

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

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

[Stefano Stabellini]

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@gmail.com>
> 
> 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.

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

[Jean Guyader]

[-- Attachment #1.1: Type: text/plain, Size: 2622 bytes --]

On 11 May 2012 17:06, Stefano Stabellini
<stefano.stabellini@eu.citrix.com>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@gmail.com>
> >
> > 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

[-- Attachment #1.2: Type: text/html, Size: 3470 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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