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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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)


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