ia64: use i386 dmi_scan.c

Andi Kleen has a patch in his x86_64 tree which enables the use of i386 dmi_scan.c on x86_64. dmi_scan.c functions are being used by the drivers/char/ipmi/ipmi_si_intf.c driver for autodetecting the ports or memory spaces where the IPMI controllers may be found.

This patch adds equivalent changes for ia64 as to what is in the x86_64 tree.

Tested on ia64 with 2.6.15, plus the 'dmi' patch from ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt which Andi has queued for submission for 2.6.16, plus a patch to ipmi_si_intf.c which will follow separately.

Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

index 199eeaf..e4c7988 100644
@@ -5,6 +5,10 @@
 
 mainmenu "IA-64 Linux Kernel Configuration"
 
+config DMI
+	bool
+	default y
+
 source "init/Kconfig"
 
 menu "Processor type and features"

index 307514f..6fc59ed 100644
@@ -7,7 +7,7 @@ extra-y	:= head.o init_task.o vmlinux.ld
 obj-y := acpi.o entry.o efi.o efi_stub.o gate-data.o fsys.o ia64_ksyms.o irq.o irq_ia64.o	\
 	 irq_lsapic.o ivt.o machvec.o pal.o patch.o process.o perfmon.o ptrace.o sal.o		\
 	 salinfo.o semaphore.o setup.o signal.o sys_ia64.o time.o traps.o unaligned.o \
-	 unwind.o mca.o mca_asm.o topology.o
+	 unwind.o mca.o mca_asm.o topology.o dmi_scan.o
 
 obj-$(CONFIG_IA64_BRL_EMU)	+= brl_emu.o
 obj-$(CONFIG_IA64_GENERIC)	+= acpi-ext.o

@@ -25,6 +25,7 @@ obj-$(CONFIG_IA64_MCA_RECOVERY)	+= mca_r
 obj-$(CONFIG_KPROBES)		+= kprobes.o jprobes.o
 obj-$(CONFIG_IA64_UNCACHED_ALLOCATOR)	+= uncached.o
 mca_recovery-y			+= mca_drv.o mca_drv_asm.o
+dmi_scan-y			+= ../../i386/kernel/dmi_scan.o
 
 # The gate DSO image is built using a special linker script.
 targets += gate.so gate-syms.o

index 5add0bc..03e0c60 100644
@@ -43,6 +43,7 @@
 #include <linux/initrd.h>
 #include <linux/platform.h>
 #include <linux/pm.h>
+#include <linux/dmi.h>
 
 #include <asm/ia32.h>
 #include <asm/machvec.h>

@@ -870,3 +871,10 @@ check_bugs (void)
 	ia64_patch_mckinley_e9((unsigned long) __start___mckinley_e9_bundles,
 			       (unsigned long) __end___mckinley_e9_bundles);
 }
+
+static int __init run_dmi_scan(void)
+{
+	dmi_scan_machine();
+	return 0;
+}
+core_initcall(run_dmi_scan);

index cf772a6..54f7457 100644
@@ -434,6 +434,11 @@ iounmap (volatile void __iomem *addr)
 
 #define ioremap_nocache(o,s)	ioremap(o,s)
 
+/* Use normal IO mappings for DMI */
+#define dmi_ioremap ioremap
+#define dmi_iounmap(x,l) iounmap(x)
+#define dmi_alloc(l) kmalloc(l, GFP_ATOMIC)
+
 # ifdef __KERNEL__
 
 /*

Can't this be done via ACPI/EFI? I'm really opposed to adding
anything to ia64 that blindly picks memory ranges and starts scanning for magic legacy tables. If nothing else, this can be found via efi.smbios. Thanks,

Alex

I'll redo this to use efi.smbios. Thanks for the tip.

Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

The DMI scan looks like it's done in try_init_smbios(). But try_init_acpi() is done first. Since every ia64 machine has ACPI, I would think try_init_acpi() should be sufficient.

Or do you have a machine that doesn't supply the SPMI table used by try_init_acpi()?

Personally, I think try_init_acpi() should be re-done so it uses the normal acpi_bus_register_driver() mechanism, which would locate the IPMI device in the ACPI namespace. I don't think there's any need to rely on the SPMI, which is primarily there to support OS's that want to do IPMI stuff early in boot, before the ACPI machinery is ready.

Bjorn

This system (Dell PowerEdge 7250, very very similar to an Intel 4-way Itanium2 server) doesn't have an SPMI table, but it does have the IPMI information in the SMBIOS table.

Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

But the IPMI device *should* be described in the ACPI namespace, so using acpi_bus_register_driver() should be sufficient.

I think that would be a better approach than using the SMBIOS table. But it is certainly a lot more work :-(

Bjorn

You mentioned on IRC that /sys/firmware/acpi/namespace didn't contain anything that looked like an IPMI device. Try dumping the actual DSDT and looking there -- I'm not sure everything makes it into /sys/firmware/acpi/...

Use the latest "pmtools" from here:
http://ftp.kernel.org/pub/linux/kernel/people/lenb/acpi/utils

and "iasl" to disassemble it.

I did this on an Intel Tiger, and didn't see any "IPI" devices in the namespace either. I think it's a firmware bug if the hardware is there but not described in the namespace.

So maybe you'd have to grub through SMBIOS to workaround the firmware defect.

Indeed. I updated the BIOS to the latest available, and it doesn't list the IPMI controller in the ACPI namespace. FWIW, this is effectively an Intel S870BN4 "Tiger4" system. FWIW2, the Dell PowerEdge x8xx line (current shipping EM64T servers) doesn't have the IPMI controller in the ACPI namespace either.

I reworked the patch, such that on EFI-capable systems, it uses efi.smbios to find the _DMI_ table header, and on all non-EFI systems, continues the brute force search from 0xF0000 as before.

Thanks,
Matt

Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

I was under the impression that the correct way to find the DMI tables on IA64 was by the EFI[ng ;)] firmware interface

Enable DMI table parsing on ia64.

Andi Kleen has a patch in his x86_64 tree which enables the use of i386 dmi_scan.c on x86_64. dmi_scan.c functions are being used by the drivers/char/ipmi/ipmi_si_intf.c driver for autodetecting the ports or memory spaces where the IPMI controllers may be found.

This patch adds equivalent changes for ia64 as to what is in the x86_64 tree. In addition, I reworked the DMI detection, such that on EFI-capable systems, it uses the efi.smbios pointer to find the table, rather than brute-force searching from 0xF0000. On non-EFI systems, it continues the brute-force search.

My test system, an Intel S870BN4 'Tiger4', aka Dell PowerEdge 7250, with latest BIOS, does not list the IPMI controller in the ACPI namespace, nor does it have an ACPI SPMI table. Also note, currently shipping Dell x8xx EM64T servers don't have these either, so DMI is the only method for obtaining the address of the IPMI controller.

Tested on ia64 with 2.6.15, plus the 'dmi' patch from ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt which Andi has queued for submission for 2.6.16, plus a patch to ipmi_si_intf.c which will follow separately.

Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

index 6a93d75..87fd0a1 100644
@@ -5,6 +5,7 @@
 #include <linux/dmi.h>
 #include <linux/bootmem.h>
 #include <linux/slab.h>
+#include <linux/efi.h>
 
 static char * __init dmi_string(struct dmi_header *dm, u8 s)
 {

@@ -184,10 +185,36 @@ static void __init dmi_decode(struct dmi
 	}
 }
 
-void __init dmi_scan_machine(void)
+static int __init dmi_present(char __iomem *p)
 {
 	u8 buf[15];
+	memcpy_fromio(buf, p, 15);
+	if ((memcmp(buf, "_DMI_", 5) = 0) && dmi_checksum(buf)) {
+		u16 num = (buf[13] << 8) | buf[12];
+		u16 len = (buf[7] << 8) | buf[6];
+		u32 base = (buf[11] << 24) | (buf[10] << 16) |
+			(buf[9] << 8) | buf[8];
+
+		/*
+		 * DMI version 0.0 means that the real version is taken from
+		 * the SMBIOS version, which we don't know at this point.
+		 */
+		if (buf[14] != 0)
+			printk(KERN_INFO "DMI %d.%d present.\n",
+			       buf[14] >> 4, buf[14] & 0xF);
+		else
+			printk(KERN_INFO "DMI present.\n");
+		if (dmi_table(base,len, num, dmi_decode) = 0)
+			return 0;
+	}
+	return 1;
+}
+
+#ifndef CONFIG_EFI
+void __init dmi_scan_machine(void)
+{
 	char __iomem *p, *q;
+	int rc;
 
 	/*
 	 * no iounmap() for that ioremap(); it would be a no-op, but it's

@@ -199,30 +226,39 @@ void __init dmi_scan_machine(void)
 		goto out;
 
 	for (q = p; q < p + 0x10000; q += 16) {
-		memcpy_fromio(buf, q, 15);
-		if ((memcmp(buf, "_DMI_", 5) = 0) && dmi_checksum(buf)) {
-			u16 num = (buf[13] << 8) | buf[12];
-			u16 len = (buf[7] << 8) | buf[6];
-			u32 base = (buf[11] << 24) | (buf[10] << 16) |
-				   (buf[9] << 8) | buf[8];
-
-			/*
-			 * DMI version 0.0 means that the real version is taken from
-			 * the SMBIOS version, which we don't know at this point.
-			 */
-			if (buf[14] != 0)
-				printk(KERN_INFO "DMI %d.%d present.\n",
-					buf[14] >> 4, buf[14] & 0xF);
-			else
-				printk(KERN_INFO "DMI present.\n");
-
-			if (dmi_table(base,len, num, dmi_decode) = 0)
-				return;
-		}
+		rc = dmi_present(q);
+		if (!rc)
+			return;
 	}
