* [RESEND PATCH v2 0/3] Update to SMBIOS 2.6
@ 2025-08-29 9:58 Teddy Astie
2025-08-29 9:58 ` [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting Teddy Astie
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Teddy Astie @ 2025-08-29 9:58 UTC (permalink / raw)
To: xen-devel
Cc: Teddy Astie, Oleksii Kurochko, Community Manager, Andrew Cooper,
Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall,
Roger Pau Monné, Stefano Stabellini
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
^ permalink raw reply [flat|nested] 17+ messages in thread* [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting 2025-08-29 9:58 [RESEND PATCH v2 0/3] Update to SMBIOS 2.6 Teddy Astie @ 2025-08-29 9:58 ` Teddy Astie 2025-09-01 13:34 ` Oleksii Kurochko ` (2 more replies) 2025-08-29 9:58 ` [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6 Teddy Astie 2025-08-29 9:58 ` [RESEND PATCH v2 3/3] CHANGELOG.md: Add SMBIOS 2.6 update statement Teddy Astie 2 siblings, 3 replies; 17+ messages in thread From: Teddy Astie @ 2025-08-29 9:58 UTC (permalink / raw) To: xen-devel Cc: Teddy Astie, Oleksii Kurochko, Community Manager, Andrew Cooper, Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting 2025-08-29 9:58 ` [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting Teddy Astie @ 2025-09-01 13:34 ` Oleksii Kurochko 2025-09-01 15:55 ` Jan Beulich 2025-12-17 15:53 ` Anthony PERARD 2 siblings, 0 replies; 17+ messages in thread From: Oleksii Kurochko @ 2025-09-01 13:34 UTC (permalink / raw) To: Teddy Astie, xen-devel Cc: Community Manager, Andrew Cooper, Anthony PERARD, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini [-- 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 --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting 2025-08-29 9:58 ` [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting Teddy Astie 2025-09-01 13:34 ` Oleksii Kurochko @ 2025-09-01 15:55 ` Jan Beulich 2025-09-03 14:24 ` Oleksii Kurochko 2025-12-17 15:53 ` Anthony PERARD 2 siblings, 1 reply; 17+ messages in thread From: Jan Beulich @ 2025-09-01 15:55 UTC (permalink / raw) To: Teddy Astie Cc: Oleksii Kurochko, Community Manager, Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall, Roger Pau Monné, Stefano Stabellini, xen-devel 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); ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting 2025-09-01 15:55 ` Jan Beulich @ 2025-09-03 14:24 ` Oleksii Kurochko 0 siblings, 0 replies; 17+ messages in thread From: Oleksii Kurochko @ 2025-09-03 14:24 UTC (permalink / raw) To: Jan Beulich, Teddy Astie Cc: Community Manager, Andrew Cooper, Anthony PERARD, Michal Orzel, Julien Grall, Roger Pau Monné, Stefano Stabellini, xen-devel [-- 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 --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting 2025-08-29 9:58 ` [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting Teddy Astie 2025-09-01 13:34 ` Oleksii Kurochko 2025-09-01 15:55 ` Jan Beulich @ 2025-12-17 15:53 ` Anthony PERARD 2 siblings, 0 replies; 17+ messages in thread From: Anthony PERARD @ 2025-12-17 15:53 UTC (permalink / raw) To: Teddy Astie Cc: xen-devel, Oleksii Kurochko, Community Manager, Andrew Cooper, Michal Orzel, Jan Beulich, Julien Grall, Roger Pau Monné, Stefano Stabellini 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6 2025-08-29 9:58 [RESEND PATCH v2 0/3] Update to SMBIOS 2.6 Teddy Astie 2025-08-29 9:58 ` [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting Teddy Astie @ 2025-08-29 9:58 ` Teddy Astie 2025-09-02 12:35 ` Jan Beulich 2025-12-17 15:22 ` Anthony PERARD 2025-08-29 9:58 ` [RESEND PATCH v2 3/3] CHANGELOG.md: Add SMBIOS 2.6 update statement Teddy Astie 2 siblings, 2 replies; 17+ messages in thread From: Teddy Astie @ 2025-08-29 9:58 UTC (permalink / raw) To: xen-devel Cc: Teddy Astie, Jan Beulich, Andrew Cooper, Roger Pau Monné, Anthony PERARD 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6 2025-08-29 9:58 ` [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6 Teddy Astie @ 2025-09-02 12:35 ` Jan Beulich 2025-09-02 13:24 ` Teddy Astie 2025-12-17 15:22 ` Anthony PERARD 1 sibling, 1 reply; 17+ messages in thread From: Jan Beulich @ 2025-09-02 12:35 UTC (permalink / raw) To: Teddy Astie Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, xen-devel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6 2025-09-02 12:35 ` Jan Beulich @ 2025-09-02 13:24 ` Teddy Astie 2025-09-02 14:10 ` Jan Beulich 0 siblings, 1 reply; 17+ messages in thread From: Teddy Astie @ 2025-09-02 13:24 UTC (permalink / raw) To: Jan Beulich Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, xen-devel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6 2025-09-02 13:24 ` Teddy Astie @ 2025-09-02 14:10 ` Jan Beulich 2025-09-03 14:02 ` Teddy Astie 0 siblings, 1 reply; 17+ messages in thread From: Jan Beulich @ 2025-09-02 14:10 UTC (permalink / raw) To: Teddy Astie Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, xen-devel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6 2025-09-02 14:10 ` Jan Beulich @ 2025-09-03 14:02 ` Teddy Astie 2025-09-03 15:05 ` Jan Beulich 0 siblings, 1 reply; 17+ messages in thread From: Teddy Astie @ 2025-09-03 14:02 UTC (permalink / raw) To: Jan Beulich Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, xen-devel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6 2025-09-03 14:02 ` Teddy Astie @ 2025-09-03 15:05 ` Jan Beulich 0 siblings, 0 replies; 17+ messages in thread From: Jan Beulich @ 2025-09-03 15:05 UTC (permalink / raw) To: Teddy Astie Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, xen-devel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6 2025-08-29 9:58 ` [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6 Teddy Astie 2025-09-02 12:35 ` Jan Beulich @ 2025-12-17 15:22 ` Anthony PERARD 2026-01-08 13:54 ` Teddy Astie 1 sibling, 1 reply; 17+ messages in thread From: Anthony PERARD @ 2025-12-17 15:22 UTC (permalink / raw) To: Teddy Astie; +Cc: xen-devel, Jan Beulich, Andrew Cooper, Roger Pau Monné 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6 2025-12-17 15:22 ` Anthony PERARD @ 2026-01-08 13:54 ` Teddy Astie 0 siblings, 0 replies; 17+ messages in thread From: Teddy Astie @ 2026-01-08 13:54 UTC (permalink / raw) To: Anthony PERARD Cc: xen-devel, Jan Beulich, Andrew Cooper, Roger Pau Monné 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [RESEND PATCH v2 3/3] CHANGELOG.md: Add SMBIOS 2.6 update statement 2025-08-29 9:58 [RESEND PATCH v2 0/3] Update to SMBIOS 2.6 Teddy Astie 2025-08-29 9:58 ` [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting Teddy Astie 2025-08-29 9:58 ` [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6 Teddy Astie @ 2025-08-29 9:58 ` Teddy Astie 2025-09-01 13:42 ` Oleksii Kurochko 2025-09-01 13:52 ` Jan Beulich 2 siblings, 2 replies; 17+ messages in thread From: Teddy Astie @ 2025-08-29 9:58 UTC (permalink / raw) To: xen-devel; +Cc: Teddy Astie, Oleksii Kurochko, Community Manager 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RESEND PATCH v2 3/3] CHANGELOG.md: Add SMBIOS 2.6 update statement 2025-08-29 9:58 ` [RESEND PATCH v2 3/3] CHANGELOG.md: Add SMBIOS 2.6 update statement Teddy Astie @ 2025-09-01 13:42 ` Oleksii Kurochko 2025-09-01 13:52 ` Jan Beulich 1 sibling, 0 replies; 17+ messages in thread From: Oleksii Kurochko @ 2025-09-01 13:42 UTC (permalink / raw) To: Teddy Astie, xen-devel; +Cc: Community Manager [-- 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 --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RESEND PATCH v2 3/3] CHANGELOG.md: Add SMBIOS 2.6 update statement 2025-08-29 9:58 ` [RESEND PATCH v2 3/3] CHANGELOG.md: Add SMBIOS 2.6 update statement Teddy Astie 2025-09-01 13:42 ` Oleksii Kurochko @ 2025-09-01 13:52 ` Jan Beulich 1 sibling, 0 replies; 17+ messages in thread From: Jan Beulich @ 2025-09-01 13:52 UTC (permalink / raw) To: Teddy Astie; +Cc: Oleksii Kurochko, Community Manager, xen-devel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-01-08 13:54 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-29 9:58 [RESEND PATCH v2 0/3] Update to SMBIOS 2.6 Teddy Astie 2025-08-29 9:58 ` [RESEND PATCH v2 1/3] xen: Define xen_domain_handle_t encoding and formatting Teddy Astie 2025-09-01 13:34 ` Oleksii Kurochko 2025-09-01 15:55 ` Jan Beulich 2025-09-03 14:24 ` Oleksii Kurochko 2025-12-17 15:53 ` Anthony PERARD 2025-08-29 9:58 ` [RESEND PATCH v2 2/3] hvmloader: Update to SMBIOS 2.6 Teddy Astie 2025-09-02 12:35 ` Jan Beulich 2025-09-02 13:24 ` Teddy Astie 2025-09-02 14:10 ` Jan Beulich 2025-09-03 14:02 ` Teddy Astie 2025-09-03 15:05 ` Jan Beulich 2025-12-17 15:22 ` Anthony PERARD 2026-01-08 13:54 ` Teddy Astie 2025-08-29 9:58 ` [RESEND PATCH v2 3/3] CHANGELOG.md: Add SMBIOS 2.6 update statement Teddy Astie 2025-09-01 13:42 ` Oleksii Kurochko 2025-09-01 13:52 ` Jan Beulich
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.