[RESEND PATCH v2 0/3] Update to SMBIOS 2.6

[RESEND PATCH v2 0/3] Update to SMBIOS 2.6

[Teddy Astie]

First patch clarifies the Xen guest handle definition as being a big
endian UUID. The second does update to SMBIOS 2.6, writing a proper
UUID in the table.

Teddy Astie (3):
  xen: Define xen_domain_handle_t encoding and formatting
  hvmloader: Update to SMBIOS 2.6
  CHANGELOG.md: Add SMBIOS 2.6 update statement

 CHANGELOG.md                            |  2 +
 tools/firmware/hvmloader/smbios.c       | 50 ++++++++++++++++++++-----
 tools/firmware/hvmloader/smbios_types.h |  9 +++++
 xen/include/public/xen.h                |  7 ++++
 4 files changed, 59 insertions(+), 9 deletions(-)

-- 
2.50.1



Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting

[Teddy Astie]

xen_domain_handle_t is defined as a opaque 16-bytes blob, but is
commonly used by toolstack and guest as a big-endian encoded and
formatted UUID (alike RFC 9562).

Clarify the definition of the type to ensure the guest and toolstack
interprets this value correctly in a way consistent with existing users
(at least with XAPI, xl, libvirt, hvmloader and Linux).

Fixes: 30ce2a9295a5 ("Store an opaque handle (tools uuid) in the domain structure")
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
v2:
 - introduced
---
 CHANGELOG.md             | 1 +
 xen/include/public/xen.h | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cd34ea87b8..8c4435c181 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
    - For x86, GCC 5.1 and Binutils 2.25, or Clang/LLVM 11
    - For ARM32 and ARM64, GCC 5.1 and Binutils 2.25
  - Linux based device model stubdomains are now fully supported.