+ out:	printk(KERN_INFO "DMI not present or invalid.\n");
+}
 
-out:	printk(KERN_INFO "DMI not present or invalid.\n");
+#else /* CONFIG_EFI */
+/* This is called as a core_initcall() because it isn't needed
+ * during early boot.  This also means we can iounmap the space
+ * when we're done with it.
+ */
+void __init dmi_scan_machine(void)
+{
+	char __iomem *p;
+	int rc;
+
+	if (!efi.smbios)
+		goto out;
+
+	p = ioremap((unsigned long)efi.smbios, 0x10000);
+	if (p = NULL)
+		goto out;
+
+	rc = dmi_present(p + 0x10); /* offset of _DMI_ string */
+	iounmap(p);
+	if (!rc)
+		return;
+
+ out:	printk(KERN_INFO "DMI not present or invalid.\n");
 }
+
+#endif /* CONFIG_EFI */
 
 
 /**

index 199eeaf..51ac4e0 100644
@@ -42,6 +42,10 @@ config TIME_INTERPOLATION
 	bool
 	default y
 
+config DMI
+	bool
+	default y
+
 config EFI
 	bool
 	default y

index 307514f..6fc59ed 100644
@@ -7,7 +7,7 @@ extra-y	:= head.o init_task.o vmlinux.ld
 obj-y := acpi.o entry.o efi.o efi_stub.o gate-data.o fsys.o ia64_ksyms.o irq.o irq_ia64.o	\
 	 irq_lsapic.o ivt.o machvec.o pal.o patch.o process.o perfmon.o ptrace.o sal.o		\
 	 salinfo.o semaphore.o setup.o signal.o sys_ia64.o time.o traps.o unaligned.o \
-	 unwind.o mca.o mca_asm.o topology.o
+	 unwind.o mca.o mca_asm.o topology.o dmi_scan.o
 
 obj-$(CONFIG_IA64_BRL_EMU)	+= brl_emu.o
 obj-$(CONFIG_IA64_GENERIC)	+= acpi-ext.o

@@ -25,6 +25,7 @@ obj-$(CONFIG_IA64_MCA_RECOVERY)	+= mca_r
 obj-$(CONFIG_KPROBES)		+= kprobes.o jprobes.o
 obj-$(CONFIG_IA64_UNCACHED_ALLOCATOR)	+= uncached.o
 mca_recovery-y			+= mca_drv.o mca_drv_asm.o
+dmi_scan-y			+= ../../i386/kernel/dmi_scan.o
 
 # The gate DSO image is built using a special linker script.
 targets += gate.so gate-syms.o

index 5add0bc..03e0c60 100644
@@ -43,6 +43,7 @@
 #include <linux/initrd.h>
 #include <linux/platform.h>
 #include <linux/pm.h>
+#include <linux/dmi.h>
 
 #include <asm/ia32.h>
 #include <asm/machvec.h>

@@ -870,3 +871,10 @@ check_bugs (void)
 	ia64_patch_mckinley_e9((unsigned long) __start___mckinley_e9_bundles,
 			       (unsigned long) __end___mckinley_e9_bundles);
 }
+
+static int __init run_dmi_scan(void)
+{
+	dmi_scan_machine();
+	return 0;
+}
+core_initcall(run_dmi_scan);

index cf772a6..54f7457 100644
@@ -434,6 +434,11 @@ iounmap (volatile void __iomem *addr)
 
 #define ioremap_nocache(o,s)	ioremap(o,s)
 
+/* Use normal IO mappings for DMI */
+#define dmi_ioremap ioremap
+#define dmi_iounmap(x,l) iounmap(x)
+#define dmi_alloc(l) kmalloc(l, GFP_ATOMIC)
+
 # ifdef __KERNEL__
 
 /*

Enable DMI table parsing on ia64.

This consolidates the two dmi_scan_machine() functions of the previous version of the patch into one function, and eliminates the #ifdef CONFIG_EFI test, as suggested by Matt Tolentino.

Andi Kleen has a patch in his x86_64 tree which enables the use of i386 dmi_scan.c on x86_64. dmi_scan.c functions are being used by the drivers/char/ipmi/ipmi_si_intf.c driver for autodetecting the ports or memory spaces where the IPMI controllers may be found.

This patch adds equivalent changes for ia64 as to what is in the x86_64 tree. In addition, I reworked the DMI detection, such that on EFI-capable systems, it uses the efi.smbios pointer to find the table, rather than brute-force searching from 0xF0000. On non-EFI systems, it continues the brute-force search.

My test system, an Intel S870BN4 'Tiger4', aka Dell PowerEdge 7250, with latest BIOS, does not list the IPMI controller in the ACPI namespace, nor does it have an ACPI SPMI table. Also note, currently shipping Dell x8xx EM64T servers don't have these either, so DMI is the only method for obtaining the address of the IPMI controller.

Tested on ia64 with 2.6.15, plus the 'dmi' patch from ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt which Andi has queued for submission for 2.6.16, plus a patch to ipmi_si_intf.c which will follow separately.

Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

index 6a93d75..2764eab 100644
@@ -5,6 +5,7 @@
 #include <linux/dmi.h>
 #include <linux/bootmem.h>
 #include <linux/slab.h>
+#include <linux/efi.h>
 
 static char * __init dmi_string(struct dmi_header *dm, u8 s)
 {

@@ -184,46 +185,71 @@ static void __init dmi_decode(struct dmi
 	}
 }
 
-void __init dmi_scan_machine(void)
+static int __init dmi_present(char __iomem *p)
 {
 	u8 buf[15];
-	char __iomem *p, *q;
+	memcpy_fromio(buf, p, 15);
+	if ((memcmp(buf, "_DMI_", 5) = 0) && dmi_checksum(buf)) {
+		u16 num = (buf[13] << 8) | buf[12];
+		u16 len = (buf[7] << 8) | buf[6];
+		u32 base = (buf[11] << 24) | (buf[10] << 16) |
+			(buf[9] << 8) | buf[8];
 
-	/*
-	 * no iounmap() for that ioremap(); it would be a no-op, but it's
-	 * so early in setup that sucker gets confused into doing what
-	 * it shouldn't if we actually call it.
-	 */
-	p = ioremap(0xF0000, 0x10000);
-	if (p = NULL)
-		goto out;
+		/*
+		 * DMI version 0.0 means that the real version is taken from
+		 * the SMBIOS version, which we don't know at this point.
+		 */
+		if (buf[14] != 0)
+			printk(KERN_INFO "DMI %d.%d present.\n",
+			       buf[14] >> 4, buf[14] & 0xF);
+		else
+			printk(KERN_INFO "DMI present.\n");
+		if (dmi_table(base,len, num, dmi_decode) = 0)
+			return 0;
+	}
+	return 1;
+}
 
