Re: [Xen-devel] [PATCH v5 9/9] ioreq-server: bring the PCI hotplug controller implementation into Xen

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Ian Campbell
> Sent: 07 May 2014 10:48
> To: Paul Durrant
> Cc: xen-devel@xxxxxxxxxxxxx; Ian Jackson; Stefano Stabellini; Jan Beulich
> Subject: Re: [PATCH v5 9/9] ioreq-server: bring the PCI hotplug controller
> implementation into Xen
> 
> On Tue, 2014-05-06 at 14:35 +0100, Paul Durrant wrote:
> > > When libxl does the qmp call to tell qemu about a new device what stops
> > > qemu from generating hotplug events (SCI or whatever the interrupt
> > > mechanism is) even if Xen is emulating the HP controller? Because Qemu
> > > will still have the controller emulation, it's just "shadowed", right?
> > >
> >
> > The controller emulation is there but since the ports are handled by
> > Xen the I/O to enable the hotplug events will never get through. So,
> > yes QEMU will go through the motions of hotplugging but it will never
> > raise the SCI.
> 
> OK. And just to make doubly sure this I/O to enable the hotplug events
> is definitely not preserved over a migration somewhere? (meaning I don't
> have to think about what happens if it is enabled in qemu and then
> migrated to a system where Xen handles it)
> 

Damn... You have a good point there - it is necessary to preserve the enabled 
bits. I'm going to drop this patch from the series to allow more testing time. 
I'll post v6 of the rest shortly.

  Paul

> 
> >
> > > > > What does "hotplugging via qemu" mean here if qemu isn't patched
> to
> > > call
> > > > > this new call?
> > > > >
> > > >
> > > > It means QEMU is implementing the hotplug device. So, if Xen is
> > > > implementing the PCIHP libxl will call the new function and it will
> > > > succeed. If QEMU is implementing the PCIHP then libxl will call the
> > > > new function, it will fail, but QEMU will implicitly do the hotplug.
> > > > Either way, the guest sees a hotplug event.
> > >
> > > Even if Xen is implementing the PCIHP qemu is still involved in plugging
> > > the device, since it has to know about it though, so in some sense
> > > hotplugging is always (partially) via qemu (which is why I'm a bit
> > > confused, but also why the lack of a Qemu side patch surprises me)
> > >
> >
> > Yes, but because QEMU's PCIHP implementation never has any of its
> > enabled bits set it remains silent.
> 
> Makes sense, might be worth mentioing this somewhere (comment, commit
> log), since it is a bit subtle.
> 
> 
> > > > > > diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> > > > > > index 44d0453..55cb8a2 100644
> > > > > > --- a/tools/libxl/libxl_pci.c
> > > > > > +++ b/tools/libxl/libxl_pci.c
> > > > > > @@ -867,6 +867,13 @@ static int do_pci_add(libxl__gc *gc, uint32_t
> > > > > domid, libxl_device_pci *pcidev, i
> > > > > >          }
> > > > > >          if ( rc )
> > > > > >              return ERROR_FAIL;
> > > > > > +
> > > > > > +        rc = xc_hvm_pci_hotplug(CTX->xch, domid, pcidev->dev, 1);
> > > > > > +        if (rc < 0 && errno != EOPNOTSUPP) {
> > > > > > +            LOGE(ERROR, "Error: xc_hvm_pci_hotplug enable failed");
> > > > > > +            return ERROR_FAIL;
> > > > > > +        }
> > > > >
> > > > > I initially thought you needed to also reset rc to indicate success in
> > > > > the errno==EOPNOTSUPP, but actually the error handling in this
> function
> > > > > is just confusing...
> > > > >
> > > > > But, have you tried hotpluging into a migrated guest?
> > > > >
> > > >
> > > > Not yet, but I can't see why that would be a problem over hotplugging
> > > > at all,
> > >
> > > It's the problems we can't imagine that I'm worried about. This stuff is
> > > certainly subtle in places WRT migration.
> > >
> >
> > I'll give it a go on my dev box. I'm also trying to get this series
> > backported onto Xen 4.4 so I can throw it at XenRT.
> 
> Thanks!
> 
> Ian.

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