diff mbox series

[5/6] xen-pt: Hide MSI-X from xen stubdoms

Message ID 20190311180216.18811-6-jandryuk@gmail.com (mailing list archive)
State New, archived
Headers show
Series Xen stubdom support | expand

Commit Message

Jason Andryuk March 11, 2019, 6:02 p.m. UTC
MSI-X is not supported in Xen stubdoms, so it must be disabled.  Use the
existing xen_pt_hide_dev_cap to hide when running under -xen-stubdom.

A compile-time patch was originally written by James McKenzie
<james.mckenzie@bromium.com>

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 hw/xen/xen_pt_config_init.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Roger Pau Monné March 12, 2019, 12:04 p.m. UTC | #1
On Mon, Mar 11, 2019 at 02:02:15PM -0400, Jason Andryuk wrote:
> MSI-X is not supported in Xen stubdoms, so it must be disabled.  Use the
> existing xen_pt_hide_dev_cap to hide when running under -xen-stubdom.

I'm afraid this requires some more context. What's the actual issue
that prevents MSI-X from working?

Roger.
Marek Marczykowski-Górecki March 12, 2019, 12:38 p.m. UTC | #2
On Tue, Mar 12, 2019 at 01:04:19PM +0100, Roger Pau Monné wrote:
> On Mon, Mar 11, 2019 at 02:02:15PM -0400, Jason Andryuk wrote:
> > MSI-X is not supported in Xen stubdoms, so it must be disabled.  Use the
> > existing xen_pt_hide_dev_cap to hide when running under -xen-stubdom.
> 
> I'm afraid this requires some more context. What's the actual issue
> that prevents MSI-X from working?

At least missing "Fix PCI passthrough for HVM with stubdomain" series,
but that's mostly on Xen side (+ one change how QEMU enable MSI-X in
config space).
Some of it can be worked around by enabling permissive mode. Jason, did
you had a chance to test it with any MSI-X device?
I'm not aware of anything thing particular that breaks MSI-X but not
MSI. Besides much less devices lying around to test MSI-X...
Jason Andryuk March 12, 2019, 1:58 p.m. UTC | #3
On Tue, Mar 12, 2019 at 8:38 AM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> On Tue, Mar 12, 2019 at 01:04:19PM +0100, Roger Pau Monné wrote:
> > On Mon, Mar 11, 2019 at 02:02:15PM -0400, Jason Andryuk wrote:
> > > MSI-X is not supported in Xen stubdoms, so it must be disabled.  Use the
> > > existing xen_pt_hide_dev_cap to hide when running under -xen-stubdom.
> >
> > I'm afraid this requires some more context. What's the actual issue
> > that prevents MSI-X from working?
>
> At least missing "Fix PCI passthrough for HVM with stubdomain" series,
> but that's mostly on Xen side (+ one change how QEMU enable MSI-X in
> config space).
> Some of it can be worked around by enabling permissive mode. Jason, did
> you had a chance to test it with any MSI-X device?
> I'm not aware of anything thing particular that breaks MSI-X but not
> MSI. Besides much less devices lying around to test MSI-X...

OpenXT and Qubes have used a compile time patch that disabled MSI-X
for a long time.  The OpenXT patch description doesn't help:
"""
Currently we do not support MSI-X setup for PCI devices passed through.

Although the specification mentions that PCI-e devices might implement only
MSI-X there is not a lot of those and mostly none that we have encountered yet.
Considering that, we force devices to use MSI by hiding the MSI-X capability.
"""

To be honest, I didn't question the reasoning and just made the
compile-time disabling into a runtime disabling.

I tested with a NEC uPD720200 XHCI controller supporting MSI-X.  There
was an error related to setting up MSI-X when I failed to pass the
"-xen-stubdom" flag.  I can pull that log when I get back to the
machine.  With this patch, MSI-X was hidden in the guest, but dom0
showed MSI-X present but unused.

Marek, is "Use xc_physdev_msi_set_enable for enabling MSI..." the QEMU
patch you are refer to?  Do you think permissive mode would allow
MSI-X to work without that patch?  I could test that out.