-	for (q = p; q < p + 0x10000; q += 16) {
-		memcpy_fromio(buf, q, 15);
-		if ((memcmp(buf, "_DMI_", 5) = 0) && dmi_checksum(buf)) {
-			u16 num = (buf[13] << 8) | buf[12];
-			u16 len = (buf[7] << 8) | buf[6];
-			u32 base = (buf[11] << 24) | (buf[10] << 16) |
-				   (buf[9] << 8) | buf[8];
-
-			/*
-			 * DMI version 0.0 means that the real version is taken from
-			 * the SMBIOS version, which we don't know at this point.
-			 */
-			if (buf[14] != 0)
-				printk(KERN_INFO "DMI %d.%d present.\n",
-					buf[14] >> 4, buf[14] & 0xF);
-			else
-				printk(KERN_INFO "DMI present.\n");
+void __init dmi_scan_machine(void)
+{
+	char __iomem *p, *q;
+	int rc;
 
-			if (dmi_table(base,len, num, dmi_decode) = 0)
+	if (efi_enabled) {
+		if (!efi.smbios)
+			goto out;
+
+               /* This is called as a core_initcall() because it isn't
+                * needed during early boot.  This also means we can
+                * iounmap the space when we're done with it.
+		*/
+		p = ioremap((unsigned long)efi.smbios, 0x10000);
+		if (p = NULL)
+			goto out;
+
+		rc = dmi_present(p + 0x10); /* offset of _DMI_ string */
+		iounmap(p);
+		if (!rc)
+			return;
+	}
+	else {
+		/*
+		 * no iounmap() for that ioremap(); it would be a no-op, but it's
+		 * so early in setup that sucker gets confused into doing what
+		 * it shouldn't if we actually call it.
+		 */
+		p = ioremap(0xF0000, 0x10000);
+		if (p = NULL)
+			goto out;
+
+		for (q = p; q < p + 0x10000; q += 16) {
+			rc = dmi_present(q);
+			if (!rc)
 				return;
 		}
 	}
-
-out:	printk(KERN_INFO "DMI not present or invalid.\n");
+ out:	printk(KERN_INFO "DMI not present or invalid.\n");
 }
-
 
 /**
  *	dmi_check_system - check system DMI data

index 199eeaf..51ac4e0 100644
@@ -42,6 +42,10 @@ config TIME_INTERPOLATION
 	bool
 	default y
 
+config DMI
+	bool
+	default y
+
 config EFI
 	bool
 	default y

index 307514f..6fc59ed 100644
@@ -7,7 +7,7 @@ extra-y	:= head.o init_task.o vmlinux.ld
 obj-y := acpi.o entry.o efi.o efi_stub.o gate-data.o fsys.o ia64_ksyms.o irq.o irq_ia64.o	\
 	 irq_lsapic.o ivt.o machvec.o pal.o patch.o process.o perfmon.o ptrace.o sal.o		\
 	 salinfo.o semaphore.o setup.o signal.o sys_ia64.o time.o traps.o unaligned.o \
-	 unwind.o mca.o mca_asm.o topology.o
+	 unwind.o mca.o mca_asm.o topology.o dmi_scan.o
 
 obj-$(CONFIG_IA64_BRL_EMU)	+= brl_emu.o
 obj-$(CONFIG_IA64_GENERIC)	+= acpi-ext.o

@@ -25,6 +25,7 @@ obj-$(CONFIG_IA64_MCA_RECOVERY)	+= mca_r
 obj-$(CONFIG_KPROBES)		+= kprobes.o jprobes.o
 obj-$(CONFIG_IA64_UNCACHED_ALLOCATOR)	+= uncached.o
 mca_recovery-y			+= mca_drv.o mca_drv_asm.o
+dmi_scan-y			+= ../../i386/kernel/dmi_scan.o
 
 # The gate DSO image is built using a special linker script.
 targets += gate.so gate-syms.o

index 5add0bc..03e0c60 100644
@@ -43,6 +43,7 @@
 #include <linux/initrd.h>
 #include <linux/platform.h>
 #include <linux/pm.h>
+#include <linux/dmi.h>
 
 #include <asm/ia32.h>
 #include <asm/machvec.h>

@@ -870,3 +871,10 @@ check_bugs (void)
 	ia64_patch_mckinley_e9((unsigned long) __start___mckinley_e9_bundles,
 			       (unsigned long) __end___mckinley_e9_bundles);
 }
+
+static int __init run_dmi_scan(void)
+{
+	dmi_scan_machine();
+	return 0;
+}
+core_initcall(run_dmi_scan);

index cf772a6..54f7457 100644
@@ -434,6 +434,11 @@ iounmap (volatile void __iomem *addr)
 
 #define ioremap_nocache(o,s)	ioremap(o,s)
 
+/* Use normal IO mappings for DMI */
+#define dmi_ioremap ioremap
+#define dmi_iounmap(x,l) iounmap(x)
+#define dmi_alloc(l) kmalloc(l, GFP_ATOMIC)
+
 # ifdef __KERNEL__
 
 /*

Not unreasonable. The DMI standard for the tables isn't specifically an x86 thing but drawn mostly from the management group stuff. The arch specific stuff is whether you scan for a table or ask the EFIng firmware

At least on i386/x86-64 it is largely used for hardware/firmware bug workaround and these have been traditionally always compiled in

Or do you want to spend a lot of time on a bug report from a user only to discover they didn't enable the workarounds for their particular platform?

You might not need that right now but I can predict that at some point you'll need board specific workarounds - and then it will be very useful to have.

Well, i suppose it will be more common in the future. Perhaps get over that particular hatered?

-Andi

Yeah, that was a dumb idea. I just need to fix whatever's currently broken and then there'll be no harm in having it all the time.

It *is* a memory attribute problem. The current code always calls ioremap() on efi.smbios. The first problem is that this is a physical address on x86, but a virtual address on ia64.

The second problem is that we don't check the supported attributes for the SMBIOS table. On HP sx1000/sx2000, these tables are in system memory, which doesn't support uncacheable access, so ioremap() does the wrong thing.

The patch below addresses both problems (but I can't test the x86 EFI change). I don't really like it, because the memory attribute checking is not complete (it only checks the first page, not the whole range), and there's already very similar code in acpi_os_map_memory(), acpi_os_read_memory(), acpi_os_write_memory(), and efi_range_is_wc().

But it's a start, and maybe the consolidation could be done later.

Index: work-mm3/arch/i386/kernel/dmi_scan.c
=================================--- work-mm3.orig/arch/i386/kernel/dmi_scan.c 2006-01-17 15:18:42.000000000 -0700
+++ work-mm3/arch/i386/kernel/dmi_scan.c 2006-01-17 16:58:11.000000000 -0700
@@ -39,9 +39,18 @@
void (*decode)(struct dmi_header *))
{
u8 *buf, *data;
- int i = 0;
+ int iomem = 1, i = 0;

- buf = dmi_ioremap(base, len);
+ if (efi_enabled) {
+ if (efi_mem_attributes(base & EFI_MEMORY_WB)) {
+ iomem = 0;
+ buf = (u8 *) phys_to_virt(base);
+ } else if (efi_mem_attributes(base & EFI_MEMORY_UC))
+ buf = dmi_ioremap(base, len);
+ else
+ buf = NULL;
+ } else
+ buf = dmi_ioremap(base, len);
if (buf = NULL)
return -1;

@@ -66,7 +75,8 @@
data += 2;
i++;
}
- dmi_iounmap(buf, len);
+ if (iomem)
+ dmi_iounmap(buf, len);
return 0;
}

@@ -216,19 +226,30 @@
int rc;

if (efi_enabled) {
- if (!efi.smbios)
+ unsigned long phys_addr = __pa(efi.smbios);
+ int iomem = 0;
+
+ if (!phys_addr)
goto out;

/* This is called as a core_initcall() because it isn't
* needed during early boot. This also means we can
* iounmap the space when we're done with it.
*/
- p = ioremap((unsigned long)efi.smbios, 0x10000);
+ if (efi_mem_attributes(phys_addr & EFI_MEMORY_WB))
+ p = (char *) phys_to_virt(phys_addr);
+ else if (efi_mem_attributes(phys_addr & EFI_MEMORY_UC)) {
+ iomem = 1;
+ p = ioremap(phys_addr, 0x10000);
+ } else
+ p = NULL;
+
if (p = NULL)
goto out;

rc = dmi_present(p + 0x10); /* offset of _DMI_ string */
- iounmap(p);
+ if (iomem)
+ iounmap(p);
if (!rc)
return;
}
Index: work-mm3/arch/i386/kernel/efi.c
=================================--- work-mm3.orig/arch/i386/kernel/efi.c 2005-10-27 18:02:08.000000000 -0600
+++ work-mm3/arch/i386/kernel/efi.c 2006-01-17 17:10:20.000000000 -0700
@@ -391,7 +391,7 @@
printk(KERN_INFO " ACPI=0x%lx ", config_tables[i].table);
} else
if (efi_guidcmp(config_tables[i].guid, SMBIOS_TABLE_GUID) = 0) {
- efi.smbios = (void *) config_tables[i].table;
+ efi.smbios = __va(config_tables[i].table);
printk(KERN_INFO " SMBIOS=0x%lx ", config_tables[i].table);
} else
if (efi_guidcmp(config_tables[i].guid, HCDP_TABLE_GUID) = 0) {

At least on x86-64/i386 the ioremap is actually cached unless a MTRR changes it, but it normally doesn't here. If one wants to force uncached access one has to use ioremap_uncached(). You're saying IA64 ioremap forces uncached access? That seems weird.

Right. On ia64, ioremap() and ioremap_nocache() are the same. You'd have to ask David about the history behind this.

Yes, hiding this all inside ioremap() is probably a good idea. It'll slow it down a lot, but I guess it's probably not used in performance paths anyway. Thanks for the advice.

Existing distros (RHEL3, RHEL4, SLES9, ..) don't have a huge problem because my target user - the ipmi driver - does its own DMI scanning on those distros still. Ugly, but works. So the only concern is SLES10 and RHEL5, both of which are still rebasing to upstream best as I can tell from their publicly posted kernels. So if it can be done "right" in 2.6.16-rc, let's do that. If it will be intrusive, and after 2.6.16 is out, than a minimally intrusive but working patch for 2.6.16-rc would be preferred.

Thanks!
Matt

Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

OK, here's a set of patches to (hopefully) clean up this area a bit:

1 Simplify efi_mem_attribute_range() so I can use it both for
/dev/mem validation and ioremap() attribute checking.

2 Make ia64 ioremap() check memory attributes, so it works for
plain memory that only supports write-back access, as well as for MMIO space that only supports uncacheable access.

3 DMI ioremaps too much space, which makes it fail on machines
where the SMBIOS table is near the end of a memory region.

4 Keep EFI table addresses as physical, not virtual. The SMBIOS
address was physical on x86 but virtual on ia64, which broke dmi_scan_machine().

5 Use smarter ioremap() implementation to remove some cruft in
acpi_os_{read,write,map}_memory(). This one's just an optional cleanup; I don't think it fixes any bugs.

-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642

Pass the size, not a pointer to the size, to efi_mem_attribute_range().

This function validates memory regions for the /dev/mem read/write/mmap paths. The pointer allows arches to reduce the size of the range, but I think that's unnecessary complexity. Simplifying it will let me use efi_mem_attribute_range() to improve the ia64 ioremap() implementation.

Index: work-mm3/arch/ia64/kernel/efi.c
===================================================================
@@ -685,27 +685,34 @@
 /*
  * Determines whether the memory at phys_addr supports the desired
  * attribute (WB, UC, etc).  If this returns 1, the caller can safely
- * access *size bytes at phys_addr with the specified attribute.
+ * access size bytes at phys_addr with the specified attribute.
  */
-static int
-efi_mem_attribute_range (unsigned long phys_addr, unsigned long *size, u64 attr)
+int
+efi_mem_attribute_range (unsigned long phys_addr, unsigned long size, u64 attr)
 {
+	unsigned long end = phys_addr + size;
 	efi_memory_desc_t *md = efi_memory_descriptor(phys_addr);
-	unsigned long md_end;
 
-	if (!md || (md->attribute & attr) != attr)
+	/*
+	 * Some firmware doesn't report MMIO regions in the EFI memory
+	 * map.  The Intel BigSur (a.k.a. HP i2000) has this problem.
+	 * On those platforms, we have to assume UC is valid everywhere.
+	 */
+	if (!md || (md->attribute & attr) != attr) {
+		if (attr == EFI_MEMORY_UC && !efi_memmap_has_mmio())
+			return 1;
 		return 0;
+	}
 
 	do {
-		md_end = efi_md_end(md);
-		if (phys_addr + *size <= md_end)
+		unsigned long md_end = efi_md_end(md);
+
+		if (end <= md_end)
 			return 1;
 
 		md = efi_memory_descriptor(md_end);
-		if (!md || (md->attribute & attr) != attr) {
-			*size = md_end - phys_addr;
-			return 1;
-		}
+		if (!md || (md->attribute & attr) != attr)
+			return 0;
 	} while (md);
 	return 0;
 }

@@ -716,7 +723,7 @@
  * control access size.
  */
 int
-valid_phys_addr_range (unsigned long phys_addr, unsigned long *size)
+valid_phys_addr_range (unsigned long phys_addr, unsigned long size)
 {
 	return efi_mem_attribute_range(phys_addr, size, EFI_MEMORY_WB);
 }

@@ -731,7 +738,7 @@
  * because that doesn't appear in the boot-time EFI memory map.
  */
 int
-valid_mmap_phys_addr_range (unsigned long phys_addr, unsigned long *size)
+valid_mmap_phys_addr_range (unsigned long phys_addr, unsigned long size)
 {
 	if (efi_mem_attribute_range(phys_addr, size, EFI_MEMORY_WB))
 		return 1;

@@ -739,14 +746,6 @@
 	if (efi_mem_attribute_range(phys_addr, size, EFI_MEMORY_UC))
 		return 1;
 
-	/*
-	 * Some firmware doesn't report MMIO regions in the EFI memory map.
-	 * The Intel BigSur (a.k.a. HP i2000) has this problem.  In this
-	 * case, we can't use the EFI memory map to validate mmap requests.
-	 */
-	if (!efi_memmap_has_mmio())
-		return 1;
-
 	return 0;
 }
 

Index: work-mm3/drivers/char/mem.c
===================================================================
@@ -88,21 +88,15 @@
 }
 
 #ifndef ARCH_HAS_VALID_PHYS_ADDR_RANGE
