[PATCH] xen: Add support for XenServer 6.1 platform device

Frediano Ziglio posted 1 patch 8 months, 1 week ago
Failed in applying to current master (apply log)
There is a newer version of this series
drivers/xen/platform-pci.c | 2 ++
include/linux/pci_ids.h    | 1 +
2 files changed, 3 insertions(+)
[PATCH] xen: Add support for XenServer 6.1 platform device
Posted by Frediano Ziglio 8 months, 1 week ago
On XenServer on Windows machine a platform device with ID 2 instead of
1 is used.
This device is mainly identical to device 1 but due to some Windows
update behaviour it was decided to use a device with a different ID.
This causes compatibility issues with Linux which expects, if Xen
is detected, to find a Xen platform device (5853:0001) otherwise code
will crash due to some missing initialization (specifically grant
tables).
The device 2 is presented by Xapi adding device specification to
Qemu command line.

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
 drivers/xen/platform-pci.c | 2 ++
 include/linux/pci_ids.h    | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
index 544d3f9010b9..9cefc7d6bcba 100644
--- a/drivers/xen/platform-pci.c
+++ b/drivers/xen/platform-pci.c
@@ -174,6 +174,8 @@ static int platform_pci_probe(struct pci_dev *pdev,
 static const struct pci_device_id platform_pci_tbl[] = {
 	{PCI_VENDOR_ID_XEN, PCI_DEVICE_ID_XEN_PLATFORM,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
+	{PCI_VENDOR_ID_XEN, PCI_DEVICE_ID_XEN_PLATFORM_XS61,
+		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
 	{0,}
 };
 
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 1a2594a38199..e4791fd97ee0 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -3241,6 +3241,7 @@
 
 #define PCI_VENDOR_ID_XEN		0x5853
 #define PCI_DEVICE_ID_XEN_PLATFORM	0x0001
+#define PCI_DEVICE_ID_XEN_PLATFORM_XS61	0x0002
 
 #define PCI_VENDOR_ID_OCZ		0x1b85
 
-- 
2.48.1
Re: [PATCH] xen: Add support for XenServer 6.1 platform device
Posted by Bjorn Helgaas 8 months, 1 week ago
On Tue, Feb 25, 2025 at 02:03:53PM +0000, Frediano Ziglio wrote:
> On XenServer on Windows machine a platform device with ID 2 instead of
> 1 is used.
> This device is mainly identical to device 1 but due to some Windows
> update behaviour it was decided to use a device with a different ID.
> This causes compatibility issues with Linux which expects, if Xen
> is detected, to find a Xen platform device (5853:0001) otherwise code
> will crash due to some missing initialization (specifically grant
> tables).
> The device 2 is presented by Xapi adding device specification to
> Qemu command line.

Add blank lines between paragraphs.

A crash seems unfortunate.  And it sounds like a user mistake, e.g., a
typo in the Qemu device specification, could also cause a crash?

If the crash is distinctive, a hint here like a dmesg line or two
might help users.

> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> ---
>  drivers/xen/platform-pci.c | 2 ++
>  include/linux/pci_ids.h    | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
> index 544d3f9010b9..9cefc7d6bcba 100644
> --- a/drivers/xen/platform-pci.c
> +++ b/drivers/xen/platform-pci.c
> @@ -174,6 +174,8 @@ static int platform_pci_probe(struct pci_dev *pdev,
>  static const struct pci_device_id platform_pci_tbl[] = {
>  	{PCI_VENDOR_ID_XEN, PCI_DEVICE_ID_XEN_PLATFORM,
>  		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
> +	{PCI_VENDOR_ID_XEN, PCI_DEVICE_ID_XEN_PLATFORM_XS61,
> +		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
>  	{0,}
>  };
>  
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 1a2594a38199..e4791fd97ee0 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -3241,6 +3241,7 @@
>  
>  #define PCI_VENDOR_ID_XEN		0x5853
>  #define PCI_DEVICE_ID_XEN_PLATFORM	0x0001
> +#define PCI_DEVICE_ID_XEN_PLATFORM_XS61	0x0002

If this is the only place PCI_DEVICE_ID_XEN_PLATFORM_XS61 is used, we
would put this in platform-pci.c, per the pci_ids.h comment:

 *      Do not add new entries to this file unless the definitions
 *      are shared between multiple drivers.

>  #define PCI_VENDOR_ID_OCZ		0x1b85
>  
> -- 
> 2.48.1
>
[PATCH v2] xen: Add support for XenServer 6.1 platform device
Posted by Frediano Ziglio 8 months, 1 week ago
On XenServer on Windows machine a platform device with ID 2 instead of
1 is used.

This device is mainly identical to device 1 but due to some Windows
update behaviour it was decided to use a device with a different ID.

This causes compatibility issues with Linux which expects, if Xen
is detected, to find a Xen platform device (5853:0001) otherwise code
will crash due to some missing initialization (specifically grant
tables). Specifically from dmesg

    RIP: 0010:gnttab_expand+0x29/0x210
    Code: 90 0f 1f 44 00 00 55 31 d2 48 89 e5 41 57 41 56 41 55 41 89 fd
          41 54 53 48 83 ec 10 48 8b 05 7e 9a 49 02 44 8b 35 a7 9a 49 02
          <8b> 48 04 8d 44 39 ff f7 f1 45 8d 24 06 89 c3 e8 43 fe ff ff
          44 39
    RSP: 0000:ffffba34c01fbc88 EFLAGS: 00010086
    ...

The device 2 is presented by Xapi adding device specification to
Qemu command line.

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
 drivers/xen/platform-pci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
index 544d3f9010b9..1db82da56db6 100644
--- a/drivers/xen/platform-pci.c
+++ b/drivers/xen/platform-pci.c
@@ -26,6 +26,8 @@
 
 #define DRV_NAME    "xen-platform-pci"
 
+#define PCI_DEVICE_ID_XEN_PLATFORM_XS61	0x0002
+
 static unsigned long platform_mmio;
 static unsigned long platform_mmio_alloc;
 static unsigned long platform_mmiolen;
@@ -174,6 +176,8 @@ static int platform_pci_probe(struct pci_dev *pdev,
 static const struct pci_device_id platform_pci_tbl[] = {
 	{PCI_VENDOR_ID_XEN, PCI_DEVICE_ID_XEN_PLATFORM,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
+	{PCI_VENDOR_ID_XEN, PCI_DEVICE_ID_XEN_PLATFORM_XS61,
+		PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
 	{0,}
 };
 
-- 
2.48.1
Re: [PATCH v2] xen: Add support for XenServer 6.1 platform device
Posted by Andrew Cooper 8 months, 1 week ago
On 27/02/2025 2:50 pm, Frediano Ziglio wrote:
> On XenServer on Windows machine a platform device with ID 2 instead of
> 1 is used.
>
> This device is mainly identical to device 1 but due to some Windows
> update behaviour it was decided to use a device with a different ID.
>
> This causes compatibility issues with Linux which expects, if Xen
> is detected, to find a Xen platform device (5853:0001) otherwise code
> will crash due to some missing initialization (specifically grant
> tables). Specifically from dmesg
>
>     RIP: 0010:gnttab_expand+0x29/0x210
>     Code: 90 0f 1f 44 00 00 55 31 d2 48 89 e5 41 57 41 56 41 55 41 89 fd
>           41 54 53 48 83 ec 10 48 8b 05 7e 9a 49 02 44 8b 35 a7 9a 49 02
>           <8b> 48 04 8d 44 39 ff f7 f1 45 8d 24 06 89 c3 e8 43 fe ff ff
>           44 39
>     RSP: 0000:ffffba34c01fbc88 EFLAGS: 00010086
>     ...
>
> The device 2 is presented by Xapi adding device specification to
> Qemu command line.
>
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>

I'm split about this.  It's just papering over the bugs that exist
elsewhere in the Xen initialisation code.

But, if we're going to take this approach, then 0xc000 needs adding too,
which is the other device ID you might find when trying to boot Linux in
a VM configured using a Windows template.

Bjorn: to answer a prior question of yours, all 3 of these devices are
identical, and exist in production for political reasons (bindings in
Windows Updates) rather than technical reasons.

~Andrew
Re: [PATCH v2] xen: Add support for XenServer 6.1 platform device
Posted by Roger Pau Monné 8 months ago
On Thu, Feb 27, 2025 at 03:41:54PM +0000, Andrew Cooper wrote:
> On 27/02/2025 2:50 pm, Frediano Ziglio wrote:
> > On XenServer on Windows machine a platform device with ID 2 instead of
> > 1 is used.
> >
> > This device is mainly identical to device 1 but due to some Windows
> > update behaviour it was decided to use a device with a different ID.
> >
> > This causes compatibility issues with Linux which expects, if Xen
> > is detected, to find a Xen platform device (5853:0001) otherwise code
> > will crash due to some missing initialization (specifically grant
> > tables). Specifically from dmesg
> >
> >     RIP: 0010:gnttab_expand+0x29/0x210
> >     Code: 90 0f 1f 44 00 00 55 31 d2 48 89 e5 41 57 41 56 41 55 41 89 fd
> >           41 54 53 48 83 ec 10 48 8b 05 7e 9a 49 02 44 8b 35 a7 9a 49 02
> >           <8b> 48 04 8d 44 39 ff f7 f1 45 8d 24 06 89 c3 e8 43 fe ff ff
> >           44 39
> >     RSP: 0000:ffffba34c01fbc88 EFLAGS: 00010086

I think the back trace might be more helpful here rather than the raw
code?

Not sure if it helps, but there's a document in upstream Xen
repository that lists the IDs:

https://xenbits.xen.org/docs/unstable/man/xen-pci-device-reservations.7.html

It would be good to record the information you have gathered about the
different devices somewhere.  Maybe xen-pci-device-reservations would
be a good place to list the intended usage of those device IDs, as
right now it just lists the allocated ranges, but no information about
what's the purpose of each device.

> >     ...
> >
> > The device 2 is presented by Xapi adding device specification to
> > Qemu command line.
> >
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> 
> I'm split about this.  It's just papering over the bugs that exist
> elsewhere in the Xen initialisation code.
> 
> But, if we're going to take this approach, then 0xc000 needs adding too,
> which is the other device ID you might find when trying to boot Linux in
> a VM configured using a Windows template.

Won't adding 0xc000 cause issues?  As then the xenpci driver will bind
to two devices on the same system (either 0001 or 0002, and
additionally c000).  As it's my understanding that the device with ID
c000 will be present in conjunction with either a device with ID 0001
or 0002.

Thanks, Roger.

Re: [PATCH v2] xen: Add support for XenServer 6.1 platform device
Posted by Frediano Ziglio 7 months, 4 weeks ago
Hi,
   after all discussions on this thread and Xen-devel, can we agree
that this patch is good as it is and can be merged upstream?
Just to make it clear, it was already acked.

Regards,
   Frediano

On Fri, Feb 28, 2025 at 12:37 PM Roger Pau Monné <roger.pau@citrix.com> wrote:
>
> On Thu, Feb 27, 2025 at 03:41:54PM +0000, Andrew Cooper wrote:
> > On 27/02/2025 2:50 pm, Frediano Ziglio wrote:
> > > On XenServer on Windows machine a platform device with ID 2 instead of
> > > 1 is used.
> > >
> > > This device is mainly identical to device 1 but due to some Windows
> > > update behaviour it was decided to use a device with a different ID.
> > >
> > > This causes compatibility issues with Linux which expects, if Xen
> > > is detected, to find a Xen platform device (5853:0001) otherwise code
> > > will crash due to some missing initialization (specifically grant
> > > tables). Specifically from dmesg
> > >
> > >     RIP: 0010:gnttab_expand+0x29/0x210
> > >     Code: 90 0f 1f 44 00 00 55 31 d2 48 89 e5 41 57 41 56 41 55 41 89 fd
> > >           41 54 53 48 83 ec 10 48 8b 05 7e 9a 49 02 44 8b 35 a7 9a 49 02
> > >           <8b> 48 04 8d 44 39 ff f7 f1 45 8d 24 06 89 c3 e8 43 fe ff ff
> > >           44 39
> > >     RSP: 0000:ffffba34c01fbc88 EFLAGS: 00010086
>
> I think the back trace might be more helpful here rather than the raw
> code?
>
> Not sure if it helps, but there's a document in upstream Xen
> repository that lists the IDs:
>
> https://xenbits.xen.org/docs/unstable/man/xen-pci-device-reservations.7.html
>
> It would be good to record the information you have gathered about the
> different devices somewhere.  Maybe xen-pci-device-reservations would
> be a good place to list the intended usage of those device IDs, as
> right now it just lists the allocated ranges, but no information about
> what's the purpose of each device.
>
> > >     ...
> > >
> > > The device 2 is presented by Xapi adding device specification to
> > > Qemu command line.
> > >
> > > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
> >
> > I'm split about this.  It's just papering over the bugs that exist
> > elsewhere in the Xen initialisation code.
> >
> > But, if we're going to take this approach, then 0xc000 needs adding too,
> > which is the other device ID you might find when trying to boot Linux in
> > a VM configured using a Windows template.
>
> Won't adding 0xc000 cause issues?  As then the xenpci driver will bind
> to two devices on the same system (either 0001 or 0002, and
> additionally c000).  As it's my understanding that the device with ID
> c000 will be present in conjunction with either a device with ID 0001
> or 0002.
>
> Thanks, Roger.
Re: [PATCH v2] xen: Add support for XenServer 6.1 platform device
Posted by Juergen Gross 7 months, 3 weeks ago
On 07.03.25 12:14, Frediano Ziglio wrote:
> Hi,
>     after all discussions on this thread and Xen-devel, can we agree
> that this patch is good as it is and can be merged upstream?
> Just to make it clear, it was already acked.

Will add it for 6.15.


Juergen
Re: [PATCH v2] xen: Add support for XenServer 6.1 platform device
Posted by Frediano Ziglio 8 months ago
On Thu, Feb 27, 2025 at 3:41 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 27/02/2025 2:50 pm, Frediano Ziglio wrote:
> > On XenServer on Windows machine a platform device with ID 2 instead of
> > 1 is used.
> >
> > This device is mainly identical to device 1 but due to some Windows
> > update behaviour it was decided to use a device with a different ID.
> >
> > This causes compatibility issues with Linux which expects, if Xen
> > is detected, to find a Xen platform device (5853:0001) otherwise code
> > will crash due to some missing initialization (specifically grant
> > tables). Specifically from dmesg
> >
> >     RIP: 0010:gnttab_expand+0x29/0x210
> >     Code: 90 0f 1f 44 00 00 55 31 d2 48 89 e5 41 57 41 56 41 55 41 89 fd
> >           41 54 53 48 83 ec 10 48 8b 05 7e 9a 49 02 44 8b 35 a7 9a 49 02
> >           <8b> 48 04 8d 44 39 ff f7 f1 45 8d 24 06 89 c3 e8 43 fe ff ff
> >           44 39
> >     RSP: 0000:ffffba34c01fbc88 EFLAGS: 00010086
> >     ...
> >
> > The device 2 is presented by Xapi adding device specification to
> > Qemu command line.
> >
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
>
> I'm split about this.  It's just papering over the bugs that exist
> elsewhere in the Xen initialisation code.
>
> But, if we're going to take this approach, then 0xc000 needs adding too,
> which is the other device ID you might find when trying to boot Linux in
> a VM configured using a Windows template.
>
> Bjorn: to answer a prior question of yours, all 3 of these devices are
> identical, and exist in production for political reasons (bindings in
> Windows Updates) rather than technical reasons.
>

Hi,
   we got some internal conversation here at XenServer trying to
understand a bit the history and situation of these devices. I'll try
to sum up.

Devices 0001 and 0002 are both "Xen Platform" devices
(https://gitlab.com/qemu-project/qemu/-/blob/master/hw/i386/xen/xen_platform.c?ref_type=heads).
Devices 0001 and 0002 are mutually exclusive. Usually on XenServer
templates for Windows present device 0002. In other words the xen
platform device is either presented as 0001 or 0002.
Device C000 is a "Xen PV Device"
(https://gitlab.com/qemu-project/qemu/-/blob/master/hw/i386/xen/xen_pvdevice.c?ref_type=heads)
which is mainly empty being a placeholder for Windows updates.

Back to this patch, as C000 is just for Windows update purpose, I
don't see the reason why Linux may care about it (I may be wrong). On
the other hand, if device 0001 is missing Linux will crash so it
should consider also device 0002 as an alternative.

I'll try to post an update for device reservations
(https://xenbits.xen.org/docs/unstable/man/xen-pci-device-reservations.7.html)
to xen-devel.

Frediano
Re: [PATCH v2] xen: Add support for XenServer 6.1 platform device
Posted by Jürgen Groß 8 months, 1 week ago
On 27.02.25 15:50, Frediano Ziglio wrote:
> On XenServer on Windows machine a platform device with ID 2 instead of
> 1 is used.
> 
> This device is mainly identical to device 1 but due to some Windows
> update behaviour it was decided to use a device with a different ID.
> 
> This causes compatibility issues with Linux which expects, if Xen
> is detected, to find a Xen platform device (5853:0001) otherwise code
> will crash due to some missing initialization (specifically grant
> tables). Specifically from dmesg
> 
>      RIP: 0010:gnttab_expand+0x29/0x210
>      Code: 90 0f 1f 44 00 00 55 31 d2 48 89 e5 41 57 41 56 41 55 41 89 fd
>            41 54 53 48 83 ec 10 48 8b 05 7e 9a 49 02 44 8b 35 a7 9a 49 02
>            <8b> 48 04 8d 44 39 ff f7 f1 45 8d 24 06 89 c3 e8 43 fe ff ff
>            44 39
>      RSP: 0000:ffffba34c01fbc88 EFLAGS: 00010086
>      ...
> 
> The device 2 is presented by Xapi adding device specification to
> Qemu command line.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>

Acked-by: Juergen Gross <jgross@suse.com>


Juergen