Regards,
Jason
Roger Pau Monné March 12, 2019, 2:13 p.m. UTC | #4
On Tue, Mar 12, 2019 at 09:58:56AM -0400, Jason Andryuk wrote:
> On Tue, Mar 12, 2019 at 8:38 AM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> >
> > On Tue, Mar 12, 2019 at 01:04:19PM +0100, Roger Pau Monné wrote:
> > > On Mon, Mar 11, 2019 at 02:02:15PM -0400, Jason Andryuk wrote:
> > > > MSI-X is not supported in Xen stubdoms, so it must be disabled.  Use the
> > > > existing xen_pt_hide_dev_cap to hide when running under -xen-stubdom.
> > >
> > > I'm afraid this requires some more context. What's the actual issue
> > > that prevents MSI-X from working?
> >
> > At least missing "Fix PCI passthrough for HVM with stubdomain" series,
> > but that's mostly on Xen side (+ one change how QEMU enable MSI-X in
> > config space).
> > Some of it can be worked around by enabling permissive mode. Jason, did
> > you had a chance to test it with any MSI-X device?
> > I'm not aware of anything thing particular that breaks MSI-X but not
> > MSI. Besides much less devices lying around to test MSI-X...
> 
> OpenXT and Qubes have used a compile time patch that disabled MSI-X
> for a long time.  The OpenXT patch description doesn't help:
> """
> Currently we do not support MSI-X setup for PCI devices passed through.
> 
> Although the specification mentions that PCI-e devices might implement only
> MSI-X there is not a lot of those and mostly none that we have encountered yet.
> Considering that, we force devices to use MSI by hiding the MSI-X capability.
> """
> 
> To be honest, I didn't question the reasoning and just made the
> compile-time disabling into a runtime disabling.
> 
> I tested with a NEC uPD720200 XHCI controller supporting MSI-X.  There
> was an error related to setting up MSI-X when I failed to pass the
> "-xen-stubdom" flag.  I can pull that log when I get back to the
> machine.  With this patch, MSI-X was hidden in the guest, but dom0
> showed MSI-X present but unused.

Given that Marek is working on a series to make both MSI and MSI-X
work for pci-passthrough and stubdomains I'm not sure how did you even
manage to get plain MSI working. Is there something I'm missing?

Thanks, Roger.
Marek Marczykowski-Górecki March 12, 2019, 2:29 p.m. UTC | #5
On Tue, Mar 12, 2019 at 09:58:56AM -0400, Jason Andryuk wrote:
> On Tue, Mar 12, 2019 at 8:38 AM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> >
> > On Tue, Mar 12, 2019 at 01:04:19PM +0100, Roger Pau Monné wrote:
> > > On Mon, Mar 11, 2019 at 02:02:15PM -0400, Jason Andryuk wrote:
> > > > MSI-X is not supported in Xen stubdoms, so it must be disabled.  Use the
> > > > existing xen_pt_hide_dev_cap to hide when running under -xen-stubdom.
> > >
> > > I'm afraid this requires some more context. What's the actual issue
> > > that prevents MSI-X from working?
> >
> > At least missing "Fix PCI passthrough for HVM with stubdomain" series,
> > but that's mostly on Xen side (+ one change how QEMU enable MSI-X in
> > config space).
> > Some of it can be worked around by enabling permissive mode. Jason, did
> > you had a chance to test it with any MSI-X device?
> > I'm not aware of anything thing particular that breaks MSI-X but not
> > MSI. Besides much less devices lying around to test MSI-X...
> 
> OpenXT and Qubes have used a compile time patch that disabled MSI-X
> for a long time.  The OpenXT patch description doesn't help:
> """
> Currently we do not support MSI-X setup for PCI devices passed through.
> 
> Although the specification mentions that PCI-e devices might implement only
> MSI-X there is not a lot of those and mostly none that we have encountered yet.
> Considering that, we force devices to use MSI by hiding the MSI-X capability.
> """
> 
> To be honest, I didn't question the reasoning and just made the
> compile-time disabling into a runtime disabling.
> 
> I tested with a NEC uPD720200 XHCI controller supporting MSI-X.  There
> was an error related to setting up MSI-X when I failed to pass the
> "-xen-stubdom" flag.  I can pull that log when I get back to the
> machine.  With this patch, MSI-X was hidden in the guest, but dom0
> showed MSI-X present but unused.
> 
> Marek, is "Use xc_physdev_msi_set_enable for enabling MSI..." the QEMU
> patch you are refer to?  Do you think permissive mode would allow
> MSI-X to work without that patch?  I could test that out.