-static inline int valid_phys_addr_range(unsigned long addr, size_t *count)
+static inline int valid_phys_addr_range(unsigned long addr, size_t count)
 {
-	unsigned long end_mem;
-
-	end_mem = __pa(high_memory);
-	if (addr >= end_mem)
+	if (addr + count > __pa(high_memory))
 		return 0;
 
-	if (*count > end_mem - addr)
-		*count = end_mem - addr;
-
 	return 1;
 }
 
-static inline int valid_mmap_phys_addr_range(unsigned long addr, size_t *size)
+static inline int valid_mmap_phys_addr_range(unsigned long addr, size_t size)
 {
 	return 1;
 }

@@ -119,7 +113,7 @@
 	ssize_t read, sz;
 	char *ptr;
 
-	if (!valid_phys_addr_range(p, &count))
+	if (!valid_phys_addr_range(p, count))
 		return -EFAULT;
 	read = 0;
 #ifdef __ARCH_HAS_NO_PAGE_ZERO_MAPPED

@@ -177,7 +171,7 @@
 	unsigned long copied;
 	void *ptr;
 
-	if (!valid_phys_addr_range(p, &count))
+	if (!valid_phys_addr_range(p, count))
 		return -EFAULT;
 
 	written = 0;

@@ -251,7 +245,7 @@
 {
 	size_t size = vma->vm_end - vma->vm_start;
 
-	if (!valid_mmap_phys_addr_range(vma->vm_pgoff << PAGE_SHIFT, &size))
+	if (!valid_mmap_phys_addr_range(vma->vm_pgoff << PAGE_SHIFT, size))
 		return -EINVAL;
 
 	vma->vm_page_prot = phys_mem_access_prot(file, vma->vm_pgoff,

Index: work-mm3/include/asm-ia64/io.h
===================================================================
@@ -88,8 +88,8 @@
 }
 
 #define ARCH_HAS_VALID_PHYS_ADDR_RANGE
-extern int valid_phys_addr_range (unsigned long addr, size_t *count); /* efi.c */
-extern int valid_mmap_phys_addr_range (unsigned long addr, size_t *count);
+extern int valid_phys_addr_range (unsigned long addr, size_t count); /* efi.c */
+extern int valid_mmap_phys_addr_range (unsigned long addr, size_t count);
 
 /*
  * The following two macros are deprecated and scheduled for removal.

Index: work-mm3/include/linux/efi.h
===================================================================
@@ -292,6 +292,8 @@
 extern u64 efi_get_iobase (void);
 extern u32 efi_mem_type (unsigned long phys_addr);
 extern u64 efi_mem_attributes (unsigned long phys_addr);
+extern int efi_mem_attribute_range (unsigned long phys_addr, unsigned long size,
+				    u64 attr);
 extern int __init efi_uart_console_only (void);
 extern void efi_initialize_iomem_resources(struct resource *code_resource,
 					struct resource *data_resource);

-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642

Check the EFI memory map so we can use the correct memory attributes for ioremap(). Previously, we always used uncacheable access, which blows up on some machines for regular system memory.

Depends on the previous efi_mem_attribute_range() patch.

Index: work-mm3/arch/ia64/mm/ioremap.c
===================================================================
@@ -0,0 +1,40 @@
+/*
+ * (c) Copyright 2006 Hewlett-Packard Development Company, L.P.
+ *	Bjorn Helgaas <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/compiler.h>
+#include <linux/efi.h>
+#include <asm/io.h>
+
+static inline void __iomem *
+__ioremap (unsigned long offset, unsigned long size)
+{
+	return (void __iomem *) (__IA64_UNCACHED_OFFSET | offset);
+}
+
+void __iomem *
+ioremap (unsigned long offset, unsigned long size)
+{
+	if (efi_mem_attribute_range(offset, size, EFI_MEMORY_UC))
+		return __ioremap(offset, size);
+
+	if (efi_mem_attribute_range(offset, size, EFI_MEMORY_WB))
+		return phys_to_virt(offset);
+
+	/*
+	 * Someday this should check ACPI resources so we
+	 * can do the right thing for hot-plugged regions.
+	 */
+	return __ioremap(offset, size);
+}
+
+void __iomem *
+ioremap_nocache (unsigned long offset, unsigned long size)
+{
+	return __ioremap(offset, size);
+}

