2004-04-30 02:23:31

by Michael Brown

[permalink] [raw]
Subject: [PATCH 2.4] add SMBIOS information to /proc/smbios -- UPDATED

Marcelo, Al,
Below is an updated patch to add SMBIOS information to /proc/smbios.
Updates have been made per Al's previous comments. Please apply.

For reference, here are previous postings:
Previous 2.4 thread:
http://marc.theaimsgroup.com/?t=108321757100001&r=1&w=1

Previous 2.6 thread:
http://marc.theaimsgroup.com/?t=108311959700002&r=1&w=1

Thank you,
Michael Brown


# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2004/04/29 21:11:19-05:00 [email protected]
# Updates per Al Viro:
# -- remove MOD_{INC,DEC}_USE_COUNT
# -- use normal read call instead of proc read call
#
# drivers/char/smbios.c
# 2004/04/29 21:11:17-05:00 [email protected] +35 -47
# Updates per comments from Al Viro.
#
# ChangeSet
# 2004/04/29 00:16:35-05:00 [email protected]
# rename smbios to proper smbios.c
#
# drivers/char/Makefile
# 2004/04/29 00:09:15-05:00 [email protected] +1 -1
# rename smbios driver to 'smbios'.
#
# drivers/char/smbios.c
# 2004/04/29 00:08:51-05:00 [email protected] +0 -0
# Rename: drivers/char/procsmbios.c -> drivers/char/smbios.c
#
# ChangeSet
# 2004/04/28 23:51:19-05:00 [email protected]
# Add SMBIOS driver that exports system SMBIOS information as files in /proc/smbios.
#
# drivers/char/smbios.h
# 2004/04/28 23:50:15-05:00 [email protected] +53 -0
#
# drivers/char/procsmbios.c
# 2004/04/28 23:50:15-05:00 [email protected] +251 -0
#
# drivers/char/smbios.h
# 2004/04/28 23:50:15-05:00 [email protected] +0 -0
# BitKeeper file /home/michael_e_brown/kernel/bk/subtrees/smbios-24-driver/drivers/char/smbios.h
#
# drivers/char/procsmbios.c
# 2004/04/28 23:50:15-05:00 [email protected] +0 -0
# BitKeeper file /home/michael_e_brown/kernel/bk/subtrees/smbios-24-driver/drivers/char/procsmbios.c
#
# drivers/char/Makefile
# 2004/04/28 23:50:15-05:00 [email protected] +1 -0
# Add SMBIOS driver to build system
#
# drivers/char/Config.in
# 2004/04/28 23:50:15-05:00 [email protected] +1 -0
# Add config option to turn on SMBIOS proc driver
#
# Documentation/Configure.help
# 2004/04/28 23:50:15-05:00 [email protected] +14 -0
# Add help text for SMBIOS driver
#
diff -Nru a/Documentation/Configure.help b/Documentation/Configure.help
--- a/Documentation/Configure.help Thu Apr 29 21:13:45 2004
+++ b/Documentation/Configure.help Thu Apr 29 21:13:45 2004
@@ -19914,6 +19914,20 @@

If unsure, say N.

+SMBIOS table information in /proc
+CONFIG_SMBIOS
+ This driver creates two files in /proc, /proc/smbios/table_entry_point
+ and /proc/smbios/table. These two files contain the contents of the
+ BIOS-generated SMBIOS tables. Generally, PC-like architectures made
+ after 1997 have this, it is safe to enable on any system.
+
+ To compile this driver as a module ( = code which can be inserted in
+ and removed from the running kernel whenever you want), say M here
+ and read <file:Documentation/modules.txt>. The module will be called
+ smbios.o.
+
+ If unsure, say N.
+
Sony Vaio Programmable I/O Control Device support
CONFIG_SONYPI
This driver enables access to the Sony Programmable I/O Control
diff -Nru a/drivers/char/Config.in b/drivers/char/Config.in
--- a/drivers/char/Config.in Thu Apr 29 21:13:45 2004
+++ b/drivers/char/Config.in Thu Apr 29 21:13:45 2004
@@ -333,6 +333,7 @@
if [ "$CONFIG_EXPERIMENTAL" = "y" -a "$CONFIG_X86" = "y" -a "$CONFIG_X86_64" != "y" ]; then
dep_tristate 'Sony Vaio Programmable I/O Control Device support (EXPERIMENTAL)' CONFIG_SONYPI $CONFIG_PCI
fi
+tristate 'SMBIOS table information in /proc' CONFIG_SMBIOS