Yes, this one. Permissive mode should work around it.
There is also another patch about IRQ permission, but I believe you
already have it in OpenXT.
Jason Andryuk March 12, 2019, 3:15 p.m. UTC | #6
On Tue, Mar 12, 2019 at 10:13 AM Roger Pau Monné <roger.pau@citrix.com> wrote:
>
> On Tue, Mar 12, 2019 at 09:58:56AM -0400, Jason Andryuk wrote:
> > On Tue, Mar 12, 2019 at 8:38 AM Marek Marczykowski-Górecki
> > <marmarek@invisiblethingslab.com> wrote:
> > >
> > > On Tue, Mar 12, 2019 at 01:04:19PM +0100, Roger Pau Monné wrote:
> > > > On Mon, Mar 11, 2019 at 02:02:15PM -0400, Jason Andryuk wrote:
> > > > > MSI-X is not supported in Xen stubdoms, so it must be disabled.  Use the
> > > > > existing xen_pt_hide_dev_cap to hide when running under -xen-stubdom.
> > > >
> > > > I'm afraid this requires some more context. What's the actual issue
> > > > that prevents MSI-X from working?
> > >
> > > At least missing "Fix PCI passthrough for HVM with stubdomain" series,
> > > but that's mostly on Xen side (+ one change how QEMU enable MSI-X in
> > > config space).
> > > Some of it can be worked around by enabling permissive mode. Jason, did
> > > you had a chance to test it with any MSI-X device?
> > > I'm not aware of anything thing particular that breaks MSI-X but not
> > > MSI. Besides much less devices lying around to test MSI-X...
> >
> > OpenXT and Qubes have used a compile time patch that disabled MSI-X
> > for a long time.  The OpenXT patch description doesn't help:
> > """
> > Currently we do not support MSI-X setup for PCI devices passed through.
> >
> > Although the specification mentions that PCI-e devices might implement only
> > MSI-X there is not a lot of those and mostly none that we have encountered yet.
> > Considering that, we force devices to use MSI by hiding the MSI-X capability.
> > """
> >
> > To be honest, I didn't question the reasoning and just made the
> > compile-time disabling into a runtime disabling.
> >
> > I tested with a NEC uPD720200 XHCI controller supporting MSI-X.  There
> > was an error related to setting up MSI-X when I failed to pass the
> > "-xen-stubdom" flag.  I can pull that log when I get back to the
> > machine.  With this patch, MSI-X was hidden in the guest, but dom0
> > showed MSI-X present but unused.
>
> Given that Marek is working on a series to make both MSI and MSI-X
> work for pci-passthrough and stubdomains I'm not sure how did you even
> manage to get plain MSI working. Is there something I'm missing?

Marek's series adds a hypercall to enable MSI/MSI-X since they are not
allowed by default in PCI passthrough.  PCI passthrough also has a
permissive mode to allow access to a device's entire PCI configuration
space including enabling MSI.

Qubes 4.0 has an out-of-tree patch that whitelists enabling MSI in
non-permissive mode - I've tested these patches on Qubes 4.0 with the
MSI-X XHCI device where MSI is enabled but not MSI-X.  Other NICs with
only MSI also work.

OpenXT linux stubdoms use permissive mode PCI passthrough, so the
stubdom can program the PCI config space to enable MSI.  I've tested
the patches there, but none of my OpenXT test systems have MSI-X.  MSI
devices work properly.

Marek also tested a "vanilla" linux stubdom version in his osstest
suite, but that doesn't test passthrough.

If Marek's pending patch series or "permissive" mode will enable
MSI/MSI-X, then maybe this patch should just be dropped in favor of
those options.  I'll test to verify whether MSI-X works with
permissive mode.

Regards,
Jason
Jason Andryuk March 13, 2019, 2:15 a.m. UTC | #7
On Tue, Mar 12, 2019 at 11:15 AM Jason Andryuk <jandryuk@gmail.com> wrote:
<snip>
> I'll test to verify whether MSI-X works with
> permissive mode.

