Zhenzhong Duan
2013-Jul-01 05:46 UTC
[PATCH] qemu-xen: free all the pirqs for msi/msix when driver unload
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>
---
hw/xen_pt_config_init.c | 6 ++++--
hw/xen_pt_msi.c | 6 ++++--
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/xen_pt_config_init.c b/hw/xen_pt_config_init.c
index 0a5f82c..68d1195 100644
--- a/hw/xen_pt_config_init.c
+++ b/hw/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_pt_msi.c b/hw/xen_pt_msi.c
index db757cd..733c991 100644
--- a/hw/xen_pt_msi.c
+++ b/hw/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.3
Stefano Stabellini
2013-Jul-01 11:54 UTC
Re: [PATCH] qemu-xen: free all the pirqs for msi/msix when driver unload
On Mon, 1 Jul 2013, Zhenzhong Duan wrote:> 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>It looks OK to me, I''ll have to trust you to have tested this patch properly too.> hw/xen_pt_config_init.c | 6 ++++-- > hw/xen_pt_msi.c | 6 ++++-- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/hw/xen_pt_config_init.c b/hw/xen_pt_config_init.c > index 0a5f82c..68d1195 100644 > --- a/hw/xen_pt_config_init.c > +++ b/hw/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_pt_msi.c b/hw/xen_pt_msi.c > index db757cd..733c991 100644 > --- a/hw/xen_pt_msi.c > +++ b/hw/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.3 >
DuanZhenzhong
2013-Jul-02 05:54 UTC
Re: [PATCH] qemu-xen: free all the pirqs for msi/msix when driver unload
Stefano Stabellini wrote:> On Mon, 1 Jul 2013, Zhenzhong Duan wrote: > >> 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> >> > > It looks OK to me, I''ll have to trust you to have tested this patch > properly too. >Not yet, it just passed build test. Pls hold off until I got permit to override the 4.1.30-OVM test env with upstream things and test. Or is there some way of testing with 4.1.30-OVM + qemu-system-i386 ? zduan
Stefano Stabellini
2013-Jul-02 10:25 UTC
Re: [PATCH] qemu-xen: free all the pirqs for msi/msix when driver unload
On Tue, 2 Jul 2013, DuanZhenzhong wrote:> Stefano Stabellini wrote: > > On Mon, 1 Jul 2013, Zhenzhong Duan wrote: > > > > > 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> > > > > > > > It looks OK to me, I''ll have to trust you to have tested this patch > > properly too. > > > Not yet, it just passed build test. > Pls hold off until I got permit to override the 4.1.30-OVM test env with > upstream things and test.OK, thanks for letting me know.> Or is there some way of testing with 4.1.30-OVM + qemu-system-i386 ?Uhm.. I guess not: you need QMP support in libxl and create VMs with xl to be able to use qemu-xen. Your best bet would be to update the hypervisor and tools to 4.2 or 4.3.