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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-10-03 at 17:19 +0100, Paul Durrant wrote:
> > -----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 thought you meant there was no implementation of the blacklisting
protocol itself, which there is, it just doesn't happen to actually
blacklist anything.

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

I'm not sure I understand what you are trying to say.

How does a driver which expects the blacklist protocol work with
xen_pvdevice which doesn't implement that protocol?

Or are does a driver which works with xen_platform not work with
xen_pvdevice and vice versa? Or do the drivers have to contain code to
determine which they are running against and only do the unplug against
xen_platform?

I'm a bit surprised xen_platform doesn't implement the unplug protocol,
I thought it was just a xen_platform with parameterised vendor id etc.

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

In which namespace is my question.

Do the drivers using these 4 and 5 codes work with xen_platform and/or
xen_pvdevice?

xen_pvdevice doesn't implement the unplug protocol AFAICT, so the list
of product codes seems to be pretty irrelevant to it.

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


Ian.


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