From patchwork Fri Jun 2 14:13:11 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean Delvare X-Patchwork-Id: 9762601 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3004260363 for ; Fri, 2 Jun 2017 14:13:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 21A9428557 for ; Fri, 2 Jun 2017 14:13:22 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1498C28565; Fri, 2 Jun 2017 14:13:22 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7567C28557 for ; Fri, 2 Jun 2017 14:13:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750955AbdFBONP (ORCPT ); Fri, 2 Jun 2017 10:13:15 -0400 Received: from mx2.suse.de ([195.135.220.15]:50476 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750813AbdFBONO (ORCPT ); Fri, 2 Jun 2017 10:13:14 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 20B08ABBD; Fri, 2 Jun 2017 14:13:13 +0000 (UTC) Date: Fri, 2 Jun 2017 16:13:11 +0200 From: Jean Delvare To: Cc: , , , , Subject: Re: dmi type 0xB1 record - unknown flag Message-ID: <20170602161311.27eab72d@endymion> In-Reply-To: <68181b62786540ed8ca6ab877151a98b@BLRX13MDC105.AMER.DELL.COM> References: <20170601143826.5f980444@endymion> <68181b62786540ed8ca6ab877151a98b@BLRX13MDC105.AMER.DELL.COM> Organization: SUSE Linux X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.31; x86_64-suse-linux-gnu) MIME-Version: 1.0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Narendra, Thanks for your answer. On Thu, 1 Jun 2017 13:28:31 +0000, Narendra.K@dell.com wrote: > > -----Original Message----- > > From: Jean Delvare [mailto:jdelvare@suse.de] > > I see the following message in my kernel log: > > > > dmi type 0xB1 record - unknown flag > > > > This is on a Dell Optiplex 9020 workstation. I see the message comes > > from: > > > > static void __init read_dmi_type_b1(const struct dmi_header *dm, > > void *private_data) { > > (...) > > switch (((*(u32 *)d) >> 9) & 0x03) { > > case 0x00: > > printk(KERN_INFO "dmi type 0xB1 record - unknown flag\n"); > > break; > > > > What is the value of this message? Is there anything which needs to be done > > to properly support such systems? > > This function was added to avoid adding systems to the 'pciprobe_dmi_table' and set breadth first sorting in a generic way. > > This flag is a hint to indicate the sort method to be used. The value 0x01 indicates that PCI breadth first sort be used. > ' find_sort_method' function checks if smbios_type_b1_flag is set to 1 and if yes, calls 'set_bf_sort '. This function sets ' pci_bf_sort' to 'pci_dmi_bf'. I can read the code, thank you ;-) > The value 0x00 is not a valid value. When the flag is 0x00, the sort method will be the default that is decided by the kernel. How can it be invalid? I have DMI dumps of several Dell systems with a type 0xB1 DMI structure, with value 0x00 for these bits. If Dell ships such systems, then this is valid by definition. > There is no additional handling required for such a system. I don't understand the complexity of the code. There are 4 possible values for these bits, the code treats all of them differently: * 0x01, you set smbios_type_b1_flag to 1 and this later triggers a call to set_bf_sort(). * 0x02, you set smbios_type_b1_flag to 2 but this has no effect. * 0x00, you print a rather cryptic info message which serves no purpose. * 0x03, you do nothing. On top of that, I can't see the value of the intermediate variable smbios_type_b1_flag. The only situations where it would make a difference is if multiple type 0xB1 structures with conflicting information were present; but I don't think this is supposed to happen in the first place. Lastly I'm not sure why you continue processing the list of DMI matches, when smbios_type_b1_flag == 1, and stop processing it if not. This seems needlessly inconsistent. I think the whole thing can be simplified like this: From: Jean Delvare Subject: x86/PCI: Simplify Dell DMI B1 quirk No need for such convoluted code, when all we need is to call one function in one specific case. Signed-off-by: Jean Delvare --- arch/x86/pci/common.c | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) What do you think? Thanks, --- linux-4.11.orig/arch/x86/pci/common.c 2017-05-01 04:47:48.000000000 +0200 +++ linux-4.11/arch/x86/pci/common.c 2017-06-02 16:04:05.737889598 +0200 @@ -24,7 +24,6 @@ unsigned int pci_probe = PCI_PROBE_BIOS unsigned int pci_early_dump_regs; static int pci_bf_sort; -static int smbios_type_b1_flag; int pci_routeirq; int noioapicquirk; #ifdef CONFIG_X86_REROUTE_FOR_BROKEN_BOOT_IRQS @@ -197,34 +196,18 @@ static int __init set_bf_sort(const stru static void __init read_dmi_type_b1(const struct dmi_header *dm, void *private_data) { - u8 *d = (u8 *)dm + 4; + u8 *data = (u8 *)dm + 4; if (dm->type != 0xB1) return; - switch (((*(u32 *)d) >> 9) & 0x03) { - case 0x00: - printk(KERN_INFO "dmi type 0xB1 record - unknown flag\n"); - break; - case 0x01: /* set pci=bfsort */ - smbios_type_b1_flag = 1; - break; - case 0x02: /* do not set pci=bfsort */ - smbios_type_b1_flag = 2; - break; - default: - break; - } + if ((((*(u32 *)data) >> 9) & 0x03) == 0x01) + set_bf_sort((const struct dmi_system_id *)private_data); } static int __init find_sort_method(const struct dmi_system_id *d) { - dmi_walk(read_dmi_type_b1, NULL); - - if (smbios_type_b1_flag == 1) { - set_bf_sort(d); - return 0; - } - return -1; + dmi_walk(read_dmi_type_b1, (void *)d); + return 0; } /*