Messages in this thread | | | Date | Tue, 28 Apr 2015 11:52:57 +0300 | From | subscivan <> | Subject | Re: [PATCH 1/2] firmware: dmi_scan: Simplified displayed version |
| |
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.
| |