[PATCH] Fix qemu-xen with PCI passthrough.

[PATCH] Fix qemu-xen with PCI passthrough.

[konrad]

Hey Anthony and Stefano,

This patch was posted in the past (see
http://lists.xen.org/archives/html/xen-devel/2013-07/msg00008.html)
and Stefano said: ""It looks OK to me, I'll have to trust you to have
tested this patch properly too.".

I don't recall if Zhenzhong did, but I just did with Xen-unstable
and the latest of qemu-xen. Without this patch, this loop

while (true)
do
 rmmod igbvf
 killall dhclient
 modprobe igbvf
 dhclient eth1
done

ends up shorlty exhausting the list of PIRQs and the driver cannot load
anymore. With this patch the loop continues on.

This has been tested with an 82576 VF.

Please pull!

 hw/xen/xen_pt_config_init.c |    6 ++++--
 hw/xen/xen_pt_msi.c         |    6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

Zhenzhong Duan (1):
      qemu-xen: free all the pirqs for msi/msix when driver unload

[PATCH] qemu-xen: free all the pirqs for msi/msix when driver unload

[konrad]

From: Zhenzhong Duan <zhenzhong.duan@oracle.com>

Pirqs are not freed when driver unload, then new pirqs are allocated when
driver reload. This could exhaust pirqs if do it in a loop.

This patch fixes the bug by freeing pirqs when ENABLE bit is cleared in
msi/msix control reg.

There is also other way of fixing it such as reuse pirqs between driver reload,
but this way is better.
Xen-devel: http://marc.info/?l=xen-devel&m=136800120304275&w=2

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 hw/xen/xen_pt_config_init.c |    6 ++++--
 hw/xen/xen_pt_msi.c         |    6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 8ccc2e4..de9a20f 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -1123,8 +1123,8 @@ static int xen_pt_msgctrl_reg_write(XenPCIPassthroughState *s,
             msi->mapped = true;
         }
         msi->flags |= PCI_MSI_FLAGS_ENABLE;
-    } else {
-        msi->flags &= ~PCI_MSI_FLAGS_ENABLE;
+    } else if (msi->mapped) {
+        xen_pt_msi_disable(s);
     }
 
     /* pass through MSI_ENABLE bit */
@@ -1397,6 +1397,8 @@ static int xen_pt_msixctrl_reg_write(XenPCIPassthroughState *s,
     if ((*val & PCI_MSIX_FLAGS_ENABLE)
         && !(*val & PCI_MSIX_FLAGS_MASKALL)) {
         xen_pt_msix_update(s);
+    } else if (!(*val & PCI_MSIX_FLAGS_ENABLE) && s->msix->enabled) {
+        xen_pt_msix_disable(s);
     }
 
     debug_msix_enabled_old = s->msix->enabled;
diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index 6fbe0cc..12b4c45 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -282,7 +282,8 @@ void xen_pt_msi_disable(XenPCIPassthroughState *s)
                      msi->initialized);
 
     /* clear msi info */
-    msi->flags = 0;
+    msi->flags &= ~PCI_MSI_FLAGS_ENABLE;
+    msi->initialized = false;
     msi->mapped = false;
     msi->pirq = XEN_PT_UNASSIGNED_PIRQ;
 }
