Hi Erwan,
On Mon, 9 Nov 2020 13:03:31 +0100, Erwan Velu wrote:
> I rebased my patch on top of yours and it works perfectly.
> Please find attached the v2 of my patch which is now into the hpe specific
> case.
Thanks for your contribution. Review:
> HPE servers encodes some information about the HDD backplane into type 236.
>
> A typical output looks like:
>
> Handle 0x002F, DMI type 236, 21 bytes
> HPE HDD Backplane FRU Information
> FRU I2C Address: 0xAE
> Box Number: 4
> NVRAM ID: 0x109
> Sas Expander WWID: 0x0
> Total SAS Bays: 4
>
> Signed-off-by: Erwan Velu <e.v...@criteo.com>
> ---
> dmioem.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/dmioem.c b/dmioem.c
> index 60b667416563..7d8931500619 100644
> --- a/dmioem.c
> +++ b/dmioem.c
> @@ -282,6 +282,32 @@ static int dmi_decode_hp(const struct dmi_header *h)
> pr_subattr("UEFI", "%s", feat & 0x1400 ? "Yes" : "No");
> break;
>
> + case 236:
> + /*
> + * Vendor Specific: HPE ProLiant HDD Backplane
> + *
> + *
> + * Offset | Name | Width | Description
> + * ---------------------------------------
> + * 0x00 | I2C Address| BYTE | Backplane FRU I2C
> Address
> + * 0x01 | Box Number | WORD | Backplane Box Number
> + * 0x03 | NVRAM ID | WORD | Backplane NVRAM Id
Id -> ID for consistency.
> + * 0x05 | WWID | 64B | SAS Expander WWWID
Following the convention that is used in the other tables in this
function, the "B" stands for bytes, not bits, so that should be "8B".
> + * 0x0D | Total Bays | WORD | Total SAS Bays
> + */
> + pr_handle_name("%s HDD Backplane FRU Information",
> company);
> +
> + /* If the record isn't 0x15, that's suspicious */
> + if (h->length != 0x15) break;
If I counted right, you actually only decode 4 + 15 = 19 bytes. Any
idea what's left in the last 2 bytes that lead to length = 21 (0x15)?
FWIW, I do have several DMI table dumps from a HP ProLiant servers
(BL460c G9, DL385 G10 Plus) where the structure length is 17 (0x11)
bytes. Without your patch, it looks like:
OEM-specific Type
Header and Data:
EC 11 B5 00 A0 01 00 AC 00 00 00 00 00 00 00 00
00
The bytes which are present make sense when decoded with the table you
provided above, just the "Total Bays" field is missing. So I think you
should make your code tolerant to this case, otherwise we'll fail to
decode type 236 on many HP/HPE systems.
The usual way to deal with variable length in dmidecode is to check for
a minimum length at start, and then add conditional breaks for all
other length values which are known to be used in practice.
> +
> + u8 *backplane = data + 0x4;
> + pr_attr("FRU I2C Address", "0x%X", backplane[0x0]);
I2C addresses are 7-bit numbers. For all the type 236 examples I've seen
so far, the values are even numbers in the 0xA0-0xAE range, which does
NOT fit in 7 bits. I suspect the 7-bit value is left-aligned (which
happens often in the I2C literature, because that's how the address is
sent on the wire, with bit 0 representing the transfer direction).
Therefore you should right-shift the value by 1 bit before you print
it, so that it matches the actual I2C address.
> + pr_attr("Box Number", "%d", WORD(backplane + 0x1));
> + pr_attr("NVRAM ID", "0x%X", WORD(backplane + 0x3));
I'm not familiar with NVRAM technology, is it usual to represent the
NVRAM ID as an hexadecimal number? I found examples of both on the web
so I'm a bit confused as what is standard.
> + pr_attr("Sas Expander WWID", "0x%X", QWORD(backplane +
> 0x5));
> + pr_attr("Total SAS Bays", "%d", WORD(backplane + 0x0D));
That one is suspicious. The values I see in my examples are all huge
(1032, 514). I have a hard time imagining a back-end with that many
bays. Considering that 1032 = 1024 + 8 and 514 = 512 + 2, I suspect
that the field is actually composed of 2 values, with the higher bits
meaning something else (or maybe these are actually 2 one-byte fields
rather than one 2-byte field?) Can you please double check with your
contact at HP?
> + break;
> +
> default:
> return 0;
> }
Thanks,
--
Jean Delvare
SUSE L3 Support
_______________________________________________
https://lists.nongnu.org/mailman/listinfo/dmidecode-devel