mainmenu_option next_comment
comment 'Ftape, the floppy tape device driver'
diff -Nru a/drivers/char/Makefile b/drivers/char/Makefile
--- a/drivers/char/Makefile Thu Apr 29 21:13:45 2004
+++ b/drivers/char/Makefile Thu Apr 29 21:13:45 2004
@@ -265,6 +265,7 @@
obj-$(CONFIG_HW_RANDOM) += hw_random.o
obj-$(CONFIG_AMD_PM768) += amd76x_pm.o
obj-$(CONFIG_BRIQ_PANEL) += briq_panel.o
+obj-$(CONFIG_SMBIOS) += smbios.o

obj-$(CONFIG_ITE_GPIO) += ite_gpio.o
obj-$(CONFIG_AU1X00_GPIO) += au1000_gpio.o
diff -Nru a/drivers/char/smbios.c b/drivers/char/smbios.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/char/smbios.c Thu Apr 29 21:13:45 2004
@@ -0,0 +1,239 @@
+/*
+ * linux/drivers/firmware/smbios.c
+ * Copyright (C) 2002, 2003, 2004 Dell Inc.
+ * by Michael Brown <[email protected]>
+ * vim:noet:ts=8:sw=8:filetype=c:textwidth=80:
+ *
+ * BIOS SMBIOS Table access
+ * conformant to DMTF SMBIOS definition
+ * at http://www.dmtf.org/standards/smbios
+ *
+ * This code takes information provided by SMBIOS tables
+ * and presents it in procfs as:
+ * /proc/smbios
+ * |--> /table_entry_point
+ * |--> /table
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License v2.0 as published by
+ * the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/proc_fs.h>
+#include <asm/io.h>
+#include <asm/uaccess.h>
+#include "smbios.h"
+
+#define SMBIOS_VERSION "1.0"
+#define SMBIOS_DATE "2004-04-29"
+
+MODULE_AUTHOR("Michael Brown <[email protected]>");
+MODULE_DESCRIPTION("procfs interface to SMBIOS information");
+MODULE_LICENSE("GPL");
+EXPORT_NO_SYMBOLS;
+
+struct smbios_device {
+ struct smbios_table_entry_point table_eps;
+ unsigned int smbios_table_real_length;
+};
+
+/* there shall be only one */
+static struct smbios_device the_smbios_device;
+
+static struct proc_dir_entry *smbios_dir, *table_eps_file, *table_file;
+
+static __init int
+checksum_eps(struct smbios_table_entry_point *table_eps)
+{
+ u8 *p = (u8 *)table_eps;
+ u8 checksum = 0;
+ int i=0;
+ for (i=0; i < table_eps->eps_length && i < sizeof(*table_eps); ++i) {
+ checksum += p[i];
+ }
+ return(checksum == 0);
+}
+
+static __init int
+find_table_entry_point(struct smbios_device *sdev)
+{
+ struct smbios_table_entry_point *table_eps = &(sdev->table_eps);
+ u32 fp = 0xF0000;
+ while (fp < 0xFFFFF) {
+ isa_memcpy_fromio(table_eps, fp, sizeof(*table_eps));
+ if (memcmp(table_eps->anchor, "_SM_", 4)==0 &&
+ checksum_eps(table_eps)) {
+ return 0;
+ }
+ fp += 16;
+ }
+
+ printk(KERN_INFO "SMBIOS table entry point not found in "
+ "0xF0000 - 0xFFFFF\n");
+ return -ENODEV;
+}
+
+static __init int
+find_table_max_address(struct smbios_device *sdev)
+{
+ /* break out on one of three conditions:
+ * -- hit table_eps.table_length
+ * -- hit number of items that table claims we have
+ * -- hit structure type 127
+ */
+
+ u8 *buf = ioremap(sdev->table_eps.table_address,
+ sdev->table_eps.table_length);
+ u8 *ptr = buf;
+ int count = 0, keep_going = 1;
+ int max_count = sdev->table_eps.table_num_structs;
+ int max_length = sdev->table_eps.table_length;
+ while(keep_going && ((ptr - buf) <= max_length) && count < max_count){
+ if (ptr[0] == 0x7F) /* ptr[0] is type */
+ keep_going = 0;
+
+ ptr += ptr[1]; /* ptr[1] is length, skip structure */
+ /* skip strings at end of structure */
+ while((ptr-buf) < max_length && (ptr[0] || ptr[1]))
+ ++ptr;
+
+ /* string area ends in double-null. skip it. */
+ ptr += 2;
+ ++count;
+ }
+ sdev->smbios_table_real_length = (ptr - buf);
+ iounmap(buf);
+
+ if (count != max_count)
+ printk(KERN_INFO "Warning: SMBIOS table structure count"
+ " does not match count specified in the"
+ " table entry point.\n"
+ " Table entry point count: %d\n"
+ " Actual count: %d\n",
+ max_count, count);
+
+ if (keep_going != 0)
+ printk(KERN_INFO "Warning: SMBIOS table does not end with a"
+ " structure type 127. This may indicate a"
+ " truncated table.");
+
+ if (sdev->smbios_table_real_length != max_length)
+ printk(KERN_INFO "Warning: BIOS specified SMBIOS table length"
+ " does not match calculated length.\n"
+ " BIOS specified: %d\n"
+ " calculated length: %d\n",
+ max_length, sdev->smbios_table_real_length);
+
+ return sdev->smbios_table_real_length;
+}
+
+/* simple procfs style. Print the whole thing and let core
+ * handle splitting it out for userspace and setting eof.
+ */
+static ssize_t
+smbios_read_table_entry_point(char *page, char **start,
+ off_t off, int count,
+ int *eof, void *data)
+{
+ unsigned int max_off = sizeof(the_smbios_device.table_eps);
+ memcpy(page, &the_smbios_device.table_eps, max_off);
+ return max_off;
+}
+
+static ssize_t smbios_read_table(struct file *file, char *buf,
+ size_t count, loff_t *ppos)
+{
+ unsigned long origppos = *ppos;
+ unsigned long max_off = the_smbios_device.smbios_table_real_length;
+ u8 *ptr;
+
+ if(*ppos >= max_off)
+ return 0;
+
+ if(count > (max_off - *ppos))
+ count = max_off - *ppos;
+
+ ptr = ioremap(the_smbios_device.table_eps.table_address, max_off);
+ if (ptr == NULL)
+ return -ENXIO;
+
+ while (*ppos < max_off) {
+ put_user(readb(ptr + *ppos), buf);
+ ++(*ppos); ++buf;
+ }
+ iounmap(ptr);
+ return *ppos - origppos;
+}
+
+static struct file_operations proc_smbios_table_operations = {
+ .read = smbios_read_table,
+};
+
+static int __init
+smbios_init(void)
+{
+ int rc=0;
+
+ printk(KERN_INFO "SMBIOS facility v%s %s\n",
+ SMBIOS_VERSION, SMBIOS_DATE);
+
+ rc = find_table_entry_point(&the_smbios_device);
+ if (rc)
+ return rc;
+
+ find_table_max_address(&the_smbios_device);
+
+ rc = -ENOMEM;
+ smbios_dir = proc_mkdir("smbios", NULL);
+ if (smbios_dir == NULL)
+ goto out;
+
+ smbios_dir->owner = THIS_MODULE;
+
+ table_eps_file = create_proc_read_entry("table_entry_point",
+ 0444, smbios_dir,
+ smbios_read_table_entry_point,
+ NULL);
+ if (table_eps_file == NULL)
+ goto no_table_eps_file;
+
+ table_eps_file->owner = THIS_MODULE;
+
+ table_file = create_proc_entry("table", 0444, smbios_dir);
+ if (table_file == NULL)
+ goto no_table_file;
+
+ table_file->proc_fops = &proc_smbios_table_operations;
+ table_file->owner = THIS_MODULE;
+
+ rc = 0;
+ goto out;
+no_table_file:
+ remove_proc_entry("table_entry_point", smbios_dir);
+
+no_table_eps_file:
+ remove_proc_entry("smbios", NULL);
+
+out:
+ return rc;
+}
+
+static void __exit
+smbios_exit(void)
+{
+ remove_proc_entry("table_entry_point", smbios_dir);
+ remove_proc_entry("table", smbios_dir);
+ remove_proc_entry("smbios", NULL);
+}
+
+module_init(smbios_init);
+module_exit(smbios_exit);
diff -Nru a/drivers/char/smbios.h b/drivers/char/smbios.h
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/char/smbios.h Thu Apr 29 21:13:45 2004
@@ -0,0 +1,53 @@
+/*
+ * linux/drivers/firmware/smbios.c
+ * Copyright (C) 2002, 2003, 2004 Dell Inc.
+ * by Michael Brown <[email protected]>
+ * vim:noet:ts=8:sw=8:filetype=c:textwidth=80:
+ *
+ * BIOS SMBIOS Table access
+ * conformant to DMTF SMBIOS definition
+ * at http://www.dmtf.org/standards/smbios
+ *
+ * This code takes information provided by SMBIOS tables
+ * and presents it in sysfs.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License v2.0 as published by
+ * the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef _LINUX_SMBIOS_H
+#define _LINUX_SMBIOS_H
+
+#include <linux/types.h>
+
+struct smbios_table_entry_point {
+ u8 anchor[4];
+ u8 checksum;
+ u8 eps_length;
+ u8 major_ver;
+ u8 minor_ver;
+ u16 max_struct_size;
+ u8 revision;
+ u8 formatted_area[5];
+ u8 dmi_anchor[5];
+ u8 intermediate_checksum;
+ u16 table_length;
+ u32 table_address;
+ u16 table_num_structs;
+ u8 smbios_bcd_revision;
+} __attribute__ ((packed));
+
+struct smbios_structure_header {
+ u8 type;
+ u8 length;
+ u16 handle;
+} __attribute__ ((packed));
+
+#endif /* _LINUX_SMBIOS_H */



