[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [dmidecode] [PATCH 1/2] dmioem: Decode HPE OEM Record 240
From: |
Jean Delvare |
Subject: |
Re: [dmidecode] [PATCH 1/2] dmioem: Decode HPE OEM Record 240 |
Date: |
Tue, 5 Jan 2021 13:56:09 +0100 |
Hi Jerry,
On Wed, 16 Dec 2020 14:18:58 -0700, Jerry Hoemann wrote:
> HP Firmware Inventory Record (Type 240)
>
> Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
> ---
> dmioem.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/dmioem.c b/dmioem.c
> index 180a95d..d8cab2c 100644
> --- a/dmioem.c
> +++ b/dmioem.c
> @@ -361,6 +361,47 @@ 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
Capital A after dot.
> + * with other SMBIOS records, such as Type 203 (which
> in turn is
> + * associated with Types 9, 41, and 228),
End with a dot instead of comma?
> + *
> + * 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 HPE Proliant Inventory Record",
> company);
Remove the hard-coded "HPE" else you end up with "HPE HPE" in the
output.
To be on the safe side, please start with a length check as is done for
regular DMI types (although apparently not all OEM types - will look
into that).
> + pr_attr("Associated Handle ", "0x%04X", WORD(data +
> 0x4));
You may want to condition that one on !(opt.flags & FLAG_QUIET) as
we do for other handle cross-references.
> + pr_attr("Package Version ", "0x%X", DWORD(data +
> 0x6));
> + pr_attr("Version String ", "%s", dmi_string(h,
> data[0x0A]));
We don't do space-alignment of attribute names in dmidecode, so it
would look weird to do it for just that one record.
(If there is a general desire to present things that way for better
readability then that would be handled globally, through the recently
introduced "output drivers".)
We also don't do space-alignment in the source code.
> +
> + if (DWORD(data + 0x0B))
> + pr_attr("Image Size (bytes)", "0x%08x",
> QWORD(data + 0xB));
Could we use dmi_print_memory_size() here so that the size is
automatically expressed in the most suitable unit?
> + else
> + pr_attr("Image Size ", "%s", "Not
> Available");
> +
> + pr_attr("Attribute Defined ", "0x%08x", QWORD(data +
> 0x13));
> + pr_attr("Attribute Set ", "0x%08x", QWORD(data +
> 0x1B));
I would use Attributes (plural) for both?
Would it make sense to go into greater details and list the individual
attributes? Or if that too low details?
> +
> + if (DWORD(data + 0x23))
> + pr_attr("Lowest Supported Version ", "%hi",
> DWORD(data + 0x23));
> + else
> + pr_attr("Lowest Supported Version ", "%s",
> "Not Available");
> + break;
I have one system where this prints -1. Should all FF's be treated as
"Not Available" too? Or does it have a special meaning?
> +
> default:
> return 0;
> }
--
Jean Delvare
SUSE L3 Support
- Re: [dmidecode] [PATCH 1/2] dmioem: Decode HPE OEM Record 240,
Jean Delvare <=