Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1278464 > unrolled thread

Re: [PATCH] Save SMBIOS Type 9 System Slots during DMI Scan

Started byJordan Hargrave <jharg93@gmail.com>
First post2015-11-26 23:10 +0100
Last post2015-11-30 12:10 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

Contents

  Re: [PATCH] Save SMBIOS Type 9 System Slots during DMI Scan Jordan Hargrave <jharg93@gmail.com> - 2015-11-26 23:10 +0100
    Re: [PATCH] Save SMBIOS Type 9 System Slots during DMI Scan Jean Delvare <jdelvare@suse.de> - 2015-11-27 13:10 +0100
      Re: [PATCH] Save SMBIOS Type 9 System Slots during DMI Scan Jean Delvare <jdelvare@suse.de> - 2015-11-28 09:50 +0100
        Re: [PATCH] Save SMBIOS Type 9 System Slots during DMI Scan Jean Delvare <jdelvare@suse.de> - 2015-11-30 12:10 +0100

#1278464 — Re: [PATCH] Save SMBIOS Type 9 System Slots during DMI Scan

FromJordan Hargrave <jharg93@gmail.com>
Date2015-11-26 23:10 +0100
SubjectRe: [PATCH] Save SMBIOS Type 9 System Slots during DMI Scan
Message-ID<qzcTM-2Dt-21@gated-at.bofh.it>
PCI address of onboard devices is currently saved but not for slots.

Created common onboard/slot save function. Verify that bus address is
not invalid.

Signed-off-by: Jordan Hargrave <jordan_hargrave@dell.com>
---
 drivers/firmware/dmi_scan.c | 53 +++++++++++++++++++++++++++++++--------------
 include/linux/dmi.h         |  1 +
 2 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index ac1ce4a..c6b2828 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -321,26 +321,30 @@ static void __init dmi_save_ipmi_device(const struct dmi_header *dm)
 	list_add_tail(&dev->list, &dmi_devices);
 }
 
-static void __init dmi_save_dev_onboard(int instance, int segment, int bus,
-					int devfn, const char *name)
+static void __init dmi_save_dev_pciaddr(int instance, int segment, int bus,
+					int devfn, const char *name, int type)
 {
-	struct dmi_dev_onboard *onboard_dev;
+	struct dmi_dev_onboard *dev;
 
-	onboard_dev = dmi_alloc(sizeof(*onboard_dev) + strlen(name) + 1);
-	if (!onboard_dev)
+	/* Ignore invalid values */
+	if ((bus == 0 && devfn == 0) || (bus == 0xFF && devfn == 0xFF))
 		return;
 
-	onboard_dev->instance = instance;
-	onboard_dev->segment = segment;
-	onboard_dev->bus = bus;
-	onboard_dev->devfn = devfn;
+	dev = dmi_alloc(sizeof(*dev) + strlen(name) + 1);
+	if (!dev)
+		return;
 
-	strcpy((char *)&onboard_dev[1], name);
-	onboard_dev->dev.type = DMI_DEV_TYPE_DEV_ONBOARD;
-	onboard_dev->dev.name = (char *)&onboard_dev[1];
-	onboard_dev->dev.device_data = onboard_dev;
+	dev->instance = instance;
+	dev->segment = segment;
+	dev->bus = bus;
+	dev->devfn = devfn;
 
-	list_add(&onboard_dev->dev.list, &dmi_devices);
+	strcpy((char *)&dev[1], name);
+	dev->dev.type = type;
+	dev->dev.name = (char *)&dev[1];
+	dev->dev.device_data = dev;
+
+	list_add(&dev->dev.list, &dmi_devices);
 }
 
 static void __init dmi_save_extended_devices(const struct dmi_header *dm)
@@ -351,11 +355,25 @@ static void __init dmi_save_extended_devices(const struct dmi_header *dm)
 	if ((*d & 0x80) == 0)
 		return;
 
-	dmi_save_dev_onboard(*(d+1), *(u16 *)(d+2), *(d+4), *(d+5),
-			     dmi_string_nosave(dm, *(d-1)));
+	dmi_save_dev_pciaddr(*(d+1), *(u16 *)(d+2), *(d+4), *(d+5),
+			     dmi_string_nosave(dm, *(d-1)),
+			     DMI_DEV_TYPE_DEV_ONBOARD);
 	dmi_save_one_device(*d & 0x7f, dmi_string_nosave(dm, *(d - 1)));
 }
 
