* [PATCH] Fix qemu traditional with PCI passthrough.
@ 2014-04-08 16:44 Konrad Rzeszutek Wilk
2014-04-08 16:44 ` [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads Konrad Rzeszutek Wilk
2014-04-08 16:56 ` [PATCH] Fix qemu traditional with PCI passthrough Ian Jackson
0 siblings, 2 replies; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-04-08 16:44 UTC (permalink / raw)
To: stefano.stabellini, xen-devel, ian.jackson; +Cc: zhenzhong.duan
Hey Ian and Stefano,
This patch was posted way back last year in July
(see http://lists.xen.org/archives/html/xen-devel/2013-07/msg00004.html) and
was mentioned to:
">From what I understand following the conversation, I think this is
> probably the right way to solve the problem, but given that it's only
> really a problem when you load and unload drivers, which is the
> uncommon case, I think at this point we should probably hold off on
> this one until 4.3.1.
>
> Stefano, thoughts?
I think that's OK. I'll wait to apply the qemu-xen patch until after the
release."
I think this patch just got lost in the Xen 4.4 release. Dusting it
off and reposting.
The issue at hand is simple - you boot an PVHVM guest with a PCI
passthrough device and in a loop do:
#!/bin/bash
while (true)
do
rmmod igbvf
killall dhclient
modprobe igbvf
dhclient eth1
done
and you find yourself in distressed to see that after a while it
cannot allocate any IRQs. I've tested it and it fixes the issue.
Now there was also an qemu-xen version of this patch posted:
http://lists.xen.org/archives/html/xen-devel/2013-07/msg00008.html
and I just reposted it, see:
http://mid.gmane.org/1396975053-16435-1-git-send-email-konrad@kernel.org
(or "[PATCH] Fix qemu-xen with PCI passthrough.")
hw/pass-through.c | 8 +++++++-
hw/pt-msi.c | 5 +++--
2 files changed, 10 insertions(+), 3 deletions(-)
Zhenzhong Duan (1):
qemu-xen-trad: free all the pirqs for msi/msix when driver unloads
Thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads 2014-04-08 16:44 [PATCH] Fix qemu traditional with PCI passthrough Konrad Rzeszutek Wilk @ 2014-04-08 16:44 ` Konrad Rzeszutek Wilk 2014-07-02 15:06 ` Ian Jackson 2014-04-08 16:56 ` [PATCH] Fix qemu traditional with PCI passthrough Ian Jackson 1 sibling, 1 reply; 14+ messages in thread From: Konrad Rzeszutek Wilk @ 2014-04-08 16:44 UTC (permalink / raw) To: stefano.stabellini, xen-devel, ian.jackson Cc: zhenzhong.duan, Konrad Rzeszutek Wilk From: Zhenzhong Duan <zhenzhong.duan@oracle.com> Pirqs are not freed when driver unloads, then new pirqs are allocated when driver reloads. 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/pass-through.c | 8 +++++++- hw/pt-msi.c | 5 +++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/hw/pass-through.c b/hw/pass-through.c index 304c438..4821182 100644 --- a/hw/pass-through.c +++ b/hw/pass-through.c @@ -3866,7 +3866,11 @@ static int pt_msgctrl_reg_write(struct pt_dev *ptdev, ptdev->msi->flags |= PCI_MSI_FLAGS_ENABLE; } else - ptdev->msi->flags &= ~PCI_MSI_FLAGS_ENABLE; + { + if (ptdev->msi->flags & PT_MSI_MAPPED) { + pt_msi_disable(ptdev); + } + } /* pass through MSI_ENABLE bit when no MSI-INTx translation */ if (!ptdev->msi_trans_en) { @@ -4013,6 +4017,8 @@ static int pt_msixctrl_reg_write(struct pt_dev *ptdev, pt_disable_msi_translate(ptdev); } pt_msix_update(ptdev); + } else if (!(*value & PCI_MSIX_ENABLE) && ptdev->msix->enabled) { + pt_msix_disable(ptdev); } ptdev->msix->enabled = !!(*value & PCI_MSIX_ENABLE); diff --git a/hw/pt-msi.c b/hw/pt-msi.c index b03b989..3f5f94b 100644 --- a/hw/pt-msi.c +++ b/hw/pt-msi.c @@ -213,7 +213,8 @@ void pt_msi_disable(struct pt_dev *dev) out: /* clear msi info */ - dev->msi->flags &= ~(MSI_FLAG_UNINIT | PT_MSI_MAPPED | PCI_MSI_FLAGS_ENABLE); + dev->msi->flags &= ~(PT_MSI_MAPPED | PCI_MSI_FLAGS_ENABLE); + dev->msi->flags |= MSI_FLAG_UNINIT; dev->msi->pirq = -1; dev->msi_trans_en = 0; } @@ -447,7 +448,7 @@ static void pci_msix_writel(void *opaque, target_phys_addr_t addr, uint32_t val) { const volatile uint32_t *vec_ctrl; - if ( entry->io_mem[offset] == val ) + if ( entry->io_mem[offset] == val && entry->pirq != -1) return; /* -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads 2014-04-08 16:44 ` [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads Konrad Rzeszutek Wilk @ 2014-07-02 15:06 ` Ian Jackson 2014-07-03 3:12 ` Zhenzhong Duan 0 siblings, 1 reply; 14+ messages in thread From: Ian Jackson @ 2014-07-02 15:06 UTC (permalink / raw) To: Konrad Rzeszutek Wilk; +Cc: xen-devel, zhenzhong.duan, stefano.stabellini Konrad Rzeszutek Wilk writes ("[Xen-devel] [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads"): > From: Zhenzhong Duan <zhenzhong.duan@oracle.com> > > Pirqs are not freed when driver unloads, then new pirqs are allocated when > driver reloads. 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. I have backported this to 4.4 and 4.3. It did not apply cleanly to 4.2 (and I have not investgated why). Thanks, Ian. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads 2014-07-02 15:06 ` Ian Jackson @ 2014-07-03 3:12 ` Zhenzhong Duan 2014-07-03 17:54 ` Konrad Rzeszutek Wilk 2014-08-07 3:29 ` Zhenzhong Duan 0 siblings, 2 replies; 14+ messages in thread From: Zhenzhong Duan @ 2014-07-03 3:12 UTC (permalink / raw) To: Ian Jackson, Konrad Rzeszutek Wilk; +Cc: xen-devel, stefano.stabellini There is a patch dependency missed. commit adf74189dd58014744a4b8c9d64407d629da5e2f Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Date: Mon Dec 10 12:43:33 2012 +0000 qemu-xen-trad/pt_msi_disable: do not clear all MSI flags "qemu-xen-trad: fix msi_translate with PV event delivery" added a pt_msi_disable() call into pt_msgctrl_reg_write, clearing the MSI flags as a consequence. MSIs get enabled again soon after by calling pt_msi_setup. However the MSI flags are only setup once in the pt_msgctrl_reg_init function, so from the QEMU point of view the device has lost some important properties, like for example PCI_MSI_FLAGS_64BIT. This patch fixes the bug by clearing only the MSI enabled/mapped/initialized flags in pt_msi_disable. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Tested-by: G.R. <firemeteor@users.sourceforge.net> Xen-devel: http://marc.info/?l=xen-devel&m=135489879503075 diff --git a/hw/pt-msi.c b/hw/pt-msi.c index 73f737d..b03b989 100644 --- a/hw/pt-msi.c +++ b/hw/pt-msi.c @@ -213,7 +213,7 @@ void pt_msi_disable(struct pt_dev *dev) out: /* clear msi info */ - dev->msi->flags = 0; + dev->msi->flags &= ~(MSI_FLAG_UNINIT | PT_MSI_MAPPED | PCI_MSI_FLAGS_ENABLE); dev->msi->pirq = -1; dev->msi_trans_en = 0; } On 2014-7-2 23:06, Ian Jackson wrote: > Konrad Rzeszutek Wilk writes ("[Xen-devel] [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads"): >> From: Zhenzhong Duan<zhenzhong.duan@oracle.com> >> >> Pirqs are not freed when driver unloads, then new pirqs are allocated when >> driver reloads. 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. > I have backported this to 4.4 and 4.3. It did not apply cleanly to > 4.2 (and I have not investgated why). > > Thanks, > Ian. ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads 2014-07-03 3:12 ` Zhenzhong Duan @ 2014-07-03 17:54 ` Konrad Rzeszutek Wilk 2014-07-04 1:29 ` Zhenzhong Duan 2014-08-07 3:29 ` Zhenzhong Duan 1 sibling, 1 reply; 14+ messages in thread From: Konrad Rzeszutek Wilk @ 2014-07-03 17:54 UTC (permalink / raw) To: Zhenzhong Duan; +Cc: xen-devel, Ian Jackson, stefano.stabellini On Thu, Jul 03, 2014 at 11:12:18AM +0800, Zhenzhong Duan wrote: > There is a patch dependency missed. Is that one upstream as well? Or does it not need to be? If it is upstream, what is the title/commit id of that one? Thank you! > > commit adf74189dd58014744a4b8c9d64407d629da5e2f > Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Date: Mon Dec 10 12:43:33 2012 +0000 > > qemu-xen-trad/pt_msi_disable: do not clear all MSI flags > > "qemu-xen-trad: fix msi_translate with PV event delivery" added a > pt_msi_disable() call into pt_msgctrl_reg_write, clearing the MSI flags > as a consequence. MSIs get enabled again soon after by calling > pt_msi_setup. > > However the MSI flags are only setup once in the pt_msgctrl_reg_init > function, so from the QEMU point of view the device has lost some > important properties, like for example PCI_MSI_FLAGS_64BIT. > > This patch fixes the bug by clearing only the MSI > enabled/mapped/initialized flags in pt_msi_disable. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Tested-by: G.R. <firemeteor@users.sourceforge.net> > Xen-devel: http://marc.info/?l=xen-devel&m=135489879503075 > > diff --git a/hw/pt-msi.c b/hw/pt-msi.c > index 73f737d..b03b989 100644 > --- a/hw/pt-msi.c > +++ b/hw/pt-msi.c > @@ -213,7 +213,7 @@ void pt_msi_disable(struct pt_dev *dev) > > out: > /* clear msi info */ > - dev->msi->flags = 0; > + dev->msi->flags &= ~(MSI_FLAG_UNINIT | PT_MSI_MAPPED | > PCI_MSI_FLAGS_ENABLE); > dev->msi->pirq = -1; > dev->msi_trans_en = 0; > } > > On 2014-7-2 23:06, Ian Jackson wrote: > >Konrad Rzeszutek Wilk writes ("[Xen-devel] [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads"): > >>From: Zhenzhong Duan<zhenzhong.duan@oracle.com> > >> > >>Pirqs are not freed when driver unloads, then new pirqs are allocated when > >>driver reloads. 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. > >I have backported this to 4.4 and 4.3. It did not apply cleanly to > >4.2 (and I have not investgated why). > > > >Thanks, > >Ian. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads 2014-07-03 17:54 ` Konrad Rzeszutek Wilk @ 2014-07-04 1:29 ` Zhenzhong Duan 2014-07-07 20:40 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 14+ messages in thread From: Zhenzhong Duan @ 2014-07-04 1:29 UTC (permalink / raw) To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Ian Jackson, stefano.stabellini On 2014-7-4 1:54, Konrad Rzeszutek Wilk wrote: > On Thu, Jul 03, 2014 at 11:12:18AM +0800, Zhenzhong Duan wrote: >> There is a patch dependency missed. > Is that one upstream as well? Or does it not need to be? > If it is upstream, what is the title/commit id of that one? > > Thank you! It's upstream, see detail below >> commit adf74189dd58014744a4b8c9d64407d629da5e2f >> Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> Date: Mon Dec 10 12:43:33 2012 +0000 >> >> qemu-xen-trad/pt_msi_disable: do not clear all MSI flags >> >> "qemu-xen-trad: fix msi_translate with PV event delivery" added a >> pt_msi_disable() call into pt_msgctrl_reg_write, clearing the MSI flags >> as a consequence. MSIs get enabled again soon after by calling >> pt_msi_setup. >> >> However the MSI flags are only setup once in the pt_msgctrl_reg_init >> function, so from the QEMU point of view the device has lost some >> important properties, like for example PCI_MSI_FLAGS_64BIT. >> >> This patch fixes the bug by clearing only the MSI >> enabled/mapped/initialized flags in pt_msi_disable. >> >> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> Tested-by: G.R. <firemeteor@users.sourceforge.net> >> Xen-devel: http://marc.info/?l=xen-devel&m=135489879503075 >> >> diff --git a/hw/pt-msi.c b/hw/pt-msi.c >> index 73f737d..b03b989 100644 >> --- a/hw/pt-msi.c >> +++ b/hw/pt-msi.c >> @@ -213,7 +213,7 @@ void pt_msi_disable(struct pt_dev *dev) >> >> out: >> /* clear msi info */ >> - dev->msi->flags = 0; >> + dev->msi->flags &= ~(MSI_FLAG_UNINIT | PT_MSI_MAPPED | >> PCI_MSI_FLAGS_ENABLE); >> dev->msi->pirq = -1; >> dev->msi_trans_en = 0; >> } >> >> On 2014-7-2 23:06, Ian Jackson wrote: >>> Konrad Rzeszutek Wilk writes ("[Xen-devel] [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads"): >>>> From: Zhenzhong Duan<zhenzhong.duan@oracle.com> >>>> >>>> Pirqs are not freed when driver unloads, then new pirqs are allocated when >>>> driver reloads. 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. >>> I have backported this to 4.4 and 4.3. It did not apply cleanly to >>> 4.2 (and I have not investgated why). >>> >>> Thanks, >>> Ian. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads 2014-07-04 1:29 ` Zhenzhong Duan @ 2014-07-07 20:40 ` Konrad Rzeszutek Wilk 2014-07-08 1:12 ` Zhenzhong Duan 0 siblings, 1 reply; 14+ messages in thread From: Konrad Rzeszutek Wilk @ 2014-07-07 20:40 UTC (permalink / raw) To: Zhenzhong Duan; +Cc: xen-devel, Ian Jackson, stefano.stabellini On Fri, Jul 04, 2014 at 09:29:38AM +0800, Zhenzhong Duan wrote: > > On 2014-7-4 1:54, Konrad Rzeszutek Wilk wrote: > >On Thu, Jul 03, 2014 at 11:12:18AM +0800, Zhenzhong Duan wrote: > >>There is a patch dependency missed. > >Is that one upstream as well? Or does it not need to be? > >If it is upstream, what is the title/commit id of that one? > > > >Thank you! > It's upstream, see detail below [konrad@build-external qemu-xen-dir]$ pwd /home/konrad/xtt-x86_64/xen/tools/qemu-xen-dir [konrad@build-external qemu-xen-dir]$ git show adf74189dd58014744a4b8c9d64407d629da5e2f fatal: bad object adf74189dd58014744a4b8c9d64407d629da5e2f [konrad@build-external qemu-xen-dir]$ git log --grep="clear all MSI" [konrad@build-external qemu-xen-dir]$ ? > >>commit adf74189dd58014744a4b8c9d64407d629da5e2f > >>Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >>Date: Mon Dec 10 12:43:33 2012 +0000 > >> > >> qemu-xen-trad/pt_msi_disable: do not clear all MSI flags > >> > >> "qemu-xen-trad: fix msi_translate with PV event delivery" added a > >> pt_msi_disable() call into pt_msgctrl_reg_write, clearing the MSI flags > >> as a consequence. MSIs get enabled again soon after by calling > >> pt_msi_setup. > >> > >> However the MSI flags are only setup once in the pt_msgctrl_reg_init > >> function, so from the QEMU point of view the device has lost some > >> important properties, like for example PCI_MSI_FLAGS_64BIT. > >> > >> This patch fixes the bug by clearing only the MSI > >> enabled/mapped/initialized flags in pt_msi_disable. > >> > >> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >> Tested-by: G.R. <firemeteor@users.sourceforge.net> > >> Xen-devel: http://marc.info/?l=xen-devel&m=135489879503075 > >> > >>diff --git a/hw/pt-msi.c b/hw/pt-msi.c > >>index 73f737d..b03b989 100644 > >>--- a/hw/pt-msi.c > >>+++ b/hw/pt-msi.c > >>@@ -213,7 +213,7 @@ void pt_msi_disable(struct pt_dev *dev) > >> > >> out: > >> /* clear msi info */ > >>- dev->msi->flags = 0; > >>+ dev->msi->flags &= ~(MSI_FLAG_UNINIT | PT_MSI_MAPPED | > >>PCI_MSI_FLAGS_ENABLE); > >> dev->msi->pirq = -1; > >> dev->msi_trans_en = 0; > >> } > >> > >>On 2014-7-2 23:06, Ian Jackson wrote: > >>>Konrad Rzeszutek Wilk writes ("[Xen-devel] [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads"): > >>>>From: Zhenzhong Duan<zhenzhong.duan@oracle.com> > >>>> > >>>>Pirqs are not freed when driver unloads, then new pirqs are allocated when > >>>>driver reloads. 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. > >>>I have backported this to 4.4 and 4.3. It did not apply cleanly to > >>>4.2 (and I have not investgated why). > >>> > >>>Thanks, > >>>Ian. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads 2014-07-07 20:40 ` Konrad Rzeszutek Wilk @ 2014-07-08 1:12 ` Zhenzhong Duan 2014-08-04 14:01 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 14+ messages in thread From: Zhenzhong Duan @ 2014-07-08 1:12 UTC (permalink / raw) To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Ian Jackson, stefano.stabellini On 2014-7-8 4:40, Konrad Rzeszutek Wilk wrote: > On Fri, Jul 04, 2014 at 09:29:38AM +0800, Zhenzhong Duan wrote: >> On 2014-7-4 1:54, Konrad Rzeszutek Wilk wrote: >>> On Thu, Jul 03, 2014 at 11:12:18AM +0800, Zhenzhong Duan wrote: >>>> There is a patch dependency missed. >>> Is that one upstream as well? Or does it not need to be? >>> If it is upstream, what is the title/commit id of that one? >>> >>> Thank you! >> It's upstream, see detail below > > [konrad@build-external qemu-xen-dir]$ pwd > /home/konrad/xtt-x86_64/xen/tools/qemu-xen-dir > [konrad@build-external qemu-xen-dir]$ git show adf74189dd58014744a4b8c9d64407d629da5e2f > fatal: bad object adf74189dd58014744a4b8c9d64407d629da5e2f > [konrad@build-external qemu-xen-dir]$ git log --grep="clear all MSI" > [konrad@build-external qemu-xen-dir]$ > > ? Sorry, I mean qemu-xen-traditional upstream git zduan >>>> commit adf74189dd58014744a4b8c9d64407d629da5e2f >>>> Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >>>> Date: Mon Dec 10 12:43:33 2012 +0000 >>>> >>>> qemu-xen-trad/pt_msi_disable: do not clear all MSI flags >>>> >>>> "qemu-xen-trad: fix msi_translate with PV event delivery" added a >>>> pt_msi_disable() call into pt_msgctrl_reg_write, clearing the MSI flags >>>> as a consequence. MSIs get enabled again soon after by calling >>>> pt_msi_setup. >>>> >>>> However the MSI flags are only setup once in the pt_msgctrl_reg_init >>>> function, so from the QEMU point of view the device has lost some >>>> important properties, like for example PCI_MSI_FLAGS_64BIT. >>>> >>>> This patch fixes the bug by clearing only the MSI >>>> enabled/mapped/initialized flags in pt_msi_disable. >>>> >>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >>>> Tested-by: G.R. <firemeteor@users.sourceforge.net> >>>> Xen-devel: http://marc.info/?l=xen-devel&m=135489879503075 >>>> >>>> diff --git a/hw/pt-msi.c b/hw/pt-msi.c >>>> index 73f737d..b03b989 100644 >>>> --- a/hw/pt-msi.c >>>> +++ b/hw/pt-msi.c >>>> @@ -213,7 +213,7 @@ void pt_msi_disable(struct pt_dev *dev) >>>> >>>> out: >>>> /* clear msi info */ >>>> - dev->msi->flags = 0; >>>> + dev->msi->flags &= ~(MSI_FLAG_UNINIT | PT_MSI_MAPPED | >>>> PCI_MSI_FLAGS_ENABLE); >>>> dev->msi->pirq = -1; >>>> dev->msi_trans_en = 0; >>>> } >>>> >>>> On 2014-7-2 23:06, Ian Jackson wrote: >>>>> Konrad Rzeszutek Wilk writes ("[Xen-devel] [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads"): >>>>>> From: Zhenzhong Duan<zhenzhong.duan@oracle.com> >>>>>> >>>>>> Pirqs are not freed when driver unloads, then new pirqs are allocated when >>>>>> driver reloads. 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. >>>>> I have backported this to 4.4 and 4.3. It did not apply cleanly to >>>>> 4.2 (and I have not investgated why). >>>>> >>>>> Thanks, >>>>> Ian. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads 2014-07-08 1:12 ` Zhenzhong Duan @ 2014-08-04 14:01 ` Konrad Rzeszutek Wilk 2014-08-05 7:31 ` Zhenzhong Duan 0 siblings, 1 reply; 14+ messages in thread From: Konrad Rzeszutek Wilk @ 2014-08-04 14:01 UTC (permalink / raw) To: Zhenzhong Duan; +Cc: xen-devel, Ian Jackson, stefano.stabellini On Tue, Jul 08, 2014 at 09:12:39AM +0800, Zhenzhong Duan wrote: > > On 2014-7-8 4:40, Konrad Rzeszutek Wilk wrote: > >On Fri, Jul 04, 2014 at 09:29:38AM +0800, Zhenzhong Duan wrote: > >>On 2014-7-4 1:54, Konrad Rzeszutek Wilk wrote: > >>>On Thu, Jul 03, 2014 at 11:12:18AM +0800, Zhenzhong Duan wrote: > >>>>There is a patch dependency missed. > >>>Is that one upstream as well? Or does it not need to be? > >>>If it is upstream, what is the title/commit id of that one? > >>> > >>>Thank you! > >>It's upstream, see detail below > > > >[konrad@build-external qemu-xen-dir]$ pwd > >/home/konrad/xtt-x86_64/xen/tools/qemu-xen-dir > >[konrad@build-external qemu-xen-dir]$ git show adf74189dd58014744a4b8c9d64407d629da5e2f > >fatal: bad object adf74189dd58014744a4b8c9d64407d629da5e2f > >[konrad@build-external qemu-xen-dir]$ git log --grep="clear all MSI" > >[konrad@build-external qemu-xen-dir]$ > > > >? > Sorry, I mean qemu-xen-traditional upstream git Ok, is that patch upstream? Should it be upstream? If so, had it been posted in the past? Thank you! > > zduan > >>>>commit adf74189dd58014744a4b8c9d64407d629da5e2f > >>>>Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >>>>Date: Mon Dec 10 12:43:33 2012 +0000 > >>>> > >>>> qemu-xen-trad/pt_msi_disable: do not clear all MSI flags > >>>> > >>>> "qemu-xen-trad: fix msi_translate with PV event delivery" added a > >>>> pt_msi_disable() call into pt_msgctrl_reg_write, clearing the MSI flags > >>>> as a consequence. MSIs get enabled again soon after by calling > >>>> pt_msi_setup. > >>>> > >>>> However the MSI flags are only setup once in the pt_msgctrl_reg_init > >>>> function, so from the QEMU point of view the device has lost some > >>>> important properties, like for example PCI_MSI_FLAGS_64BIT. > >>>> > >>>> This patch fixes the bug by clearing only the MSI > >>>> enabled/mapped/initialized flags in pt_msi_disable. > >>>> > >>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > >>>> Tested-by: G.R. <firemeteor@users.sourceforge.net> > >>>> Xen-devel: http://marc.info/?l=xen-devel&m=135489879503075 > >>>> > >>>>diff --git a/hw/pt-msi.c b/hw/pt-msi.c > >>>>index 73f737d..b03b989 100644 > >>>>--- a/hw/pt-msi.c > >>>>+++ b/hw/pt-msi.c > >>>>@@ -213,7 +213,7 @@ void pt_msi_disable(struct pt_dev *dev) > >>>> > >>>> out: > >>>> /* clear msi info */ > >>>>- dev->msi->flags = 0; > >>>>+ dev->msi->flags &= ~(MSI_FLAG_UNINIT | PT_MSI_MAPPED | > >>>>PCI_MSI_FLAGS_ENABLE); > >>>> dev->msi->pirq = -1; > >>>> dev->msi_trans_en = 0; > >>>> } > >>>> > >>>>On 2014-7-2 23:06, Ian Jackson wrote: > >>>>>Konrad Rzeszutek Wilk writes ("[Xen-devel] [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads"): > >>>>>>From: Zhenzhong Duan<zhenzhong.duan@oracle.com> > >>>>>> > >>>>>>Pirqs are not freed when driver unloads, then new pirqs are allocated when > >>>>>>driver reloads. 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. > >>>>>I have backported this to 4.4 and 4.3. It did not apply cleanly to > >>>>>4.2 (and I have not investgated why). > >>>>> > >>>>>Thanks, > >>>>>Ian. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads 2014-08-04 14:01 ` Konrad Rzeszutek Wilk @ 2014-08-05 7:31 ` Zhenzhong Duan 2014-08-05 15:21 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 14+ messages in thread From: Zhenzhong Duan @ 2014-08-05 7:31 UTC (permalink / raw) To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Ian Jackson, stefano.stabellini On 2014-8-4 22:01, Konrad Rzeszutek Wilk wrote: > On Tue, Jul 08, 2014 at 09:12:39AM +0800, Zhenzhong Duan wrote: >> On 2014-7-8 4:40, Konrad Rzeszutek Wilk wrote: >>> On Fri, Jul 04, 2014 at 09:29:38AM +0800, Zhenzhong Duan wrote: >>>> On 2014-7-4 1:54, Konrad Rzeszutek Wilk wrote: >>>>> On Thu, Jul 03, 2014 at 11:12:18AM +0800, Zhenzhong Duan wrote: >>>>>> There is a patch dependency missed. >>>>> Is that one upstream as well? Or does it not need to be? >>>>> If it is upstream, what is the title/commit id of that one? >>>>> >>>>> Thank you! >>>> It's upstream, see detail below >>> [konrad@build-external qemu-xen-dir]$ pwd >>> /home/konrad/xtt-x86_64/xen/tools/qemu-xen-dir >>> [konrad@build-external qemu-xen-dir]$ git show adf74189dd58014744a4b8c9d64407d629da5e2f >>> fatal: bad object adf74189dd58014744a4b8c9d64407d629da5e2f >>> [konrad@build-external qemu-xen-dir]$ git log --grep="clear all MSI" >>> [konrad@build-external qemu-xen-dir]$ >>> >>> ? >> Sorry, I mean qemu-xen-traditional upstream git > Ok, is that patch upstream? Should it be upstream? If so, had it been > posted in the past? It's already in qemu-xen upstream, see below void xen_pt_msi_disable(XenPCIPassthroughState *s) { ....... /* clear msi info */ msi->flags &= ~PCI_MSI_FLAGS_ENABLE; msi->initialized = false; msi->mapped = false; msi->pirq = XEN_PT_UNASSIGNED_PIRQ; } ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads 2014-08-05 7:31 ` Zhenzhong Duan @ 2014-08-05 15:21 ` Konrad Rzeszutek Wilk 0 siblings, 0 replies; 14+ messages in thread From: Konrad Rzeszutek Wilk @ 2014-08-05 15:21 UTC (permalink / raw) To: Zhenzhong Duan; +Cc: xen-devel, Ian Jackson, stefano.stabellini On Tue, Aug 05, 2014 at 03:31:34PM +0800, Zhenzhong Duan wrote: > > On 2014-8-4 22:01, Konrad Rzeszutek Wilk wrote: > >On Tue, Jul 08, 2014 at 09:12:39AM +0800, Zhenzhong Duan wrote: > >>On 2014-7-8 4:40, Konrad Rzeszutek Wilk wrote: > >>>On Fri, Jul 04, 2014 at 09:29:38AM +0800, Zhenzhong Duan wrote: > >>>>On 2014-7-4 1:54, Konrad Rzeszutek Wilk wrote: > >>>>>On Thu, Jul 03, 2014 at 11:12:18AM +0800, Zhenzhong Duan wrote: > >>>>>>There is a patch dependency missed. > >>>>>Is that one upstream as well? Or does it not need to be? > >>>>>If it is upstream, what is the title/commit id of that one? > >>>>> > >>>>>Thank you! > >>>>It's upstream, see detail below > >>>[konrad@build-external qemu-xen-dir]$ pwd > >>>/home/konrad/xtt-x86_64/xen/tools/qemu-xen-dir > >>>[konrad@build-external qemu-xen-dir]$ git show adf74189dd58014744a4b8c9d64407d629da5e2f > >>>fatal: bad object adf74189dd58014744a4b8c9d64407d629da5e2f > >>>[konrad@build-external qemu-xen-dir]$ git log --grep="clear all MSI" > >>>[konrad@build-external qemu-xen-dir]$ > >>> > >>>? > >>Sorry, I mean qemu-xen-traditional upstream git > >Ok, is that patch upstream? Should it be upstream? If so, had it been > >posted in the past? > It's already in qemu-xen upstream, see below > > void xen_pt_msi_disable(XenPCIPassthroughState *s) > { > ....... > /* clear msi info */ > msi->flags &= ~PCI_MSI_FLAGS_ENABLE; > msi->initialized = false; > msi->mapped = false; > msi->pirq = XEN_PT_UNASSIGNED_PIRQ; > } OK, and that is part of the giant '3854ca57': commit 3854ca577dad92c4fe97b4a6ebce360e25407af7 Author: Jiang Yunhong <yunhong.jiang@intel.com> Date: Thu Jun 21 15:42:35 2012 +0000 Introduce Xen PCI Passthrough, MSI A more complete history can be found here: git://xenbits.xensource.com/qemu-xen-unstable.git Signed-off-by: Jiang Yunhong <yunhong.jiang@intel.com> Signed-off-by: Shan Haitao <haitao.shan@intel.com> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> And the git tree it points to has tons of tags but nothing really saying 'pci'. <sigh> Seems like the only option is to spin out a tiny patch that has the fix and document where it was (aka point to the giant commit). Duan, I can do it in two-three weeks time-frame. If you want to take a stab at it before then, don't hesitate! ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads 2014-07-03 3:12 ` Zhenzhong Duan 2014-07-03 17:54 ` Konrad Rzeszutek Wilk @ 2014-08-07 3:29 ` Zhenzhong Duan 1 sibling, 0 replies; 14+ messages in thread From: Zhenzhong Duan @ 2014-08-07 3:29 UTC (permalink / raw) To: Ian Jackson, Konrad Rzeszutek Wilk; +Cc: xen-devel, stefano.stabellini Hi Ian, Could you cherry-pick commit adf74189dd58014744a4b8c9d64407d629da5e2f before backport my patch? It seems 4.3 and 4.4 branch already has commit adf74189dd but 4.2 not. thanks zduan On 2014-7-3 11:12, Zhenzhong Duan wrote: > There is a patch dependency missed. > > commit adf74189dd58014744a4b8c9d64407d629da5e2f > Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Date: Mon Dec 10 12:43:33 2012 +0000 > > qemu-xen-trad/pt_msi_disable: do not clear all MSI flags > > "qemu-xen-trad: fix msi_translate with PV event delivery" added a > pt_msi_disable() call into pt_msgctrl_reg_write, clearing the MSI > flags > as a consequence. MSIs get enabled again soon after by calling > pt_msi_setup. > > However the MSI flags are only setup once in the pt_msgctrl_reg_init > function, so from the QEMU point of view the device has lost some > important properties, like for example PCI_MSI_FLAGS_64BIT. > > This patch fixes the bug by clearing only the MSI > enabled/mapped/initialized flags in pt_msi_disable. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Tested-by: G.R. <firemeteor@users.sourceforge.net> > Xen-devel: http://marc.info/?l=xen-devel&m=135489879503075 > > diff --git a/hw/pt-msi.c b/hw/pt-msi.c > index 73f737d..b03b989 100644 > --- a/hw/pt-msi.c > +++ b/hw/pt-msi.c > @@ -213,7 +213,7 @@ void pt_msi_disable(struct pt_dev *dev) > > out: > /* clear msi info */ > - dev->msi->flags = 0; > + dev->msi->flags &= ~(MSI_FLAG_UNINIT | PT_MSI_MAPPED | > PCI_MSI_FLAGS_ENABLE); > dev->msi->pirq = -1; > dev->msi_trans_en = 0; > } > > On 2014-7-2 23:06, Ian Jackson wrote: >> Konrad Rzeszutek Wilk writes ("[Xen-devel] [PATCH] qemu-xen-trad: >> free all the pirqs for msi/msix when driver unloads"): >>> From: Zhenzhong Duan<zhenzhong.duan@oracle.com> >>> >>> Pirqs are not freed when driver unloads, then new pirqs are >>> allocated when >>> driver reloads. 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. >> I have backported this to 4.4 and 4.3. It did not apply cleanly to >> 4.2 (and I have not investgated why). >> >> Thanks, >> Ian. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix qemu traditional with PCI passthrough. 2014-04-08 16:44 [PATCH] Fix qemu traditional with PCI passthrough Konrad Rzeszutek Wilk 2014-04-08 16:44 ` [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads Konrad Rzeszutek Wilk @ 2014-04-08 16:56 ` Ian Jackson 2014-06-25 14:59 ` [PATCH] Fix qemu traditional with PCI passthrough. [and 2 more messages] Ian Jackson 1 sibling, 1 reply; 14+ messages in thread From: Ian Jackson @ 2014-04-08 16:56 UTC (permalink / raw) To: Konrad Rzeszutek Wilk; +Cc: xen-devel, zhenzhong.duan, stefano.stabellini Konrad Rzeszutek Wilk writes ("[PATCH] Fix qemu traditional with PCI passthrough."): > Hey Ian and Stefano, > > This patch was posted way back last year in July > (see http://lists.xen.org/archives/html/xen-devel/2013-07/msg00004.html) and > was mentioned to: > > ">From what I understand following the conversation, I think this is > > probably the right way to solve the problem, but given that it's only > > really a problem when you load and unload drivers, which is the > > uncommon case, I think at this point we should probably hold off on > > this one until 4.3.1. > > > > Stefano, thoughts? > > I think that's OK. I'll wait to apply the qemu-xen patch until after the > release." > > I think this patch just got lost in the Xen 4.4 release. Dusting it > off and reposting. Quite likely. I'll wait for Stefano to comment on the qemu-xen patch but I don't see a problem with applying this to staging, and it should probably go on my backport list. Thanks, Ian. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix qemu traditional with PCI passthrough. [and 2 more messages] 2014-04-08 16:56 ` [PATCH] Fix qemu traditional with PCI passthrough Ian Jackson @ 2014-06-25 14:59 ` Ian Jackson 0 siblings, 0 replies; 14+ messages in thread From: Ian Jackson @ 2014-06-25 14:59 UTC (permalink / raw) To: Konrad Rzeszutek Wilk; +Cc: xen-devel, zhenzhong.duan, stefano.stabellini Ian Jackson writes ("Re: [PATCH] Fix qemu traditional with PCI passthrough."): > Konrad Rzeszutek Wilk writes ("[PATCH] Fix qemu traditional with PCI passthrough."): > > I think this patch just got lost in the Xen 4.4 release. Dusting it > > off and reposting. > > Quite likely. > > I'll wait for Stefano to comment on the qemu-xen patch but I don't see > a problem with applying this to staging, and it should probably go on > my backport list. I see this is in our qemu-upstream tree (as d0395cc49b2ec6d1723c01f1daf2394b9264ca29). I have applied it to qemu-xen-traditional and added it to my backport list. Thanks, Ian. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-08-07 3:29 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-04-08 16:44 [PATCH] Fix qemu traditional with PCI passthrough Konrad Rzeszutek Wilk 2014-04-08 16:44 ` [PATCH] qemu-xen-trad: free all the pirqs for msi/msix when driver unloads Konrad Rzeszutek Wilk 2014-07-02 15:06 ` Ian Jackson 2014-07-03 3:12 ` Zhenzhong Duan 2014-07-03 17:54 ` Konrad Rzeszutek Wilk 2014-07-04 1:29 ` Zhenzhong Duan 2014-07-07 20:40 ` Konrad Rzeszutek Wilk 2014-07-08 1:12 ` Zhenzhong Duan 2014-08-04 14:01 ` Konrad Rzeszutek Wilk 2014-08-05 7:31 ` Zhenzhong Duan 2014-08-05 15:21 ` Konrad Rzeszutek Wilk 2014-08-07 3:29 ` Zhenzhong Duan 2014-04-08 16:56 ` [PATCH] Fix qemu traditional with PCI passthrough Ian Jackson 2014-06-25 14:59 ` [PATCH] Fix qemu traditional with PCI passthrough. [and 2 more messages] Ian Jackson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.