Index: work-mm3/include/asm-ia64/io.h
===================================================================
@@ -416,25 +416,14 @@
 # define outl_p		outl
 #endif
 
-/*
- * An "address" in IO memory space is not clearly either an integer or a pointer. We will
- * accept both, thus the casts.
- *
- * On ia-64, we access the physical I/O memory space through the uncached kernel region.
- */
-static inline void __iomem *
-ioremap (unsigned long offset, unsigned long size)
-{
-	return (void __iomem *) (__IA64_UNCACHED_OFFSET | (offset));
-}
+extern void __iomem * ioremap(unsigned long offset, unsigned long size);
+extern void __iomem * ioremap_nocache (unsigned long offset, unsigned long size);
 
 static inline void
 iounmap (volatile void __iomem *addr)
 {
 }
 
-#define ioremap_nocache(o,s)	ioremap(o,s)
-
 /* Use normal IO mappings for DMI */
 #define dmi_ioremap ioremap
 #define dmi_iounmap(x,l) iounmap(x)

Index: work-mm3/arch/ia64/mm/Makefile
===================================================================
@@ -2,7 +2,7 @@
 # Makefile for the ia64-specific parts of the memory manager.
 #
 
-obj-y := init.o fault.o tlb.o extable.o
+obj-y := init.o fault.o tlb.o extable.o ioremap.o
 
 obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
 obj-$(CONFIG_NUMA)	   += numa.o

-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642

dmi_scan_machine() tries to ioremap 0x10000 (64K) bytes, even though it only looks at the first 32 bytes or so. If the SMBIOS table is near the end of a memory region, the ioremap() may fail when it shouldn't.

This is in the efi_enabled path, so it really only affects ia64 at the moment.

Index: work-mm3/arch/i386/kernel/dmi_scan.c
=================================--- work-mm3.orig/arch/i386/kernel/dmi_scan.c 2006-01-19 11:26:17.000000000 -0700
+++ work-mm3/arch/i386/kernel/dmi_scan.c 2006-01-19 11:30:21.000000000 -0700
@@ -223,7 +223,7 @@
* needed during early boot. This also means we can
* iounmap the space when we're done with it.
*/
- p = ioremap((unsigned long)efi.smbios, 0x10000);
+ p = ioremap((unsigned long)efi.smbios, 32);
if (p = NULL)
goto out;

Almost all users of the table addresses from the EFI system table want physical addresses. So rather than doing the pa->va->pa conversion, just keep physical addresses in struct efi.

This fixes a DMI bug: the efi structure contained the physical SMBIOS address on x86 but the virtual address on ia64, so dmi_scan_machine() used ioremap() on a virtual address on ia64.

This is essentially the same as an earlier patch by Matt Tolentino:

    http://marc.theaimsgroup.com/?l=linux-kernel&m2130292316281&w=2

except that this changes all table addresses, not just ACPI addresses.

Matt's original patch was backed out because it caused MCAs on HP sx1000 systems. That problem is resolved by the ioremap() attribute checking added for ia64.

Depends on the previous ia64 ioremap patch.

Index: work-mm3/arch/i386/kernel/efi.c
=================================--- work-mm3.orig/arch/i386/kernel/efi.c 2006-01-19 11:26:17.000000000 -0700
+++ work-mm3/arch/i386/kernel/efi.c 2006-01-19 11:37:23.000000000 -0700
@@ -377,29 +377,38 @@
if (config_tables = NULL)
printk(KERN_ERR PFX "Could not map EFI Configuration Table!\n");

