Skip to contentSkip to conversation

dmi type 0xB1 record - unknown flag

DISCUSSION Applied Jean Delvare 3 participants 9 messages Jun 1, 2017 – Jun 16, 2017

Subsystems (2)
x86 PCI
lore
Share & tools
lore

Jean Delvare

author
#0 lore raw

Hi all,

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?

Thanks,

Signature · 2 lines
Jean Delvare
SUSE L3 Support
#1 lore raw
Quoted 11 lines

-----Original Message-----
From: Jean Delvare [mailto:[email protected]]
Sent: Thursday, June 1, 2017 6:08 PM
To: [email protected]; [email protected]
Cc: K, Narendra <[email protected]>; Hargrave, Jordan <[email protected]>; Iyer, Shyam <[email protected]>; Bjorn Helgaas <[email protected]>
Subject: dmi type 0xB1 record - unknown flag
=20
Hi all,
=20

Hi Jean,

Quoted 8 lines

I see the following message in my kernel log:
=20
dmi type 0xB1 record - unknown flag
=20
This is on a Dell Optiplex 9020 workstation. I see the message comes from:
=20
static void __init read_dmi_type_b1(const struct dmi_header *dm,

Quoted 5 lines
                                    void *private_data) {
	(...)
        switch (((*(u32 *)d) >> 9) & 0x03) {
        case 0x00:
                printk(KERN_INFO "dmi type 0xB1 record - unknown flag\n")=

;

Quoted 3 lines
                break;

=20
What is the value of this message? Is there anything which needs to be do=

ne

Quoted 1 line

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'.

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.

There is no additional handling required for such a system.

With regards,
Narendra K

#2 lore raw
Quoted 41 lines · On Thu, Jun 01, 2017 at 01:28:31PM +0000, [email protected] wrote:
Quoted 11 lines

-----Original Message-----
From: Jean Delvare [mailto:[email protected]]
Sent: Thursday, June 1, 2017 6:08 PM
To: [email protected]; [email protected]
Cc: K, Narendra <[email protected]>; Hargrave, Jordan <[email protected]>; Iyer, Shyam <[email protected]>; Bjorn Helgaas <[email protected]>
Subject: dmi type 0xB1 record - unknown flag

Hi all,

Hi Jean,

Quoted 15 lines

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;
Quoted 2 lines

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'.

Quoted 4 lines

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.

There is no additional handling required for such a system.

That was added by 6e8af08dfa40 ("PCI: enable pci=bfsort by default on future Dell systems").

The fact that BIOS supplied a B1 record with an unknown value (0) looks like a Dell BIOS defect, which is ironic, considering that Dell added this thing in the first place.

I think we should remove the message, since it is alarming to users and they can't do anything about it anyway.

Bjorn

#3 lore raw
Quoted 5 lines

-----Original Message-----
From: Bjorn Helgaas [mailto:[email protected]]
Sent: Thursday, June 1, 2017 10:30 PM
To: K, Narendra <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; Hargrave=

,

Quoted 3 lines

Jordan <[email protected]>; Iyer, Shyam <[email protected]>; [email protected]
Subject: Re: dmi type 0xB1 record - unknown flag

[...]

Quoted 9 lines
Quoted 4 lines

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.

There is no additional handling required for such a system.

=20
That was added by 6e8af08dfa40 ("PCI: enable pci=3Dbfsort by default on future Dell systems").
=20
The fact that BIOS supplied a B1 record with an unknown value (0) looks l=

ike a

Quoted 1 line

Dell BIOS defect, which is ironic, considering that Dell added this thing=

in the

Quoted 3 lines

first place.
=20
I think we should remove the message, since it is alarming to users and t=

hey

Quoted 2 lines

can't do anything about it anyway.
=20

Hi Bjorn,

Thank you for the feedback. We will submit a patch to remove the message.=20

With regards,
Narendra K

author
#4 lore raw
In reply to [email protected]

Hi Narendra,

Thanks for your answer.

Quoted 17 lines · On Thu, 1 Jun 2017 13:28:31 +0000, [email protected] wrote:
Quoted 17 lines

-----Original Message-----
From: Jean Delvare [mailto:[email protected]]
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;
Quoted 7 lines
Quoted 2 lines

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 ;-)

Quoted 1 line

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.

Quoted 1 line

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 <[email protected]>
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 <[email protected]>
---
 arch/x86/pci/common.c |   27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)
