2004-06-05 00:52:54

by David Mosberger

[permalink] [raw]
Subject: EFI-support for SMBIOS driver

Michael,

The patch below adds EFI support to the SMBIOS driver. Since EFI
already knows the address of the SMBIOS, this avoids having to scan
for the table. It also enables use of the driver on ia64 machines.
The patch also adds code to handle the case where the SMBIOS table
resides in memory, which is the case at least for HP's zx1-based ia64
machines. If CONFIG_EFI is off, the resulting code should be
unchanged (except for replacing a readb() loop into a
memcpy_fromio()).

One observation: I believe find_table_max_address() is missing some
readb() calls (it's dereferencing ioremap'pped addresses directly). I
didn't try to fix that since I wasn't sure why it wasn't done in the
first place.

Do you have a test-program that's using /sys/firmware/smbios?

If the patch looks OK, please apply.

Thanks,

--david

===== drivers/firmware/smbios.c 1.2 vs edited =====
--- 1.2/drivers/firmware/smbios.c Wed Apr 28 04:50:49 2004
+++ edited/drivers/firmware/smbios.c Fri Jun 4 17:48:20 2004
@@ -27,6 +27,7 @@


#include <linux/module.h>
+#include <linux/efi.h>
#include <linux/init.h>
#include <linux/device.h>
#include <asm/io.h>
@@ -53,6 +54,37 @@
* show them. */
static struct sysfs_ops smbios_attr_ops = { };

+#ifdef CONFIG_EFI
+ static int smbios_in_memory; /* does smbios reside in normal memory? */
+#else
+# define smbios_in_memory 0
+#endif
+
+static inline void *
+smbios_map (unsigned long addr, unsigned long size)
+{
+ if (smbios_in_memory)
+ return phys_to_virt(addr);
+ else
+ return ioremap(addr, size);
+}
+
+static inline void
+smbios_unmap (void *addr)
+{
+ if (!smbios_in_memory)
+ iounmap(addr);
+}
+
+static inline void
+smbios_memcpy (void *dst, void *src, size_t len)
+{
+ if (smbios_in_memory)
+ memcpy(dst, src, len);
+ else
+ memcpy_fromio(dst, src, len);
+}
+
static __init int
checksum_eps(struct smbios_table_entry_point *table_eps)
{
@@ -65,15 +97,44 @@
return( checksum == 0 );
}

+static int
+valid_table (struct smbios_table_entry_point *table_eps)
+{
+ return (memcmp(table_eps->anchor, "_SM_", 4) == 0 &&
+ checksum_eps(table_eps));
+}
+
static __init int
find_table_entry_point(struct smbios_device *sdev)
{
struct smbios_table_entry_point *table_eps = &(sdev->table_eps);
+#ifdef CONFIG_EFI
+ unsigned long addr;
+ void *header;
+
+ if (!efi.smbios) {
+ printk(KERN_INFO "SMBIOS table entry point not found\n");
+ return -ENODEV;
+ }
+ addr = virt_to_phys(efi.smbios);
+ smbios_in_memory = (efi_mem_attributes(addr) & EFI_MEMORY_WB) != 0;
+
+ header = smbios_map(addr, sizeof(*table_eps));
+ if (!header)
+ return -ENODEV;
+
+ smbios_memcpy(table_eps, header, sizeof(*table_eps));
+ smbios_unmap(header);
+
+ if (valid_table(table_eps))
+ return 0;
+
+ printk(KERN_INFO "SMBIOS table at %p appears to be invalid\n", efi.smbios);
+#else
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)) {
+ if (valid_table(table_eps)) {
return 0;
}
fp += 16;
@@ -81,6 +142,7 @@

printk(KERN_INFO "SMBIOS table entry point not found in "
"0xF0000 - 0xFFFFF\n");
+#endif
return -ENODEV;
}

@@ -93,8 +155,8 @@
* -- hit structure type 127
*/

- u8 *buf = ioremap(sdev->table_eps.table_address,
- sdev->table_eps.table_length);
+ u8 *buf = smbios_map(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;
@@ -113,7 +175,7 @@
++count;
}
sdev->smbios_table_real_length = (ptr - buf);
- iounmap(buf);
+ smbios_unmap(buf);

if( count != max_count )
printk(KERN_INFO "Warning: SMBIOS table structure count"
@@ -158,22 +220,16 @@
struct smbios_device *sdev = &the_smbios_device;
u8 *buf;
unsigned int count = sdev->smbios_table_real_length - pos;
- int i = 0;
count = count < size ? count : size;

if (pos > sdev->smbios_table_real_length)
return 0;

- buf = ioremap(sdev->table_eps.table_address, sdev->smbios_table_real_length);
+ buf = smbios_map(sdev->table_eps.table_address, sdev->smbios_table_real_length);
if (buf == NULL)
return -ENXIO;
-
- /* memcpy( buffer, buf+pos, count ); */
- for (i = 0; i < count; ++i) {
- buffer[i] = readb( buf+pos+i );
- }
-
- iounmap(buf);
+ smbios_memcpy(buffer, buf + pos, count);
+ smbios_unmap(buf);

return count;
}


