Re: [PATCH] docs: Add some details on XenServer PCI devices

["Alejandro Vallejo" <alejandro.vallejo@xxxxxxxxx>]

For the record, my remarks are fairly inconsequential. This patch is a net
positive addition to the man page and I think it should go in.

On Tue Mar 4, 2025 at 12:29 PM GMT, Roger Pau Monné wrote:
> On Tue, Mar 04, 2025 at 11:17:42AM +0000, Frediano Ziglio wrote:
> > On Tue, Mar 4, 2025 at 11:08 AM Roger Pau Monné <roger.pau@xxxxxxxxxx> 
> > wrote:
> > >
> > > On Tue, Mar 04, 2025 at 10:21:52AM +0000, Alejandro Vallejo wrote:
> > > > Hi,
> > > >
> > > > On Fri Feb 28, 2025 at 3:21 PM GMT, Frediano Ziglio wrote:
> > > > > Describe the usage of devices 5853:0002 and 5853:C000.
> > > > >
> > > > > Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
> > > > > ---
> > > > >  docs/man/xen-pci-device-reservations.7.pod | 9 +++++++++
> > > > >  1 file changed, 9 insertions(+)
> > > > >
> > > > > diff --git a/docs/man/xen-pci-device-reservations.7.pod 
> > > > > b/docs/man/xen-pci-device-reservations.7.pod
> > > > > index 9ddf3a18ad..62f3bd2105 100644
> > > > > --- a/docs/man/xen-pci-device-reservations.7.pod
> > > > > +++ b/docs/man/xen-pci-device-reservations.7.pod
> > > > > @@ -10,6 +10,8 @@ use of this is with device ID 0x0001 to advertise 
> > > > > the Xen Platform PCI
> > > > >  device - the presence of this virtual device enables a guest 
> > > > > Operating
> > > > >  System (subject to the availability of suitable drivers) to make use 
> > > > > of
> > > > >  paravirtualisation features such as disk and network devices etc.
> > > > > +XenServer, for Windows machines, presents Xen Platform device with 
> > > > > device
> > > > > +ID 0x0002 instead of 0x0001.
> > > >
> > > > nit: in the interest of future-proofing the doc 's/presents/may 
> > > > present/'?
> > > >
> > > > >
> > > > >  Some Xen vendors wish to provide alternative and/or additional guest 
> > > > > drivers
> > > > >  that can bind to virtual devices[1]. This may be done using the Xen 
> > > > > PCI
> > > > > @@ -86,4 +88,11 @@ and unplug protocol.
> > > > >  libxl provides support for creation of a single additional 
> > > > > xen-pvdevice.
> > > > >  See the vendor_device parameter in xl.cfg(5).
> > > > >
> > > > > +=item 2.
> > > > > +
> > > > > +XenServer, for Windows machines, presents a device with ID 0xC000.
> > > > > +This device is a placeholders for Windows update.
> > > > > +Device 0xC000 is presented with a Xen Platform PCI device, usually 
> > > > > with ID
> > > > > +0x0002.
> > > > > +
> > > > >  =back
> > > >
> > > > Wouldn't this be better covered under "=item 1"? Device 0xc000 is a
> > > > xen-pvdevice, so it could be simplified to a single line of "XenServer 
> > > > uses
> > > > device-id=0xc000 for its pvdevice on Windows guests", or something like 
> > > > that.
> > >
> > > I think it's important to note that c000 always appears in conjunction
> > > with 0001 or 0002, and it's not a replacement for either of those
> > > devices.

That's already stated in the item1 description.

  Note that the presence of the Xen Platform PCI device is generally a
  pre-requisite for an additional xen-pvdevice as it is the platform
  device that provides that IO ports necessary for unplugging emulated
  devices. See hvm-emulated-unplug.markdown for details of the IO ports
  and unplug protocol.

> > >
> > 
> > Do you have something more precise in mind? Can you suggest what to write?
>
> I'm fine with your proposed text, my reply was to Alejandro to note
> that I think his proposed text was missing information that was on
> your original proposal.
>
> "XenServer might present a device with ID 0xC000.  Such device is a
> placeholder for Windows update usage and is always exposed in
> conjunction with a Xen Platform PCI device, usually with ID 0x0002."
>
> I don't care much whether this is on a separate item or not.  My
> preference would be for adding a second item, as to prevent cluttering
> the first one.
>
> I've also looked at xl.cfg, and it mentions:
>
> vendor_device="VENDOR_DEVICE"
>
> Selects which variant of the QEMU xen-pvdevice should be used for this
> guest. Valid values are:
>
>   none The xen-pvdevice should be omitted. This is the default.
>
>   xenserver The xenserver variant of the xen-pvdevice (device-id=C000)
>   will be specified, enabling the use of XenServer PV drivers in the
>   guest.
>
> Isn't this wrong, as selecting `xenserver` should instead use
> device-id=0002 but not C000?  Maybe I'm not understanding how this is
> supported to work.

I think there's some confusion going on.

xen-pvdevice != xen-platformdevice

pvdevice is 0xc000. platformdevice is 0001 or 0002.

platformdevice is the PCI device used for device unplug, grant table
initialization and so on. pvdevice is a true dummy device present for the
sole purpose of letting Windows bind the drivers to an actual device that
can't be 0002 for (afaiui) very, very, very stupid and non-technical reasons.

The man page HINTS at this when talking about binding drivers to devices.

>
> > > Likewise it's important to note that 0001 and 0002 are to my
> > > understanding mutually exclusive, and only one of those must be
> > > exposed.
> > 
> > Not exactly sure if this is a must or a should. From my testing,
> > presenting 2 devices (well, they are mostly the same) works. But, as
> > they do the same things it seems reasonable to avoid the duplication.
> > It looks like a good recommendation.
>
> I was expecting it to not work, as I imagined Linux would then attempt
> to initialize the grant tables twice for example.
>
> Thanks, Roger.

It may very well be a case of "happens to work, but it really shouldn't". The
code does go through `gnttab_init()` twice if the probe succeeds twice. Not
that it couldn't be changed to behave differently, but it currently doesn't.

Anyhow, I think you're right in that 0001 and 0002 must not coexist even if
mechanically they may be able to. In the name of everyone's sanity.

Cheers,
Alejandro