On Tue, Jan 12, 2016 at 02:22:41PM -0500, Cole Robinson wrote:
SMBIOS 2.6+ stores the UUID in a different format, with the first 3 fields in little endian format. This is what modern qemu delivers and what dmidecode also handles, so let's follow suit too. More info at this thread:
http://www.seabios.org/pipermail/seabios/2015-November/010031.html
The only place this affects is when reporting the UUID at startup.
https://bugzilla.redhat.com/show_bug.cgi?id=1284259
src/fw/biostables.c | 51 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 11 deletions(-)
diff --git a/src/fw/biostables.c b/src/fw/biostables.c index cb74396..2e15f82 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -306,17 +306,46 @@ display_uuid(void) if (memcmp(uuid, empty_uuid, sizeof(empty_uuid)) == 0) return;
printf("Machine UUID"
" %02x%02x%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x%02x%02x%02x%02x\n"
, uuid[ 0], uuid[ 1], uuid[ 2], uuid[ 3]
, uuid[ 4], uuid[ 5]
, uuid[ 6], uuid[ 7]
, uuid[ 8], uuid[ 9]
, uuid[10], uuid[11], uuid[12], uuid[13], uuid[14], uuid[15]);
/*
* comment borrowed from dmidecode:
*
* As of version 2.6 of the SMBIOS specification, the first 3
* fields of the UUID are supposed to be encoded on little-endian.
* The specification says that this is the defacto standard,
* however I've seen systems following RFC 4122 instead and use
* network byte order, so I am reluctant to apply the byte-swapping
* for older versions.
*/
I find this comment confusing - how about something like:
According to SMBIOS v2.6 the first three fields are encoded in little-endian format. Versions prior to v2.6 did not specify the encoding, but we follow dmidecode and assume big-endian encoding.
if (SMBiosAddr->smbios_major_version > 2 ||
(SMBiosAddr->smbios_major_version == 2 &&
SMBiosAddr->smbios_minor_version >= 6)) {
printf("Machine UUID"
" %02x%02x%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x%02x%02x%02x%02x\n"
, uuid[ 3], uuid[ 2], uuid[ 1], uuid[ 0]
, uuid[ 5], uuid[ 4]
, uuid[ 7], uuid[ 6]
, uuid[ 8], uuid[ 9]
, uuid[10], uuid[11], uuid[12]
, uuid[13], uuid[14], uuid[15]);
} else {
printf("Machine UUID"
" %02x%02x%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x%02x%02x%02x%02x\n"
, uuid[ 0], uuid[ 1], uuid[ 2], uuid[ 3]
, uuid[ 4], uuid[ 5]
, uuid[ 6], uuid[ 7]
, uuid[ 8], uuid[ 9]
, uuid[10], uuid[11], uuid[12]
, uuid[13], uuid[14], uuid[15]);
}
The above is okay to me, but I'd like to get Gerd's comments as well, as I think he had some concerns the last time it came up.
-Kevin
On 01/14/2016 12:22 PM, Kevin O'Connor wrote:
On Tue, Jan 12, 2016 at 02:22:41PM -0500, Cole Robinson wrote:
SMBIOS 2.6+ stores the UUID in a different format, with the first 3 fields in little endian format. This is what modern qemu delivers and what dmidecode also handles, so let's follow suit too. More info at this thread:
http://www.seabios.org/pipermail/seabios/2015-November/010031.html
The only place this affects is when reporting the UUID at startup.
https://bugzilla.redhat.com/show_bug.cgi?id=1284259
src/fw/biostables.c | 51 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 11 deletions(-)
diff --git a/src/fw/biostables.c b/src/fw/biostables.c index cb74396..2e15f82 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -306,17 +306,46 @@ display_uuid(void) if (memcmp(uuid, empty_uuid, sizeof(empty_uuid)) == 0) return;
printf("Machine UUID"
" %02x%02x%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x%02x%02x%02x%02x\n"
, uuid[ 0], uuid[ 1], uuid[ 2], uuid[ 3]
, uuid[ 4], uuid[ 5]
, uuid[ 6], uuid[ 7]
, uuid[ 8], uuid[ 9]
, uuid[10], uuid[11], uuid[12], uuid[13], uuid[14], uuid[15]);
/*
* comment borrowed from dmidecode:
*
* As of version 2.6 of the SMBIOS specification, the first 3
* fields of the UUID are supposed to be encoded on little-endian.
* The specification says that this is the defacto standard,
* however I've seen systems following RFC 4122 instead and use
* network byte order, so I am reluctant to apply the byte-swapping
* for older versions.
*/
I find this comment confusing - how about something like:
According to SMBIOS v2.6 the first three fields are encoded in little-endian format. Versions prior to v2.6 did not specify the encoding, but we follow dmidecode and assume big-endian encoding.
That would be fine with me. If Gerd gives his ACK do you want to fix up the comment or should I post a v2?
Thanks, Cole
On Thu, Jan 14, 2016 at 12:24:29PM -0500, Cole Robinson wrote:
On 01/14/2016 12:22 PM, Kevin O'Connor wrote:
According to SMBIOS v2.6 the first three fields are encoded in little-endian format. Versions prior to v2.6 did not specify the encoding, but we follow dmidecode and assume big-endian encoding.
That would be fine with me. If Gerd gives his ACK do you want to fix up the comment or should I post a v2?
Can you post a v2 and include a signed-off-by line?
Thanks, -Kevin
On 01/14/16 18:22, Kevin O'Connor wrote:
On Tue, Jan 12, 2016 at 02:22:41PM -0500, Cole Robinson wrote:
SMBIOS 2.6+ stores the UUID in a different format, with the first 3 fields in little endian format. This is what modern qemu delivers and what dmidecode also handles, so let's follow suit too. More info at this thread:
http://www.seabios.org/pipermail/seabios/2015-November/010031.html
The only place this affects is when reporting the UUID at startup.
https://bugzilla.redhat.com/show_bug.cgi?id=1284259
src/fw/biostables.c | 51 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 11 deletions(-)
diff --git a/src/fw/biostables.c b/src/fw/biostables.c index cb74396..2e15f82 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -306,17 +306,46 @@ display_uuid(void) if (memcmp(uuid, empty_uuid, sizeof(empty_uuid)) == 0) return;
printf("Machine UUID"
" %02x%02x%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x%02x%02x%02x%02x\n"
, uuid[ 0], uuid[ 1], uuid[ 2], uuid[ 3]
, uuid[ 4], uuid[ 5]
, uuid[ 6], uuid[ 7]
, uuid[ 8], uuid[ 9]
, uuid[10], uuid[11], uuid[12], uuid[13], uuid[14], uuid[15]);
/*
* comment borrowed from dmidecode:
*
* As of version 2.6 of the SMBIOS specification, the first 3
* fields of the UUID are supposed to be encoded on little-endian.
* The specification says that this is the defacto standard,
* however I've seen systems following RFC 4122 instead and use
* network byte order, so I am reluctant to apply the byte-swapping
* for older versions.
*/
I find this comment confusing - how about something like:
According to SMBIOS v2.6 the first three fields are encoded in little-endian format. Versions prior to v2.6 did not specify the encoding, but we follow dmidecode and assume big-endian encoding.
if (SMBiosAddr->smbios_major_version > 2 ||
(SMBiosAddr->smbios_major_version == 2 &&
SMBiosAddr->smbios_minor_version >= 6)) {
printf("Machine UUID"
" %02x%02x%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x%02x%02x%02x%02x\n"
, uuid[ 3], uuid[ 2], uuid[ 1], uuid[ 0]
, uuid[ 5], uuid[ 4]
, uuid[ 7], uuid[ 6]
, uuid[ 8], uuid[ 9]
, uuid[10], uuid[11], uuid[12]
, uuid[13], uuid[14], uuid[15]);
} else {
printf("Machine UUID"
" %02x%02x%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x"
"-%02x%02x%02x%02x%02x%02x\n"
, uuid[ 0], uuid[ 1], uuid[ 2], uuid[ 3]
, uuid[ 4], uuid[ 5]
, uuid[ 6], uuid[ 7]
, uuid[ 8], uuid[ 9]
, uuid[10], uuid[11], uuid[12]
, uuid[13], uuid[14], uuid[15]);
}
The above is okay to me, but I'd like to get Gerd's comments as well, as I think he had some concerns the last time it came up.
I don't recall Gerd's comments from last time, but I think the case is that some combination will be unavoidably broken.
Some older QEMU machine types exists (I checked them soon after Cole posted this patch, but have forgotten them by now) that expose SMBIOS version 2.8 but encode the data nonetheless in BE. If the above code is fixed in SeaBIOS, then the UUID will be displayed incorrectly on those old machine types; if the code is not fixed in SeaBIOS, then the UUID will be displayed incorrectly on the recent machine types.
I think this SeaBIOS patch should go in; after all, this is the right thing to do. Those old machine types are simply buggy (in this aspect) in QEMU, the QEMU bugfix was restricted to new machine types for compat reasons only. So I think this is the way forward.
(Of course this code runs on physical hardware as well, so users who depend on the UUID display on their phys boxes might be surprised, when they upgrade SeaBIOS.)
Ultimately this was broken by Microsoft, who ignored the preexistent BE recommendation / practice, and went ahead with the LE encoding, ignoring everyone else. (As I heard.) This created confusion, fragmented implementations, and now we get to spread around the crap.
I think the patch should go in, but I agree the comment lifted from dmidecode is confusing, and should be cleaned up.
Laszlo
Hi,
The above is okay to me, but I'd like to get Gerd's comments as well, as I think he had some concerns the last time it came up.
I don't recall Gerd's comments from last time, but I think the case is that some combination will be unavoidably broken.
Yes, that is the problem. But there are both broken and fixed qemu versions in the wild and there is no reliable way to figure whenever qemu is broken or not.
So, fixing seabios and living with the breakage for some qemu versions is the only option we have. The problem should go away over time with people moving on to newer versions.
Lets merge it (with the improved comment).
cheers, Gerd