+static void __init dmi_save_system_slot(const struct dmi_header *dm)
+{
+	const u8 *d = (u8 *)dm;
+
+	/* Need SMBIOS 2.6+ structure */
+	if (dm->length < 0x11)
+		return;
+	dmi_save_dev_pciaddr(*(u16 *)(d + 0x9), *(u16 *)(d + 0xD),
+			     *(d + 0xF), *(d + 0x10),
+			     dmi_string_nosave(dm, *(d + 0x4)),
+			     DMI_DEV_TYPE_DEV_SLOT);
+}
+
 static void __init count_mem_devices(const struct dmi_header *dm, void *v)
 {
 	if (dm->type != DMI_ENTRY_MEM_DEVICE)
@@ -426,6 +444,9 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
 		dmi_save_ident(dm, DMI_CHASSIS_SERIAL, 7);
 		dmi_save_ident(dm, DMI_CHASSIS_ASSET_TAG, 8);
 		break;
+	case 9:		/* System Slots */
+	       dmi_save_system_slot(dm);
+	       break;
 	case 10:	/* Onboard Devices Information */
 		dmi_save_devices(dm);
 		break;
diff --git a/include/linux/dmi.h b/include/linux/dmi.h
index 5055ac3..5e9c74c 100644
--- a/include/linux/dmi.h
+++ b/include/linux/dmi.h
@@ -22,6 +22,7 @@ enum dmi_device_type {
 	DMI_DEV_TYPE_IPMI = -1,
 	DMI_DEV_TYPE_OEM_STRING = -2,
 	DMI_DEV_TYPE_DEV_ONBOARD = -3,
+	DMI_DEV_TYPE_DEV_SLOT = -4,
 };
 
 enum dmi_entry_type {
-- 
1.9.1

--
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/

[toc] | [next] | [standalone]

#1278774

FromJean Delvare <jdelvare@suse.de>
Date2015-11-27 13:10 +0100
Message-ID<qzq0G-2Gm-7@gated-at.bofh.it>
In reply to#1278464
Hi Jordan,

When sending a new version of a patch, please start a new thread, and
tag it with [PATCH v2] instead of [PATCH]. A changelog before the
diffstat is also appreciated.

On Thu, 26 Nov 2015 16:02:24 -0600, Jordan Hargrave wrote:
> PCI address of onboard devices is currently saved but not for slots.
> 
> Created common onboard/slot save function. Verify that bus address is
> not invalid.
> 
> Signed-off-by: Jordan Hargrave <jordan_hargrave@dell.com>
> ---
>  drivers/firmware/dmi_scan.c | 53 +++++++++++++++++++++++++++++++--------------
>  include/linux/dmi.h         |  1 +
>  2 files changed, 38 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index ac1ce4a..c6b2828 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -321,26 +321,30 @@ static void __init dmi_save_ipmi_device(const struct dmi_header *dm)
>  	list_add_tail(&dev->list, &dmi_devices);
>  }
>  
> -static void __init dmi_save_dev_onboard(int instance, int segment, int bus,
> -					int devfn, const char *name)
> +static void __init dmi_save_dev_pciaddr(int instance, int segment, int bus,
> +					int devfn, const char *name, int type)
>  {
> -	struct dmi_dev_onboard *onboard_dev;
> +	struct dmi_dev_onboard *dev;
>  
> -	onboard_dev = dmi_alloc(sizeof(*onboard_dev) + strlen(name) + 1);
> -	if (!onboard_dev)
> +	/* Ignore invalid values */
> +	if ((bus == 0 && devfn == 0) || (bus == 0xFF && devfn == 0xFF))
>  		return;

As discussed before, this is questionable, as you may filter out valid
entries (especially as you do not check the segment value.) It's
probably better to record invalid entries than to take the risk of
filtering out valid ones?

Also I said before that 00:00.0 could be considered invalid for slots
because it was always an on-board device, but now
dmi_save_dev_pciaddr() will also handle on-board devices. I'm not sure
why someone would want to record the root host bridge in the DMI table,
but the specification allows it.

>  
> -	onboard_dev->instance = instance;
> -	onboard_dev->segment = segment;
> -	onboard_dev->bus = bus;
> -	onboard_dev->devfn = devfn;
> +	dev = dmi_alloc(sizeof(*dev) + strlen(name) + 1);
> +	if (!dev)
> +		return;
>  
> -	strcpy((char *)&onboard_dev[1], name);
> -	onboard_dev->dev.type = DMI_DEV_TYPE_DEV_ONBOARD;
> -	onboard_dev->dev.name = (char *)&onboard_dev[1];
> -	onboard_dev->dev.device_data = onboard_dev;
> +	dev->instance = instance;
> +	dev->segment = segment;
> +	dev->bus = bus;
> +	dev->devfn = devfn;
>  
> -	list_add(&onboard_dev->dev.list, &dmi_devices);
> +	strcpy((char *)&dev[1], name);
> +	dev->dev.type = type;
> +	dev->dev.name = (char *)&dev[1];
> +	dev->dev.device_data = dev;
> +
> +	list_add(&dev->dev.list, &dmi_devices);
>  }
>  
>  static void __init dmi_save_extended_devices(const struct dmi_header *dm)
> @@ -351,11 +355,25 @@ static void __init dmi_save_extended_devices(const struct dmi_header *dm)
>  	if ((*d & 0x80) == 0)
>  		return;
>  
> -	dmi_save_dev_onboard(*(d+1), *(u16 *)(d+2), *(d+4), *(d+5),
> -			     dmi_string_nosave(dm, *(d-1)));
> +	dmi_save_dev_pciaddr(*(d+1), *(u16 *)(d+2), *(d+4), *(d+5),
> +			     dmi_string_nosave(dm, *(d-1)),
> +			     DMI_DEV_TYPE_DEV_ONBOARD);
>  	dmi_save_one_device(*d & 0x7f, dmi_string_nosave(dm, *(d - 1)));
>  }
>  
> +static void __init dmi_save_system_slot(const struct dmi_header *dm)
> +{
> +	const u8 *d = (u8 *)dm;
> +
> +	/* Need SMBIOS 2.6+ structure */
> +	if (dm->length < 0x11)
> +		return;
> +	dmi_save_dev_pciaddr(*(u16 *)(d + 0x9), *(u16 *)(d + 0xD),
> +			     *(d + 0xF), *(d + 0x10),
> +			     dmi_string_nosave(dm, *(d + 0x4)),
> +			     DMI_DEV_TYPE_DEV_SLOT);
> +}
> +
>  static void __init count_mem_devices(const struct dmi_header *dm, void *v)
>  {
>  	if (dm->type != DMI_ENTRY_MEM_DEVICE)
> @@ -426,6 +444,9 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
>  		dmi_save_ident(dm, DMI_CHASSIS_SERIAL, 7);
>  		dmi_save_ident(dm, DMI_CHASSIS_ASSET_TAG, 8);
>  		break;
> +	case 9:		/* System Slots */
> +	       dmi_save_system_slot(dm);
> +	       break;

Indentation is broken, you used spaces instead of tabs.

>  	case 10:	/* Onboard Devices Information */
>  		dmi_save_devices(dm);
>  		break;
> diff --git a/include/linux/dmi.h b/include/linux/dmi.h
> index 5055ac3..5e9c74c 100644
> --- a/include/linux/dmi.h
> +++ b/include/linux/dmi.h
> @@ -22,6 +22,7 @@ enum dmi_device_type {
>  	DMI_DEV_TYPE_IPMI = -1,
>  	DMI_DEV_TYPE_OEM_STRING = -2,
>  	DMI_DEV_TYPE_DEV_ONBOARD = -3,
> +	DMI_DEV_TYPE_DEV_SLOT = -4,
>  };
>  
>  enum dmi_entry_type {


-- 
Jean Delvare
SUSE L3 Support
--
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/

[toc] | [prev] | [next] | [standalone]

#1279114

FromJean Delvare <jdelvare@suse.de>
Date2015-11-28 09:50 +0100
Message-ID<qzJmF-6xW-5@gated-at.bofh.it>
In reply to#1278774
Hi Jordan,

Once again: please keep the list included in your replies. Others may
be able to help. Also the list is archived for later reference.

On Fri, 27 Nov 2015 20:45:21 -0600, Jordan Hargrave wrote:
> On Fri, Nov 27, 2015 at 6:04 AM, Jean Delvare <jdelvare@suse.de> wrote:
> > When sending a new version of a patch, please start a new thread, and
> > tag it with [PATCH v2] instead of [PATCH]. A changelog before the
> > diffstat is also appreciated.
> 
> Hmm, not sure how to do that, it always generates N diffs for me whenever I
> do a new commit.

I can't parse this, sorry.

> > As discussed before, this is questionable, as you may filter out valid
> > entries (especially as you do not check the segment value.) It's
> > probably better to record invalid entries than to take the risk of
> > filtering out valid ones?
> 
> Segment is even more broken. it's either 0 or ffff, and bus maybe 0 or ff
> on top of that.
> And are there even any machines that use segments anyway? The last one I
> know of was 10+ years ago, and didn't support SMBIOS 2.6

We have a few machines in the SUSE Labs network which use PCI segments.
SGI UV3000 uses segments 0000, 0001, 1000, 1001, 1002 and 1003, and it
implements SMBIOS 2.7. SGI UV100 uses segments 0000, 0001, 1000 and
1001. Fujitsu PrimeQuest 2800E uses segments 0000 and 0001, and it
implements SMBIOS 2.7.

Also SGI Altix 450 (ia64) uses PCI segments 0001, 0002, 0011, 0012 and
0021. It doesn't seem to have a valid DMI table though.

So yes, PCI bus segments are used, although not often.

> > Also I said before that 00:00.0 could be considered invalid for slots
> > because it was always an on-board device, but now
> > dmi_save_dev_pciaddr() will also handle on-board devices. I'm not sure
> > why someone would want to record the root host bridge in the DMI table,
> > but the specification allows it.
>
> Never seen 00:00.0 as a valid value.  Yeah according to spec it's not
> invalid.... but real world BIOS is different.

It is the PCI root bridge on almost every x86 system I've seen, and
a few ia64 and arm64 systems too:

00:00.0  Host bridge [0600]: Intel Corporation 5520/5500/X58 I/O Hub to ESI Port [8086:3407] (rev 22)
00:00.0  Host bridge [0600]: Intel Corporation 5400 Chipset Memory Controller Hub [8086:4003] (rev 20)
00:00.0  Host bridge: Intel Corporation 5500 I/O Hub to ESI Port (rev 13)
00:00.0  Host bridge [0600]: Intel Corporation Xeon E7 v2/Xeon E5 v2/Core i7 DMI2 [8086:0e00] (rev 07)
00:00.0  Host bridge: ATI Technologies Inc RS690 Host Bridge
00:00.0  Host bridge [0600]: Advanced Micro Devices [AMD] nee ATI RD890 PCI to PCI bridge (external gfx0 port A) [1002:5a13] (rev 02)
00:00.0  Host bridge [0600]: ATI Technologies Inc RD890 Northbridge only dual slot (2x16) PCI-e GFX Hydra part [1002:5a10] (rev 02)
00:00.0  Host bridge: ATI Technologies Inc RD890 Northbridge only dual slot (2x16) PCI-e GFX Hydra part (rev 02)
00:00.0  Host bridge [0600]: Advanced Micro Devices, Inc. [AMD] Device [1022:1a00]

-- 
Jean Delvare
SUSE L3 Support
--
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/

[toc] | [prev] | [next] | [standalone]

#1279771

FromJean Delvare <jdelvare@suse.de>
Date2015-11-30 12:10 +0100
Message-ID<qAuvf-2LA-13@gated-at.bofh.it>
In reply to#1279114
Hi Jordan,

Le Saturday 28 November 2015 à 20:54 -0600, Jordan Hargrave a écrit :
> 
> 
> On Sat, Nov 28, 2015 at 2:46 AM, Jean Delvare <jdelvare@suse.de>
> wrote:
>         Hi Jordan,
>         
>         Once again: please keep the list included in your replies.
>         Others may
>         be able to help. Also the list is archived for later
>         reference.
>         
> 
> 
> The list wasn't in the cc:?  I sent the diff with git send-email --cc

When you send the patches, the list is there. However when I review and
then you reply using your regular email client, this is HTML and list is
dropped. Not good.

>(...)
>         We have a few machines in the SUSE Labs network which use PCI
>         segments.
>         SGI UV3000 uses segments 0000, 0001, 1000, 1001, 1002 and
>         1003, and it
>         implements SMBIOS 2.7. SGI UV100 uses segments 0000, 0001,
>         1000 and
>         1001. Fujitsu PrimeQuest 2800E uses segments 0000 and 0001,
>         and it
>         implements SMBIOS 2.7.
>         
>         Also SGI Altix 450 (ia64) uses PCI segments 0001, 0002, 0011,
>         0012 and
>         0021. It doesn't seem to have a valid DMI table though.
>         
>         So yes, PCI bus segments are used, although not often.
> 
> 
> Can you send me a full dmidecode -u and lspci -vvvxxxx on those
> systems? 

Sure, will send privately.


-- 
Jean Delvare
SUSE L3 Support

--
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web