arch/x86/pci/common.c +5 -22
@@ -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;
 }
 
 /*

What do you think?

Thanks,

Signature · 2 lines
Jean Delvare
SUSE L3 Support
#5 lore raw
Quoted 8 lines

-----Original Message-----
From: Jean Delvare [mailto:[email protected]]
Sent: Friday, June 2, 2017 7:43 PM
To: K, Narendra <[email protected]>
Cc: [email protected]; [email protected]; Hargrave, Jordan <[email protected]>; Iyer, Shyam <[email protected]>; [email protected]
Subject: Re: dmi type 0xB1 record - unknown flag

[...]

Quoted 1 line
Quoted 1 line

The value 0x00 is not a valid value. When the flag is 0x00, the sort me=

thod

Quoted 3 lines

will be the default that is decided by the kernel.
=20
How can it be invalid? I have DMI dumps of several Dell systems with a ty=

pe

Quoted 1 line

0xB1 DMI structure, with value 0x00 for these bits. If Dell ships such sy=

stems,

Quoted 5 lines

then this is valid by definition.
=20

Quoted 1 line

There is no additional handling required for such a system.

=20
I don't understand the complexity of the code. There are 4 possible value=

s

Quoted 10 lines

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.
=20
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 pre=

sent;

Quoted 4 lines

but I don't think this is supposed to happen in the first place. =20
Lastly I'm not sure why you continue processing the list of DMI matches, when smbios_type_b1_flag =3D=3D 1, and stop processing it if not.

Quoted 8 lines

This seems needlessly inconsistent.
=20
I think the whole thing can be simplified like this:
=20
From: Jean Delvare <[email protected]>
Subject: x86/PCI: Simplify Dell DMI B1 quirk
=20
No need for such convoluted code, when all we need is to call one functio=

n in

arch/x86/pci/common.c static int __init set_bf_sort(const stru static vo=

Quoted 19 lines

one specific case.
=20

Signed-off-by Jean Delvare <[email protected]>
---
 arch/x86/pci/common.c |   27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

=20
--- 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

(quoted diff) +0 -0
@@ -24,7 +24,6 @@ unsigned int pci_probe =3D PCI_PROBE_BIOS

=20
unsigned int pci_early_dump_regs;
static int pci_bf_sort;

arch/x86/pci/common.c +0 -1
-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 vo=

id

Quoted 36 lines
(quoted diff) +1 -1
 __init read_dmi_type_b1(const struct dmi_header *dm,
 				    void *private_data)
 {
-	u8 *d =3D (u8 *)dm + 4;
+	u8 *data =3D (u8 *)dm + 4;

=20

arch/x86/pci/common.c +2 -13
 	if (dm->type !=3D 0xB1)
 		return;
-	switch (((*(u32 *)d) >> 9) & 0x03) {
-	case 0x00:
-		printk(KERN_INFO "dmi type 0xB1 record - unknown flag\n");
-		break;
-	case 0x01: /* set pci=3Dbfsort */
-		smbios_type_b1_flag =3D 1;
-		break;
-	case 0x02: /* do not set pci=3Dbfsort */
-		smbios_type_b1_flag =3D 2;
-		break;
-	default:
-		break;
-	}
+	if ((((*(u32 *)data) >> 9) & 0x03) =3D=3D 0x01)
+		set_bf_sort((const struct dmi_system_id *)private_data);
 }

=20
static int __init find_sort_method(const struct dmi_system_id *d) {

arch/x86/pci/common.c +2 -7
-	dmi_walk(read_dmi_type_b1, NULL);
-
-	if (smbios_type_b1_flag =3D=3D 1) {
-		set_bf_sort(d);
-		return 0;
-	}
-	return -1;
+	dmi_walk(read_dmi_type_b1, (void *)d);
+	return 0;
 }
Quoted 4 lines

=20
/*
=20
What do you think?

Hi Jean,

Thank you for reviewing and providing feedback. We will look into this and = provide feedback.

With regards,
Narendra K

Applied
#6 lore raw
In reply to Jean Delvare
Quoted 10 lines · Jean Delvare

...
I think the whole thing can be simplified like this:

From: Jean Delvare <[email protected]>
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 <[email protected]>

Applied to pci/misc for v4.13, thanks!

As far as I can tell, the new code is functionally equivalent to the old code, so I don't think there's any risk here.

Quoted 61 lines
---
 arch/x86/pci/common.c |   27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)
arch/x86/pci/common.c +5 -22
@@ -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;
 }
 
 /*

What do you think?

Thanks,

Signature · 2 lines
Jean Delvare
SUSE L3 Support
#7 lore raw
Quoted 16 lines

-----Original Message-----
From: Bjorn Helgaas [mailto:[email protected]]
Sent: Friday, June 16, 2017 3:10 AM
To: Jean Delvare <[email protected]>
Cc: K, Narendra <[email protected]>; [email protected]; linux- [email protected]; Hargrave, Jordan <[email protected]>; Iyer, Shyam <[email protected]>; [email protected]
Subject: Re: dmi type 0xB1 record - unknown flag
=20

Quoted 6 lines · Jean Delvare

...
I think the whole thing can be simplified like this:

From: Jean Delvare <[email protected]>
Subject: x86/PCI: Simplify Dell DMI B1 quirk

Quoted 8 lines
Quoted 4 lines

No need for such convoluted code, when all we need is to call one function in one specific case.

Signed-off-by Jean Delvare <[email protected]>

=20
Applied to pci/misc for v4.13, thanks!
=20
As far as I can tell, the new code is functionally equivalent to the old =

code, so

Quoted 2 lines

I don't think there's any risk here.
=20

Hi Jean/Bjorn,

Sorry for the delay. I applied the patch to the mainline kernel version 4.1= 2-rc5 and performed sanity test on a DellEMC PowerEdge R730XD system. It = is working as=20
expected. 'dmesg' shows the message that 'pci=3Dbfsort' is enabled.

I also booted kernel version 4.12-rc5 with patch applied on a DellEMC Power= Edge 1950 system which is listed in the ' pciprobe_dmi_table' quicks table. It is working as expected and has not caused regression on this system. 'dm= esg' shows the message that 'pci=3Dbfsort' is enabled.

[...]

Quoted 2 lines
Quoted 2 lines

What do you think?

=20
Thank you for simplifying the code and for the patch.=20

With regards,
Narendra K

author
#8 lore raw
Quoted 5 lines · On ven., 2017-06-16 at 09:36 +0000, [email protected] wrote:

Sorry for the delay. I applied the patch to the mainline kernel version 4.12-rc5 and performed sanity test on a DellEMC PowerEdge R730XD system. It is working as expected. 'dmesg' shows the message that 'pci=bfsort' is enabled.

I also booted kernel version 4.12-rc5 with patch applied on a DellEMC PowerEdge 1950 system which is listed in the ' pciprobe_dmi_table' quicks table. It is working as expected and has not caused regression on this system. 'dmesg' shows the message that 'pci=bfsort' is enabled.

Thank you for testing it on real systems, this is very useful and appreciated.

Signature · 2 lines
Jean Delvare
SUSE L3 Support