On Tue, 12 Jan 2021 01:05:03 -0700, Jerry Hoemann wrote:
> HP Firmware Inventory Record (Type 240)
>
> Signed-off-by: Jerry Hoemann <jerry.hoem...@hpe.com>
> ---
> dmioem.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/dmioem.c b/dmioem.c
> index 180a95d..4a7ff2b 100644
> --- a/dmioem.c
> +++ b/dmioem.c
> @@ -27,6 +27,7 @@
> #include "dmidecode.h"
> #include "dmioem.h"
> #include "dmioutput.h"
> +#include "dmiopt.h"
>
> /*
> * Globals for vendor-specific decodes
> @@ -186,6 +187,23 @@ static int dmi_hpegen(const char *s)
> return (dmi_vendor == VENDOR_HPE) ? G10P : G6;
> }
>
> +static void dmi_hp_240_attr(const char *fname, u64 code)
> +{
> + char attr[80] = "";
> +
> + if (code.l & (1ULL << 0))
> + strcat(attr, "Updatable ");
> + if (code.l & (1ULL << 1))
> + strcat(attr, "ResetRequired ");
> + if (code.l & (1ULL << 2))
> + strcat(attr, "AuthenticationRequired ");
> + if (code.l & (1ULL << 3))
> + strcat(attr, "InUse ");
> + if (code.l & (1ULL << 4))
> + strcat(attr, "UefiImage");
> + pr_attr(fname, "%s", attr);
> +}
> +
> static int dmi_decode_hp(const struct dmi_header *h)
> {
> u8 *data = h->data;
> @@ -345,7 +363,6 @@ static int dmi_decode_hp(const struct dmi_header *h)
> * 0x14 | Name | STRING| (deprecated) Backplane
> Name
> */
> pr_handle_name("%s HDD Backplane FRU Information",
> company);
> -
> pr_attr("FRU I2C Address", "0x%X raw(0x%X)", data[0x4]
> >> 1, data[0x4]);
> pr_attr("Box Number", "%d", WORD(data + 0x5));
> pr_attr("NVRAM ID", "0x%X", WORD(data + 0x7));
That blank line change shouldn't be there, I guess it sneaked in while
you were looking into fixing OEM type 236. I'll remove it before
applying.
> @@ -361,6 +378,49 @@ static int dmi_decode_hp(const struct dmi_header *h)
> }
> break;
>
> + case 240:
> + /*
> + * Vendor Specific: HPE Proliant Inventory Record
> + *
> + * Reports firmware version information for devices
> that report their
> + * firmware using their UEFI drivers. Additionally
> provides association
> + * with other SMBIOS records, such as Type 203 (which
> in turn is
> + * associated with Types 9, 41, and 228).
> + *
> + * Offset | Name | Width | Description
> + * ---------------------------------------
> + * 0x00 | Type | BYTE | 0xF0, HP Firmware
> Inventory Record
> + * 0x01 | Length | BYTE | Length of structure
> + * 0x02 | Handle | WORD | Unique handle
> + * 0x04 | Hndl Assoc | WORD | Handle to map to Type
> 203
> + * 0x06 | Pkg Vers | DWORD | FW Vers Release of All
> FW in Device
> + * 0x0A | Ver String | STRING| FW Version String
> + * 0x0B | Image Size | QWORD | FW image size (bytes)
> + * 0x13 | Attributes | QWORD | Bitfield: Is attribute
> defined?
> + * 0x1B | Attr Set | QWORD | BitField: If defined,
> is attribute set?
> + * 0x23 | Version | DWORD | Lowest supported
> version.
> + */
> + pr_handle_name("%s Proliant Inventory Record", company);
> + if (h->length < 0x24) break;
Sorry for telling you wrong in my original review. You have a DWORD at
0x23, meaning the last byte used is at offset 0x26, in turn meaning
that you need to check for h->length < 0x27, not 0x24. I'll fix it
before applying.
> + if (!(opt.flags & FLAG_QUIET))
> + pr_attr("Associated Handle", "0x%04X",
> WORD(data + 0x4));
> + pr_attr("Package Version", "0x%08X", DWORD(data + 0x6));
> + pr_attr("Version String", "%s", dmi_string(h,
> data[0x0A]));
> +
> + if (DWORD(data + 0x0B))
> + dmi_print_memory_size("Image Size", QWORD(data
> + 0xB), 0);
> + else
> + pr_attr("Image Size", "%s", "Not Available");
Note that format isn't mandatory. We can simply use:
pr_attr("Image Size", "Not Available");
Same below. I'll fix both.
> +
> + dmi_hp_240_attr("Attributes Def", QWORD(data + 0x13));
> + dmi_hp_240_attr("Attributes Set", QWORD(data + 0x1B));
I'm not completely satisfied with the way this presents the attributes.
But we can commit this first and I'll see if I can come up with
something better later.
> +
> + if (DWORD(data + 0x23))
> + pr_attr("Lowest Supported Version", "0x%08X",
> DWORD(data + 0x23));
> + else
> + pr_attr("Lowest Supported Version", "%s", "Not
> Available");
I'm curious what this "version" is about. On my samples, I have seen
values 1, 2, 3, which do not look like hexadecimal values. But also
0x00000000 (properly handled) and 0x0000FFFF (displayed as is).
Are you sure this is a 32-bit field and not 2 16-bit fields?
> + break;
> +
> default:
> return 0;
> }
--
Jean Delvare
SUSE L3 Support
_______________________________________________
https://lists.nongnu.org/mailman/listinfo/dmidecode-devel
