Re: [Xen-devel] [PATCH] Register PV driver product numbers 4 and 5.

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Ian Campbell
> Sent: 03 October 2013 15:33
> To: Paul Durrant
> Cc: xen-devel@xxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH] Register PV driver product numbers 4 and
> 5.
> 
> On Thu, 2013-10-03 at 15:19 +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Ian Campbell
> > > Sent: 03 October 2013 14:19
> > > To: Paul Durrant
> > > Cc: xen-devel@xxxxxxxxxxxxx
> > > Subject: Re: [Xen-devel] [PATCH] Register PV driver product numbers 4
> and
> > > 5.
> > >
> > > On Mon, 2013-09-30 at 12:18 +0100, Paul Durrant wrote:
> > > > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > > > ---
> > > >  xen/include/public/hvm/pvdrivers.h |   12 +++++++-----
> > > >  1 file changed, 7 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/xen/include/public/hvm/pvdrivers.h
> > > b/xen/include/public/hvm/pvdrivers.h
> > > > index 4c6b705..77994d2 100644
> > > > --- a/xen/include/public/hvm/pvdrivers.h
> > > > +++ b/xen/include/public/hvm/pvdrivers.h
> > > > @@ -38,10 +38,12 @@
> > > >   * indicate a driver which is yet to be released.
> > > >   */
> > > >
> > > > -#define PVDRIVERS_PRODUCT_LIST(EACH)                         \
> > > > -        EACH("xensource-windows", 0x0001) /* Citrix */       \
> > > > -        EACH("gplpv-windows",     0x0002) /* James Harper */ \
> > > > -        EACH("linux",             0x0003)                    \
> > > > -        EACH("experimental",      0xffff)
> > > > +#define PVDRIVERS_PRODUCT_LIST(EACH)                               \
> > > > +        EACH("xensource-windows",       0x0001) /* Citrix */       \
> > > > +        EACH("gplpv-windows",           0x0002) /* James Harper */ \
> > > > +        EACH("linux",                   0x0003)                    \
> > > > +        EACH("xenserver-windows-v7.0+", 0x0004) /* Citrix */       \
> > > > +        EACH("xenserver-windows-v7.2+", 0x0005) /* Citrix */       \
> > >
> > > The unplug protocol includes
> > >         4. The drivers write a four-byte build number to IO port `0x10`.
> > > Can't you use that instead of defining a new product for each version of
> > > your drivers?
> > >
> >
> > Unfortunately I need to grandfather in 4 because it's used in
> > XenServer (and cause the patched version of QEMU therein to behave in
> > a slightly funky way). I wanted to reserve 5 because it's not been
> > used in XenServer so far and therefore has no odd semantics attached
> > to it and so I intend to use it for the newer Windows-Update-ready
> > drivers, which I don't need to be able to blacklist in the 'old' way
> > as they're designed to work with upstream QEMU where there is no
> > implementation of blacklisting.
> 
> ./hw/xen/xen_platform.c seems to have support for blacklisting in it. Or
> did you mean hw/xen/xen_pvdevice?
> 

No, I meant xen_platform; that's still the home of the fixed IO ports. From my 
reading of the code in xen_platform.c I don't see anything that ever sets 
s->drivers_blacklisted - am I missing something subtle?

> > I was originally intending to use 1, but that also has been messed
> > with in XenServer to work around some compatibility problems.
> >
> > > How does this interact with the use of the alternative platform device
> > > thing which you added? Does docs/misc/hvm-emulated-
> unplug.markdown
> > > need
> > > expanding to cover that case?
> > >
> >
> > No, I still use the traditional unplug protocol and will respect
> > blacklisting
> 
> Hrm, what is the traditional unplug protocol? Something other than
> docs/misc/hvm-emulated-unplug.markdown?
> 

That's the one I mean. XenServer has another which we don't propose to upstream.

> Assuming it is that, xen_pvdevice doesn't seem to implement any of it,
> won't the drivers therefore abort at #1?"  If the magic number doesn't
> match, the drivers don't do anything."?
> 

xen_pvdevice doesn't need to, xen_platform is always there.

> >  if anyone cares to implement it for product number 5 but, as I said,
> > I don't anticipate the need for it.
> 
> So product 5 is intended to be the same thing no matter whether you use
> xen_platform or xen_pvdevice and no matter which vendor/device ID is
> configured? You can only make this true for Citrix vendor ids I think,
> or do you intend this to be binding for everyone?
> 

The whole point of having this header file is that it is the canonical list of 
product codes; it should apply to everyone. That is why I'm reserving 4 and 5 - 
to make sure that no-one else tries to use them.

> I think that doc should be expanded to at least mention what is going on
> here, since there are a lot of reasonable options and it's not clear to
> me at least which one we are following.
> 
> > > I notice that docs/misc/hvm-emulated-unplug.markdown also says:
> > >         3. The drivers write a two-byte product number to IO port `0x12`. 
> > >  At
> > >            the moment, the only drivers using this protocol are our
> > >            closed-source ones, which use product number 1.
> > > which isn't accurate any longer...
> > >
> >
> > True, but then Linux PVonHVM didn't register product number 3 either,
> > which is what led to a lot of the craziness leading to me now having
> > to register 4 and 5.
> 
> I'm not sure I follow. How does PVHVM using 3 mean that you *have* to
> reserve anything at all? I can see how given you need to register
> something it now has to be 4 etc but that seems normal not crazy.
> 

The problem was that we started using 3 in the belief that this was safe, 
because no-one had put any code into qemu that suggested 3 was already in use 
by anyone (and this header in xen did not exist). We then used the build number 
for product in a new way so we could blacklist drivers according to their PCI 
binding, only then to discover that PVonHVM linux no longer worked properly. 
So, we hastily moved to product code 4 with the same semantics for build 
number. That is why I want make sure that is reserved, and reserve 5 for future 
use.

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