2004-04-30 03:34:18

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH 2.4] add SMBIOS information to /proc/smbios -- UPDATED

On Thu, Apr 29, 2004 at 09:21:52PM -0500, Michael Brown wrote:
> + u32 fp = 0xF0000;
> + while (fp < 0xFFFFF) {
> + isa_memcpy_fromio(table_eps, fp, sizeof(*table_eps));
> + if (memcmp(table_eps->anchor, "_SM_", 4)==0 &&
> + checksum_eps(table_eps)) {
> + return 0;
> + }
> + fp += 16;
> + }

Stilistic note:
for (fp = 0xf0000; fp < 0xfffff; fp += 16) {
isa_memcpy_fromio(table_eps, fp, sizeof(*table_eps));
if (memcmp(table_eps->anchor, "_SM_", 4) != 0)
continue;
if (checksum_eps(table_eps))
return 0;
}

> + while(keep_going && ((ptr - buf) <= max_length) && count < max_count){
> + if (ptr[0] == 0x7F) /* ptr[0] is type */
> + keep_going = 0;
> +
> + ptr += ptr[1]; /* ptr[1] is length, skip structure */
> + /* skip strings at end of structure */
> + while((ptr-buf) < max_length && (ptr[0] || ptr[1]))
> + ++ptr;

It looks like an off-by-one - if ptr reaches buf + max_length - 1, ptr[1]
appears to be beyond the area it's OK to dereference.

> + size_t count, loff_t *ppos)
> +{
> + unsigned long origppos = *ppos;
> + unsigned long max_off = the_smbios_device.smbios_table_real_length;
> + u8 *ptr;
> +
> + if(*ppos >= max_off)
> + return 0;

Note that *ppos is signed here. llseek() to negative and you've got a problem.

> + while (*ppos < max_off) {
> + put_user(readb(ptr + *ppos), buf);
> + ++(*ppos); ++buf;
> + }

Eeek...

a) that's called copy_to_user()
b) you'd better check the return value (either of put_user() or
copy_to_user()).

