[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [dmidecode] [PATCH 1/1] dmioem: HPE OEM Record 199
From: |
Jean Delvare |
Subject: |
Re: [dmidecode] [PATCH 1/1] dmioem: HPE OEM Record 199 |
Date: |
Tue, 2 Mar 2021 13:57:43 +0100 |
On Mon, 1 Mar 2021 23:11:23 -0700, Jerry Hoemann wrote:
> On Mon, Mar 01, 2021 at 07:56:31PM +0100, Jean Delvare wrote:
> > I don't think you want to use %2x and %4x. Numbers shorter than 2
> > (resp. 4) digits would have spaces inserted before them, so you could
> > have a date printed as "12/25/ 800" (if someone would be brave enough
> > to release a new CPU firmware the day Charlemagne was crowned emperor).
> >
> > Also I think I would always print the month with 2 digits, as this
> > seems to be the most popular way of formatting dates in the BIOS world
> > (315 to 1 on my samples). So I would go for "%02x/%02x/%x"
> >
> > Alternatively, we could use the standard, unambiguous ISO 8601 date
> > format. While not as popular in the BIOS world, I can still see a few
> > occurrences (13) in my samples.
>
>
> my quick read of the Wikipedia's description of the ISO 8601 format
> would be: YYYY[-]MM[-]DD, which I like as it sorts lexicographically.
I didn't even know hyphens were optional, I always put them.
And yes, the fact that it sorts easily (even more so for the next 7979
years) is very convenient.
> I'd like to use the optional hyphens as IMO it makes it a bit easier
> to read. Also keeping the hyphens helps to distinguish the display
> from the raw data which is in a different order.
Agreed.
> So format would be "04x-02x-02x".
"%x-%02x-%02x", methinks.
> > > + cpuid = DWORD(data + ptr + 2 * sizeof(u32));
> > > + pr_attr("CPUID", "%02X %02X %02X %02X",
> > > + cpuid & 0xff, (cpuid >> 8) &
> > > 0xff,
> > > + (cpuid >> 16) & 0xff, (cpuid >>
> > > 24) & 0xff);
> >
> > This doesn't seem to be the most popular way to display an x86 CPUID
> > value. I know of two ways to present that information:
> >
> > 1* Optional type/cpu family/model/stepping, as 3 or 4 decimal numbers.
> >
> > 2* A single, raw hexadecimal number.
> >
> > Splitting into 4 hexadecimal bytes doesn't really make sense, as this
> > doesn't match the different fields within the 32-bit number.
>
>
> This field contains the output of the CPUID instruction and the printing is
> consistent with how dmidecode displays the type 4 "ID" field.
Not really. That field in type 4 is displayed twice. As a raw "ID"
first, before taking the architecture into account. And then once again
in an arch-specific way. For i686 and later, this is decoded as
"Signature" with format strings "Type %u, Family %u, Model %u, Stepping
%u" (for Intel) or "Family %u, Model %u, Stepping %u" (for AMD).
> I'm not opposed to changing how we display this, but we should probably
> also change the display of the type 4 ID field as well to keep consistent.
> That would make it easier to determine which patch would be applied to
> the current hardware by matching up the output to: "dmidecode -t 4 -t 199"
Reusing the same strings I mentioned above for consistency would be
perfect, agreed.
> > (...)
> > On a broader scope, I think we can improve the readability by making it
> > clearer which lines belong to the same group. There aren't many
> > examples of that in dmidecode yet, but maybe we can do something
> > similar to how type 40 is handled:
> >
> > Handle 0x0013, DMI type 40, 18 bytes
> > Additional Information 1
> > Referenced Handle: 0x0004
> > Referenced Offset: 0x05
> > String: PCIExpressx16
> > Value: 0xaa
> > Additional Information 2
> > Referenced Handle: 0x0000
> > Referenced Offset: 0x05
> > String: Compiler Version: VC 9.0
> > Value: 0x05dc
> >
> > Alternatively, it would be possible to add another level of indentation
> > by using pr_subattr(), as is done in complex type 42. Please check if
> > either option would work for you.
>
> I think having pr_attr("Patch", ...) and date and cpuid as pr_subattr looks
> better.
>
> An example still using the 4 byte display of cpu id and printing patch id as
> decimal:
>
>
> Handle 0x0001, DMI type 199, 52 bytes
> HPE ProLiant CPU Microcode Patch Support Info
> Patch: 33554537
> Date: 2019-12-20
> CPUID: 54 06 05 00
> Patch: 50331663
> Date: 2018-10-08
> CPUID: 55 06 05 00
> Patch: 67120896
> Date: 2020-01-14
> CPUID: 56 06 05 00
> Patch: 83898112
> Date: 2020-01-14
> CPUID: 57 06 05 00
>
>
> Is the above more like what you were thinking?
That was one of the options, yes, even though I did not mention it
explicitly. If you like it this way, let's go with it :-)
--
Jean Delvare
SUSE L3 Support