Re: [dmidecode] [PATCH] update dmidecode to parse Modern Management Controller blocks

[Neil Horman]

On Sat, Aug 04, 2018 at 10:59:39PM +0200, Jean Delvare wrote:
> Hi Neil,
> 
> On Thu, 2 Aug 2018 10:51:32 -0400, Neil Horman wrote:
> > On Thu, Aug 02, 2018 at 02:17:42PM +0000, Elliott, Robert (Persistent 
> > Memory) wrote:
> > > Also be careful of endianness; these are all little-endian values. Does
> > > dmidecode support being run on a big-endian machine?  If so, it should
> > > still print the same characters.  For example, Intel's PCI vendor ID
> > > should always print as "0x8086", not '0x8680" on some machines.
> >
> > Thats an interesting question.  Currently as far as I know smbios is only
> > supported on intel, which is a little endian system.  The application is 
> > written
> > to expect that, so were good.  
> 
> Not true. First of all, all of Intel isn't fully little-endian. IA-64 is
> (well, was...) bi-endian. Secondly, SMBIOS is supported on ARM these
> days, which is also bi-endian. But... I think that both default to
> little-endian indeed, and people stick to that?
> 
Hm, not sure on that.  ARM can still be a bit of a wild west show.  Its
stabilizing with larger distribution support, but smaller stuff could still go
either way I think.

> The original SMBIOS specification (when I started working on dmidecode
> 2.0 back in 2003) did not specify the byte order in the SMBIOS entry
> point and DMI table. I assumed native byte order of the host, and
> that's the reason why we have these macros defined in types.h. (In fact
> we have these macros exactly because the specification was not clear
> and I'd rather update the macros once than change it everywhere
> throughout the source code.)
> 
Well, sure, but the macros only work if you know the endianess at the source.

> But now that you are raising the point again, I took a look at the most
> recent specification and found the following note (apparently added in
> 2.8.0, but I did not pay attention at that time):
> 
> NOTE The Entry Point Structure and all SMBIOS structures assume a 
> little-endian ordering convention, unless
> explicitly specified otherwise, i.e., multi-byte numbers (WORD, DWORD, etc.) 
> are stored with the low-order
> byte at the lowest address and the high-order byte at the highest address.
> 
Ok, cool, so this suggests that we should be using the ntoh* macros
consistently.

> Which means I got it wrong and types.h needs to be revisited. This also
> suggests that nobody ever tried dmidecode on a big-endian system, or
> they would have noticed.
> 
The only system I know of that would have been consistently big endian I think
would have been old pre power-9 systems, which IIRC never supported smbios.

> > That said, dmidecode can also read from a dump
> > file, so its possible that someone might take a dump on an intel system, 
> > and go
> > read it on an old power system.
> 
> This is a good point. The endianness of the original system is not
> recorded in the dump, so if the data endianness would depend on the
> original host, we would be in trouble. But as it seems now clear that
> all DMI data must be little-endian, we should be on the safe side. It's
> currently broken, but easy to fix.
> 
Agreed, if we just assume consistent little endian from the source (be it
in-memory or file), we can move forward.

> Reading a dump from a powerpc system might be challenge because
> typically distributions don't package dmidecode on architectures which
> do not implement SMBIOS. But I could try building it from sources on
> the machine directly.
> 
> > I would say, it wouldn't hurt to update dmidecode to be endian aware, but 
> > such
> > an update is likely outside the scope of this patch.
> 
> Actually we need to update it to be endian agnostic ;-) but yes, that
> would be done in a separate patch.
> 
Ok, so are you comfortable with this patch as it is?  I'm happy to start a
review of the code to consistently use the endian conversion macros if you like.

Neil

> Thanks,
> -- 
> Jean Delvare
> SUSE L3 Support
>