2004-04-30 04:38:55

by Michael Brown

[permalink] [raw]
Subject: Re: [PATCH 2.4] add SMBIOS information to /proc/smbios -- UPDATED

Good stuff! Thanks for the feedback Al. Give me a few minutes and I will
send an updated patch.

Comments/Questions below.

On Thu, 2004-04-29 at 22:34, [email protected]
wrote:
> On Thu, Apr 29, 2004 at 09:21:52PM -0500, Michael Brown wrote:
> > + u32 fp = 0xF0000;
> > + while (fp < 0xFFFFF) {
> > + isa_memcpy_fromio(table_eps, fp, sizeof(*table_eps));
> > + if (memcmp(table_eps->anchor, "_SM_", 4)==0 &&
> > + checksum_eps(table_eps)) {
> > + return 0;
> > + }
> > + fp += 16;
> > + }
>
> Stilistic note:
> for (fp = 0xf0000; fp < 0xfffff; fp += 16) {
> isa_memcpy_fromio(table_eps, fp, sizeof(*table_eps));
> if (memcmp(table_eps->anchor, "_SM_", 4) != 0)
> continue;
> if (checksum_eps(table_eps))
> return 0;
> }

Will change. I like your version.

Originally copied from Alan Cox's stuff, so if Alan's style is off, oh
well... :-)

