Re: [PATCH] hvmloader: Update to SMBIOS 2.6

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/08/2025 9:27 pm, Teddy Astie wrote:
> Le 27/08/2025 à 19:49, Andrew Cooper a écrit :
>> On 22/08/2025 2:47 pm, 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 fixup the UUID
>>> written in the table; which effectively fix this endianness mismatch with
>>> OVMF; while the UUID displayed by Linux is still the same for SeaBIOS.
>>>
>>> Signed-off-by: Teddy Astie <teddy.astie@xxxxxxxxxx>
>>> ---
>>> This effectively changes the UUID seen with UEFI guests as it was
>>> actually inconsistent with SeaBIOS and SMBIOS expectations.
>>> ---
>> I agree this is a real bug and needs fixing.  However, ...
>>
>>
>>>   tools/firmware/hvmloader/smbios.c | 20 ++++++++++++++++++--
>>>   1 file changed, 18 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/firmware/hvmloader/smbios.c 
>>> b/tools/firmware/hvmloader/smbios.c
>>> index 6bcdcc233a..f4822ae6f8 100644
>>> --- a/tools/firmware/hvmloader/smbios.c
>>> +++ b/tools/firmware/hvmloader/smbios.c
>>> @@ -352,7 +352,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);
>>> @@ -462,7 +462,23 @@ 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 uses OSF DCE UUIDs which is fully big endian, 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.
>> ... this is not an accurate statement.
>>
>> Xen specifically tries to treat a xen_domain_handle_t as an opaque blob.
>>
>> The only two areas I can see ascribing any structure are the 'q'
>> debugkey (not exactly a strong ABI statement), and the arinc635
>> scheduler whose use is buggy (uuids are not unique in Xen; it's the
>> domid which is).
>>
>> It is an error that a format isn't stated, but the format comes from the
>> toolstack.  We'd better hope that all toolstacks use OSF DCE UUIDs, or
>> this is going to badly wrong.
>>
> I agree in principle. maybe OSF DCE UUID is not the proper definition 
> (even though it implies the same) but I should rather use RFC 9562 UUIDs 
> but refering to the string representation rather than the UUID meaning 
> itself.
>
> The RFC 9562 defines the UUID as being sequenced as big endian and 
> string represented as > UUID     = 4hexOctet "-"
>>            2hexOctet "-"
>>            2hexOctet "-"
>>            2hexOctet "-"
>>            6hexOctet
>> hexOctet = HEXDIG HEXDIG
>> DIGIT    = %x30-39
>> HEXDIG   = DIGIT / "A" / "B" / "C" / "D" / "E" / "F"
> This matches the UUID encoding provided by XEN_DEFINE_UUID and is used 
> by libxl, libvirt and XAPI and considered by Linux when reading the 
> UUID. However, it may always not be a "valid" UUID strictly speaking but 
> it doesn't really matter since we only care about its binary/string 
> representation.
>
>> And on that note, the toolstacks are not the same.  Xapi for example
>> uses reads 16 bytes out of /dev/urandom.
>>
>> Whatever we end up doing, the fix must include a change to
>> xen/include/public/version.h stating the format of the UUID.
>>
> Something like
>
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index 04fc891353..3241e8dd2b 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -975,6 +975,10 @@ 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 guest handled provided by toolstack encoded as a UUID in
> + * big-endian order. Its string representation follows RFC 9562.
> + */
>   typedef uint8_t xen_domain_handle_t[16];
>
>   __DEFINE_XEN_GUEST_HANDLE(uint8,  uint8_t);
>
> ?
>
> So that we're converting between big-endian encoded UUID (RFC 9562) and 
> Microsoft GUID (which doesn't care about its content but only about its 
> endianness regarding formatting).

I'd be tempted to be rather more explicit.

diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index 82b9c05a76b7..f1592dc059e2 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);


~Andrew