Mailing List Archive: [PATCH] qemu-xen-trad/pt_msi_disable: do not clear all MSI flags

[PATCH] qemu-xen-trad/pt_msi_disable: do not clear all MSI flags

Dec 10, 2012, 4:36 AM

Post #1 of 3 (301 views)

"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;
}

Re: [PATCH] qemu-xen-trad/pt_msi_disable: do not clear all MSI flags [ In reply to ]

linux at eikelenboom

Dec 10, 2012, 5:17 AM

Post #2 of 3 (293 views)

Permalink

Monday, December 10, 2012, 1:36:41 PM, you wrote:

> "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;
> }

Seems this should be fixed for qemu-upstream as well ?
I think since switching to qemu-upstream as default for xen-unstable / 4.3 seems around the corner,
it's perhaps wise for all patches to qemu-traditional, to also check if qemu-upstream needs the same fix (to prevent regressions after the switch) ?

--
Sander

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

Re: [PATCH] qemu-xen-trad/pt_msi_disable: do not clear all MSI flags [ In reply to ]

stefano.stabellini at eu

Dec 10, 2012, 5:38 AM

Post #3 of 3 (292 views)

Permalink

On Mon, 10 Dec 2012, Sander Eikelenboom wrote:
> Monday, December 10, 2012, 1:36:41 PM, you wrote:
>
> > "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;
> > }
>
>
> Seems this should be fixed for qemu-upstream as well ?
> I think since switching to qemu-upstream as default for xen-unstable / 4.3 seems around the corner,
> it's perhaps wise for all patches to qemu-traditional, to also check if qemu-upstream needs the same fix (to prevent regressions after the switch) ?
>

That is true and you do right to remind me, but this is one of the few
cases of PCI passthrough code where upstream QEMU and
qemu-xen-traditional differ: MSI translation is not implemented at all
in upstream QEMU so there is no need for this patch there.