@@ -446,7 +447,8 @@ static void pci_msix_write(void *opaque, hwaddr addr,
     if (offset != PCI_MSIX_ENTRY_VECTOR_CTRL) {
         const volatile uint32_t *vec_ctrl;
 
-        if (get_entry_value(entry, offset) == val) {
+        if (get_entry_value(entry, offset) == val
+            && entry->pirq != XEN_PT_UNASSIGNED_PIRQ) {
             return;
         }
 
-- 
1.7.7.6

Re: [PATCH] qemu-xen: free all the pirqs for msi/msix when driver unload

[Stefano Stabellini]

On Tue, 8 Apr 2014, konrad@kernel.org wrote:
> From: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> 
> Pirqs are not freed when driver unload, then new pirqs are allocated when
> driver reload. This could exhaust pirqs if do it in a loop.
> 
> This patch fixes the bug by freeing pirqs when ENABLE bit is cleared in
> msi/msix control reg.
> 
> There is also other way of fixing it such as reuse pirqs between driver reload,
> but this way is better.
> Xen-devel: http://marc.info/?l=xen-devel&m=136800120304275&w=2
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

I'll add it to my queue.
At the moment they are busy with the QEMU 2.0 release, I would rather
wait for the next merge window.


>  hw/xen/xen_pt_config_init.c |    6 ++++--
>  hw/xen/xen_pt_msi.c         |    6 ++++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
> index 8ccc2e4..de9a20f 100644
> --- a/hw/xen/xen_pt_config_init.c
> +++ b/hw/xen/xen_pt_config_init.c
> @@ -1123,8 +1123,8 @@ static int xen_pt_msgctrl_reg_write(XenPCIPassthroughState *s,
>              msi->mapped = true;
>          }
>          msi->flags |= PCI_MSI_FLAGS_ENABLE;
> -    } else {
> -        msi->flags &= ~PCI_MSI_FLAGS_ENABLE;
> +    } else if (msi->mapped) {
> +        xen_pt_msi_disable(s);
>      }
>  
>      /* pass through MSI_ENABLE bit */
> @@ -1397,6 +1397,8 @@ static int xen_pt_msixctrl_reg_write(XenPCIPassthroughState *s,
>      if ((*val & PCI_MSIX_FLAGS_ENABLE)
>          && !(*val & PCI_MSIX_FLAGS_MASKALL)) {
>          xen_pt_msix_update(s);
> +    } else if (!(*val & PCI_MSIX_FLAGS_ENABLE) && s->msix->enabled) {
> +        xen_pt_msix_disable(s);
>      }
>  
>      debug_msix_enabled_old = s->msix->enabled;
> diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
> index 6fbe0cc..12b4c45 100644
> --- a/hw/xen/xen_pt_msi.c
> +++ b/hw/xen/xen_pt_msi.c
> @@ -282,7 +282,8 @@ void xen_pt_msi_disable(XenPCIPassthroughState *s)
>                       msi->initialized);
>  
>      /* clear msi info */
> -    msi->flags = 0;
> +    msi->flags &= ~PCI_MSI_FLAGS_ENABLE;
> +    msi->initialized = false;
>      msi->mapped = false;
>      msi->pirq = XEN_PT_UNASSIGNED_PIRQ;
>  }
> @@ -446,7 +447,8 @@ static void pci_msix_write(void *opaque, hwaddr addr,
>      if (offset != PCI_MSIX_ENTRY_VECTOR_CTRL) {
>          const volatile uint32_t *vec_ctrl;
>  
> -        if (get_entry_value(entry, offset) == val) {
> +        if (get_entry_value(entry, offset) == val
> +            && entry->pirq != XEN_PT_UNASSIGNED_PIRQ) {
>              return;
>          }
>  
> -- 
> 1.7.7.6
> 

Re: [PATCH] qemu-xen: free all the pirqs for msi/msix when driver unload

[Konrad Rzeszutek Wilk]

On Tue, Apr 08, 2014 at 06:22:09PM +0100, Stefano Stabellini wrote:
> On Tue, 8 Apr 2014, konrad@kernel.org wrote:
> > From: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> > 
> > Pirqs are not freed when driver unload, then new pirqs are allocated when
> > driver reload. This could exhaust pirqs if do it in a loop.
> > 
> > This patch fixes the bug by freeing pirqs when ENABLE bit is cleared in
> > msi/msix control reg.
> > 
> > There is also other way of fixing it such as reuse pirqs between driver reload,
> > but this way is better.
> > Xen-devel: http://marc.info/?l=xen-devel&m=136800120304275&w=2
> > 
> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> I'll add it to my queue.

Yeey!
> At the moment they are busy with the QEMU 2.0 release, I would rather
> wait for the next merge window.

OK. When should I ping you so that in the unlikely event it gets lost
I would ping you?

Thanks!

Re: [PATCH] qemu-xen: free all the pirqs for msi/msix when driver unload

[Stefano Stabellini]

On Tue, 8 Apr 2014, Konrad Rzeszutek Wilk wrote:
> On Tue, Apr 08, 2014 at 06:22:09PM +0100, Stefano Stabellini wrote:
> > On Tue, 8 Apr 2014, konrad@kernel.org wrote:
> > > From: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> > > 
> > > Pirqs are not freed when driver unload, then new pirqs are allocated when
> > > driver reload. This could exhaust pirqs if do it in a loop.
> > > 
> > > This patch fixes the bug by freeing pirqs when ENABLE bit is cleared in
> > > msi/msix control reg.
> > > 
> > > There is also other way of fixing it such as reuse pirqs between driver reload,
> > > but this way is better.
> > > Xen-devel: http://marc.info/?l=xen-devel&m=136800120304275&w=2
> > > 
> > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > 
> > I'll add it to my queue.
> 
> Yeey!
> > At the moment they are busy with the QEMU 2.0 release, I would rather
> > wait for the next merge window.
> 
> OK. When should I ping you so that in the unlikely event it gets lost
> I would ping you?

As soon as QEMU makes the 2.0 release.

Re: [PATCH] qemu-xen: free all the pirqs for msi/msix when driver unload

[Konrad Rzeszutek Wilk]

On Tue, Apr 08, 2014 at 01:29:45PM -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Apr 08, 2014 at 06:22:09PM +0100, Stefano Stabellini wrote:
> > On Tue, 8 Apr 2014, konrad@kernel.org wrote:
> > > From: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> > > 
> > > Pirqs are not freed when driver unload, then new pirqs are allocated when
> > > driver reload. This could exhaust pirqs if do it in a loop.
> > > 
> > > This patch fixes the bug by freeing pirqs when ENABLE bit is cleared in
> > > msi/msix control reg.
> > > 
> > > There is also other way of fixing it such as reuse pirqs between driver reload,
> > > but this way is better.
> > > Xen-devel: http://marc.info/?l=xen-devel&m=136800120304275&w=2
> > > 
> > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > 
> > I'll add it to my queue.
> 
> Yeey!
> > At the moment they are busy with the QEMU 2.0 release, I would rather
> > wait for the next merge window.
> 
> OK. When should I ping you so that in the unlikely event it gets lost
> I would ping you?

ping! Seems QEMU 2.0 is out
> 
> Thanks!