On Mon, Jul 07, 2025 at 05:39:39PM +0200, Jean Delvare wrote:
> Hi Jerry,
> 
> On Thu,  3 Jul 2025 16:42:21 -0600, Jerry Hoemann wrote:
> > DIMM Atributes Record
> 
> Spelling: Attributes
> 
Fixed.


> > 
> > Signed-off-by: Jerry Hoemann <[email protected]>
> > ---
> >  dmioem.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 86 insertions(+)
> > 
> > diff --git a/dmioem.c b/dmioem.c
> > index 31efde6..710852f 100644
> > --- a/dmioem.c
> > +++ b/dmioem.c
> > @@ -725,6 +725,22 @@ static void dmi_hp_230_method_bus_seg_addr(u8 code, u8 
> > bus_seg, u8 addr)
> >             pr_attr("I2C Address", "0x%02x", addr >> 1);
> >  }
> >  
> > +static void dmi_hp_232_encrypt(u8 code)
> > +{
> > +   const char *str = "Reserved";
> > +   static const char * const status[] = {
> > +           "Not Encrypted",
> > +           "Encrypted",
> > +           "Unknown",
> > +           "Not Supported",
> > +   };
> > +
> > +   if (code < ARRAY_SIZE(status))
> > +           str = status[code];
> > +
> > +   pr_attr("Encryption Status", "%s", str);
> > +}
> > +
> >  static void dmi_hp_238_loc(const char *fname, unsigned int code)
> >  {
> >     const char *str = "Reserved";
> > @@ -1525,6 +1541,76 @@ static int dmi_decode_hp(const struct dmi_header *h)
> >                     dmi_hp_230_method_bus_seg_addr(data[0x08], data[0x09], 
> > data[0x0A]);
> >                     break;
> >  
> > +           case 232:
> > +                   /*
> > +                    * Vendor Specific: DIMM Atributes Record
> 
> Spelling: Attributes.

Fixed.


> 
> > +                    *
> > +                    * This record is used to communicate information about 
> > DIMMs that is not
> > +                    * available via Industry Standard SMBIOS Records.
> > +                    *
> > +                    * There will be one Record Type 232 for each DIMM 
> > socket possible in the
> > +                    * system (just like Type 17 Records).
> > +                    *
> > +                    * Offset| Name        | Width | Description
> > +                    * -----------------------------------------
> > +                    *  0x00 | Type        | BYTE  | 0xE8, Power Supply 
> > Information Indicator
> 
> Definitely not ^^

The cut/paste error on "Power Supply ...."  ?

Fixed.


> 
> > +                    *  0x01 | Length      | BYTE  | Length of structure
> > +                    *  0x02 | Handle      | WORD  | Unique handle
> > +                    *  0x04 | Assoc Handle| WORD  | Associated Handle 
> > (Type 17)
> > +                    *  0x06 | DIMM Attr   | DWORD | Attributes Bitfield 
> > (Defined in code)
> > +                    *  0x0A | Min Voltage | WORD  | Minimum operating 
> > voltage in milivolts
> > +                    *  0x0C | Con Voltage | WORD  | Configured operating 
> > voltage in milivolts
> 
> Spelling: millivolts.

Fixed.

> 
> "Con" if an unusual abbreviation for "Configured". Maybe "Conf" or
> "Cfg"?

Fixed.

> 
> > +                    *  0x0E | RESERVED
> > +                    *  ....   RESERVED
> > +                    *  0X021  RESERVED
> 
> Please write it 0x21 for consistency.

Done.

> 
> > +                    *  0x22 | Map-Out     | BYTE  | Bit Field reason for 
> > DIMM being mapped out
> > +                    *  0x23 | Encryption  | BYTE  | Encryption status
> > +                    *
> 
> Useless blank line in comment.
> 

Removed.

> > +                    */
> > +
> > +                   if (gen < G9) return 0;
> > +                   pr_handle_name("%s DIMM Attributes Record", company);
> > +
> > +                   if (h->length < 0x0E) break;
> > +                   if (!(opt.flags & FLAG_QUIET))
> > +                           pr_attr("Associated Handle", "0x%04X", 
> > WORD(data + 0x4));
> 
> 0x04 for consistency.

Done.
> 
> > +
> > +                   feat = DWORD(data + 0x06);
> > +                   pr_attr("Attributes", "0x%08X", feat);
> > +                   /* Bit [1:0] HP SmartMemory */
> > +                   pr_subattr("HPE Smart Memory",
> > +                                   (feat & 0x03) == 0 ? "No" :
> > +                                   (feat & 0x03) == 1 ? "Yes" : "Unknown");
> > +                   /* Bit [3:2] Indicator if DIMM is Load Reduced (LR) */
> > +                   /* Bit [2]: 1 = Field Supported */
> > +                   if (feat & (1 << 2))
> > +                           pr_subattr("Load Reduced DIMM installed",
> > +                                   (feat & (1 << 3)) ? "Yes" : "No");
> > +                   /* Bit [5:4] HP Standard DIMM Indicator */
> > +                   /* Bit [4]: 1 = Field Supported */
> > +                   if (feat & (1 << 4))
> > +                           pr_subattr("HPE Standard Memory Installed",
> > +                                   (feat & (1 << 5)) ? "Yes" : "No");
> > +                   if (WORD(data + 0x0A))
> > +                           pr_attr("Minimum Voltage", "%d", WORD(data + 
> > 0x0A));
> 
> Please include the unit (mV).

Done.
> 
> > +                   else
> > +                           pr_attr("Minimum Voltage", "Unknown");
> > +
> > +                   if (WORD(data + 0x0C))
> > +                           pr_attr("Configured Voltage", "%d", WORD(data + 
> > 0x0C));
> > +                   else
> > +                           pr_attr("Configured Voltage", "Unknown");
> 
> Ditto.

Done.
> 
> As a side note, I wonder if maybe we should have a helper function to
> print voltage values with the most appropriate unit BTW, like we have
> for storage size and memory size. I think it makes more sense to print
> it in Volts for values 1 V and above, and millivolts if under 1 V.
> 
> > +
> > +                   if (h->length < 0x24) break;
> 
> I have several dumps in my collection where type 232 records have size
> 35 (length == 0x23): ProLiant DL385 G10, ProLiant DL560 G10, ProLiant
> DL580 G10 and ProLiant SY480 G10. So I think you should also test this
> record length, to always decode all available fields.
> 

Fixed.

> > +                   feat = data[0x22];
> > +                   if (feat) {
> > +                           pr_attr("Map-Out Reason", "0x%0X", feat);
> > +                           pr_subattr("Configuration Error", (feat & 0x01) 
> > ? "Yes" : "No");
> > +                           pr_subattr("Training Error", (feat & 0x02) ? 
> > "Yes" : "No");
> > +                   }
> > +                   dmi_hp_232_encrypt(data[0x23]);
> > +                   break;
> > +
> >             case 233:
> >                     /*
> >                      * Vendor Specific: HPE ProLiant NIC MAC Information
> 
> Thanks,
> -- 
> Jean Delvare
> SUSE L3 Support

-- 

-----------------------------------------------------------------------------
Jerry Hoemann                  Software Engineer   Hewlett Packard Enterprise
-----------------------------------------------------------------------------

Reply via email to