Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753603AbbDPGWy (ORCPT ); Thu, 16 Apr 2015 02:22:54 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:24955 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750919AbbDPGWp (ORCPT ); Thu, 16 Apr 2015 02:22:45 -0400 Message-ID: <552F5535.9010302@oracle.com> Date: Thu, 16 Apr 2015 14:22:45 +0800 From: Zhenzhong Duan Reply-To: zhenzhong.duan@oracle.com Organization: Oracle User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Jean Delvare , LKML CC: Ben Hutchings , Artem Savkov , Ivan Khoronzhuk , Matt Fleming Subject: Re: [PATCH] firmware: dmi_scan: Fix ordering of product_uuid References: <20150415110222.194867c1@endymion.delvare> In-Reply-To: <20150415110222.194867c1@endymion.delvare> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3602 Lines: 93 On 2015/4/15 17:02, Jean Delvare wrote: > In function dmi_present(), dmi_walk_early() calls dmi_table(), which > calls dmi_decode(), which ultimately calls dmi_save_uuid(). This last > function makes a decision based on the value of global variable > dmi_ver. The problem is that this variable is set right _after_ > dmi_walk_early() returns. So dmi_save_uuid() always sees dmi_ver == 0 > regardless of the actual version implemented. > > This causes /sys/class/dmi/id/product_uuid to always use the old > ordering even on systems implementing DMI/SMBIOS 2.6 or later, which > should use the new ordering. > > This is broken since kernel v3.8 for legacy DMI implementations and > since kernel v3.10 for SMBIOS 2 implementations. SMBIOS 3 > implementations with the 64-bit entry point are not affected. > > The first breakage does not matter much as in practice legacy DMI > implementations are always for versions older than 2.6, which is when > the UUID ordering changed. The second breakage is more problematic as > it affects the vast majority of x86 systems manufactured since 2009. > > Signed-off-by: Jean Delvare > Fixes: 9f9c9cbb6057 ("drivers/firmware/dmi_scan.c: fetch dmi version from SMBIOS if it exists") I think above line should be removed as dmi_ver is set before dmi_walk_early with the commit, see below clip. We did get right UUID order with SMBIOS 2.6 per customer test. +static int __init smbios_present(const char __iomem *p) +{ + u8 buf[32]; + int offset = 0; + + memcpy_fromio(buf, p, 32); + if ((buf[5] < 32) && dmi_checksum(buf, buf[5])) { + dmi_ver = (buf[6] << 8) + buf[7]; > Fixes: 79bae42d51a5 ("dmi_scan: refactor dmi_scan_machine(), {smbios,dmi}_present()") > Cc: Zhenzhong Duan > Cc: Ben Hutchings > Cc: Artem Savkov > Cc: Ivan Khoronzhuk > Cc: Matt Fleming > Cc: stable@vger.kernel.org [v3.10+] > --- > drivers/firmware/dmi_scan.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > --- linux-4.0.orig/drivers/firmware/dmi_scan.c 2015-04-13 00:12:50.000000000 +0200 > +++ linux-4.0/drivers/firmware/dmi_scan.c 2015-04-15 10:24:37.556994240 +0200 > @@ -499,18 +499,19 @@ static int __init dmi_present(const u8 * > buf += 16; > > if (memcmp(buf, "_DMI_", 5) == 0 && dmi_checksum(buf, 15)) { > + if (smbios_ver) > + dmi_ver = smbios_ver; > + else > + dmi_ver = (buf[14] & 0xF0) << 4 | (buf[14] & 0x0F); > dmi_num = get_unaligned_le16(buf + 12); > dmi_len = get_unaligned_le16(buf + 6); > dmi_base = get_unaligned_le32(buf + 8); > > if (dmi_walk_early(dmi_decode) == 0) { > if (smbios_ver) { > - dmi_ver = smbios_ver; > pr_info("SMBIOS %d.%d present.\n", > dmi_ver >> 8, dmi_ver & 0xFF); > } else { > - dmi_ver = (buf[14] & 0xF0) << 4 | > - (buf[14] & 0x0F); > pr_info("Legacy DMI %d.%d present.\n", > dmi_ver >> 8, dmi_ver & 0xFF); > } > > The basic idea is right, but you ignore the case dmi_walk_early may fail, though looks impossible when bootup. Better to add below for robust. @@ -521,6 +521,6 @@ static int __init dmi_present(const u8 * return 0; } } + dmi_ver = 0; return 1; } zduan -- 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/