>
> > + while(keep_going && ((ptr - buf) <= max_length) && count < max_count){
> > + if (ptr[0] == 0x7F) /* ptr[0] is type */
> > + keep_going = 0;
> > +
> > + ptr += ptr[1]; /* ptr[1] is length, skip structure */
> > + /* skip strings at end of structure */
> > + while((ptr-buf) < max_length && (ptr[0] || ptr[1]))
> > + ++ptr;
>
> It looks like an off-by-one - if ptr reaches buf + max_length - 1, ptr[1]
> appears to be beyond the area it's OK to dereference.

Great spot. Updating, changed "<= max_length" to "<= (max_length-1)".

>
> > + size_t count, loff_t *ppos)
> > +{
> > + unsigned long origppos = *ppos;
> > + unsigned long max_off = the_smbios_device.smbios_table_real_length;
> > + u8 *ptr;
> > +
> > + if(*ppos >= max_off)
> > + return 0;
>
> Note that *ppos is signed here. llseek() to negative and you've got a problem.

Added "|| *ppos < 0" to the check.

>
> > + while (*ppos < max_off) {
> > + put_user(readb(ptr + *ppos), buf);
> > + ++(*ppos); ++buf;
> > + }
>
> Eeek...
>
> a) that's called copy_to_user()
> b) you'd better check the return value (either of put_user() or
> copy_to_user()).

Ok, will update, but I have one question. for (A), is this equivalent to
copy_to_user() even with the readb() in there? Sorry if this is a stupid
question.

If I get a bad return from either of these, is "return -EINVAL"
appropriate?
--
Michael






2004-04-30 19:23:05

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH 2.4] add SMBIOS information to /proc/smbios -- UPDATED

Michael Brown <[email protected]> writes:

> Below is an updated patch to add SMBIOS information to /proc/smbios.
> Updates have been made per Al's previous comments. Please apply.

What is this good for? There are tools to read this from
/dev/mem; and that is fine because the information is static.
There is no reason to bloat the kernel with this.

-Andi

2004-05-01 03:31:45

by Michael Brown

[permalink] [raw]
Subject: Re: [PATCH 2.4] add SMBIOS information to /proc/smbios -- UPDATED

On Fri, 2004-04-30 at 14:22, Andi Kleen wrote:
> Michael Brown <[email protected]> writes:
>
> > Below is an updated patch to add SMBIOS information to /proc/smbios.
> > Updates have been made per Al's previous comments. Please apply.
>
> What is this good for? There are tools to read this from
> /dev/mem; and that is fine because the information is static.
> There is no reason to bloat the kernel with this.

As I mentioned in my first posting of this to l-k, there are three
reasons why this driver is necessary:

-- This information is, in the very near future, _not_ going to be
static anymore. There will be systems that update the information in
dynamically during SMIs.

-- SMBIOS consists of two things, the table entry point and the table
itself. The table entry point is always in 0xF0000 - 0xFFFFF.
Traditionally, the actual table has been here as well. BIOS is running
out of space here and future systems are moving this information to high
memory. /dev/mem will not allow access to memory above top of system
RAM.

-- Red Hat has a /dev/mem patch in their tree that restricts access to
RAM above 1MB.

Because of all of these reasons, we feel it is a good thing to have a
stable method to get to the SMBIOS information that will work into the
future. Our userspace libs will try to use this driver to access SMBIOS,
but fall back to /dev/mem if this driver is not available. (with the
caveat that nothing will work if table >1MB and this driver not
present.)

As for the "bloat" argument: this driver is about the most trivial
driver I can conceive of that does useful work. It is 250 raw lines of
code, comparable in size to /proc/meminfo or /proc/cpuinfo.

This 250 line driver allows us to move a few thousand lines of code from
Dell's current, proprietary systems management driver into userspace. If
this approach is accepted, I am pushing to work on opening up other
pieces of Dell's current proprietary drivers. This work is a
proof-of-concept to management that this approach can work.
--
Michael