: SMBIOS: Add initial code and export version via sysfs
Subsystems (2)
PATCH 1/1 : SMBIOS: Add initial code and export version via sysfs
The existing DMI code, drivers/firmware/dmi.c, is not really parsing DMI. It is actually SMBIOS code that is using the old DMI infrastructure to expose SMBIOS entries.
We should be doing the following:
1. Looking for SMBIOS (either EFI or mapped to it's standard location,
0xF0000)
2. If found, look for the SMBIOS' Intermediate Structure (aka "DMI" entry)
3. If not found, look for an "old" DMI structure.
DMI is a predecessor of SMBIOS, so we should start treating it as such.
For this patch I have introduced drivers/firmware/smbios.c, exported the SMBIOS version through sysfs, and modified the DMI code such that
a) dmi_scan_machine() is now called from init_smbios(), and
b) if an SMBIOS is found, the values in the SMBIOS STEP structure are used in dmi_scan_machine().
Right now, removing the DMI code is not an option. It is used by common initscripts, etc., to bringup the system. Later modifications in this area will include additional parsing of the SMBIOS structs and a simultaneous cleanup of the DMI code.
arch/ia64/Kconfig +5 -0
@@ -94,6 +94,11 @@ config HAVE_SETUP_PER_CPU_AREA config DMI bool + depends on SMBIOS + default y + +config SMBIOS + bool default y config EFI
arch/ia64/include/asm/smbios.h new file +12 -0
@@ -0,0 +1,12 @@ +#ifndef _ASM_SMBIOS_H +#define _ASM_SMBIOS_H + +#include <linux/slab.h> +#include <asm/io.h> + +/* Use normal IO mappings for SMBIOS */ +#define smbios_ioremap ioremap +#define smbios_iounmap (x, l) iounmap(x) +#define smbios_alloc (l) kmalloc(l, GFP_ATOMIC) + +#endif
arch/ia64/kernel/setup.c +3 -3
@@ -1061,9 +1061,9 @@ check_bugs (void) (unsigned long) __end___mckinley_e9_bundles); } -static int __init run_dmi_scan (void ) +static int __init run_smbios_scan (void ) { -dmi_scan_machine (); +init_smbios ();return 0 ; } -core_initcall (run_dmi_scan); +core_initcall (run_smbios_scan);
arch/x86/Kconfig +12 -0
@@ -637,6 +637,7 @@ config APB_TIMER # The code disables itself when not needed. config DMI default y + depends on SMBIOS bool "Enable DMI scanning" if EXPERT ---help--- Enabled scanning of DMI to identify machine quirks. Say Y
@@ -644,6 +645,17 @@ config DMI affected by entries in the DMI blacklist. Required by PNP BIOS code. +# Mark as expert because too many people got it wrong. +# The code disables itself when not needed. +config SMBIOS + default y + bool "Enable SMBIOS scan" if EXPERT + ---help--- + Enables scanning of SMBIOS to identify machine quirks. Say Y + here unless you have verified that your setup is not + affected by entries in the DMI and SMBIOS blacklists. Required by + PNP BIOS code + config GART_IOMMU bool "GART IOMMU support" if EXPERT default y
arch/x86/include/asm/smbios.h new file +19 -0
@@ -0,0 +1,19 @@ +#ifndef _ASM_X86_SMBIOS_H +#define _ASM_X86_SMBIOS_H + +#include <linux/compiler.h> +#include <linux/init.h> + +#include <asm/io.h> +#include <asm/setup.h> + +static __always_inline __init void *smbios_alloc(unsigned len) +{ +return extend_brk (len,sizeof (int )); +} + +/* Use early IO mappings for SMIBIOS because it's initialized early */ +#define smbios_ioremap early_ioremap +#define smbios_iounmap early_iounmap + +#endif /* _ASM_X86_SMBIOS_H */
arch/x86/kernel/setup.c +6 -2
@@ -68,6 +68,7 @@#include <linux/percpu.h> #include <linux/crash_dump.h> #include <linux/tboot.h> +#include <linux/smbios.h> #include <video/edid.h>
@@ -126,6 +127,9 @@ unsigned long max_pfn_mapped;#ifdef CONFIG_DMI RESERVE_BRK (dmi_alloc,65536 );#endif +#ifdef CONFIG_SMBIOS +RESERVE_BRK (smbios_alloc,65536 ); +#endif static __initdata unsigned long _brk_start = (unsigned long )__brk_base;
@@ -879,11 +883,11 @@ void __init setup_arch(char **cmdline_p)if (efi_enabled)efi_init (); -dmi_scan_machine (); +init_smbios ();/* * VMware detection requires dmi to be available, so this -* needs to be done after dmi_scan_machine, for the BP. +* needs to be done after init_smbios(), for the BP. */ init_hypervisor_platform ();
drivers/firmware/Makefile +1 -0
@@ -11,3 +11,4 @@ obj-$(CONFIG_DMIID) += dmi-id.o obj-$(CONFIG_ISCSI_IBFT_FIND) += iscsi_ibft_find.o obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o obj-$(CONFIG_FIRMWARE_MEMMAP) += memmap.o +obj-$(CONFIG_SMBIOS) += smbios.o
drivers/firmware/dmi_scan.c +18 -29
73 lines not on this page · raw message
drivers/firmware/smbios.c new file +175 -0
175 lines not on this page · raw message
include/linux/smbios.h new file +51 -0
51 lines not on this page · raw message
drivers/firmware/smbios.c line 32
Quoted 4 lines
drivers/firmware/smbios.c new file +4
-0+static struct class smbios_class = { + .name ="smbios" , + .dev_release = (void (*)(struct device *)) kfree, +};
I thought the policy was no new classes- Greg ?
drivers/firmware/smbios.c line 101
Quoted 3 lines
drivers/firmware/smbios.c new file +3
-0+smbios_iounmap (buf,sizeof (*smbios_step)); +printk (KERN_INFO "SMBIOS: Invalid STEP table checksum = %u\n" , + chksum1);
Surely this isn't INFO ?
Quoted 7 lines · Alan Cox
drivers/firmware/smbios.c line 32
Quoted 5 lines
drivers/firmware/smbios.c new file +4
-0+static struct class smbios_class = { + .name ="smbios" , + .dev_release = (void (*)(struct device *)) kfree, +};I thought the policy was no new classes- Greg ?
Greg, eventually the SMBIOS code could replace the mixed DMI/SMBIOS code we have right now. If there is a "no new classes" policy, could you give me an idea of what I should do/use instead? The DMI stuff is currently in its own class ...
Quoted 8 lines
drivers/firmware/smbios.c line 101
Quoted 5 lines
drivers/firmware/smbios.c new file +3
-0+smbios_iounmap (buf,sizeof (*smbios_step)); +printk (KERN_INFO "SMBIOS: Invalid STEP table checksum = %u\n" , + chksum1);Surely this isn't INFO ?
Good point :) I can certainly make that higher or a WARN_ON in the next version.
P.
Quoted 6 lines · Prarit Bhargava
Quoted 3 lines · Alan Cox
drivers/firmware/smbios.c line 31
Quoted 3 lines
drivers/firmware/smbios.c new file +3
-0+static struct class smbios_class = { + .name ="smbios" , + .dev_release = (void (*)(struct device *)) kfree,
Wait, what? Heh, nice try, you can't do this. If you can, your code is doing something really wrong. Or hacky like trying to put the device structure as the first thing in your structure. If that gets moved, this will break badly. Don't.
Quoted 9 lines
Quoted 4 lines
Quoted 2 lines
+};
I thought the policy was no new classes- Greg ?
Greg, eventually the SMBIOS code could replace the mixed DMI/SMBIOS code we have right now. If there is a "no new classes" policy, could you give me an idea of what I should do/use instead? The DMI stuff is currently in its own class ...
Ideally you should use a bus instead. Let me go look at your original patch and get back to you on this...
thanks,
greg k-h
Quoted 4 lines · Prarit Bhargava
The existing DMI code,
drivers/firmware/dmi.c, is not really parsing DMI. It is actually SMBIOS code that is using the old DMI infrastructure to expose SMBIOS entries.
Quoted 8 lines
We should be doing the following:
1. Looking for SMBIOS (either EFI or mapped to it's standard location,
0xF0000)2. If found, look for the SMBIOS' Intermediate Structure (aka "DMI" entry)
3. If not found, look for an "old" DMI structure.DMI is a predecessor of SMBIOS, so we should start treating it as such.
Why? What is this change going to buy us in the end? Heck, right now, what's the benefit?
Quoted 11 lines
For this patch I have introduced
drivers/firmware/smbios.c, exported the SMBIOS version through sysfs, and modified the DMI code such thata)
dmi_scan_machine()is now called frominit_smbios(), and
b) if an SMBIOS is found, the values in the SMBIOS STEP structure are used indmi_scan_machine().Right now, removing the DMI code is not an option. It is used by common initscripts, etc., to bringup the system. Later modifications in this area will include additional parsing of the SMBIOS structs and a simultaneous cleanup of the DMI code.
Care to show follow-on patches that convert things to a manner which you think it should look like?
Quoted 5 lines
drivers/firmware/smbios.c new file +2
-0@@ -0,0 +1,175 @@ +#include <linux/dmi.h> +#include <linux/efi.h>
drivers/firmware/smbios.c line 12
Quoted 10 lines
drivers/firmware/smbios.c new file +10
-0+#include <linux/init.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/smbios.h> + +#include <asm/setup.h> + +int smbios_present =0 ; +struct smbios_step *smbios_step; +static struct device *smbios_dev;
A "raw" struct device? Why? You should never do this, that's not the way to use a 'struct device'.
Let's back up here, what exactly are you trying to show in sysfs? Where are your Documentation/ABI/ entries that show these new files, what they contain, and how to use them?
drivers/firmware/smbios.c line 16
Quoted 4 lines
drivers/firmware/smbios.c new file +4
-0+/* The actual address of the SMBIOS */ +static unsigned long smbios_addr =0 ; + +#ifdef CONFIG_SYSFS
You should never need this in your .c file.
drivers/firmware/smbios.c line 21
Quoted 5 lines
drivers/firmware/smbios.c new file +5
-0+static ssize_t version_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return snprintf (buf,PAGE_SIZE ,"%u.%u\n" , smbios_step->major, + smbios_step->minor);
As you "know" you are only returning a simple value, why the snprintf()? Hint, don't, it's overkill and how do you "know" that the buffer is PAGE_SIZE big? Hint, you don't, so stick to a small buffer like you are, and all is fine.
drivers/firmware/smbios.c line 27
Quoted 6 lines
drivers/firmware/smbios.c new file +6
-0+} + +/* add additional sysfs files here */ +static struct device_attribute smbios_attrs[] = { +__ATTR_RO (version), +};
So a whole new class just for a single version file in sysfs?
I'm confused, what exactly are you trying to show here that you can't show in the existing device/class structure?
drivers/firmware/smbios.c line 31
Quoted 4 lines
drivers/firmware/smbios.c new file +4
-0+ +static struct class smbios_class = { + .name ="smbios" , + .dev_release = (void (*)(struct device *)) kfree,
As stated before, nice try, but no. :)
Quoted 17 lines
drivers/firmware/smbios.c new file +17
-0+}; + +static struct device *smbios_dev; + +static int __init smbios_setup_sysfs (void ) +{ +int attr, i, ret =0 ; + + ret =class_register (&smbios_class); +if (ret) + goto out; + + smbios_dev =kzalloc (sizeof (*smbios_dev),GFP_KERNEL ); +if (!smbios_dev) { + ret = -ENOMEM ; + goto out_cleanup_class; + }
drivers/firmware/smbios.c line 58
Quoted 10 lines
drivers/firmware/smbios.c new file +10
-0+ + smbios_dev->class = &smbios_class; +dev_set_name (smbios_dev,"id" ); + + ret =device_register (smbios_dev); +if (ret) + goto out_cleanup_smbios_dev; + +for (attr =0 ; attr <ARRAY_SIZE (smbios_attrs); attr++) { + ret =device_create_file (smbios_dev, &smbios_attrs[attr]);
You just raced userspace when you created this file, causing it massive problems if it was expecting when the hotplug event happened that this file would be present. Please don't do that, we have the infrastructure to do this properly, please use it.
thanks,
greg k-h
Quoted 13 lines · Greg KH
Quoted 11 lines · Prarit Bhargava
The existing DMI code,
drivers/firmware/dmi.c, is not really parsing DMI. It is actually SMBIOS code that is using the old DMI infrastructure to expose SMBIOS entries.We should be doing the following:
1. Looking for SMBIOS (either EFI or mapped to it's standard location,
0xF0000)2. If found, look for the SMBIOS' Intermediate Structure (aka "DMI" entry)
3. If not found, look for an "old" DMI structure.
Quoted 5 lines
Quoted 2 lines
DMI is a predecessor of SMBIOS, so we should start treating it as such.
Why? What is this change going to buy us in the end? Heck, right now, what's the benefit?
It's wrong. DMI doesn't give us SMBIOS information -- it's the other way around. There's also a bunch of other stuff from SMBIOS being used in the kernel (see pci slots and hotplug drivers).
It would be nice to have a single place for this, no?
Quoted 8 lines
Quoted 8 lines
For this patch I have introduced
drivers/firmware/smbios.c, exported the SMBIOS version through sysfs, and modified the DMI code such thata)
dmi_scan_machine()is now called frominit_smbios(), and
b) if an SMBIOS is found, the values in the SMBIOS STEP structure are used indmi_scan_machine().
Quoted 9 lines
Quoted 5 lines
Right now, removing the DMI code is not an option. It is used by common initscripts, etc., to bringup the system. Later modifications in this area will include additional parsing of the SMBIOS structs and a simultaneous cleanup of the DMI code.
Care to show follow-on patches that convert things to a manner which you think it should look like?
I haven't written anything yet :) -- I was planning on exporting the SMBIOS structs (all types) though.
Quoted 10 lines
Quoted 10 lines
drivers/firmware/smbios.c new file +7
-0@@ -0,0 +1,175 @@ +#include <linux/dmi.h> +#include <linux/efi.h> +#include <linux/init.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/smbios.h> +
Quoted 10 lines
drivers/firmware/smbios.c line 12
Quoted 6 lines
drivers/firmware/smbios.c new file +5
-0+#include <asm/setup.h> + +int smbios_present =0 ; +struct smbios_step *smbios_step; +static struct device *smbios_dev;A "raw" struct device? Why? You should never do this, that's not the way to use a 'struct device'.
Ah, okay -- my familiarity with sysfs is very minimal. I'll ask on kernel-mentors for a pointer to "good" code.
Quoted 4 lines
Let's back up here, what exactly are you trying to show in sysfs? Where are your
Documentation/ABI/entries that show these new files, what they contain, and how to use them?
Obviously missing -- I will add this on my next patch.
Quoted 9 lines
drivers/firmware/smbios.c line 16
Quoted 6 lines
drivers/firmware/smbios.c new file +4
-0+/* The actual address of the SMBIOS */ +static unsigned long smbios_addr =0 ; + +#ifdef CONFIG_SYSFS You should never need this in your .c file.
Will remove.
Quoted 11 lines
drivers/firmware/smbios.c line 21
Quoted 6 lines
drivers/firmware/smbios.c new file +5
-0+static ssize_t version_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return snprintf (buf,PAGE_SIZE ,"%u.%u\n" , smbios_step->major, + smbios_step->minor);As you "know" you are only returning a simple value, why the
snprintf()? Hint, don't, it's overkill and how do you "know" that the buffer isPAGE_SIZEbig? Hint, you don't, so stick to a small buffer like you are, and all is fine.
:) Will do. Thanks :)
Quoted 14 lines
drivers/firmware/smbios.c line 27
Quoted 8 lines
drivers/firmware/smbios.c new file +6
-0+} + +/* add additional sysfs files here */ +static struct device_attribute smbios_attrs[] = { +__ATTR_RO (version), +};So a whole new class just for a single version file in sysfs?
I'm confused, what exactly are you trying to show here that you can't show in the existing device/class structure?
Quoted 8 lines
drivers/firmware/smbios.c line 31
Quoted 5 lines
drivers/firmware/smbios.c new file +4
-0+ +static struct class smbios_class = { + .name ="smbios" , + .dev_release = (void (*)(struct device *)) kfree,As stated before, nice try, but no. :)
Right -- I'll go off and try to figure out the proper way to do this. Sorry ... I was following along with what the existing DMI code does.
Quoted 25 lines
Quoted 25 lines
drivers/firmware/smbios.c new file +24
-0+}; + +static struct device *smbios_dev; + +static int __init smbios_setup_sysfs (void ) +{ +int attr, i, ret =0 ; + + ret =class_register (&smbios_class); +if (ret) + goto out; + + smbios_dev =kzalloc (sizeof (*smbios_dev),GFP_KERNEL ); +if (!smbios_dev) { + ret = -ENOMEM ; + goto out_cleanup_class; + } + + smbios_dev->class = &smbios_class; +dev_set_name (smbios_dev,"id" ); + + ret =device_register (smbios_dev); +if (ret) + goto out_cleanup_smbios_dev;
Quoted 10 lines
drivers/firmware/smbios.c line 58
Quoted 4 lines
drivers/firmware/smbios.c new file +3
-0+ +for (attr =0 ; attr <ARRAY_SIZE (smbios_attrs); attr++) { + ret =device_create_file (smbios_dev, &smbios_attrs[attr]);You just raced userspace when you created this file, causing it massive problems if it was expecting when the hotplug event happened that this file would be present. Please don't do that, we have the infrastructure to do this properly, please use it.
np -- thanks for the info Greg.
P.
Quoted 4 lines
thanks,
greg k-h
Sorry Greg, I missed replying to one of your questions.
Quoted 13 lines
Quoted 13 lines
drivers/firmware/smbios.c line 27
Quoted 10 lines
drivers/firmware/smbios.c new file +6
-0+} + +/* add additional sysfs files here */ +static struct device_attribute smbios_attrs[] = { +__ATTR_RO (version), +};So a whole new class just for a single version file in sysfs?
Right now it is only the version file. Eventually I was thinking that this would grow to include sysfs entries for the BIOS info (date, vendor, etc.) and Product and Chassis info, not to mention all the various SMBIOS structs. We have at least one driver that goes through a lot of trouble to get this info, and IIRC, the PCI code now outputs type41 and type9 fields for the network device naming stuff.
I thought that meant it needed a new class but, as you pointed out, that isn't the case. I'll ping kernel-mentors and see if anyone there might point me to the right class for this data.
Quoted 4 lines
Quoted 4 lines
I'm confused, what exactly are you trying to show here that you can't show in the existing device/class structure?
I want to show the version of the SMBIOS on the system. But it doesn't seem to make sense to me to expose it in the DMI sysfs code. The SMBIOS provides the DMI code, not the other way around. We've become used to the DMI code, and we're adding SMBIOS decoding to it but the other parts of the kernel are doing their own decoding of the SMBIOS to get additional info. To me, at least, this means we should have a central place for it.
Thanks again,
P.
Quoted 21 lines · Prarit Bhargava
Quoted 13 lines
Quoted 13 lines
drivers/firmware/smbios.c line 27
Quoted 10 lines
drivers/firmware/smbios.c new file +6
-0+} + +/* add additional sysfs files here */ +static struct device_attribute smbios_attrs[] = { +__ATTR_RO (version), +};So a whole new class just for a single version file in sysfs?
Right now it is only the version file. Eventually I was thinking that this would grow to include sysfs entries for the BIOS info (date, vendor, etc.) and Product and Chassis info, not to mention all the various SMBIOS structs. We have at least one driver that goes through a lot of trouble to get this info, and IIRC, the PCI code now outputs type41 and type9 fields for the network device naming stuff.
Quoted 3 lines
I thought that meant it needed a new class but, as you pointed out, that isn't the case. I'll ping kernel-mentors and see if anyone there might point me to the right class for this data.
See my other response for how to go about this.
Quoted 5 lines
Quoted 4 lines
Quoted 4 lines
I'm confused, what exactly are you trying to show here that you can't show in the existing device/class structure?
Quoted 7 lines
I want to show the version of the SMBIOS on the system. But it doesn't seem to make sense to me to expose it in the DMI sysfs code. The SMBIOS provides the DMI code, not the other way around. We've become used to the DMI code, and we're adding SMBIOS decoding to it but the other parts of the kernel are doing their own decoding of the SMBIOS to get additional info. To me, at least, this means we should have a central place for it.
Ok, that's fine. But you can't remove the existing DMI files and symlinks, so how are you anticipating handling all of that?
This seems like a lot of work at the moment just for a version file, let's see some more use of this which would justify it being added to the kernel first.
thanks,
greg k-h
Quoted 22 lines · Prarit Bhargava
Quoted 13 lines · Greg KH
Quoted 11 lines · Prarit Bhargava
The existing DMI code,
drivers/firmware/dmi.c, is not really parsing DMI. It is actually SMBIOS code that is using the old DMI infrastructure to expose SMBIOS entries.We should be doing the following:
1. Looking for SMBIOS (either EFI or mapped to it's standard location,
0xF0000)2. If found, look for the SMBIOS' Intermediate Structure (aka "DMI" entry)
3. If not found, look for an "old" DMI structure.Quoted 5 lines
Quoted 2 lines
DMI is a predecessor of SMBIOS, so we should start treating it as such.
Why? What is this change going to buy us in the end? Heck, right now, what's the benefit?
Quoted 5 lines
It's wrong. DMI doesn't give us SMBIOS information -- it's the other way around. There's also a bunch of other stuff from SMBIOS being used in the kernel (see pci slots and hotplug drivers).
It would be nice to have a single place for this, no?
Ok, yes it would.
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
For this patch I have introduced
drivers/firmware/smbios.c, exported the SMBIOS version through sysfs, and modified the DMI code such thata)
dmi_scan_machine()is now called frominit_smbios(), and
b) if an SMBIOS is found, the values in the SMBIOS STEP structure are used indmi_scan_machine().Right now, removing the DMI code is not an option. It is used by common initscripts, etc., to bringup the system. Later modifications in this area will include additional parsing of the SMBIOS structs and a simultaneous cleanup of the DMI code.
Quoted 7 lines
Quoted 4 lines
Care to show follow-on patches that convert things to a manner which you think it should look like?
I haven't written anything yet :) -- I was planning on exporting the SMBIOS structs (all types) though.
Sounds good, how are you going to do this? In binary way (like efivars does it) or in text files like the current DMI code does?
Quoted 21 lines
Quoted 10 lines
Quoted 10 lines
drivers/firmware/smbios.c new file +7
-0@@ -0,0 +1,175 @@ +#include <linux/dmi.h> +#include <linux/efi.h> +#include <linux/init.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/smbios.h> +Quoted 10 lines
drivers/firmware/smbios.c line 12
Quoted 6 lines
drivers/firmware/smbios.c new file +5
-0+#include <asm/setup.h> + +int smbios_present =0 ; +struct smbios_step *smbios_step; +static struct device *smbios_dev;A "raw" struct device? Why? You should never do this, that's not the way to use a 'struct device'.
Quoted 2 lines
Ah, okay -- my familiarity with sysfs is very minimal. I'll ask on kernel-mentors for a pointer to "good" code.
It all depends on what you are trying to do. What exactly are you trying to achive? What do you want to show in sysfs and how do you expect it to look?
Quoted 1 line
From there we can then work backwards and create the code to do this.
thanks,
greg k-h
Quoted 2 lines · gregkh
Sounds good, how are you going to do this? In binary way (like efivars does it) or in text files like the current DMI code does?
Quoted 3 lines · gregkh
What exactly are you
trying to achive? What do you want to show in sysfs and how do you expect it to look?
Greg, sorry for repeating myself in some of the below. You're right in your implication that I was not explaining myself very well. I figured the best thing to do was to start my argument from the beginning...
The last update to the DMI specification was in 2003. It has since been replaced by SMBIOS. A new SMBIOS specification was released in February 2011.
While looking at the SMBIOS related code in the kernel, I noticed that the DMI code was actually being overloaded to include SMBIOS structures and that other sections of code (ia64 setup, x86 setup & pci, Dell platform driver(s), the new PCI sysfs labe codel, hotplug drivers, the HP watchdog driver, and the sigmatel sound driver, all used a method to derive values from the SMBIOS structs. Please note, that this list is very likely incomplete. I only did a search on 'smbios' in the kernel.
As I previously said, it would be nice if we did this in a single location instead of throughout the kernel, and fix the overloading of the DMI code currently in the kernel.
For now, my intention is to leave the code as similar to the DMI code as possible. That is, export the SMBIOS information via text files in sysfs (although the layout of the files will obviously change, see below).
In this way we would limit the amount of change to external packages (ie, initscripts, network init, etc.) to hopefully only filename changes.
I will create a new sysfs class, to temporarily exist next to the current /sys/devices/virtual/dmi entry, which would export values from SMBIOS. I want to introduce this slowly and in several stages:
1. Introduce the new SMBIOS infrastructure in a [relatively] simple patch, and export the smbios version # to the kernel.
The model I had in mind was:
/sys/devices/virtual/smbios/id/
/sys/devices/virtual/smbios/id/[type#]/...
ie) a straight mapping of the smbios tables. It should be up to the individual
drivers & subsystems to link back to the files. Of course, the SMBIOS code would provide lookup functions similar to the ones provided by the existing DMI code to aid drivers in their lookups.
2. Export all of the SMBIOS information in the current (Feb 2011) spec. If all goes well I think the correct thing to do is to update feature-removal-schedule.txt with a well-defined timeline at this point.
3. Modify drivers to use the new SMBIOS table lookups, etc, instead of DMI table lookups. This is relatively easy, IMO. I will try to keep dmi* functions identical to the new smbios* lookup functions. That code works
well, and isn't in need of change.
4. Notify external userspace programs of the change. To be honest, I haven't
thought much about this other than to take a couple of systems and see what is using those values and let them know of the impending change.
[This next part is wishful thinking ;)] ... I suppose at some point it would be
possible to put in a printk into the *existing* DMI sysfs files to indicate that they were being deprecated. I can certainly see how that would be frowned upon; while it would be a loud nasty warning, it also might be one of those warnings people just learn to ignore.
5. After a release (or some other set period of time defined in feature-removal-schedule.txt), remove the old DMI code.
I'm hoping to lean on what you did for the udev change, Greg. IIRC, you kept the old code and the new udev code in place for a while and then made the final change after some time. I can see that would be the best idea here...
P.
Quoted 2 lines
In this way we would limit the amount of change to external packages (ie, initscripts, network init, etc.) to hopefully only filename changes.
Why change them at all. SMBIOS provides everything DMI BIOS interfaces did, so we should just continue to provide them to userspace exactly as before, but possibly sourced from other locations. There simply isn't a need to break anything user side. Maybe in five or ten years time someone will delete the old stuff as nobody uses it any more but it's not an item that needs hurrying.
Also distinguish between DMI and the DMI BIOS interfaces, the two are rather different beasts.
Quoted 3 lines
drivers & subsystems to link back to the files. Of course, the SMBIOS code would provide lookup functions similar to the ones provided by the existing DMI code to aid drivers in their lookups.
I'm wondering why put all the SMBIOS complexity into the kernel, but then I wonder likewise about some of the DMI stuff still 8)
Quoted 5 lines
3. Modify drivers to use the new SMBIOS table lookups, etc, instead of DMI table lookups. This is relatively easy, IMO. I will try to keep dmi* functions identical to the new smbios* lookup functions. That code works
well, and isn't in need of change.
I'd actually suggest one change. In using dmi_ naming and DMI alone the original code made the assumption it was a PC specific tool for doing fixups and the like. If we are going to tweak it then we should probably recognize that want the kernel needs to provide to drivers isn't (for the most case) SMBIOS lookup it's generic "Platform name" etc and these could have been populated from things other than SMBIOS (OpenFirmware, DeviceTree, whatever).
It doesn't really change the interface but if you are going to be renaming things maybe a better naming would be worth it ?
Quoted 2 lines
5. After a release (or some other set period of time defined in feature-removal-schedule.txt), remove the old DMI code.
This is a user API so really it wants to hang around for a long time, and given SMBIOS can just provide the same names as well trivially it might as well just stay there until no distro is using it, then get a _LEGACY config, then vanish. Think multiples of years.
Alan
Quoted 10 lines · Alan Cox
Quoted 3 lines
In this way we would limit the amount of change to external packages (ie, initscripts, network init, etc.) to hopefully only filename changes.
Why change them at all. SMBIOS provides everything DMI BIOS interfaces did, so we should just continue to provide them to userspace exactly as before, but possibly sourced from other locations. There simply isn't a need to break anything user side. Maybe in five or ten years time someone will delete the old stuff as nobody uses it any more but it's not an item that needs hurrying.
I'm sorry Alan -- I meant to say that the change would be minimal -- it is likely with a new sysfs mapping that /sys/devices/virtual/id/dmi would stay the same, but I'm not willing to guarantee it.
It *might* turn out to be /sys/devices/virtual/id/smbios/dmi .
I'm really not sure which I prefer.
Quoted 4 lines
Also distinguish between DMI and the DMI BIOS interfaces, the two are rather different beasts.
Quoted 8 lines
Quoted 4 lines
drivers & subsystems to link back to the files. Of course, the SMBIOS code would provide lookup functions similar to the ones provided by the existing DMI code to aid drivers in their lookups.
I'm wondering why put all the SMBIOS complexity into the kernel, but then I wonder likewise about some of the DMI stuff still 8)
Well, like I said before, there are several drivers that are each doing their own thing. Given the past history of the kernel this should be centralized to one location. Having drivers each "dig" through mem is not the best thing IMO.
Quoted 6 lines
Quoted 6 lines
3. Modify drivers to use the new SMBIOS table lookups, etc, instead of DMI table lookups. This is relatively easy, IMO. I will try to keep dmi* functions identical to the new smbios* lookup functions. That code works
well, and isn't in need of change.
Quoted 9 lines
I'd actually suggest one change. In using
dmi_naming and DMI alone the original code made the assumption it was a PC specific tool for doing fixups and the like. If we are going to tweak it then we should probably recognize that want the kernel needs to provide to drivers isn't (for the most case) SMBIOS lookup it's generic "Platform name" etc and these could have been populated from things other than SMBIOS (OpenFirmware, DeviceTree, whatever).
Good idea :) Thanks -- I'll incorporate that into my next proposal. IMO that's much better ... and it lends its hand to other arches if they wish to do it.
Quoted 4 lines
It doesn't really change the interface but if you are going to be renaming things maybe a better naming would be worth it ?
IMO yes. The more general the better IMO. Why have several different Platform Firmware implementations.
So .. maybe /sys/devices/virtual/platform ? I dunno ... maybe someone reading can come up with a better idea.
Quoted 9 lines
Quoted 3 lines
5. After a release (or some other set period of time defined in feature-removal-schedule.txt), remove the old DMI code.
This is a user API so really it wants to hang around for a long time, and given SMBIOS can just provide the same names as well trivially it might as well just stay there until no distro is using it, then get a
_LEGACYconfig, then vanish. Think multiples of years.
Okay. Will do.
P.
Quoted 2 lines
Alan