Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c

[Bjorn Helgaas <bjorn.helgaas@hp.com>]

On Friday 13 January 2006 17:24, Bjorn Helgaas wrote:
> ... the
> DMI stuff crashes HP sx2000 (and probably sx1000) boxes, probably
> because of some memory attribute problem.  So I'll have more
> feedback after I debug that ;-)

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) {