Dropping this patch and enabling permissive mode allowed MSI-X to work.

{"execute": "device_add", "arguments": {"driver":
"xen-pci-passthrough", "id": "xen-pci-pt_0000-03-00.0", "hostaddr":
"0000:00:01.00", "machine_addr": "0000:03:00.0", "permissive": true}}
[00:07.0] xen_pt_realize: Assigning real physical device 00:01.0 to devfn 0x38
[00:07.0] xen_pt_register_regions: IO region 0 registered
(size=0x00002000 base_addr=0xe1900000 type: 0x4)
[00:07.0] xen_pt_config_reg_init: Offset 0x000e mismatch!
Emulated=0x0080, host=0x0000, syncing to 0x0000.
[00:07.0] xen_pt_config_reg_init: Offset 0x0010 mismatch!
Emulated=0x0000, host=0xe1900004, syncing to 0xe1900004.
[00:07.0] xen_pt_config_reg_init: Offset 0x0052 mismatch!
Emulated=0x0000, host=0x01c3, syncing to 0x0003.
[00:07.0] xen_pt_config_reg_init: Offset 0x0072 mismatch!
Emulated=0x0000, host=0x0086, syncing to 0x0080.
[00:07.0] xen_pt_config_reg_init: Offset 0x00a4 mismatch!
Emulated=0x0000, host=0x8fc0, syncing to 0x8fc0.
[00:07.0] xen_pt_config_reg_init: Offset 0x00aa mismatch!
Emulated=0x0000, host=0x0010, syncing to 0x0010.
[00:07.0] xen_pt_config_reg_init: Offset 0x00b2 mismatch!
Emulated=0x0000, host=0x1011, syncing to 0x1011.
[00:07.0] xen_pt_msix_init: get MSI-X table BAR base 0xe1900000
[00:07.0] xen_pt_msix_init: table_off = 0x1000, total_entries = 8
[00:07.0] xen_pt_msix_init: mapping physical MSI-X table to 0x7ad6a82e4000
[00:07.0] xen_pt_config_reg_init: Offset 0x0092 mismatch!
Emulated=0x0000, host=0x0007, syncing to 0x0007.
[00:07.0] xen_pt_pci_intx: intx=1
[00:07.0] xen_pt_realize: Real physical device 00:01.0 registered successfully
{"return": {}}

hw/xen/xen_pt_msi.c:xen_pt_msix_init() calls open(/dev/mem) and mmaps
it, so I had to add CONFIG_DEVMEM to the stubdom linux .config.
Without /dev/mem:
[00:07.0] xen_pt_msix_init: get MSI-X table BAR base 0xe1900000
[00:07.0] xen_pt_msix_init: Error: Can't open /dev/mem: No such file
or directory
[00:07.0] xen_pt_msix_size_init: Error: Internal error: Invalid
xen_pt_msix_init.
Failed to initialize 12/15, type = 0x1, rc: -2
[00:07.0] xen_pt_msi_set_enable: disabling MSI.
free(): double free detected in tcache 2

[user@sys-usb ~]$ sudo lspci -v -s 00:07.0
00:07.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host
Controller (rev 04) (prog-if 30 [XHCI])
    Subsystem: Lenovo Device 21d2
    Physical Slot: 7
    Flags: bus master, fast devsel, latency 0, IRQ 44
    Memory at f2024000 (64-bit, non-prefetchable) [size=8K]
    Capabilities: [50] Power Management version 3
    Capabilities: [70] MSI: Enable- Count=1/1 Maskable- 64bit+
    Capabilities: [90] MSI-X: Enable+ Count=8 Masked-
    Capabilities: [a0] Express Endpoint, MSI 00
    Kernel driver in use: xhci_hcd
    Kernel modules: xhci_pci

Regards,
Jason
diff mbox series

Patch

diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 31ec5add1d..b827a493ea 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -54,6 +54,9 @@  static int xen_pt_hide_dev_cap(const XenHostPCIDevice *d, uint8_t grp_id)
             return 1;
         }
         break;
+    case PCI_CAP_ID_MSIX:
+        /* stubdoms don't support MSI-X so skip it. */
+        return xen_stubdom_enabled();
     }
     return 0;
 }