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

From: subscivan
Date: Tue Apr 28 2015 - 04:53:27 EST




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@xxxxxxx>
Fixes: 95be58df74a5 ("firmware: dmi_scan: Use full dmi version for SMBIOS3")
Cc: Ivan Khoronzhuk <ivan.khoronzhuk@xxxxxxxxxx>
---
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/