Re: [dmidecode] [PATCH] dmidecode: Skip details of uninstalled memory modules

[Jean Delvare]

Hi Petter,

On Mon, 19 Oct 2020 15:18:41 +0200, Petter Reinholdtsen wrote:
> [Jean Delvare]
> > If a memory slot is empty then by definition the fields containing
> > the memory module details are irrelevant. Best case they are filled
> > with "Unused" and "None", but in some cases they are even invalid
> > because the manufacturer did not bother setting the fields to
> > valid neutral values. So it is better to not print these fields
> > at all, so as to not confuse the user.  
> 
> How are the users supposed to identify hardware where the manufacturer
> did not bother with sensible quality control of the DMI data if
> dmidecode hide bogus values?  It could even be used by the QA people of
> the manufacturer to discover such uninitialized values.

Thanks for your feedback. This raises the interesting question of the
target audience for dmidecode. I wish it was used by manufacturers to
validate their DMI tables, but I'm not sure if this is actually the
case. I guess that the primary target would be users willing to know
more about their hardware without opening the box.

Just to clarify, in general I do agree with you. I want dmidecode to
hide as few errors as possible, because I want problems to be fixed
where they are rather that being wallpapered over somewhere else.

Pragmatism made me add a few exceptions already though: the
ascii_filter() and dmi_fixup_type_34() functions, as well as

static void dmi_memory_device_width(const char *attr, u16 code)
{
        /*
         * If no memory module is present, width may be 0
         */
        if (code == 0xFFFF || code == 0)
                pr_attr(attr, "Unknown");
        (...)

> Perhaps a better idea to so flag potential bogus values as such in the
> output, instead of hiding them?

That's what dmidecode was doing so far, and what I received complaints
about.

I believe that the reason is that it's not clear if the remaining
fields must be set when a memory module is not installed. It's pretty
obvious that most of the fields make absolutely no sense in this case
and that would be the reason why some vendors do not bother setting
these fields. I suppose that most hardware inventory tools check the
memory module size first, and if it is 0, they just say "No memory
module installed" or something like that. In dmidecode I chose to
decode the fields in the order in which they appear in the table as
much as possible (or a more correct way to put it is that I never gave
it much thought so it just ended up this way).

The SMBIOS specification includes a section specifying the requirements
for each type. For type 17 (Memory Device) it states:

 "Device Locator string is set to a non-null value. Memory Array Handle
  contains the handle associated with the Physical Memory Array
  structure to which this device belongs. Data Width, Size, Form Factor,
  and Device Set are all set to “known” values. If the device is present
  (for instance, Size is non-zero), the Total Width field is not set to
  0xFFFF (Unknown)."

So basically the requirements are that fields at 0x04, 0x0A, 0x0C,
0x0E, 0x0F and 0x10 are set right unconditionally. Field at 0x08
doesn't have to be set for unpopulated sockets. Nothing is said about
fields beyond offset 0x10. My proposal is to decode up to 0x13
unconditionally, and skip everything after that for unpopulated
sockets. I chose that limit based on the many DMI tables I have in my
test suite. It is compatible with the specification, in the sense that
all fields listed above are decoded unconditionally.

You could argue that this change belongs to --quiet mode, and to some
degree I would agree with that, however I'm afraid that users just run
"dmidecode" and want a nice-looking output by default, without having
to tinker with command-line options. So doing that would probably not
make the complaints go away.

One thing I am considering, in order to make everyone happy, is to add
a --dumb mode to dmidecode, which would explicitly disable any attempt
to make the output look better, and instead would just decode everything
strictly according to the specification. That mode would be targeted at
hardware manufacturers, so that they can use dmidecode to validate their
work. Such a mode would disable the change I'm just proposing, and also
skip the call to dmi_fixup_type_34() and the quirk in
dmi_memory_device_width().

-- 
Jean Delvare
SUSE L3 Support