On Tue, Sep 23, 2025 at 04:06:15PM +0200, Jean Delvare wrote:
> Hi Jerry,
>
> On Mon, 15 Sep 2025 14:23:11 -0600, Jerry Hoemann wrote:
> > DIMM Current Configuration Record
> >
> > Signed-off-by: Jerry Hoemann <[email protected]>
> > ---
> > dmioem.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 91 insertions(+)
> >
> > diff --git a/dmioem.c b/dmioem.c
> > index cffa49d..1f40fa7 100644
> > --- a/dmioem.c
> > +++ b/dmioem.c
> > @@ -949,6 +949,27 @@ static void dmi_hp_242_speed(const char *attr, u16
> > speed)
> > pr_attr(attr, "%s", "Unknown");
> > }
> >
> > +static void dmi_hp_244_health(u8 code)
> > +{
> > + const char *str = "Reserved";
>
> This initial string is never used.
>
> > + char buf[80];
> > + static const char * const health[] = {
> > + "Healthy", /* 0x00 */
> > + "DIMM Missing",
> > + "Config Inactive",
> > + "SPA Missing",
> > + "New Goal",
> > + "Locked", /* 0x05 */
> > + };
> > + if (code < ARRAY_SIZE(health))
> > + str = health[code];
> > + else {
> > + sprintf(buf, "%d", code);
> > + str = buf;
> > + }
> > + pr_attr("Interleave Set Health", "%s", str);
>
> The typical way to handle unexpected values is to simply return
> "Reserved", which is what you have done in many other HPE-specific
> types before. I would stick to that for consistency and simplicity
> (avoids the intermediate buffer).
Agreed. a bit of debugging code i failed to clean up.
Done.
>
> > +}
> > +
> > static void dmi_hp_245_pcie_riser(const struct dmi_header *h)
> > {
> > const char *str = "Reserved";
> > @@ -1919,6 +1940,76 @@ static int dmi_decode_hp(const struct dmi_header *h)
> > dmi_hp_242_speed("Capable Speed", WORD(data + 0x3C));
> > break;
> >
> > +
>
> Extra blank line.
Fixed.
>
> > + case 244:
> > + /*
> > + * Vendor Specific: HPE DIMM Current Configuration
> > Record
> > + *
> > + * This record is used to communicate information about
> > the currently
> > + * configured memory regions on DIMMs installed in the
> > system.
> > + *
> > + * There will be at least one Type 244 Record for each
> > DIMM installed
> > + * in the system with configured non-volatile or
> > volatile memory.
> > + *
> > + * The number of DIMM Configuration Records for each
> > DIMM is specified by
> > + * the Configured Region Count field in its
> > corresponding Type 232 DIMM
> > + * Capabilities record. Each record represents a memory
> > region on the
> > + * DIMM, labeled by its Region ID. The Memory Type
> > field can be used to
> > + * determine the currently configured memory type for
> > the region. When
> > + * set to Volatile, the data on the memory region is
> > lost on a power
> > + * cycle. When set to Byte Accessible Persistent, the
> > data on
> > + * the memory region is retained through a reset. The
> > Region Memory Size
> > + * field contains the size of the configured region in
> > MiB.
> > + *
> > + * The Passphrase State specifies the enable/disable
> > state of the
> > + * Passphrase requirement and is only applicable when
> > the DIMM region is
> > + * configured as Byte Accessible Persistent. The
> > Interleave Set Index
> > + * specifies which interleave set the DIMM region
> > belongs to, if any.
> > + * Regions with identical * interleave set indices mean
> > the DIMM regions
>
> Stray "*".
Fixed.
>
> > + * are interleaved. These indices should match what is
> > found in the DIMM's
> > + * PCAT Interleave Information tables. Interleave set
> > indices are
> > + * 1-based. This will be a unique value per interleave
> > set. If the DIMM
> > + * region is not interleaved, the interleave set index
> > will
> > + * still be a unique value.
> > + *
> > + * Offset | Name | Width | Description
> > + * ---------------------------------------
> > + * 0x00 | Type | BYTE | 0xF4, DIMM Current
> > Configuration Record
> > + * 0x01 | Length | BYTE | Length of structure
> > + * 0x02 | Handle | WORD | Unique handle
> > + * 0x04 | Hndl Assoc | WORD | Handle of Coresponding
> > Type 17 record
>
> Spelling: corresponding.
Fixed.
>
> > + * 0x06 | Region ID | BYTE | Unique ID of memory
> > region on DIMM
> > + * 0x07 | Region Type| WORD | Persistence Type. Bit
> > Field: See code
> > + * 0x09 | Region Size| QWORD | Size of memory region
> > in MiB
> > + * 0x11 | Passphrase | BYTE | Current state of
> > Passphrase. See code
> > + * 0x12 | Set Index | WORD | Interleave set index.
> > Region w/ same index
> > + * | are part of same
> > interlave set.
>
> Spelling: interleave.
Fixed.
>
> > + * 0x14 | DIMM Count | BYTE | Number of DIMMs in
> > interleave set
> > + * 0x15 | Health | BYTE | Health of Interleave
> > set defined in code
> > + */
> > +
> > + pr_handle_name("%s DIMM Current Configuration",
> > company);
> > + if (h->length < 0x14) break;
> > + if (!(opt.flags & FLAG_QUIET))
> > + pr_attr("Associated Handle", "0x%04X",
> > WORD(data + 0x04));
> > + pr_attr("Region ID", "%d", data[0x06]);
> > + feat = WORD(data + 0x07);
> > + pr_attr("Persistence Type", "0x%0x", feat);
> > + /* Volatile implies normal RDIMM or AEP DIMM in memory
> > mode */
> > + pr_subattr("Volatile", (feat & 0x01) ? "Yes" : "No");
> > + pr_subattr("Byte Accessible Persistent", ((feat >> 1) &
> > 0x01) ? "Yes" : "No");
> > + pr_subattr("Block I/O Persistent", ((feat >> 2) & 0x01)
> > ? "Yes" : "No");
>
> Encoding is odd. I can't imagine a region being volatile and persistent
> at the same time. Likewise "Byte Accessible Persistent" and "Block I/O
> Persistent" sound mutually exclusive. As a matter of fact, in all my
> samples, one and only one of the 3 sub-attributes is set to "Yes".
>
> Are there other values possible than 1, 2 and 4?
This is a bit confusing. The FW is overloading this field to
account for both normal RDIMM and AEP DIMM (i.e. nvram).
A fuller description from the spec is:
Bit[0] –
0 = Memory region not
configured as Volatile
1 = Memory region configured
as Volatile (normal RDIMM or
AEP DIMM in memory mode)
Bit[1]:
0 = Memory region not
configured as Byte Accessible
Persistent
1 = Memory region configured
as Byte Accessible Persistent
Bit[2]:
0 = Memory region not
configured as Block I/O
Persistence
1 = Memory region configured
as Block I/O Persistence
So normal memory will always have just Bit[0] set. However, AEP memory would
have exactly one of the three bit set dependeing on how the AEP memory is
configured.
At least that is my recollection as it has been a while.
There weren't a lot of systems w/ AEP on them and the feature has not
been supported on Gen11 going forward.
>
> My feeling is that this could be displayed as a single attribute
> "Persistence Type" and a single string.
I think I undestand what you want here and will modify accordingly.
>
> > +
> > + dmi_print_memory_size("Size", QWORD(data + 0x09), 2);
> > + pr_attr("Passphrase Enabled", "%s", data[0x11] ? "Yes"
> > : "No");
> > + pr_attr("Interleave Set Index", "%d", WORD(data +
> > 0x12));
> > + if (h->length < 0x15) break;
> > + pr_attr("Interleave DIMM Count", "%d", data[0x14]);
> > + if (h->length < 0x16) break;
> > + dmi_hp_244_health(data[0x15]);
>
> Based on the clarification from your Firmware team (and consistent with
> the fact that interleave set indices are 1-based), do we want to hide
> these last 3 fields if the interleave set index is 0?
Done.
>
> > +
>
> Stray blank line.
Fixed.
>
> > + break;
> > +
> > case 245:
> > /*
> > * Vendor Specific: HPE Extension Board Inventory Record
>
> Looks pretty good overall.
>
> --
> Jean Delvare
> SUSE L3 Support
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------