Subject: Re: EFI-support for SMBIOS driver


It seems SMBIOS driver is gone.

http://linus.bkbits.net:8080/linux-2.5/cset@40b6702axansvQIxOVjTSIp7_DVoHA

On Saturday 05 of June 2004 02:52, David Mosberger wrote:
> Michael,
>
> The patch below adds EFI support to the SMBIOS driver. Since EFI
> already knows the address of the SMBIOS, this avoids having to scan
> for the table. It also enables use of the driver on ia64 machines.
> The patch also adds code to handle the case where the SMBIOS table
> resides in memory, which is the case at least for HP's zx1-based ia64
> machines. If CONFIG_EFI is off, the resulting code should be
> unchanged (except for replacing a readb() loop into a
> memcpy_fromio()).
>
> One observation: I believe find_table_max_address() is missing some
> readb() calls (it's dereferencing ioremap'pped addresses directly). I
> didn't try to fix that since I wasn't sure why it wasn't done in the
> first place.
>
> Do you have a test-program that's using /sys/firmware/smbios?
>
> If the patch looks OK, please apply.
>
> Thanks,
>
> --david

2004-06-05 01:17:44

by David Mosberger

[permalink] [raw]
Subject: Re: EFI-support for SMBIOS driver

>>>>> On Sat, 5 Jun 2004 03:09:36 +0200, Bartlomiej Zolnierkiewicz <[email protected]> said:

Bartlomiej> It seems SMBIOS driver is gone.

Bartlomiej> http://linus.bkbits.net:8080/linux-2.5/cset@40b6702axansvQIxOVjTSIp7_DVoHA

Man, talk about going in circles!

If folks are serious about discouraging /dev/mem, then the SMBIOS
driver makes tons of sense.

--david

2004-06-05 03:33:02

by Greg KH

[permalink] [raw]
Subject: Re: EFI-support for SMBIOS driver

On Fri, Jun 04, 2004 at 05:52:21PM -0700, David Mosberger wrote:
> Michael,
>
> The patch below adds EFI support to the SMBIOS driver.

The smbios driver is gone in 2.6.7-rc. You don't need a driver for
this, as you can do everything from userspace.

thanks,

greg k-h

2004-06-05 03:49:03

by David Mosberger

[permalink] [raw]
Subject: Re: EFI-support for SMBIOS driver

>>>>> On Fri, 4 Jun 2004 20:29:02 -0700, Greg KH <[email protected]> said:

Greg> On Fri, Jun 04, 2004 at 05:52:21PM -0700, David Mosberger
Greg> wrote:

>> The patch below adds EFI support to the SMBIOS driver.

Greg> The smbios driver is gone in 2.6.7-rc. You don't need a
Greg> driver for this, as you can do everything from userspace.

I know full well that it can be done in user-level --- via /dev/mem,
which lots of people dislike. I certainly don't feel strongly about
it, but the SMBIOS driver made sense to me.

--david

2004-06-06 00:14:25

by Michael E Brown

[permalink] [raw]
Subject: Re: EFI-support for SMBIOS driver

Sorry David.

I was the original submitter. Somebody kindly pointed out to me
(offline) the error of my ways, and I was able to get my userspace lib
to work just fine using mmap() instead of read(). This takes care of two
out of three of the main reasons I orignally submitted this driver.

Since my library was the only user of this lib, and I no longer need it,
I asked several people if it would make sense to continue to support it
or to pull it. The consensus was not to "bloat" the kernel with extra
stuff if it can be done via /dev/mem.

If there are strong sentiments the other way, the code is still out
there if somebody else wants to take over. I looked at what it would
take to add EFI support for this, and it would be trivial. I just didn't
want to become yet another absentee-maintainer if I no longer need the
code.
--
Michael Brown

On Fri, 2004-06-04 at 22:48, David Mosberger wrote:
> >>>>> On Fri, 4 Jun 2004 20:29:02 -0700, Greg KH <[email protected]> said:
>
> Greg> On Fri, Jun 04, 2004 at 05:52:21PM -0700, David Mosberger
> Greg> wrote:
>
> >> The patch below adds EFI support to the SMBIOS driver.
>
> Greg> The smbios driver is gone in 2.6.7-rc. You don't need a
> Greg> driver for this, as you can do everything from userspace.
>
> I know full well that it can be done in user-level --- via /dev/mem,
> which lots of people dislike. I certainly don't feel strongly about
> it, but the SMBIOS driver made sense to me.
>
> --david