+	efi.mps        = EFI_INVALID_TABLE_ADDR;
+	efi.acpi       = EFI_INVALID_TABLE_ADDR;
+	efi.acpi20     = EFI_INVALID_TABLE_ADDR;
+	efi.smbios     = EFI_INVALID_TABLE_ADDR;
+	efi.sal_systab = EFI_INVALID_TABLE_ADDR;
+	efi.boot_info  = EFI_INVALID_TABLE_ADDR;
+	efi.hcdp       = EFI_INVALID_TABLE_ADDR;
+	efi.uga        = EFI_INVALID_TABLE_ADDR;

+
for (i = 0; i < num_config_tables; i++) {
if (efi_guidcmp(config_tables[i].guid, MPS_TABLE_GUID) = 0) {
- efi.mps = (void *)config_tables[i].table;
+ efi.mps = config_tables[i].table;
printk(KERN_INFO " MPS=0x%lx ", config_tables[i].table);
} else
if (efi_guidcmp(config_tables[i].guid, ACPI_20_TABLE_GUID) = 0) {
- efi.acpi20 = __va(config_tables[i].table);
+ efi.acpi20 = config_tables[i].table;
printk(KERN_INFO " ACPI 2.0=0x%lx ", config_tables[i].table);
} else
if (efi_guidcmp(config_tables[i].guid, ACPI_TABLE_GUID) = 0) {
- efi.acpi = __va(config_tables[i].table);
+ efi.acpi = config_tables[i].table;
printk(KERN_INFO " ACPI=0x%lx ", config_tables[i].table);
} else
if (efi_guidcmp(config_tables[i].guid, SMBIOS_TABLE_GUID) = 0) {
- efi.smbios = (void *) config_tables[i].table;
+ efi.smbios = config_tables[i].table;
printk(KERN_INFO " SMBIOS=0x%lx ", config_tables[i].table);
} else
if (efi_guidcmp(config_tables[i].guid, HCDP_TABLE_GUID) = 0) {
- efi.hcdp = (void *)config_tables[i].table;
+ efi.hcdp = config_tables[i].table;
printk(KERN_INFO " HCDP=0x%lx ", config_tables[i].table);
} else
if (efi_guidcmp(config_tables[i].guid, UGA_IO_PROTOCOL_GUID) = 0) {
- efi.uga = (void *)config_tables[i].table;
+ efi.uga = config_tables[i].table;
printk(KERN_INFO " UGA=0x%lx ", config_tables[i].table);
}
}
Index: work-mm3/arch/ia64/kernel/efi.c
=================================--- work-mm3.orig/arch/ia64/kernel/efi.c 2006-01-19 11:36:52.000000000 -0700
+++ work-mm3/arch/ia64/kernel/efi.c 2006-01-19 11:37:23.000000000 -0700
@@ -466,24 +466,33 @@
printk(KERN_INFO "EFI v%u.%.02u by %s:",
efi.systab->hdr.revision >> 16, efi.systab->hdr.revision & 0xffff, vendor);

+	efi.mps        = EFI_INVALID_TABLE_ADDR;
+	efi.acpi       = EFI_INVALID_TABLE_ADDR;
+	efi.acpi20     = EFI_INVALID_TABLE_ADDR;
+	efi.smbios     = EFI_INVALID_TABLE_ADDR;
+	efi.sal_systab = EFI_INVALID_TABLE_ADDR;
+	efi.boot_info  = EFI_INVALID_TABLE_ADDR;
+	efi.hcdp       = EFI_INVALID_TABLE_ADDR;
+	efi.uga        = EFI_INVALID_TABLE_ADDR;

+
for (i = 0; i < (int) efi.systab->nr_tables; i++) {
if (efi_guidcmp(config_tables[i].guid, MPS_TABLE_GUID) = 0) {
- efi.mps = __va(config_tables[i].table);
+ efi.mps = config_tables[i].table;
printk(" MPS=0x%lx", config_tables[i].table);
} else if (efi_guidcmp(config_tables[i].guid, ACPI_20_TABLE_GUID) = 0) {
- efi.acpi20 = __va(config_tables[i].table);
+ efi.acpi20 = config_tables[i].table;
printk(" ACPI 2.0=0x%lx", config_tables[i].table);
} else if (efi_guidcmp(config_tables[i].guid, ACPI_TABLE_GUID) = 0) {
- efi.acpi = __va(config_tables[i].table);
+ efi.acpi = config_tables[i].table;
printk(" ACPI=0x%lx", config_tables[i].table);
} else if (efi_guidcmp(config_tables[i].guid, SMBIOS_TABLE_GUID) = 0) {
- efi.smbios = __va(config_tables[i].table);
+ efi.smbios = config_tables[i].table;
printk(" SMBIOS=0x%lx", config_tables[i].table);
} else if (efi_guidcmp(config_tables[i].guid, SAL_SYSTEM_TABLE_GUID) = 0) {
- efi.sal_systab = __va(config_tables[i].table);
+ efi.sal_systab = config_tables[i].table;
printk(" SALsystab=0x%lx", config_tables[i].table);
} else if (efi_guidcmp(config_tables[i].guid, HCDP_TABLE_GUID) = 0) {
- efi.hcdp = __va(config_tables[i].table);
+ efi.hcdp = config_tables[i].table;
printk(" HCDP=0x%lx", config_tables[i].table);
}
}
Index: work-mm3/drivers/firmware/efivars.c
=================================--- work-mm3.orig/drivers/firmware/efivars.c 2006-01-19 11:26:17.000000000 -0700
+++ work-mm3/drivers/firmware/efivars.c 2006-01-19 11:37:23.000000000 -0700
@@ -568,20 +568,20 @@
if (!entry || !buf)
return -EINVAL;

- if (efi.mps)
- str += sprintf(str, "MPS=0x%lx\n", __pa(efi.mps));
- if (efi.acpi20)
- str += sprintf(str, "ACPI20=0x%lx\n", __pa(efi.acpi20));
- if (efi.acpi)
- str += sprintf(str, "ACPI=0x%lx\n", __pa(efi.acpi));
- if (efi.smbios)
- str += sprintf(str, "SMBIOS=0x%lx\n", __pa(efi.smbios));
- if (efi.hcdp)
- str += sprintf(str, "HCDP=0x%lx\n", __pa(efi.hcdp));
- if (efi.boot_info)
- str += sprintf(str, "BOOTINFO=0x%lx\n", __pa(efi.boot_info));
- if (efi.uga)
- str += sprintf(str, "UGA=0x%lx\n", __pa(efi.uga));
+ if (efi.mps != EFI_INVALID_TABLE_ADDR)
+ str += sprintf(str, "MPS=0x%lx\n", efi.mps);
+ if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
+ str += sprintf(str, "ACPI20=0x%lx\n", efi.acpi20);
+ if (efi.acpi != EFI_INVALID_TABLE_ADDR)
+ str += sprintf(str, "ACPI=0x%lx\n", efi.acpi);
+ if (efi.smbios != EFI_INVALID_TABLE_ADDR)
+ str += sprintf(str, "SMBIOS=0x%lx\n", efi.smbios);
+ if (efi.hcdp != EFI_INVALID_TABLE_ADDR)
+ str += sprintf(str, "HCDP=0x%lx\n", efi.hcdp);
+ if (efi.boot_info != EFI_INVALID_TABLE_ADDR)
+ str += sprintf(str, "BOOTINFO=0x%lx\n", efi.boot_info);
+ if (efi.uga != EFI_INVALID_TABLE_ADDR)
+ str += sprintf(str, "UGA=0x%lx\n", efi.uga);

