issue in unplug qemu PCI devices

issue in unplug qemu PCI devices

[Zhai, Edwin]

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.

Re: issue in unplug qemu PCI devices

[Stefano Stabellini]

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

Re: issue in unplug qemu PCI devices

[Ky Srinivasan]

>>> 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

Re: issue in unplug qemu PCI devices

[Ian Jackson]

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.

Re: issue in unplug qemu PCI devices

[Stefano Stabellini]

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.

Re: issue in unplug qemu PCI devices

[Edwin Zhai]

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

Re: issue in unplug qemu PCI devices

[Stefano Stabellini]

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++) {

Re: issue in unplug qemu PCI devices

[Zhai, Edwin]

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

Re: issue in unplug qemu PCI devices

[Ian Jackson]

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.