Re: [PATCH 1/2] firmware: dmi_scan: Simplified displayed version

[subscivan <subscivan@gmail.com>]

On 28.04.15 11:15, Jean Delvare wrote:
> Hi Ivan,
>
> On Mon, 27 Apr 2015 19:10:05 +0300, subscivan wrote:
>> On 21.04.15 15:45, Jean Delvare wrote:
>>> The trailing .x adds no information for the reader, and if anyone
>>> tries to parse that line, this is more work as they have 3 different
>>> formats to handle instead of 2. Plus, this makes backporting fixes
>>> harder.
>>>
>>> Signed-off-by: Jean Delvare <jdelvare@suse.de>
>>> Fixes: 95be58df74a5 ("firmware: dmi_scan: Use full dmi version for SMBIOS3")
>>> Cc: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>>> ---
>>> It doesn't actually "fix" the mentioned commit, as there is no bug, but
>>> if anyone backports dmi-related commits, picking this one will make
>>> his/her life easier.
>>>
>>>    drivers/firmware/dmi_scan.c |    5 ++---
>>>    1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> --- linux-4.0.orig/drivers/firmware/dmi_scan.c	2015-04-17 10:35:56.959512401 +0200
>>> +++ linux-4.0/drivers/firmware/dmi_scan.c	2015-04-17 10:38:02.090156803 +0200
>>> @@ -506,9 +506,8 @@ static int __init dmi_present(const u8 *
>>>    		if (dmi_walk_early(dmi_decode) == 0) {
>>>    			if (smbios_ver) {
>>>    				dmi_ver = smbios_ver;
>>> -				pr_info("SMBIOS %d.%d%s present.\n",
>>> -					dmi_ver >> 8, dmi_ver & 0xFF,
>>> -					(dmi_ver < 0x0300) ? "" : ".x");
>>> +				pr_info("SMBIOS %d.%d present.\n",
>>> +				       dmi_ver >> 8, dmi_ver & 0xFF);
>>>    			} else {
>>>    				dmi_ver = (buf[14] & 0xF0) << 4 |
>>>    					   (buf[14] & 0x0F);
>>>
>>>
>> The main idea here was that dmi version after 3 is in format x.x.x
>> And after v3 it's expected to see such format. But in case if (I hope that
>> will never happen) firmware has 32 bit version of SMBIOS3 the table doesn't
> Oh, it will happen. Given that the v3 entry point format is
> incompatible with the v2 entry point format, I expect (at least x86)
> vendors to provide both whenever possible for several years to come, for
> compatibility reasons. Our code scanning the memory for SMBIOS entry
> points will pick the first one it finds (both in the kernel and in
> dmidecode). I hope that vendors will be smart enough to place the v3
> entry point first, but I expect to be disappointed by some.
>
>> have fields to hold revision number, that's why, to warn user about trimming
>> of revision the .x was added. IMHO the 3.2.x is more informative then 3.2
>> 3.2 can be wrongly interpreted as 3.2.0. If script (or else) needs to see
>> version in usual way, it can parse tables recently exposed.
> I don't think so. 3.2.x and 3.2 mean exactly the same, none if more
> informative than the other. For example if I say "openSUSE 13.2 is
> based on kernel 3.16", that doesn't mean exactly kernel version 3.16.0.
> Same here.
>
>> But if you insist on 3.2, maybe it be good to warn user in some way like
>> printing pr_info("SMBIOS doc revision cannot be accessible");
> That would be replacing a bit of over-engineering with another. No,
> thanks.
>
> The doc revision number has been omitted so far because the
> specification made no room to carry it. People and tools are used to
> that. And to be honest I'm surprised they added it in v3. The revision
> number is not so interesting IMHO, I never missed it in dmidecode.
> Thankfully the additions to the specification are incremental and
> almost always backward compatible so we seldom need to make decoding
> decisions based on the version. Whenever a significant change happens,
> at least the minor version number should be incremented. Bumps of the
> doc revision should only translate to new enumerated values and maybe
> new fields, all of which can be implemented unconditionally.
>
> I suspect that they added a field for the doc revision number in the v3
> entry point simply to avoid a mistake that has happened a couple times
> in the past where vendors would attempt to encode the minor version
> _and_ the doc revision in the minor version byte. When the SMBIOS 2.3.1
> specification was released, a number of vendors encoded the version as
> 2.31 instead of 2.3. This was the first time the doc revision number
> was used AFAIK and apparently some vendors failed to understand how to
> handle it. Maybe the DMTF took note back then that, if the entry point
> format ever changed, they should include a separate field for the doc
> revision number to clear the confusion.
>
> But what I do expect now is the opposite: the doc revision number
> doesn't really matter, so I wouldn't be surprised if in the future some
> vendors don't set it or forget to bump it on BIOS update. So we can
> report it where available but I don't plan to make any use of it.
>
> Anyway, my point here is: let's keep things simple and just report what
> is encoded in the entry point. If it's a v3 entry point, the doc
> revision is there, print it; if it's a v2 entry point, it's not, don't
> print it. Easy as that.
>

Sorry, but you probably meant if it's a 64-bit version of v3
print it, if it's a 32-bit v3 don't print it. It's no the same as
with v2. In case of v2 it's printed as usual w/o this patch, like "2.3".
.x is added only for 32-bit version of v3.