return str - buf;
}
Index: work-mm3/include/linux/efi.h
=================================--- work-mm3.orig/include/linux/efi.h 2006-01-19 11:28:43.000000000 -0700
+++ work-mm3/include/linux/efi.h 2006-01-19 11:37:23.000000000 -0700
@@ -240,19 +240,21 @@
unsigned long desc_size;
};

+#define EFI_INVALID_TABLE_ADDR (~0UL)
+
/*
* All runtime access to EFI goes through this structure:
*/
extern struct efi {
efi_system_table_t *systab; /* EFI system table */
- void *mps; /* MPS table */
- void *acpi; /* ACPI table (IA64 ext 0.71) */
- void *acpi20; /* ACPI table (ACPI 2.0) */
- void *smbios; /* SM BIOS table */
- void *sal_systab; /* SAL system table */
- void *boot_info; /* boot info table */
- void *hcdp; /* HCDP table */
- void *uga; /* UGA table */
+ unsigned long mps; /* MPS table */
+ unsigned long acpi; /* ACPI table (IA64 ext 0.71) */
+ unsigned long acpi20; /* ACPI table (ACPI 2.0) */
+ unsigned long smbios; /* SM BIOS table */
+ unsigned long sal_systab; /* SAL system table */
+ unsigned long boot_info; /* boot info table */
+ unsigned long hcdp; /* HCDP table */
+ unsigned long uga; /* UGA table */
efi_get_time_t *get_time;
efi_set_time_t *set_time; efi_get_wakeup_time_t *get_wakeup_time;
Index: work-mm3/arch/i386/kernel/acpi/boot.c
=================================--- work-mm3.orig/arch/i386/kernel/acpi/boot.c 2006-01-19 11:26:17.000000000 -0700
+++ work-mm3/arch/i386/kernel/acpi/boot.c 2006-01-19 11:37:23.000000000 -0700
@@ -659,10 +659,10 @@
unsigned long rsdp_phys = 0;

if (efi_enabled) {
- if (efi.acpi20)
- return __pa(efi.acpi20);
- else if (efi.acpi)
- return __pa(efi.acpi);
+ if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
+ return efi.acpi20;
+ else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
+ return efi.acpi;
}
/*
* Scan memory looking for the RSDP signature. First search EBDA (low
Index: work-mm3/arch/ia64/kernel/acpi.c
=================================--- work-mm3.orig/arch/ia64/kernel/acpi.c 2006-01-19 11:26:17.000000000 -0700
+++ work-mm3/arch/ia64/kernel/acpi.c 2006-01-19 11:37:23.000000000 -0700
@@ -618,9 +618,9 @@
{
unsigned long rsdp_phys = 0;

- if (efi.acpi20)
- rsdp_phys = __pa(efi.acpi20);
- else if (efi.acpi)
+ if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
+ rsdp_phys = efi.acpi20;
+ else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
printk(KERN_WARNING PREFIX
"v1.0/r0.71 tables no longer supported\n");
return rsdp_phys;
Index: work-mm3/drivers/acpi/osl.c
=================================--- work-mm3.orig/drivers/acpi/osl.c 2006-01-19 11:26:18.000000000 -0700
+++ work-mm3/drivers/acpi/osl.c 2006-01-19 11:37:23.000000000 -0700
@@ -156,12 +156,10 @@
{
if (efi_enabled) {
addr->pointer_type = ACPI_PHYSICAL_POINTER;
- if (efi.acpi20)
- addr->pointer.physical - (acpi_physical_address) virt_to_phys(efi.acpi20);
- else if (efi.acpi)
- addr->pointer.physical - (acpi_physical_address) virt_to_phys(efi.acpi);
+ if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
+ addr->pointer.physical = efi.acpi20;
+ else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
+ addr->pointer.physical = efi.acpi;
else {
printk(KERN_ERR PREFIX
"System description tables not found\n");
Index: work-mm3/arch/i386/kernel/dmi_scan.c
=================================--- work-mm3.orig/arch/i386/kernel/dmi_scan.c 2006-01-19 11:37:04.000000000 -0700
+++ work-mm3/arch/i386/kernel/dmi_scan.c 2006-01-19 11:37:58.000000000 -0700
@@ -216,14 +216,14 @@
int rc;

if (efi_enabled) {
- if (!efi.smbios)
+ if (efi.smbios = EFI_INVALID_TABLE_ADDR)
goto out;

/* This is called as a core_initcall() because it isn't
* needed during early boot. This also means we can
* iounmap the space when we're done with it.
*/
- p = ioremap((unsigned long)efi.smbios, 32);
+ p = ioremap(efi.smbios, 32);
if (p = NULL)
goto out;

Index: work-mm3/arch/ia64/kernel/setup.c
=================================--- work-mm3.orig/arch/ia64/kernel/setup.c 2006-01-19 11:26:17.000000000 -0700
+++ work-mm3/arch/ia64/kernel/setup.c 2006-01-19 11:37:23.000000000 -0700
@@ -444,7 +444,7 @@
find_memory();

/* process SAL system table: */
- ia64_sal_init(efi.sal_systab);
+ ia64_sal_init(__va(efi.sal_systab));

#ifdef CONFIG_SMP
cpu_physical_id(0) = hard_smp_processor_id();
Index: work-mm3/arch/ia64/sn/kernel/setup.c
=================================--- work-mm3.orig/arch/ia64/sn/kernel/setup.c 2006-01-19 11:26:17.000000000 -0700
+++ work-mm3/arch/ia64/sn/kernel/setup.c 2006-01-19 11:37:23.000000000 -0700
@@ -339,10 +339,11 @@
struct pcdp_interface_pci if_pci; extern struct efi efi;

- pcdp = efi.hcdp;
- if (! pcdp)
+ if (efi.hcdp = EFI_INVALID_TABLE_ADDR)
return; /* no hcdp/pcdp table */

+ pcdp = __va(efi.hcdp);
+
if (pcdp->rev < 3)
return; /* only support PCDP (rev >= 3) */

Index: work-mm3/drivers/firmware/pcdp.c
=================================--- work-mm3.orig/drivers/firmware/pcdp.c 2006-01-19 11:26:17.000000000 -0700
+++ work-mm3/drivers/firmware/pcdp.c 2006-01-19 11:37:23.000000000 -0700
@@ -89,19 +89,20 @@
struct pcdp_uart *uart;
struct pcdp_device *dev, *end;
int i, serial = 0;
+ int rc = -ENODEV;

- pcdp = efi.hcdp;
- if (!pcdp)
+ if (efi.hcdp = EFI_INVALID_TABLE_ADDR)
return -ENODEV;

- printk(KERN_INFO "PCDP: v%d at 0x%lx\n", pcdp->rev, __pa(pcdp));
+ pcdp = ioremap(efi.hcdp, 4096);
+ printk(KERN_INFO "PCDP: v%d at 0x%lx\n", pcdp->rev, efi.hcdp);

if (strstr(cmdline, "console=hcdp")) {
if (pcdp->rev < 3)
serial = 1;
} else if (strstr(cmdline, "console=")) {
printk(KERN_INFO "Explicit \"console=\"; ignoring PCDP\n");
- return -ENODEV;
+ goto out;
}

if (pcdp->rev < 3 && efi_uart_console_only())
@@ -110,7 +111,8 @@
for (i = 0, uart = pcdp->uart; i < pcdp->num_uarts; i++, uart++) {
if (uart->flags & PCDP_UART_PRIMARY_CONSOLE || serial) {
if (uart->type = PCDP_CONSOLE_UART) {
- return setup_serial_console(uart);
+ rc = setup_serial_console(uart);
+ goto out;
}
}
}
@@ -121,10 +123,13 @@
dev = (struct pcdp_device *) ((u8 *) dev + dev->length)) {
if (dev->flags & PCDP_PRIMARY_CONSOLE) {
if (dev->type = PCDP_CONSOLE_VGA) {
- return setup_vga_console(dev);
+ rc = setup_vga_console(dev);
+ goto out;
}
}
}

