* [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] 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
* 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
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.