Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933133AbbD1IxX (ORCPT ); Tue, 28 Apr 2015 04:53:23 -0400 Received: from mail-lb0-f172.google.com ([209.85.217.172]:36299 "EHLO mail-lb0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932356AbbD1IxD (ORCPT ); Tue, 28 Apr 2015 04:53:03 -0400 Message-ID: <553F4A69.9080106@gmail.com> Date: Tue, 28 Apr 2015 11:52:57 +0300 From: subscivan User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Jean Delvare , "Ivan.khoronzhuk" CC: LKML , Ivan Khoronzhuk Subject: Re: [PATCH 1/2] firmware: dmi_scan: Simplified displayed version References: <20150421144555.35e2828d@endymion.delvare> <553E5F5D.60302@gmail.com> <20150428101546.3a27a119@endymion.delvare> In-Reply-To: <20150428101546.3a27a119@endymion.delvare> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5390 Lines: 106 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 >>> Fixes: 95be58df74a5 ("firmware: dmi_scan: Use full dmi version for SMBIOS3") >>> Cc: Ivan Khoronzhuk >>> --- >>> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/