Hi Robert,

On Sat, 12 Apr 2025 16:38:13 +0000, Elliott, Robert \(Servers\) via 
dmidecode-devel wrote:
> > -----Original Message-----
> > Sent: Saturday, April 12, 2025 11:17 AM
> > Subject: [PATCH] dmidecode: Use binary unit prefixes
> > 
> > The SMBIOS 3.7.1 specification updated all unit size references to
> > comply with IEC 60027, that is, use binary prefixes (KiB, MiB...) to
> > express memory size instead of the inaccurate SI prefixes (kB,
> > MB...). Update the code to embrace this clarification.  
> ...
> 
> >  void dmi_print_memory_size(const char *attr, u64 code, int shift)
> > +           "bytes", "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "ZiB"  
> 
> > @@ -331,7 +331,7 @@ static void dmi_bios_runtime_size(u32 co
> > +           format = "%u KiB";  
> 
> 
> >  static void dmi_bios_rom_size(u8 code1, u16 code2)
> > +           "MiB", "GiB", out_of_spec, out_of_spec  
> 
> > @@ -1709,7 +1709,7 @@ static void dmi_memory_module_size(const
> > +                   pr_attr(attr, "%u MiB%s", 1 << (code & 0x7F),  
> 
> 
> > @@ -2767,11 +2767,11 @@ static void dmi_memory_device_extended_s
> > +           pr_attr("Size", "%lu MiB", (unsigned long)code);
> > +           pr_attr("Size", "%lu GiB", (unsigned long)code >> 10);
> > +           pr_attr("Size", "%lu TiB", (unsigned long)code >> 20);  
> 
> >  static void dmi_memory_voltage_value(const char *attr, u16 code)
> > +                   pr_attr("Maximum Memory Module Size", "%u MiB",
> > +                   pr_attr("Maximum Total Memory Size", "%u MiB",  
> 
> As discussed in 2018, I still think it'd be good to have a generic helper
> that selects the biggest unit size without rounding and use that for
> all the size prints. See
> https://lists.gnu.org/archive/html/dmidecode-devel/2018-12/msg00005.html

Interesting that you are quoting the last message of the thread, which
is mine and includes both my reasoning leading to the conclusion that
I'm happy with the current implementation, and also a question which
was never answered ;-)

But as it was 7 years ago, I'm fine opening the discussion again.

> Make any unexpected values be more obvious.

I suppose this is actually the answer to my question back then ("which
problem are you trying to solve?) But this in fact brings more
questions. Why would you like unexpected values to be more obvious? Are
you (at HPE) using dmidecode to check the validity of your DMI tables
at the time you develop your system firmware, and you'd appreciate if
dmidecode reported (or at least hinted) potential issues to you? Or do
you mean as a user (system owner)? I'm not sure what the end user could
do about the information anyway. Report to the hardware manufacturer? In
my experience it's pretty hard to get hardware manufacturers to fix
small issues in their firmware.

I checked again in my DMI table dump collection for systems where the
code in dmi_print_memory_size() rounds (truncates, actually) some of
the reported memory size values. There's still the system we discussed
originally:

Memory Array Mapped Address
        Starting Address: 0x0000000020000000
        Ending Address: 0x000000007EFBFFFF
        Range Size: 1519 MiB

The number 1519 is already unexpected per se (it would be 1536 if the
whole range was mapped). This should be somewhat obvious to anyone who
is used to work with binary numbers. Also note that the same DMI table
includes 11 more Memory Array Mapped Address entries with sizes each
one more funky than the last: 44928 KiB, 208832 KiB... and happily
overlapping each other, so nothing I would trust anyway.

As my collection grew, I now have a few other samples where the printed
memory size value is truncated. This is from an Asus Vivobook laptop:

Cache Information
        Socket Designation: CPU Internal L1
        (...)
        Installed Size: 3938 GiB
        Maximum Size: 1619 GiB

Cache Information
        Socket Designation: CPU Internal L2
        (...)
        Installed Size: 9154 GiB
        Maximum Size: 100885 GiB

Besides the fact that the installed L1 cache size is greater than the
maximum size, which is impossible, all 4 values are unexpected and
definitely wrong. The system has an Intel Celeron N4000 CPU, known to
have a 4 MiB L2 cache. The values in the table are way too large and
make no sense at all. I checked the raw dump of these records, and the
respective sizes are encoded as (little-endian):
        3E CE 33 65
        05 95 D8 83
        62 72 85 E2
        68 82 F0 88
Looks like completely random bits to me. So here again, rounding or not
rounding makes no difference whatsoever.

And my last example is from an HPE ProLiant DL380 G9 server:

HP Proliant Inventory Record
        (...)
        Image Size: 4095 MiB

The raw dump shows the following data (little-endian):
        FF FF FF FF 00 00 00 00
so this is really 4 GiB minus one byte. This looks suspiciously large
for a firmware image size, so most likely the field was in fact never
set and we are seeing an arbitrary default value. But then again, to
someone familiar with binary numbers, 4095 looks already suspicious
(you would expect 4096 MiB, and thus you'd expect it to be displayed as
4 GiB). Displaying it without rounding at all would be "4294967295
bytes", which I agree would look suspicious too, but wouldn't
immediately translate to "way too large to be realistic as a firmware
image size".

As a summary, in practice I don't think we are really losing valuable
information by truncating the unexpected memory size values. It would
be different if we were rounding to the closest value, possibly
rounding up and hiding values which are short by a few bytes.
Truncating make these values still look suspicious.

I would also like to mention that since you proposed this change 7
years ago, nobody else voiced up to support it, nor complained about
the current implementation. So there doesn't seem to be a general
preference for changing this piece of code.

Lastly, I can provide an update on 2 points you mentioned back then:

* Switching to IEC-defined prefixes for binary multiples. The DMTF
  finally made the move with version 3.7.1 of the SMBIOS specification,
  and we will reflect that move in dmidecode, effectively clearing the
  (relative) ambiguity caused by the improper use of regular
  power-of-ten SI prefixes so far.

* Using a common helper function to display memory size values.
  Function dmi_print_memory_size() serves that purpose now, and since
  last time we discussed this, it has become more widely used, as can be
  seen from git log:

commit b381d53c1199895aecccad543210ae1d40534493
Author: Deomid rojer Ryabkov
Date:   Mon Aug 26 14:20:15 2019 +0200

    Use larger units for memory device and BIOS size

commit 60197d25465b51e682b2349b3c3780b1449fb0a4
Author: Jerry Hoemann
Date:   Fri Jan 15 17:20:30 2021 +0100

    dmioem: Decode HPE OEM Record 240

As a matter of fact, dmi_print_memory_size() is being called from 10
different places by now. Let's go through the remaining open-coded
memory size printing functions:

* dmi_bios_rom_size(). The proper unit is supposed to be already
  encoded in the DMI record itself, so I think it makes sense to trust
  the record and use the requested unit.

* dmi_memory_module_size() and dmi_decode(), case 5. These could make
  use of dmi_print_memory_size(), however this only affects DMI types 5
  and 6 which are deprecated since SMBIOS 2.1 (June 1997) so I don't
  expect such a change to make much of a difference in practice for any
  system still in use these days.

* dmi_memory_device_extended_size(). That one could probably make use
  of dmi_print_memory_size(), I'll look into it.

* dmi_bios_runtime_size(). That one could probably make use
  of dmi_print_memory_size(), I'll look into it.

* dmi_decode(), case 15. The log area length is currently printed in
  bytes regardless of its size. We could use dmi_print_memory_size(),
  however I'm not sure if it's worth it, considering that the maximum
  supported size is 64 KiB which isn't that large, and in practice,
  when the size is properly set, it is generally much smaller than this
  (second largest I've seen is 2049 bytes) and not a multiple of 1 KiB.

-- 
Jean Delvare2049 bytes
SUSE L3 Support

Reply via email to