- return -ENODEV;
+out:
+ iounmap(pcdp);
+ return rc;
}
Index: work-mm3/include/asm-ia64/sn/sn_sal.h
=================================--- work-mm3.orig/include/asm-ia64/sn/sn_sal.h 2006-01-19 11:26:17.000000000 -0700
+++ work-mm3/include/asm-ia64/sn/sn_sal.h 2006-01-19 11:37:23.000000000 -0700
@@ -158,7 +158,7 @@
static inline u32
sn_sal_rev(void)
{
- struct ia64_sal_systab *systab = efi.sal_systab;
+ struct ia64_sal_systab *systab = __va(efi.sal_systab);

return (u32)(systab->sal_b_rev_major << 8 | systab->sal_b_rev_minor);
}

ia64 ioremap is now smart enough to use the correct memory attributes, so remove the EFI checks from osl.c.

Depends on the previous ia64 ioremap patch.

Index: work-mm3/drivers/acpi/osl.c
===================================================================
@@ -180,22 +180,14 @@
 acpi_os_map_memory(acpi_physical_address phys, acpi_size size,
 		   void __iomem ** virt)
 {
-	if (efi_enabled) {
-		if (EFI_MEMORY_WB & efi_mem_attributes(phys)) {
-			*virt = (void __iomem *)phys_to_virt(phys);
-		} else {
-			*virt = ioremap(phys, size);
-		}
-	} else {
-		if (phys > ULONG_MAX) {
-			printk(KERN_ERR PREFIX "Cannot map memory that high\n");
-			return AE_BAD_PARAMETER;
-		}
-		/*
-		 * ioremap checks to ensure this is in reserved space
-		 */
-		*virt = ioremap((unsigned long)phys, size);
+	if (phys > ULONG_MAX) {
+		printk(KERN_ERR PREFIX "Cannot map memory that high\n");
+		return AE_BAD_PARAMETER;
 	}
+	/*
+	 * ioremap checks to ensure this is in reserved space
+	 */
+	*virt = ioremap((unsigned long)phys, size);
 
 	if (!*virt)
 		return AE_NO_MEMORY;

@@ -405,18 +397,8 @@
 {
 	u32 dummy;
 	void __iomem *virt_addr;
-	int iomem = 0;
 
-	if (efi_enabled) {
-		if (EFI_MEMORY_WB & efi_mem_attributes(phys_addr)) {
-			/* HACK ALERT! We can use readb/w/l on real memory too.. */
-			virt_addr = (void __iomem *)phys_to_virt(phys_addr);
-		} else {
-			iomem = 1;
-			virt_addr = ioremap(phys_addr, width);
-		}
-	} else
-		virt_addr = (void __iomem *)phys_to_virt(phys_addr);
+	virt_addr = ioremap(phys_addr, width);
 	if (!value)
 		value = &dummy;
 

@@ -434,10 +416,7 @@
 		BUG();
 	}
 
-	if (efi_enabled) {
-		if (iomem)
-			iounmap(virt_addr);
-	}
+	iounmap(virt_addr);
 
 	return AE_OK;
 }

@@ -446,18 +425,8 @@
 acpi_os_write_memory(acpi_physical_address phys_addr, u32 value, u32 width)
 {
 	void __iomem *virt_addr;
-	int iomem = 0;
 
-	if (efi_enabled) {
-		if (EFI_MEMORY_WB & efi_mem_attributes(phys_addr)) {
-			/* HACK ALERT! We can use writeb/w/l on real memory too */
-			virt_addr = (void __iomem *)phys_to_virt(phys_addr);
-		} else {
-			iomem = 1;
-			virt_addr = ioremap(phys_addr, width);
-		}
-	} else
-		virt_addr = (void __iomem *)phys_to_virt(phys_addr);
+	virt_addr = ioremap(phys_addr, width);
 
 	switch (width) {
 	case 8:

@@ -473,8 +442,7 @@
 		BUG();
 	}
 
-	if (iomem)
-		iounmap(virt_addr);
+	iounmap(virt_addr);
 
 	return AE_OK;
 }

-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642

This set of patches works for me on my IA64 Tiger4 (Dell PowerEdge 7250) in kernel 2.6.16-rc1-mm4. modprobe ipmi_si successfully automatically finds the IPMI controller now.

Thanks Bjorn for doing a more complete cleanup in support of this!

Thanks,
Matt

Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

There's a patch in Andi's queue (ftp://ftp.firstfloor.org/pub/ak/x86_64/quilt-current/patches/feature/dmi-early) which adds

#include <asm/dmi.h>

into arch/i386/kernel/dmi_scan.c.

hence with both your patch and Andi's patch applied, ia64 won't compile due to missing asm/dmi.h.

I've been reverting Andi's patch due to this, but it seems reasonable that dmi_scan.c be able to include asm/dmi.h..

So to get these thing to play together properly we'll need an include/asm-ia64/dmi.h which does the stuff which is in the two dmi.h's which Andi's patch creates.

Is that something you could take a look at, please?

You'll need a kernel to patch, so I'll include this in next -mm:

@@ -0,0 +1,6 @@
+#ifndef _ASM_DMI_H
+#define _ASM_DMI_H 1
+
+#include <asm/io.h>
+
+#endif

diff -puN arch/i386/kernel/dmi_scan.c~ia64-use-i386-dmi_scanc-fix arch/i386/kernel/dmi_scan.c
@@ -224,7 +224,7 @@ void __init dmi_scan_machine(void)
                 * needed during early boot.  This also means we can
                 * iounmap the space when we're done with it.
 		*/
-		p = ioremap((unsigned long)efi.smbios, 0x10000);
+		p = dmi_ioremap((unsigned long)efi.smbios, 0x10000);
 		if (p == NULL)
 			goto out;
 

@@ -235,11 +235,11 @@ void __init dmi_scan_machine(void)
 	}
 	else {
 		/*
-		 * no iounmap() for that ioremap(); it would be a no-op, but it's
-		 * so early in setup that sucker gets confused into doing what
-		 * it shouldn't if we actually call it.
+		 * no iounmap() for that ioremap(); it would be a no-op, but
+		 * it's so early in setup that sucker gets confused into doing
+		 * what it shouldn't if we actually call it.
 		 */
-		p = ioremap(0xF0000, 0x10000);
+		p = dmi_ioremap(0xF0000, 0x10000);
 		if (p == NULL)
 			goto out;
 

_

I assume that those ioremap->dmi_ioremap conversions are appropriate.

It could be that Andi's changes break the ia64 dmi impementation - I don't know. I guess it's OK if ia64 is not doing a scan "early".

The above might not compile, but I'll make sure that it does so before releasing next -mm.

So. Bottom line: please test the ia64 dmi patches in next -mm, send any needed fixups, thanks.

We should move this code into drivers/ or something - #including other architecture's stuff like this is awful.

It's not done "early", because at this point it's only needed for drivers. On i386 it's done "early" to catch some chipsets (coincidentally, Dell).

Built 2.6.16-rc6-mm2 on ia64 Itanium2 (Dell PowerEdge 7250, aka Intel Tiger4). Compiled clean, loaded clean, works as expected. Thanks!

I haven't tried same on x86_64 yet, will get to that ASAP.

Thanks,
Matt

Built 2.6.16-rc6-mm2 on x86_64 Dell PowerEdge 2800. Compiled clean, loaded clean, works as expected. Thanks!

-Matt

--
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux Linux on Dell mailing lists @ http://lists.us.dell.com

Sweet, thanks.