From b44ec2fb65dfb115fc5485ea9eb7f0281cdb6a05 Mon Sep 17 00:00:00 2001 From: Alexander Amelkin Date: Mon, 30 Jul 2018 16:08:47 +0300 Subject: [PATCH] mc: guid: Fix byte ordering to follow IPMI spec Before this commit the bytes for the GUID 'node' part were displayed in reverse order as if they were reported by BMC according to SMBIOS specification, not accordint to IPMI. The field order in guid_t was also specified according to RFC4122/SMBIOS, which is the reverse of IPMI specification. It has now been fixed. The time_low field of GUID was taken directly, without taking in account the ipmitool host endianness. It is now properly converted from IPMI little-endian to host byte order. Other GUID fields are also properly converted now. As of now, ipmitool does not generally work properly when built for a big-endian target, but that's out of scope of this commit. Please note that this commit most probably breaks the output of `ipmitool mc guid` with most existing BMC implementations, but that's just an indication of them being broken. A follow-up commit will re-add support for the broken behavior. Partially resolves ipmitool/ipmitool#25 Signed-off-by: Alexander Amelkin --- include/ipmitool/ipmi_mc.h | 15 ++++++++++----- lib/ipmi_mc.c | 19 +++++++++++++------ 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/include/ipmitool/ipmi_mc.h b/include/ipmitool/ipmi_mc.h index 4e177e5..a4096fd 100644 --- a/include/ipmitool/ipmi_mc.h +++ b/include/ipmitool/ipmi_mc.h @@ -103,12 +103,17 @@ struct ipm_devid_rsp { #pragma pack(1) #endif struct ipmi_guid_t { - uint32_t time_low; /* timestamp low field */ - uint16_t time_mid; /* timestamp middle field */ - uint16_t time_hi_and_version; /* timestamp high field and version number */ - uint8_t clock_seq_hi_variant;/* clock sequence high field and variant */ - uint8_t clock_seq_low; /* clock sequence low field */ uint8_t node[6]; /* node */ + union { + struct { + uint8_t clock_seq_low; /* clock sequence low field */ + uint8_t clock_seq_hi_variant;/* clock sequence high field and variant */ + }; + uint16_t clock_seq_variant; + }; + uint16_t time_hi_and_version; /* timestamp high field and version number */ + uint16_t time_mid; /* timestamp middle field */ + uint32_t time_low; /* timestamp low field */ } ATTRIBUTE_PACKING; #ifdef HAVE_PRAGMA_PACK #pragma pack(0) diff --git a/lib/ipmi_mc.c b/lib/ipmi_mc.c index ee75e95..34f9978 100644 --- a/lib/ipmi_mc.c +++ b/lib/ipmi_mc.c @@ -533,14 +533,21 @@ ipmi_mc_print_guid(struct ipmi_guid_t guid) memset(tbuf, 0, 40); struct tm *tm; - /* Kipp - changed order of last field (node) to follow specification */ printf("System GUID : %08x-%04x-%04x-%04x-%02x%02x%02x%02x%02x%02x\n", - guid.time_low, guid.time_mid, guid.time_hi_and_version, - guid.clock_seq_hi_variant << 8 | guid.clock_seq_low, - guid.node[0], guid.node[1], guid.node[2], - guid.node[3], guid.node[4], guid.node[5]); + /* We're displaying GUID as hex integers. Thus we need them to be + * in host byte order before we display them. However, according to + * IPMI 2.0, all GUID fields are in LSB first (Little Endian) + * format. Hence the ipmiXtoh() calls: + */ + ipmi32toh(&guid.time_low), + ipmi16toh(&guid.time_mid), + ipmi16toh(&guid.time_hi_and_version), + ipmi16toh(&guid.clock_seq_variant), + /* The node part is shown as bytes, so no additional conversion */ + guid.node[5], guid.node[4], guid.node[3], + guid.node[2], guid.node[1], guid.node[0]); - s = (time_t)guid.time_low; /* Kipp - removed the BSWAP_32, it was not needed here */ + s = (time_t)ipmi32toh(&guid.time_low); if(time_in_utc) tm = gmtime(&s); else