On Thu, 25 Sep 2025 10:40:13 -0600, Jerry Hoemann wrote:
> Reserved Memory Location
> 
> Signed-off-by: Jerry Hoemann <[email protected]>
> ---
>  dmioem.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/dmioem.c b/dmioem.c
> index b8eac5c..8ea944b 100644
> --- a/dmioem.c
> +++ b/dmioem.c
> @@ -1642,6 +1642,55 @@ static int dmi_decode_hp(const struct dmi_header *h)
>                       pr_attr("Serial Number", "%s", dmi_string(h, 
> data[0x14]));
>                       break;
>  
> +             case 229:
> +                     /*
> +                      * Vendor Specific: Reserved Memory Location
> +                      *
> +                      * This OEM SMBIOS Record is used to communicate the 
> physical address
> +                      * location of memory regions reserved during POST by 
> System Firmware.
> +                      * These memory regions will be reserved for various 
> purposes. It is
> +                      * intended that this OEM SMBIOS Record be expandable 
> to support any
> +                      * future POST reserved memory requirements.  The 
> regions reserved by
> +                      * POST will typically be reported by INT15h E820h as 
> reserved memory.
> +                      * This record was initially defined to communicate to 
> iLO FW and Smart
> +                      * Array Storage FW the location of a memory buffer 
> reserved for passing
> +                      * information between the Smart Array Controller and 
> iLO FW for providing
> +                      * hard drive temperatures to iLO FW fan control.
> +                      *
> +                      * Note: Multiple Type 229 Records may exist in the 
> SMBIOS Table because
> +                      * each SMBIOS Record has a maximum length of 256 bytes 
> and it is possible
> +                      * that there eventually would be enough reserved 
> memory locations such
> +                      * that a single record could exceed this limit (each 
> reserved memory
> +                      * location utilizes 16 bytes). Software utilizing the 
> Type 229 Record
> +                      * should be written to handle the possibility of 
> multiple records.
> +                      *
> +                      * Offset| Name        | Width | Description
> +                      * -----------------------------------------
> +                      *  0x00 | Type        | BYTE  | 0xE5, Reserved Memory 
> Location
> +                      *  0x01 | Length      | BYTE  | Length of structure
> +                      *  0x02 | Handle      | WORD  | Unique handle
> +                      *  0x04 | Signature   | DWORD | Enumerated value that 
> indicates the type
> +                      *                             | of memory described by 
> this Reserved
> +                      *                             | Memory Location

An "enumerated value" on a 32-bit field is kind of surprising. More
about that below.

> +                      *  0x08 | Phys ADDR   | QWORD | 64-Bitphysical memory 
> address

"Addr" for consistency. Missing space between "Bit" and "physical".

> +                      *  0x10 | Size of Loc | DWORD | Bit[30:0] - Size of 
> the Memory Location
> +                      *                             | Bit[31] - Indicates 
> whether the size field in
> +                      *                             | Bits[30:0] is in 1 
> byte or 1 Kbyte granularity
> +                      *                             | 0 = Byte Granularity
> +                      *                             | 1 = Kbyte Granularity
> +                      *  0x14 | Mem Entries         | 16 Bytes per Reserved 
> Memory Entry
> +                      */
> +                     pr_handle_name("%s Reserved Information", company);

Not a good description, "%s Reserved Memory Location" would be better.

> +                     if (h->length < 0x14) break;
> +                     pr_attr("Handle", "%X", WORD(data + 0x02));

This is already printed in the main loop in dmi_table_decode(), no need
to repeat it.

> +                     for (ptr = 0x04;  ptr < h->length; ptr += 16) {

Extra space.

The test is not robust, if the record is incorrectly encoded we could
continue over a partial entry. We should ensure that a whole entry fits
within the record before attempting to decode it, that is, ptr + 16 <=
h->length.

> +                             pr_attr("Reserved Memory Signature", "%d: 
> 0x%0X", ptr, DWORD(data + ptr));

Printing the value of ptr here is confusing, it is the offset of the
entry within the record, not something the user would be interested in.

If the intention is to enumerate the entries, then we should count them
linearly, starting at 1. This number should be included in all
attribute names to avoid confusion. Another option (cleaner IMHO) would
be to go with sub-attributes, as is done in type 42 (see function
dmi_parse_protocol_record).

About the "signature" itself, in my samples they appear to always be 4
printable ASCII characters. So I think it would make sense to print
this field as a string rather than a number.

> +                             pr_attr("Physical Address", "0x%X", QWORD(data 
> + ptr + 0x04));

I think we want to use "0x%016llX" for consistency with how physical
addresses are displayed by dmidecode.

> +                             feat = DWORD(data + ptr + 0x0c);

0x0C (capital C) for consistency.

> +                             dmi_print_memory_size("Size", feat & 
> 0x7fffffff, feat >> 31);
> +                     }
> +                     break;
> +
>               case 230:
>                       /*
>                        * Vendor Specific: Power Supply Information OEM SMBIOS 
> Record


-- 
Jean Delvare
SUSE L3 Support

Reply via email to