Re: [PATCH 1/1] dmioem: Decode HPE OEM Record 245

[Jean Delvare]

Hi Jerry,

On Mon, 2023-05-08 at 11:32 -0600, Jerry Hoemann wrote:
> Decode HPE OEM Record 245: Extension Board Inventory Record.
> 
> Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
> ---
>  dmioem.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/dmioem.c b/dmioem.c
> index 04a89ab..5ccd30e 100644
> --- a/dmioem.c
> +++ b/dmioem.c
> @@ -771,6 +771,28 @@ static void dmi_hp_242_speed(const char *attr, u16 speed)
>                 pr_attr(attr, "%s", "Unknown");
>  }
>  
> +static void dmi_hp_245_pcie_riser(const struct dmi_header *h)
> +{
> +       const char *str = "Reserved";
> +       u8 *data = h->data;
> +
> +       pr_attr("Board Type", "PCIe Riser");
> +       if (h->length < 0x09) return;
> +       switch (data[0x05])
> +       {
> +               case 1: str = "Primary"; break;
> +               case 2: str = "Secondary"; break;
> +               case 3: str = "Tertiary"; break;
> +               case 4: str = "Quaternary"; break;
> +               case 10: str = "Front"; break;
> +       }
> +       pr_attr("Riser Position", "%s", str);
> +       pr_attr("Riser ID", "%d", data[0x06]);

In the sample you provided, there are 2 riser records and both have
this field set to 2. I'd expect this ID to be unique within a given
system. The code looks right though, so I suppose the problem is with
the DMI table data?

> +       if (data[0x07])
> +               pr_attr("CPLD Version", "%c:%x", (data[0x07] >> 7) ? 'B' : 
> 'R', (data[0x07] & 0x7F));

Hexadecimal number printed without a prefix can be confusing. Is there
any standard regarding how a CPLD version should be presented?

> +       pr_attr("Riser Name", dmi_string(h, data[0x08]));
> +}
> +
>  static int dmi_decode_hp(const struct dmi_header *h)
>  {
>         u8 *data = h->data;
> @@ -1490,6 +1512,40 @@ static int dmi_decode_hp(const struct dmi_header *h)
>                         dmi_hp_242_speed("Negotiated Speed", WORD(data + 
> 0x3A));
>                         dmi_hp_242_speed("Capable Speed", WORD(data + 0x3C));
>                         break;
> +
> +               case 245:
> +                       /*
> +                        * Vendor Specific: HPE Extension Board Inventory 
> Record
> +                        *
> +                        * This record provides a mechanism for software to 
> retrieve installed
> +                        * Extension Boards in system, such as Riser Cards, 
> etc. Each extension
> +                        * board discovered in system boot time has a 
> corresponding record

"at system boot time" would sound better, methinks.

> +                        * produced in SMBIOS Type 245. This record is 
> currently applicable
> +                        * for ML, DL and Alletra series servers in Gen11 and 
> will be backward
> +                        * compatible with next generations
> +                        *
> +                        * This is a variant record. Definition of fields 
> 0x05 ... vary based
> +                        * upon field 0x04 Board Type.
> +                        *
> +                        * Offset |  Name      | Width | Description
> +                        * ---------------------------------------
> +                        *  0x00  | Type       | BYTE  | 0xF5, Extension 
> Board Inventory Record
> +                        *  0x01  | Length     | BYTE  | Length of structure
> +                        *  0x02  | Handle     | WORD  | Unique handle
> +                        *  0x04  | Board Type | WORD  | 0: PCIe Riser, Other 
> Reserved
> +                        *
> +                        *  If Board Type == 0
> +                        *  0x05  | Riser Pos  | WORD  |
> +                        *  0x06  | Riser ID   | BYTE  |
> +                        *  0x07  | CPLD Vers  | BTYE  | 0-> No CPLD. Bits 
> [7][6:0] Release:Vers
> +                        *  0x08  | Riser Name | STRING|
> +                        */
> +                       pr_handle_name("%s ProLiant Extension Board Inventory 
> Record", company);
> +                       if (h->length < 0x05) break;
> +                       if (data[0x04] == 0)
> +                               dmi_hp_245_pcie_riser(h);
> +                       break;
> +
>                 default:
>                         return 0;
>         }

Everything else looks good to me.

-- 
Jean Delvare
SUSE L3 Support