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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@xxxxxxxxxx]
> Sent: 14 March 2014 18:07
> To: Paul Durrant
> Cc: Ian Campbell; xen-devel@xxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH v3 6/6] ioreq-server: bring the PCI hotplug
> controller implementation into Xen
> 
> On Fri, Mar 14, 2014 at 03:18:12PM +0000, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Ian Campbell
> > > Sent: 14 March 2014 15:02
> > > To: Paul Durrant
> > > Cc: xen-devel@xxxxxxxxxxxxx
> > > Subject: Re: [Xen-devel] [PATCH v3 6/6] ioreq-server: bring the PCI
> hotplug
> > > controller implementation into Xen
> > >
> > > On Fri, 2014-03-14 at 14:31 +0000, Paul Durrant wrote:
> > > > > -----Original Message-----
> > > > > From: Ian Campbell
> > > > > Sent: 14 March 2014 14:09
> > > > > To: Paul Durrant
> > > > > Cc: xen-devel@xxxxxxxxxxxxx
> > > > > Subject: Re: [Xen-devel] [PATCH v3 6/6] ioreq-server: bring the PCI
> > > hotplug
> > > > > controller implementation into Xen
> > > > >
> > > > > On Fri, 2014-03-14 at 13:25 +0000, Paul Durrant wrote:
> > > > > > > -----Original Message-----
> > > > > > > From: Ian Campbell
> > > > > > > Sent: 14 March 2014 11:58
> > > > > > > To: Paul Durrant
> > > > > > > Cc: xen-devel@xxxxxxxxxxxxx
> > > > > > > Subject: Re: [Xen-devel] [PATCH v3 6/6] ioreq-server: bring the
> PCI
> > > > > hotplug
> > > > > > > controller implementation into Xen
> > > > > > >
> > > > > > > On Wed, 2014-03-05 at 14:48 +0000, Paul Durrant wrote:
> > > > > > > > diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> > > > > > > > index 2e52470..4176440 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_enable(ctx->xch, domid, pcidev-
> > > >dev);
> > > > > > > > +        if (rc < 0) {
> > > > > > > > +            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Error:
> > > > > > > xc_hvm_pci_hotplug_enable failed");
> > > > > > > > +            return ERROR_FAIL;
> > > > > > > > +        }
> > > > > > >
> > > > > > > Perhaps I'm misreading this but does this imply that you cannot
> > > hotplug
> > > > > > > PCI devices into an HVM guest which wasn't started with a PCI
> > > device?
> > > > > > > That doesn't sound right/desirable.
> > > > > > >
> > > > > >
> > > > > > I don't think that is the case. The extra code here is because we're
> > > > > > intercepting the hotplug controller IO space in Xen so QEMU may
> well
> > > > > > play with its hotplug controller device model, but the guest will
> > > > > > never see it.
> > > > >
> > > > > That wasn't what I meant.
> > > > >
> > > > > Unless the guest has a PCI device enabled the above code will never
> be
> > > > > called, so we will never setup the hotplug controller within Xen.
> > > > >
> > > >
> > > > I don't follow. The hotplug controller is set up by the call to
> > > > gpe_init() in hvm_domain_initialize(). The above code is there to tell
> > > > the hotplug controller a new device has appeared. Am I missing
> > > > something?
> > >
> > > No, I was, didn't realise this was per-device setup.
> > >
> > > I assume this is ok to call for both cold- and hotplug
> > >
> >
> > I believe so. I've certainly seen no fallout in testing my new VGA device
> (which is cold plugged to a paused domain).
> >
> > > > > > > Is there no problem with the availability of the i/o space for the
> > > > > > > different versions of qemu (i.e. they are both the same today?)
> The
> > > AML
> > > > > > > looked like it poked a different thing in the trad case -- so is
> 0xae00
> > > > > > > unused there?
> > > > > > >
> > > > > >
> > > > > > QEMU will still emulate a PCI hotplug controller but the guest will 
> > > > > > no
> > > > > > longer see it. In the case of upstream that io range is now handled
> by
> > > > > > xen, so it really really can't get to it. If trad is used then the
> > > > > > hotplug controller would still be visible if the guest talks to the
> > > > > > old IO ranges, but since they are not specified in the ACPI table 
> > > > > > any
> > > > > > more it shouldnât have anything to do with them. If you think that's
> a
> > > > > > problem then I could hook those IO ranges in Xen too and stop the
> IO
> > > > > > getting through.
> > > > >
> > > > > What I meant was what if there was something else at 0xae00 on
> trad?
> > > >
> > > > I don't believe so.
> > > >
> > > > > (since trad seems to have its hotplug controller somewhere else this
> is
> > > > > possible). That something will now be shadowed by the hotplug
> > > controller
> > > > > in Xen. If that something was important for some other reason this is
> a
> > > > > problem. IOW is there a hole in the io port address map at this
> location
> > > > > on both qemus?
> > > > >
> > > >
> > > > The new implementation in Xen directly overlays the upstream QEMU
> > > > controller.
> > >
> > > I got this part.
> > >
> > > >  I believe those IO ports are unimplemented by trad.
> > >
> > > That's the important thing (although turning "believe" into "have
> > > confirmed" would make me sleep easier).
> >
> > Sure, I can check.
> 
> What about future version of QEMU? If they moved the addresses in the
> future
> (or decided to expand the existing ones to do some extra stuff) - what is our
> path to deal with this?

I don't think we have to worry about that now. If our AML always points at 
Xen's hotplug controller implementation then, if QEMU decides to add some new 
IO ports for its hotplug controller implementation then it's no different from 
the machine type getting a new device model that we don't expose to the guest: 
we have to decide whether itâs a problem on a case-by-case basis.

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