IOEMU change set e7911109 uses Magic ioport (0x10) protocol for negotating with guest PV drivers during startup, and allowing PV drivers to disable hardware emulations thus preventing guest from seeing the same device through two paths. But when I tried PV drivers in xen upstream, this unplug logic never succeed. PV driver uses following io sequence to interact with platform_pci device in qemu: 1. inw 0x10 2. inb 0x12 3. outw(0x12, 0xbeef) 4. outl(0x10,0xdead) But I only saw 1&2 happened in xen io instruction emulation(x86_emulate), while 3&4 seemed to lost so the unplug will never happen. Are you aware of this issue? Or some suggestion for debugging? BTW, the unplug logic itself has some issues also: 1. Pass-through NICs are also unplugged, although them have different path with vnif and emulated NIC. 2. Unplug happens as long as inserting the xen_platform_pci module regardless of existence of PVed device in the config file. End user likely to unplug all the PCI device by accident. 3. Inserting xen_platform_pci module would unplug all the devices, which is not reasonable. E.g. end user only has vbd driver, but all NICs are also unplugged. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, 12 Feb 2010, Zhai, Edwin wrote:> IOEMU change set e7911109 uses Magic ioport (0x10) protocol for > negotating with guest PV drivers during startup, and allowing PV drivers > to disable hardware emulations thus preventing guest from seeing the > same device through two paths. > > But when I tried PV drivers in xen upstream, this unplug logic never > succeed. PV driver uses following io sequence to interact with > platform_pci device in qemu: > 1. inw 0x10 > 2. inb 0x12 > 3. outw(0x12, 0xbeef) > 4. outl(0x10,0xdead) > > But I only saw 1&2 happened in xen io instruction > emulation(x86_emulate), while 3&4 seemed to lost so the unplug will > never happen. > > Are you aware of this issue? Or some suggestion for debugging? > > BTW, the unplug logic itself has some issues also: > 1. Pass-through NICs are also unplugged, although them have different > path with vnif and emulated NIC. > 2. Unplug happens as long as inserting the xen_platform_pci module > regardless of existence of PVed device in the config file. End user > likely to unplug all the PCI device by accident. > 3. Inserting xen_platform_pci module would unplug all the devices, which > is not reasonable. E.g. end user only has vbd driver, but all NICs are > also unplugged. >You are right, that is a bug and this patch should fix it. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- diff --git a/hw/pci.c b/hw/pci.c index d7c516e..01eff8d 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -826,7 +826,8 @@ void pci_unplug_netifs(void) dev = bus->devices[x]; if (dev && dev->config[0xa] == 0 && - dev->config[0xb] == 2) { + dev->config[0xb] == 2 && + test_pci_slot(x >> 3) != 1) { /* Found a netif. Remove it from the bus. Note that we don''t free it here, since there could still be references to it floating around. There are only _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> On 2/12/2010 at 12:14 PM, in message<alpine.DEB.2.00.1002121606100.1147@kaball-desktop>, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:> On Fri, 12 Feb 2010, Zhai, Edwin wrote: >> IOEMU change set e7911109 uses Magic ioport (0x10) protocol for >> negotating with guest PV drivers during startup, and allowing PV drivers >> to disable hardware emulations thus preventing guest from seeing the >> same device through two paths. >> >> But when I tried PV drivers in xen upstream, this unplug logic never >> succeed. PV driver uses following io sequence to interact with >> platform_pci device in qemu: >> 1. inw 0x10 >> 2. inb 0x12 >> 3. outw(0x12, 0xbeef) >> 4. outl(0x10,0xdead) >> >> But I only saw 1&2 happened in xen io instruction >> emulation(x86_emulate), while 3&4 seemed to lost so the unplug will >> never happen.Looks like in 3&4 have the parameters reversed - the first parameter I think should be the value and the second parameter should be the port. Regards, K. Y>> >> Are you aware of this issue? Or some suggestion for debugging? >> >> BTW, the unplug logic itself has some issues also: >> 1. Pass-through NICs are also unplugged, although them have different >> path with vnif and emulated NIC. >> 2. Unplug happens as long as inserting the xen_platform_pci module >> regardless of existence of PVed device in the config file. End user >> likely to unplug all the PCI device by accident. >> 3. Inserting xen_platform_pci module would unplug all the devices, which >> is not reasonable. E.g. end user only has vbd driver, but all NICs are >> also unplugged. >> > > You are right, that is a bug and this patch should fix it. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > diff --git a/hw/pci.c b/hw/pci.c > index d7c516e..01eff8d 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -826,7 +826,8 @@ void pci_unplug_netifs(void) > dev = bus->devices[x]; > if (dev && > dev->config[0xa] == 0 && > - dev->config[0xb] == 2) { > + dev->config[0xb] == 2 && > + test_pci_slot(x >> 3) != 1) { > /* Found a netif. Remove it from the bus. Note that > we don''t free it here, since there could still be > references to it floating around. There are only > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Zhai, Edwin writes ("issue in unplug qemu PCI devices"):> But I only saw 1&2 happened in xen io instruction > emulation(x86_emulate), while 3&4 seemed to lost so the unplug will > never happen. > > Are you aware of this issue? Or some suggestion for debugging?No, I''m not aware of this issue. (I just got back from a week away, so sorry for the delay ...) Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini writes ("Re: [Xen-devel] issue in unplug qemu PCI devices"):> On Fri, 12 Feb 2010, Zhai, Edwin wrote: > > 1. Pass-through NICs are also unplugged, although them have different > > path with vnif and emulated NIC....> You are right, that is a bug and this patch should fix it.I''ve applied this. I assume it wants backporting to 3.4 too ? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Mon, 15 Feb 2010, Ian Jackson wrote:> Stefano Stabellini writes ("Re: [Xen-devel] issue in unplug qemu PCI devices"): > > On Fri, 12 Feb 2010, Zhai, Edwin wrote: > > > 1. Pass-through NICs are also unplugged, although them have different > > > path with vnif and emulated NIC. > ... > > You are right, that is a bug and this patch should fix it. > > I''ve applied this. I assume it wants backporting to 3.4 too ? >Yes, good idea. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano, Thanks for your fix. I''m in leave now and will have a test after back. BTW, do you think we need fix issue 2? You know some OSV will install and load the platform_pci module in guest by default. If end user doesn''t config valid PV device in config file, guest would crash after unplugging emulated device. Is it necessary that we check if having a valid backend in xenstore before unplug related emulated device? Thanks, On Sat, Feb 13, 2010 at 1:14 AM, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:> On Fri, 12 Feb 2010, Zhai, Edwin wrote: >> BTW, the unplug logic itself has some issues also: >> 1. Pass-through NICs are also unplugged, although them have different >> path with vnif and emulated NIC. >> 2. Unplug happens as long as inserting the xen_platform_pci module >> regardless of existence of PVed device in the config file. End user >> likely to unplug all the PCI device by accident. >> 3. Inserting xen_platform_pci module would unplug all the devices, which >> is not reasonable. E.g. end user only has vbd driver, but all NICs are >> also unplugged. >> > > You are right, that is a bug and this patch should fix it. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > diff --git a/hw/pci.c b/hw/pci.c > index d7c516e..01eff8d 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -826,7 +826,8 @@ void pci_unplug_netifs(void) > dev = bus->devices[x]; > if (dev && > dev->config[0xa] == 0 && > - dev->config[0xb] == 2) { > + dev->config[0xb] == 2 && > + test_pci_slot(x >> 3) != 1) { > /* Found a netif. Remove it from the bus. Note that > we don''t free it here, since there could still be > references to it floating around. There are only > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel >-- Best Rgds Zhai Edwin _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Wed, 17 Feb 2010, Edwin Zhai wrote:> Stefano, > Thanks for your fix. I''m in leave now and will have a test after back. > BTW, do you think we need fix issue 2? You know some OSV will install > and load the platform_pci module in guest by default. If end user > doesn''t config valid PV device in config file, guest would crash after > unplugging emulated device. Is it necessary that we check if having a > valid backend in xenstore before unplug related emulated device? >You are right, qemu has to honor disable_pf for ioports as well. --- diff --git a/hw/pc.c b/hw/pc.c index 129e9d9..9375951 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -1047,9 +1047,10 @@ vga_bios_error: #endif /* !CONFIG_DM */ if (pci_enabled) { disable_pf = xenstore_parse_disable_pf_config(); - if (disable_pf != 1) + if (disable_pf != 1) { pci_xen_platform_init(pci_bus); - platform_fixed_ioport_init(); + platform_fixed_ioport_init(); + } } for(i = 0; i < MAX_SERIAL_PORTS; i++) { _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano, I have tested the latest ioemu, your 2 fixes really work. Thanks! Stefano Stabellini wrote:> On Wed, 17 Feb 2010, Edwin Zhai wrote: > >> Stefano, >> Thanks for your fix. I''m in leave now and will have a test after back. >> BTW, do you think we need fix issue 2? You know some OSV will install >> and load the platform_pci module in guest by default. If end user >> doesn''t config valid PV device in config file, guest would crash after >> unplugging emulated device. Is it necessary that we check if having a >> valid backend in xenstore before unplug related emulated device? >> >> > > You are right, qemu has to honor disable_pf for ioports as well. > > --- > > diff --git a/hw/pc.c b/hw/pc.c > index 129e9d9..9375951 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -1047,9 +1047,10 @@ vga_bios_error: > #endif /* !CONFIG_DM */ > if (pci_enabled) { > disable_pf = xenstore_parse_disable_pf_config(); > - if (disable_pf != 1) > + if (disable_pf != 1) { > pci_xen_platform_init(pci_bus); > - platform_fixed_ioport_init(); > + platform_fixed_ioport_init(); > + } > } > > for(i = 0; i < MAX_SERIAL_PORTS; i++) { > >-- best rgds, edwin _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel