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.as...@vates.tech>
>>> ---
>>> 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