+ - Clarify guest UUIDs as being big-endian encoded.
 
  - On x86:
    - Restrict the cache flushing done as a result of guest physical memory map
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index 82b9c05a76..a219ef870f 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -973,6 +973,13 @@ typedef struct dom0_vga_console_info {
 #define xen_vga_console_info dom0_vga_console_info
 #define xen_vga_console_info_t dom0_vga_console_info_t
 
+/*
+ * The domain handle is chosen by the toolstack, and intended to hold a UUID
+ * conforming to RFC 9562 (i.e. big endian).
+ *
+ * Certain cases (e.g. SMBios) transform it to a Microsoft GUID (little
+ * endian) for presentation to the guest.
+ */
 typedef uint8_t xen_domain_handle_t[16];
 
 __DEFINE_XEN_GUEST_HANDLE(uint8,  uint8_t);
-- 
2.50.1



Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting

[Oleksii Kurochko]

[-- Attachment #1: Type: text/plain, Size: 2130 bytes --]


On 8/29/25 11:58 AM, Teddy Astie wrote:
> xen_domain_handle_t is defined as a opaque 16-bytes blob, but is
> commonly used by toolstack and guest as a big-endian encoded and
> formatted UUID (alike RFC 9562).
>
> Clarify the definition of the type to ensure the guest and toolstack
> interprets this value correctly in a way consistent with existing users
> (at least with XAPI, xl, libvirt, hvmloader and Linux).
>
> Fixes: 30ce2a9295a5 ("Store an opaque handle (tools uuid) in the domain structure")
> Suggested-by: Andrew Cooper<andrew.cooper3@citrix.com>
> Signed-off-by: Teddy Astie<teddy.astie@vates.tech>

LGTM: Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

> ---
> v2:
>   - introduced
> ---
>   CHANGELOG.md             | 1 +
>   xen/include/public/xen.h | 7 +++++++
>   2 files changed, 8 insertions(+)
>
> diff --git a/CHANGELOG.md b/CHANGELOG.md
> index cd34ea87b8..8c4435c181 100644
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
>      - For x86, GCC 5.1 and Binutils 2.25, or Clang/LLVM 11
>      - For ARM32 and ARM64, GCC 5.1 and Binutils 2.25
>    - Linux based device model stubdomains are now fully supported.
> + - Clarify guest UUIDs as being big-endian encoded.
>   
>    - On x86:
>      - Restrict the cache flushing done as a result of guest physical memory map
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index 82b9c05a76..a219ef870f 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -973,6 +973,13 @@ typedef struct dom0_vga_console_info {
>   #define xen_vga_console_info dom0_vga_console_info
>   #define xen_vga_console_info_t dom0_vga_console_info_t
>   
> +/*
> + * The domain handle is chosen by the toolstack, and intended to hold a UUID
> + * conforming to RFC 9562 (i.e. big endian).
> + *
> + * Certain cases (e.g. SMBios) transform it to a Microsoft GUID (little
> + * endian) for presentation to the guest.
> + */
>   typedef uint8_t xen_domain_handle_t[16];
>   
>   __DEFINE_XEN_GUEST_HANDLE(uint8,  uint8_t);

[-- Attachment #2: Type: text/html, Size: 2944 bytes --]

Re: [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting

[Jan Beulich]

On 29.08.2025 11:58, Teddy Astie wrote:
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
>     - For x86, GCC 5.1 and Binutils 2.25, or Clang/LLVM 11
>     - For ARM32 and ARM64, GCC 5.1 and Binutils 2.25
>   - Linux based device model stubdomains are now fully supported.
> + - Clarify guest UUIDs as being big-endian encoded.

Is something like this really in need of having a ChangeLog entry?

Jan

> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -973,6 +973,13 @@ typedef struct dom0_vga_console_info {
>  #define xen_vga_console_info dom0_vga_console_info
>  #define xen_vga_console_info_t dom0_vga_console_info_t
>  
> +/*
> + * The domain handle is chosen by the toolstack, and intended to hold a UUID
> + * conforming to RFC 9562 (i.e. big endian).
> + *
> + * Certain cases (e.g. SMBios) transform it to a Microsoft GUID (little
> + * endian) for presentation to the guest.
> + */
>  typedef uint8_t xen_domain_handle_t[16];
>  
>  __DEFINE_XEN_GUEST_HANDLE(uint8,  uint8_t);

Re: [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting

[Oleksii Kurochko]

[-- Attachment #1: Type: text/plain, Size: 1267 bytes --]


On 9/1/25 5:55 PM, Jan Beulich wrote:
> On 29.08.2025 11:58, Teddy Astie wrote:
>> --- a/CHANGELOG.md
>> +++ b/CHANGELOG.md
>> @@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
>>      - For x86, GCC 5.1 and Binutils 2.25, or Clang/LLVM 11
>>      - For ARM32 and ARM64, GCC 5.1 and Binutils 2.25
>>    - Linux based device model stubdomains are now fully supported.
>> + - Clarify guest UUIDs as being big-endian encoded.
> Is something like this really in need of having a ChangeLog entry?

Perhaps, you are right and there is no a lot of sense to have such item in ChangeLog.

~ Oleksii

>> --- a/xen/include/public/xen.h
>> +++ b/xen/include/public/xen.h
>> @@ -973,6 +973,13 @@ typedef struct dom0_vga_console_info {
>>   #define xen_vga_console_info dom0_vga_console_info
>>   #define xen_vga_console_info_t dom0_vga_console_info_t
>>   
>> +/*
>> + * The domain handle is chosen by the toolstack, and intended to hold a UUID
>> + * conforming to RFC 9562 (i.e. big endian).
>> + *
>> + * Certain cases (e.g. SMBios) transform it to a Microsoft GUID (little
>> + * endian) for presentation to the guest.
>> + */
>>   typedef uint8_t xen_domain_handle_t[16];
>>   
>>   __DEFINE_XEN_GUEST_HANDLE(uint8,  uint8_t);

[-- Attachment #2: Type: text/html, Size: 2105 bytes --]

Re: [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting

[Anthony PERARD]

On Fri, Aug 29, 2025 at 09:58:15AM +0000, Teddy Astie wrote:
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index 82b9c05a76..a219ef870f 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -973,6 +973,13 @@ typedef struct dom0_vga_console_info {
>  #define xen_vga_console_info dom0_vga_console_info
>  #define xen_vga_console_info_t dom0_vga_console_info_t
>  
> +/*
> + * The domain handle is chosen by the toolstack, and intended to hold a UUID
> + * conforming to RFC 9562 (i.e. big endian).
> + *
> + * Certain cases (e.g. SMBios) transform it to a Microsoft GUID (little
> + * endian) for presentation to the guest.
> + */

So, there's already a definition about this, but it's a comment on
XENVER_guest_handle (in public/version.h):

> /* arg == xen_domain_handle_t.
>  *
>  * The toolstack fills it out for guest consumption. It is intended to hold
>  * the UUID of the guest.
>  */

We can add that the binary format is most significant byte or network
byte order, but that's already the format stated in RFC 4122 and its
successor RFC 9562.

Also, I don't think it is useful here to point out how that handle/UUID
might be used/transformed by guest.

>  typedef uint8_t xen_domain_handle_t[16];

Cheers,

-- 

--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

[RESEND PATCH v2 3/3] CHANGELOG.md: Add SMBIOS 2.6 update statement

[Teddy Astie]

Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
v2:
 - introduced
---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8c4435c181..80a8273d7e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -34,6 +34,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
    - Support in hvmloader for new SMBIOS tables: 7 (Cache Info), 8 (Port
      Connector), 9 (System Slots), 26 (Voltage Probe), 27 (Cooling Device),
      and 28 (Temperature Probe).
+   - Updated SMBIOS version to 2.6.
 
  - On Arm:
     - Ability to enable stack protector
-- 
2.50.1



Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [RESEND PATCH v2 3/3] CHANGELOG.md: Add SMBIOS 2.6 update statement

[Oleksii Kurochko]

[-- Attachment #1: Type: text/plain, Size: 971 bytes --]


On 8/29/25 11:58 AM, Teddy Astie wrote:
> Signed-off-by: Teddy Astie<teddy.astie@vates.tech>
> ---
> v2:
>   - introduced
> ---
>   CHANGELOG.md | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/CHANGELOG.md b/CHANGELOG.md
> index 8c4435c181..80a8273d7e 100644
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -34,6 +34,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
>      - Support in hvmloader for new SMBIOS tables: 7 (Cache Info), 8 (Port
>        Connector), 9 (System Slots), 26 (Voltage Probe), 27 (Cooling Device),
>        and 28 (Temperature Probe).
> +   - Updated SMBIOS version to 2.6.

Generally looks good to me, but I think it would be nice to add a short explanation why it was done.
Something like:
  - Update SMBIOS to 2.6 to fix UUID endianness mismatch with OVMF and ensure consistent
    Linux guest UUIDs.
Does it make sense?

~ Oleksii


>   
>    - On Arm:
>       - Ability to enable stack protector

[-- Attachment #2: Type: text/html, Size: 1698 bytes --]

Re: [RESEND PATCH v2 3/3] CHANGELOG.md: Add SMBIOS 2.6 update statement

[Jan Beulich]

On 29.08.2025 11:58, Teddy Astie wrote:
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -34,6 +34,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
>     - Support in hvmloader for new SMBIOS tables: 7 (Cache Info), 8 (Port
>       Connector), 9 (System Slots), 26 (Voltage Probe), 27 (Cooling Device),
>       and 28 (Temperature Probe).
> +   - Updated SMBIOS version to 2.6.

Like the earlier SMBIOS related entry, this should be more specific. At the very
least it needs to similarly mention this is about hvmloader.

Jan

[RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6

[Teddy Astie]

Currently, hvmloader uses SMBIOS 2.4, however, when using OVMF, the
SMBIOS is patched to 2.8, which has clarified the UUID format (as GUID).

In Linux, if the SMBIOS version is >= 2.6, the GUID format is used, else
(undefined as per SMBIOS spec), big endian is used (used by Xen). Therefore,
you have a endian mismatch causing the UUIDs to mismatch in the guest.

$ cat /sys/hypervisor/uuid
e865e63f-3d30-4f0b-83e0-8fdfc1e30eb7
$ cat /sys/devices/virtual/dmi/id/product_uuid
3fe665e8-303d-0b4f-83e0-8fdfc1e30eb7
$ cat /sys/devices/virtual/dmi/id/product_serial
e865e63f-3d30-4f0b-83e0-8fdfc1e30eb7

This patch updates the SMBIOS version from 2.4 to 2.6 and does the appropriate
modifications of the table. This effectively fix this endianness mismatch with
OVMF; while the UUID displayed by Linux is still the same for SeaBIOS.

Fixes: c683914ef913 ("Add code to generate SMBIOS tables to hvmloader.")
(SMBIOS versions before 2.6 has a ill-defined UUID definition)
Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
---
v2:
 - rebase onto staging
 - introduce missing SMBIOS 2.5-2.6 fields
 - check for new SMBIOS 2.6 table lengths
 - update UUID conversion comment
 - add Fixes: note
 ---
 tools/firmware/hvmloader/smbios.c       | 50 ++++++++++++++++++++-----
 tools/firmware/hvmloader/smbios_types.h |  9 +++++
 2 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/tools/firmware/hvmloader/smbios.c b/tools/firmware/hvmloader/smbios.c
index 76c7090d16..e445475d78 100644
--- a/tools/firmware/hvmloader/smbios.c
+++ b/tools/firmware/hvmloader/smbios.c
@@ -396,7 +396,7 @@ smbios_entry_point_init(void *start,
     memcpy(ep->anchor_string, "_SM_", 4);
     ep->length = 0x1f;
     ep->smbios_major_version = 2;
-    ep->smbios_minor_version = 4;
+    ep->smbios_minor_version = 6;
     ep->max_structure_size = max_structure_size;
     ep->entry_point_revision = 0;
     memcpy(ep->intermediate_anchor_string, "_DMI_", 5);
@@ -505,7 +505,22 @@ smbios_type_1_init(void *start, const char *xen_version,
     p->version_str = 3;
     p->serial_number_str = 4;
 
-    memcpy(p->uuid, uuid, 16);
+    /*
+     * Xen toolstack uses big endian UUIDs, however GUIDs (which requirement
+     * is clarified by SMBIOS >= 2.6) has the first 3 components appearing as
+     * being little endian and the rest as still being big endian.
+     */
+    /* First component */
+    for ( unsigned int i = 0; i < 4; i++ )
+        p->uuid[i] = uuid[4 - i - 1];
+    /* Second component */
+    p->uuid[4] = uuid[5];
+    p->uuid[5] = uuid[4];
+    /* Third component */
+    p->uuid[6] = uuid[7];
+    p->uuid[7] = uuid[6];
+    /* Rest */
+    memcpy(p->uuid + 8, uuid + 8, 8);
 
     p->wake_up_type = 0x06; /* power switch */
     p->sku_str = 0;
@@ -705,8 +720,8 @@ smbios_type_4_init(
     struct smbios_type_4 *p = start;
     uint32_t eax, ebx, ecx, edx;
 
-    /* Specification says Type 4 table has length of 23h for v2.3+. */
-    BUILD_BUG_ON(sizeof(*p) != 35);
+    /* Specification says Type 4 table has length of 2Ah for v2.6. */
+    BUILD_BUG_ON(sizeof(*p) != 42);
 
     memset(p, 0, sizeof(*p));
 
@@ -716,7 +731,7 @@ smbios_type_4_init(
 
     p->socket_designation_str = 1;
     p->processor_type = 0x03; /* CPU */
-    p->processor_family = 0x01; /* other */
+    p->processor_family = p->processor_family_2 = 0x01; /* other */
     p->manufacturer_str = 2;
 
     cpuid(1, &eax, &ebx, &ecx, &edx);
@@ -736,6 +751,22 @@ smbios_type_4_init(
     p->l2_cache_handle = 0xffff; /* No cache information structure provided. */
     p->l3_cache_handle = 0xffff; /* No cache information structure provided. */
 
+    /*
+     * We have a smbios type 4 table per vCPU (which is per socket),
+     * which means here that we have 1 socket per vCPU.
+     */
+    p->core_count = p->core_enabled = p->thread_count = 1;
+
+    /*
+     * We set 64-bits, execute protection and enhanced virtualization.
+     * We don't set Multi-Core (bit 3) because this individual processor
+     * (as being a single vCPU) is only having one core.
+     *
+     * SMBIOS specification says that these bits don't state anything
+     * regarding the actual availability of such features.
+     */
+    p->processor_characteristics = 0x64;
+
     start += sizeof(*p);
 
     strncpy(buf, "CPU ", sizeof(buf));
@@ -780,8 +811,8 @@ smbios_type_8_init(void *start)
 static void *
 smbios_type_9_init(void *start)
 {
-    /* Specification says Type 9 table has length of 0Dh for v2.1-2.5. */
-    BUILD_BUG_ON(sizeof(struct smbios_type_9) != 13);
+    /* Specification says Type 9 table has length of 11h for v2.6+. */
+    BUILD_BUG_ON(sizeof(struct smbios_type_9) != 17);
 
     /* Only present when passed in. */
     return smbios_pt_copy(start, 9, SMBIOS_HANDLE_TYPE9,
@@ -870,8 +901,8 @@ smbios_type_17_init(void *start, uint32_t memory_size_mb, int instance)
     char buf[16];
     struct smbios_type_17 *p = start;
 
-    /* Specification says Type 17 table has length of 1Bh for v2.3-2.6. */
-    BUILD_BUG_ON(sizeof(*p) != 27);
+    /* Specification says Type 17 table has length of 1Ch for v2.6. */
+    BUILD_BUG_ON(sizeof(*p) != 28);
 
     memset(p, 0, sizeof(*p));
 
@@ -890,6 +921,7 @@ smbios_type_17_init(void *start, uint32_t memory_size_mb, int instance)
     p->bank_locator_str = 0;
     p->memory_type = 0x07; /* RAM */
     p->type_detail = 0;
+    p->attributes = 0;
 
     start += sizeof(*p);
     strcpy(start, "DIMM ");
diff --git a/tools/firmware/hvmloader/smbios_types.h b/tools/firmware/hvmloader/smbios_types.h
index c04b435d31..860617d86a 100644
--- a/tools/firmware/hvmloader/smbios_types.h
+++ b/tools/firmware/hvmloader/smbios_types.h
@@ -147,6 +147,11 @@ struct smbios_type_4 {
     uint8_t serial_number_str;
     uint8_t asset_tag_str;
     uint8_t part_number_str;
+    uint8_t core_count;
+    uint8_t core_enabled;
+    uint8_t thread_count;
+    uint16_t processor_characteristics;
+    uint16_t processor_family_2;
 } __attribute__ ((packed));
 
 /* SMBIOS type 7 - Cache Information */
@@ -185,6 +190,9 @@ struct smbios_type_9 {
     uint16_t slot_id;
     uint8_t slot_characteristics_1;
     uint8_t slot_characteristics_2;
+    uint16_t sgn_base;
+    uint8_t bus_number_base;
+    uint8_t devfn_base;
 } __attribute__ ((packed));
 
 /* SMBIOS type 11 - OEM Strings */
@@ -227,6 +235,7 @@ struct smbios_type_17 {
     uint8_t serial_number_str;
     uint8_t asset_tag_str;
     uint8_t part_number_str;
+    uint8_t attributes;
 } __attribute__ ((packed));
 
 /* SMBIOS type 19 - Memory Array Mapped Address */
-- 
2.50.1



Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6

[Jan Beulich]

On 29.08.2025 11:58, Teddy Astie wrote:
> @@ -505,7 +505,22 @@ smbios_type_1_init(void *start, const char *xen_version,
>      p->version_str = 3;
>      p->serial_number_str = 4;
>  
> -    memcpy(p->uuid, uuid, 16);
> +    /*
> +     * Xen toolstack uses big endian UUIDs, however GUIDs (which requirement
> +     * is clarified by SMBIOS >= 2.6) has the first 3 components appearing as
> +     * being little endian and the rest as still being big endian.
> +     */

The SMBIOS spec I'm looking at (2.7.1) doesn't mention the term GUID at all
(except of course when discussing the EFI System Table entry). It's all UUID
there. Here and in the description I think this needs expressing better, to
not raise extra questions.

As to endian-ness: Since everything from byte 8 onwards are merely bytes, I
don't think it makes much sense to talk of endian-ness for that latter half.

> @@ -716,7 +731,7 @@ smbios_type_4_init(
>  
>      p->socket_designation_str = 1;
>      p->processor_type = 0x03; /* CPU */
> -    p->processor_family = 0x01; /* other */
> +    p->processor_family = p->processor_family_2 = 0x01; /* other */

In the hypervisor we need to avoid such double assignments for Misra's
sake. I think we're better off avoiding them in hvmloader as well.

> @@ -736,6 +751,22 @@ smbios_type_4_init(
>      p->l2_cache_handle = 0xffff; /* No cache information structure provided. */
>      p->l3_cache_handle = 0xffff; /* No cache information structure provided. */
>  
> +    /*
> +     * We have a smbios type 4 table per vCPU (which is per socket),
> +     * which means here that we have 1 socket per vCPU.
> +     */
> +    p->core_count = p->core_enabled = p->thread_count = 1;

Might we be better off keeping them all at 0 (unknown)?

> +    /*
> +     * We set 64-bits, execute protection and enhanced virtualization.
> +     * We don't set Multi-Core (bit 3) because this individual processor
> +     * (as being a single vCPU) is only having one core.
> +     *
> +     * SMBIOS specification says that these bits don't state anything
> +     * regarding the actual availability of such features.
> +     */
> +    p->processor_characteristics = 0x64;

Unless nested virt is enabled for the guest, I think we'd better avoid
setting the Enhanced Virtualization bit.

> @@ -870,8 +901,8 @@ smbios_type_17_init(void *start, uint32_t memory_size_mb, int instance)
>      char buf[16];
>      struct smbios_type_17 *p = start;
>  
> -    /* Specification says Type 17 table has length of 1Bh for v2.3-2.6. */
> -    BUILD_BUG_ON(sizeof(*p) != 27);
> +    /* Specification says Type 17 table has length of 1Ch for v2.6. */
> +    BUILD_BUG_ON(sizeof(*p) != 28);
>  
>      memset(p, 0, sizeof(*p));

With this, ...

> @@ -890,6 +921,7 @@ smbios_type_17_init(void *start, uint32_t memory_size_mb, int instance)
>      p->bank_locator_str = 0;
>      p->memory_type = 0x07; /* RAM */
>      p->type_detail = 0;
> +    p->attributes = 0;

... I don't think we really need this. In fact I was considering to make
a patch to strip all the unnecessary assignments of zero.

> --- a/tools/firmware/hvmloader/smbios_types.h
> +++ b/tools/firmware/hvmloader/smbios_types.h
> @@ -147,6 +147,11 @@ struct smbios_type_4 {
>      uint8_t serial_number_str;
>      uint8_t asset_tag_str;
>      uint8_t part_number_str;
> +    uint8_t core_count;
> +    uint8_t core_enabled;
> +    uint8_t thread_count;
> +    uint16_t processor_characteristics;
> +    uint16_t processor_family_2;
>  } __attribute__ ((packed));
>  
>  /* SMBIOS type 7 - Cache Information */
> @@ -185,6 +190,9 @@ struct smbios_type_9 {
>      uint16_t slot_id;
>      uint8_t slot_characteristics_1;
>      uint8_t slot_characteristics_2;
> +    uint16_t sgn_base;
> +    uint8_t bus_number_base;
> +    uint8_t devfn_base;

Where do the _base suffixes come from? Nothing like that is said in the
spec I'm looking at. Also "sgn" is imo too much of an abbreviation.

Jan

Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6

[Teddy Astie]

Le 02/09/2025 à 14:38, Jan Beulich a écrit :
> On 29.08.2025 11:58, Teddy Astie wrote:
>> @@ -505,7 +505,22 @@ smbios_type_1_init(void *start, const char *xen_version,
>>       p->version_str = 3;
>>       p->serial_number_str = 4;
>>
>> -    memcpy(p->uuid, uuid, 16);
>> +    /*
>> +     * Xen toolstack uses big endian UUIDs, however GUIDs (which requirement
>> +     * is clarified by SMBIOS >= 2.6) has the first 3 components appearing as
>> +     * being little endian and the rest as still being big endian.
>> +     */
>
> The SMBIOS spec I'm looking at (2.7.1) doesn't mention the term GUID at all
> (except of course when discussing the EFI System Table entry). It's all UUID
> there. Here and in the description I think this needs expressing better, to
> not raise extra questions.
>

Yes (this is also true for SMBIOS 3.9.0 spec). Not sure how to express
that aside saying that UUID encoding differs between SMBIOS
specification and Xen representation.

> As to endian-ness: Since everything from byte 8 onwards are merely bytes, I
> don't think it makes much sense to talk of endian-ness for that latter half.
>

It's more to insist that the byte ordering differs with the first parts.

>> @@ -716,7 +731,7 @@ smbios_type_4_init(
>>
>>       p->socket_designation_str = 1;
>>       p->processor_type = 0x03; /* CPU */
>> -    p->processor_family = 0x01; /* other */
>> +    p->processor_family = p->processor_family_2 = 0x01; /* other */
>
> In the hypervisor we need to avoid such double assignments for Misra's
> sake. I think we're better off avoiding them in hvmloader as well.
>

yes

>> @@ -736,6 +751,22 @@ smbios_type_4_init(
>>       p->l2_cache_handle = 0xffff; /* No cache information structure provided. */
>>       p->l3_cache_handle = 0xffff; /* No cache information structure provided. */
>>
>> +    /*
>> +     * We have a smbios type 4 table per vCPU (which is per socket),
>> +     * which means here that we have 1 socket per vCPU.
>> +     */
>> +    p->core_count = p->core_enabled = p->thread_count = 1;
>
> Might we be better off keeping them all at 0 (unknown)?
>

Making it Unknown would feel a bit weird, unless we only keep only one
(instead of the number of vCPUs) of these table with core_count,
core_enabled, thread_count as 0 (unknown) ?

>> +    /*
>> +     * We set 64-bits, execute protection and enhanced virtualization.
>> +     * We don't set Multi-Core (bit 3) because this individual processor
>> +     * (as being a single vCPU) is only having one core.
>> +     *
>> +     * SMBIOS specification says that these bits don't state anything
>> +     * regarding the actual availability of such features.
>> +     */
>> +    p->processor_characteristics = 0x64;
>
> Unless nested virt is enabled for the guest, I think we'd better avoid
> setting the Enhanced Virtualization bit.
>

I am not sure how to interpret the SMBIOS spec on this

 > Enhanced Virtualization indicates that the processor can execute
 > enhanced virtualization instructions. This bit does not indicate the
 > present state of this feature

I see it as: Virtualization is available or can be enabled (with nested
virtualization).
Or as : We have virtualization related instructions.

>> @@ -870,8 +901,8 @@ smbios_type_17_init(void *start, uint32_t memory_size_mb, int instance)
>>       char buf[16];
>>       struct smbios_type_17 *p = start;
>>
>> -    /* Specification says Type 17 table has length of 1Bh for v2.3-2.6. */
>> -    BUILD_BUG_ON(sizeof(*p) != 27);
>> +    /* Specification says Type 17 table has length of 1Ch for v2.6. */
>> +    BUILD_BUG_ON(sizeof(*p) != 28);
>>
>>       memset(p, 0, sizeof(*p));
>
> With this, ...
>
>> @@ -890,6 +921,7 @@ smbios_type_17_init(void *start, uint32_t memory_size_mb, int instance)
>>       p->bank_locator_str = 0;
>>       p->memory_type = 0x07; /* RAM */
>>       p->type_detail = 0;
>> +    p->attributes = 0;
>
> ... I don't think we really need this. In fact I was considering to make
> a patch to strip all the unnecessary assignments of zero.
>
>> --- a/tools/firmware/hvmloader/smbios_types.h
>> +++ b/tools/firmware/hvmloader/smbios_types.h
>> @@ -147,6 +147,11 @@ struct smbios_type_4 {
>>       uint8_t serial_number_str;
>>       uint8_t asset_tag_str;
>>       uint8_t part_number_str;
>> +    uint8_t core_count;
>> +    uint8_t core_enabled;
>> +    uint8_t thread_count;
>> +    uint16_t processor_characteristics;
>> +    uint16_t processor_family_2;
>>   } __attribute__ ((packed));
>>
>>   /* SMBIOS type 7 - Cache Information */
>> @@ -185,6 +190,9 @@ struct smbios_type_9 {
>>       uint16_t slot_id;
>>       uint8_t slot_characteristics_1;
>>       uint8_t slot_characteristics_2;
>> +    uint16_t sgn_base;
>> +    uint8_t bus_number_base;
>> +    uint8_t devfn_base;
>
> Where do the _base suffixes come from? Nothing like that is said in the
> spec I'm looking at. Also "sgn" is imo too much of an abbreviation.
>

My version of the specification (3.9.0) says

0Dh 2.6+ Segment Group Number (Base)
0Fh 2.6+ Bus Number (Base)
10h 2.6+ Device/Function Number (Base)

Regarding sgn, maybe we can use `segment` instead ?

> Jan
>

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6

[Jan Beulich]

On 02.09.2025 15:24, Teddy Astie wrote:
> Le 02/09/2025 à 14:38, Jan Beulich a écrit :
>> On 29.08.2025 11:58, Teddy Astie wrote:
>>> @@ -505,7 +505,22 @@ smbios_type_1_init(void *start, const char *xen_version,
>>>       p->version_str = 3;
>>>       p->serial_number_str = 4;
>>>   
>>> -    memcpy(p->uuid, uuid, 16);
>>> +    /*
>>> +     * Xen toolstack uses big endian UUIDs, however GUIDs (which requirement
>>> +     * is clarified by SMBIOS >= 2.6) has the first 3 components appearing as
>>> +     * being little endian and the rest as still being big endian.
>>> +     */
>>
>> The SMBIOS spec I'm looking at (2.7.1) doesn't mention the term GUID at all
>> (except of course when discussing the EFI System Table entry). It's all UUID
>> there. Here and in the description I think this needs expressing better, to
>> not raise extra questions.
> 
> Yes (this is also true for SMBIOS 3.9.0 spec). Not sure how to express 
> that aside saying that UUID encoding differs between SMBIOS 
> specification and Xen representation.

Maybe

    /*
     * Xen toolstack uses big endian UUIDs, whereas the UUIDs used by SMBIOS,
     * also known as GUIDs, have the first 3 components appearing in little
     * endian (with this requirement clarified by SMBIOS >= 2.6).
     */

?

>>> @@ -736,6 +751,22 @@ smbios_type_4_init(
>>>       p->l2_cache_handle = 0xffff; /* No cache information structure provided. */
>>>       p->l3_cache_handle = 0xffff; /* No cache information structure provided. */
>>>   
>>> +    /*
>>> +     * We have a smbios type 4 table per vCPU (which is per socket),
>>> +     * which means here that we have 1 socket per vCPU.
>>> +     */
>>> +    p->core_count = p->core_enabled = p->thread_count = 1;
>>
>> Might we be better off keeping them all at 0 (unknown)?
> 
> Making it Unknown would feel a bit weird, unless we only keep only one 
> (instead of the number of vCPUs) of these table with core_count, 
> core_enabled, thread_count as 0 (unknown) ?

I don't see how unknown or not is related to how many structure instances
we surface. Your suggestion of using 1 in all three fields isn't quite
what we'd like to present to guests. Once we sort virtual topology in Xen,
we may want to expose sane values here. Yet if we expose 1-s now, that
adjustment would need to happen in lock-step with the hypervisor changes.
Whereas when we expose "unknown" that can be done at a convenient later
time.

>>> +    /*
>>> +     * We set 64-bits, execute protection and enhanced virtualization.
>>> +     * We don't set Multi-Core (bit 3) because this individual processor
>>> +     * (as being a single vCPU) is only having one core.
>>> +     *
>>> +     * SMBIOS specification says that these bits don't state anything
>>> +     * regarding the actual availability of such features.
>>> +     */
>>> +    p->processor_characteristics = 0x64;
>>
>> Unless nested virt is enabled for the guest, I think we'd better avoid
>> setting the Enhanced Virtualization bit.
> 
> I am not sure how to interpret the SMBIOS spec on this
> 
>  > Enhanced Virtualization indicates that the processor can execute
>  > enhanced virtualization instructions. This bit does not indicate the
>  > present state of this feature
> 
> I see it as: Virtualization is available or can be enabled (with nested 
> virtualization).
> Or as : We have virtualization related instructions.

You want to view what we expose to the guest from the guest's perspective
on its own little world, I think.

>>> --- a/tools/firmware/hvmloader/smbios_types.h
>>> +++ b/tools/firmware/hvmloader/smbios_types.h
>>> @@ -147,6 +147,11 @@ struct smbios_type_4 {
>>>       uint8_t serial_number_str;
>>>       uint8_t asset_tag_str;
>>>       uint8_t part_number_str;
>>> +    uint8_t core_count;
>>> +    uint8_t core_enabled;
>>> +    uint8_t thread_count;
>>> +    uint16_t processor_characteristics;
>>> +    uint16_t processor_family_2;
>>>   } __attribute__ ((packed));
>>>   
>>>   /* SMBIOS type 7 - Cache Information */
>>> @@ -185,6 +190,9 @@ struct smbios_type_9 {
>>>       uint16_t slot_id;
>>>       uint8_t slot_characteristics_1;
>>>       uint8_t slot_characteristics_2;
>>> +    uint16_t sgn_base;
>>> +    uint8_t bus_number_base;
>>> +    uint8_t devfn_base;
>>
>> Where do the _base suffixes come from? Nothing like that is said in the
>> spec I'm looking at. Also "sgn" is imo too much of an abbreviation.
>>
> 
> My version of the specification (3.9.0) says
> 
> 0Dh 2.6+ Segment Group Number (Base)
> 0Fh 2.6+ Bus Number (Base)
> 10h 2.6+ Device/Function Number (Base)

Without any clarification what "(Base)" means, afaics.

> Regarding sgn, maybe we can use `segment` instead ?

Why not segment_group or seg_group (seeing how devfn also is an abbreviation)?
And if we omit _number there and on devfn, it's hard to see why it shouldn't
be just "bus" then as well.

Jan

Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6

[Teddy Astie]

Le 02/09/2025 à 16:10, Jan Beulich a écrit :
> On 02.09.2025 15:24, Teddy Astie wrote:
>> Le 02/09/2025 à 14:38, Jan Beulich a écrit :
>>> On 29.08.2025 11:58, Teddy Astie wrote:
>>>> @@ -505,7 +505,22 @@ smbios_type_1_init(void *start, const char *xen_version,
>>>>        p->version_str = 3;
>>>>        p->serial_number_str = 4;
>>>>
>>>> -    memcpy(p->uuid, uuid, 16);
>>>> +    /*
>>>> +     * Xen toolstack uses big endian UUIDs, however GUIDs (which requirement
>>>> +     * is clarified by SMBIOS >= 2.6) has the first 3 components appearing as
>>>> +     * being little endian and the rest as still being big endian.
>>>> +     */
>>>
>>> The SMBIOS spec I'm looking at (2.7.1) doesn't mention the term GUID at all
>>> (except of course when discussing the EFI System Table entry). It's all UUID
>>> there. Here and in the description I think this needs expressing better, to
>>> not raise extra questions.
>>
>> Yes (this is also true for SMBIOS 3.9.0 spec). Not sure how to express
>> that aside saying that UUID encoding differs between SMBIOS
>> specification and Xen representation.
>
> Maybe
>
>      /*
>       * Xen toolstack uses big endian UUIDs, whereas the UUIDs used by SMBIOS,
>       * also known as GUIDs, have the first 3 components appearing in little
>       * endian (with this requirement clarified by SMBIOS >= 2.6).
>       */
>
> ?
>

Sounds good to me.

>>>> @@ -736,6 +751,22 @@ smbios_type_4_init(
>>>>        p->l2_cache_handle = 0xffff; /* No cache information structure provided. */
>>>>        p->l3_cache_handle = 0xffff; /* No cache information structure provided. */
>>>>
>>>> +    /*
>>>> +     * We have a smbios type 4 table per vCPU (which is per socket),
>>>> +     * which means here that we have 1 socket per vCPU.
>>>> +     */
>>>> +    p->core_count = p->core_enabled = p->thread_count = 1;
>>>
>>> Might we be better off keeping them all at 0 (unknown)?
>>
>> Making it Unknown would feel a bit weird, unless we only keep only one
>> (instead of the number of vCPUs) of these table with core_count,
>> core_enabled, thread_count as 0 (unknown) ?
>
> I don't see how unknown or not is related to how many structure instances
> we surface. Your suggestion of using 1 in all three fields isn't quite
> what we'd like to present to guests. Once we sort virtual topology in Xen,
> we may want to expose sane values here. Yet if we expose 1-s now, that
> adjustment would need to happen in lock-step with the hypervisor changes.
> Whereas when we expose "unknown" that can be done at a convenient later
> time.
>

It's mostly regarding this snippet that I am not sure it is a good idea
to expose "unknown" counts.

     for ( cpu_num = 1; cpu_num <= vcpus; cpu_num++ )
         do_struct(smbios_type_4_init(p, cpu_num, cpu_manufacturer));

AFAIU, we have as much smbios type 4 tables as we have vCPUs, things
would be very confusing if the CPU count of each exposed "socket" (per
vcpu) is unknown.

To me, either we should have 1 smbios type 4 table (i.e one socket) with
unknown core count, or what we currently have, but with one core per
"socket".

>>>> +    /*
>>>> +     * We set 64-bits, execute protection and enhanced virtualization.
>>>> +     * We don't set Multi-Core (bit 3) because this individual processor
>>>> +     * (as being a single vCPU) is only having one core.
>>>> +     *
>>>> +     * SMBIOS specification says that these bits don't state anything
>>>> +     * regarding the actual availability of such features.
>>>> +     */
>>>> +    p->processor_characteristics = 0x64;
>>>
>>> Unless nested virt is enabled for the guest, I think we'd better avoid
>>> setting the Enhanced Virtualization bit.
>>
>> I am not sure how to interpret the SMBIOS spec on this
>>
>>   > Enhanced Virtualization indicates that the processor can execute
>>   > enhanced virtualization instructions. This bit does not indicate the
>>   > present state of this feature
>>
>> I see it as: Virtualization is available or can be enabled (with nested
>> virtualization).
>> Or as : We have virtualization related instructions.
>
> You want to view what we expose to the guest from the guest's perspective
> on its own little world, I think.
>

Most softwares will expose it as-is as said in the SMBIOS specification;
i.e "Enhanced Virtualization". Especially since it's not bound to
hardware (here virtualized) capability.
But yes, it would make more sense to only expose it when we have
meaningful nested virtualization.

>>>> --- a/tools/firmware/hvmloader/smbios_types.h
>>>> +++ b/tools/firmware/hvmloader/smbios_types.h
>>>> @@ -147,6 +147,11 @@ struct smbios_type_4 {
>>>>        uint8_t serial_number_str;
>>>>        uint8_t asset_tag_str;
>>>>        uint8_t part_number_str;
>>>> +    uint8_t core_count;
>>>> +    uint8_t core_enabled;
>>>> +    uint8_t thread_count;
>>>> +    uint16_t processor_characteristics;
>>>> +    uint16_t processor_family_2;
>>>>    } __attribute__ ((packed));
>>>>
>>>>    /* SMBIOS type 7 - Cache Information */
>>>> @@ -185,6 +190,9 @@ struct smbios_type_9 {
>>>>        uint16_t slot_id;
>>>>        uint8_t slot_characteristics_1;
>>>>        uint8_t slot_characteristics_2;
>>>> +    uint16_t sgn_base;
>>>> +    uint8_t bus_number_base;
>>>> +    uint8_t devfn_base;
>>>
>>> Where do the _base suffixes come from? Nothing like that is said in the
>>> spec I'm looking at. Also "sgn" is imo too much of an abbreviation.
>>>
>>
>> My version of the specification (3.9.0) says
>>
>> 0Dh 2.6+ Segment Group Number (Base)
>> 0Fh 2.6+ Bus Number (Base)
>> 10h 2.6+ Device/Function Number (Base)
>
> Without any clarification what "(Base)" means, afaics.
>

Not a lot is said, apart that there are also "Peer" devices (SMBIOS
3.2+) defined as (7.10.9 Peer Devices)

 > Because some slots can be partitioned into smaller electrical widths,
 > additional peer device Segment/Bus/Device/Function are defined. These
 > peer groups are defined in Table 52. The base device is the lowest
 > ordered Segment/Bus/Device/Function and is listed first (offsets
 > 0Dh-11h). Peer devices are listed in the peer grouping section.

With Table 52 having the same layout as the segment/bus/... values we
have for the "base" ones.

>> Regarding sgn, maybe we can use `segment` instead ?
>
> Why not segment_group or seg_group (seeing how devfn also is an abbreviation)?
> And if we omit _number there and on devfn, it's hard to see why it shouldn't
> be just "bus" then as well.

So overall

  uint16_t segment_group;
  uint8_t bus;
  uint8_t devfn;

?

segment_group looks a bit off compared with the rest.
We could use `seg` like we do in Xen PCI code, as it is about PCI
segment here.

>
> Jan

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6

[Jan Beulich]

On 03.09.2025 16:02, Teddy Astie wrote:
> Le 02/09/2025 à 16:10, Jan Beulich a écrit :
>> On 02.09.2025 15:24, Teddy Astie wrote:
>>> Regarding sgn, maybe we can use `segment` instead ?
>>
>> Why not segment_group or seg_group (seeing how devfn also is an abbreviation)?
>> And if we omit _number there and on devfn, it's hard to see why it shouldn't
>> be just "bus" then as well.
> 
> So overall
> 
>   uint16_t segment_group;
>   uint8_t bus;
>   uint8_t devfn;
> 
> ?
> 
> segment_group looks a bit off compared with the rest.
> We could use `seg` like we do in Xen PCI code, as it is about PCI 
> segment here.

I wouldn't mind that, yet I wonder why the spec says "group". If there's a
(good) reason, carrying this through into our naming may be advisable.

Jan

Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6

[Anthony PERARD]

On Fri, Aug 29, 2025 at 09:58:16AM +0000, Teddy Astie wrote:
> Currently, hvmloader uses SMBIOS 2.4, however, when using OVMF, the
> SMBIOS is patched to 2.8, which has clarified the UUID format (as GUID).
> 
> In Linux, if the SMBIOS version is >= 2.6, the GUID format is used, else
> (undefined as per SMBIOS spec), big endian is used (used by Xen). Therefore,
> you have a endian mismatch causing the UUIDs to mismatch in the guest.
> 
> $ cat /sys/hypervisor/uuid
> e865e63f-3d30-4f0b-83e0-8fdfc1e30eb7
> $ cat /sys/devices/virtual/dmi/id/product_uuid
> 3fe665e8-303d-0b4f-83e0-8fdfc1e30eb7
> $ cat /sys/devices/virtual/dmi/id/product_serial
> e865e63f-3d30-4f0b-83e0-8fdfc1e30eb7
> 
> This patch updates the SMBIOS version from 2.4 to 2.6 and does the appropriate
> modifications of the table. This effectively fix this endianness mismatch with
> OVMF; while the UUID displayed by Linux is still the same for SeaBIOS.

Just curious, why change hvmloader to generate an SMBIOS v2.6 table
when OVMF later say it's v2.8 ? Why do we change hvmloader when the
problem is OVMF making change to the table before giving it to the OS?

As far as I can tell, OVMF doesn't take into account the version of the
SMBIOS table provided by hvmloader and just use the default set at build
time. This can be changed with the PCD
gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion, it's currently set to
v2.8. The main used of the version number by OVMF seems to be to check
the max sizes.

Before making any changes in hvmloader, we should first fix OvmfXen to
take into account the version of the SMBIOS table that is received.
Otherwise, we just add more tech dept. We might one day want to use a
newer version of SMBIOS, OVMF should be ready by then.


Now, for the change of SMBIOS version. I think that starting to provide
a v2.6 is a good move, but we should list the correct reason for doing
so. OVMF can mostly stay out of the picture here, and be fix separately.
One main issue we can state is that Linux and Windows interpret the UUID
in the SMBIOS differently, when the version is v2.4. I've booted Windows
with SeaBIOS and read the UUID from the SMBIOS table with

    wmic csproduct get uuid

and the UUID return is read according to the new definition propose in
SMBIOS v2.6, even if the table is said to be v2.4, so the UUID is
different from the one set by the toolstack. Moving to v2.6 would indeed
fix this discrepancy.


Next, we are going to want a way to fallback to v2.4 when a guest needs
to observe no changes across reboot. `xl` already have
`smbios_firmware=` which is passthrough `hvmloader` via xenstore entry
`hvmloader/smbios/{address,length}`, but that interface would allow to
only replace a structure (like replacing the type 1 structure) and
doesn't allow to change the version. (And that would duplicate SMBIOS
table generation between hvmloader and the toolstack, which isn't ideal.)
So, will need some changes to `xl`, `libxl`, and `hvmloader`.


> Fixes: c683914ef913 ("Add code to generate SMBIOS tables to hvmloader.")
> (SMBIOS versions before 2.6 has a ill-defined UUID definition)

This space in a commit description is usually reserved for tags and
sometime comment of change made to a patch while
committing/cherry-picking. Comment about a previous commit can be before
these tags, like stating that "commit a1b2c3 made some unhelpful
changes and still have the Fixes:a1b2c3 (...) tag.

> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>

Thanks,

-- 

--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6

[Teddy Astie]

Hello,

Le 17/12/2025 à 16:26, Anthony PERARD a écrit :
> On Fri, Aug 29, 2025 at 09:58:16AM +0000, Teddy Astie wrote:
>> Currently, hvmloader uses SMBIOS 2.4, however, when using OVMF, the
>> SMBIOS is patched to 2.8, which has clarified the UUID format (as GUID).
>>
>> In Linux, if the SMBIOS version is >= 2.6, the GUID format is used, else
>> (undefined as per SMBIOS spec), big endian is used (used by Xen). Therefore,
>> you have a endian mismatch causing the UUIDs to mismatch in the guest.
>>
>> $ cat /sys/hypervisor/uuid
>> e865e63f-3d30-4f0b-83e0-8fdfc1e30eb7
>> $ cat /sys/devices/virtual/dmi/id/product_uuid
>> 3fe665e8-303d-0b4f-83e0-8fdfc1e30eb7
>> $ cat /sys/devices/virtual/dmi/id/product_serial
>> e865e63f-3d30-4f0b-83e0-8fdfc1e30eb7
>>
>> This patch updates the SMBIOS version from 2.4 to 2.6 and does the appropriate
>> modifications of the table. This effectively fix this endianness mismatch with
>> OVMF; while the UUID displayed by Linux is still the same for SeaBIOS.
>
> Just curious, why change hvmloader to generate an SMBIOS v2.6 table
> when OVMF later say it's v2.8 ? Why do we change hvmloader when the
> problem is OVMF making change to the table before giving it to the OS?
>
> As far as I can tell, OVMF doesn't take into account the version of the
> SMBIOS table provided by hvmloader and just use the default set at build
> time. This can be changed with the PCD
> gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion, it's currently set to
> v2.8. The main used of the version number by OVMF seems to be to check
> the max sizes.
>
> Before making any changes in hvmloader, we should first fix OvmfXen to
> take into account the version of the SMBIOS table that is received.
> Otherwise, we just add more tech dept. We might one day want to use a
> newer version of SMBIOS, OVMF should be ready by then.
>

That would be better, but with a small issue. SMBIOS 2.4 doesn't define
EFI as a way to query SMBIOS location. That probably doesn't really
matter, as the table content of 2.4 still has a meaning in the next
SMBIOS specifications; but is still worth noting.

>
> Now, for the change of SMBIOS version. I think that starting to provide
> a v2.6 is a good move, but we should list the correct reason for doing
> so. OVMF can mostly stay out of the picture here, and be fix separately.
> One main issue we can state is that Linux and Windows interpret the UUID
> in the SMBIOS differently, when the version is v2.4. I've booted Windows
> with SeaBIOS and read the UUID from the SMBIOS table with
>
>      wmic csproduct get uuid
>
> and the UUID return is read according to the new definition propose in
> SMBIOS v2.6, even if the table is said to be v2.4, so the UUID is
> different from the one set by the toolstack. Moving to v2.6 would indeed
> fix this discrepancy.
>
On Windows, only doing the endianness change (expected for v2.6) would
fix what Windows reads, as AFAIU it always considers GUID encoding.

>
> Next, we are going to want a way to fallback to v2.4 when a guest needs
> to observe no changes across reboot. `xl` already have
> `smbios_firmware=` which is passthrough `hvmloader` via xenstore entry
> `hvmloader/smbios/{address,length}`, but that interface would allow to
> only replace a structure (like replacing the type 1 structure) and
> doesn't allow to change the version. (And that would duplicate SMBIOS
> table generation between hvmloader and the toolstack, which isn't ideal.)
> So, will need some changes to `xl`, `libxl`, and `hvmloader`.
>

I had in mind the idea of moving the entire SMBIOS table creation to be
fully done in the toolstack (to give it full control on this); so
hvmloader will no longer have to generate it. The caveats is that it
implies moving various things around.

>
>> Fixes: c683914ef913 ("Add code to generate SMBIOS tables to hvmloader.")
>> (SMBIOS versions before 2.6 has a ill-defined UUID definition)
>
> This space in a commit description is usually reserved for tags and
> sometime comment of change made to a patch while
> committing/cherry-picking. Comment about a previous commit can be before
> these tags, like stating that "commit a1b2c3 made some unhelpful
> changes and still have the Fixes:a1b2c3 (...) tag.
>
>> Signed-off-by: Teddy Astie <teddy.astie@vates.tech>
>
> Thanks,
>

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech