linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RE: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
@ 2006-01-06 19:03 Tolentino, Matthew E
  2006-01-06 22:36 ` Matt Domsch
  0 siblings, 1 reply; 17+ messages in thread
From: Tolentino, Matthew E @ 2006-01-06 19:03 UTC (permalink / raw)
  To: Matt Domsch, linux-ia64, ak, openipmi-developer, akpm; +Cc: linux-kernel

Matt Domsch <> wrote:
> Enable DMI table parsing on ia64.
...
> +#ifndef CONFIG_EFI
> +void __init dmi_scan_machine(void)
> +{
>  	char __iomem *p, *q;
> +	int rc;

Hi Matt,

You could potentially consolidate the two dmi_scan_machine functions
and lose the ifdef (and duplication) by checking efi_enabled instead.  
'efi_enabled' is already ifdef'd in the EFI header (defined to 1 for 
ia64) specifically for this situation.  

matt

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-01-06 19:03 [PATCH 2.6.15] ia64: use i386 dmi_scan.c Tolentino, Matthew E
@ 2006-01-06 22:36 ` Matt Domsch
  0 siblings, 0 replies; 17+ messages in thread
From: Matt Domsch @ 2006-01-06 22:36 UTC (permalink / raw)
  To: Tolentino, Matthew E
  Cc: linux-ia64, ak, openipmi-developer, akpm, linux-kernel

On Fri, Jan 06, 2006 at 11:03:17AM -0800, Tolentino, Matthew E wrote:
> Matt Domsch <> wrote:
> > Enable DMI table parsing on ia64.
> ...
> > +#ifndef CONFIG_EFI
> > +void __init dmi_scan_machine(void)
> > +{
> >  	char __iomem *p, *q;
> > +	int rc;
> 
> Hi Matt,
> 
> You could potentially consolidate the two dmi_scan_machine functions
> and lose the ifdef (and duplication) by checking efi_enabled instead.  
> 'efi_enabled' is already ifdef'd in the EFI header (defined to 1 for 
> ia64) specifically for this situation.  

Good idea, patch to follow.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-03-18 15:43         ` Matt Domsch
@ 2006-03-18 19:51           ` Andrew Morton
  0 siblings, 0 replies; 17+ messages in thread
From: Andrew Morton @ 2006-03-18 19:51 UTC (permalink / raw)
  To: Matt Domsch
  Cc: linux-ia64, ak, openipmi-developer, matthew.e.tolentino, linux-kernel

Matt Domsch <Matt_Domsch@dell.com> wrote:
>
> > Built 2.6.16-rc6-mm2 on ia64 Itanium2 (Dell PowerEdge 7250, aka Intel
>  > Tiger4).  Compiled clean, loaded clean, works as expected.  Thanks!
> 
>  Built 2.6.16-rc6-mm2 on x86_64 Dell PowerEdge 2800.  Compiled clean,
>  loaded clean, works as expected.  Thanks!

Sweet, thanks.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-03-18 14:56       ` Matt Domsch
@ 2006-03-18 15:43         ` Matt Domsch
  2006-03-18 19:51           ` Andrew Morton
  0 siblings, 1 reply; 17+ messages in thread
From: Matt Domsch @ 2006-03-18 15:43 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-ia64, ak, openipmi-developer, matthew.e.tolentino, linux-kernel

On Sat, Mar 18, 2006 at 08:56:21AM -0600, Matt Domsch wrote:
> On Fri, Mar 17, 2006 at 03:54:45PM -0800, Andrew Morton wrote:
> > 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".
> 
> 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).
> 
> > 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.
> 
> 
> Built 2.6.16-rc6-mm2 on ia64 Itanium2 (Dell PowerEdge 7250, aka Intel
> Tiger4).  Compiled clean, loaded clean, works as expected.  Thanks!

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-03-17 23:54     ` Andrew Morton
@ 2006-03-18 14:56       ` Matt Domsch
  2006-03-18 15:43         ` Matt Domsch
  0 siblings, 1 reply; 17+ messages in thread
From: Matt Domsch @ 2006-03-18 14:56 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-ia64, ak, openipmi-developer, matthew.e.tolentino, linux-kernel

On Fri, Mar 17, 2006 at 03:54:45PM -0800, Andrew Morton wrote:
> 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".

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

> 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.


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-01-06 22:39   ` Matt Domsch
  2006-01-14  0:24     ` Bjorn Helgaas
@ 2006-03-17 23:54     ` Andrew Morton
  2006-03-18 14:56       ` Matt Domsch
  1 sibling, 1 reply; 17+ messages in thread
From: Andrew Morton @ 2006-03-17 23:54 UTC (permalink / raw)
  To: Matt Domsch
  Cc: linux-ia64, ak, openipmi-developer, matthew.e.tolentino, linux-kernel

Matt Domsch <Matt_Domsch@dell.com> wrote:
>
> +dmi_scan-y			+= ../../i386/kernel/dmi_scan.o
>

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:

--- /dev/null	Thu Apr 11 07:25:15 2002
+++ 25-akpm/include/asm-ia64/dmi.h	Fri Mar 17 15:49:50 2006
@@ -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
--- 25/arch/i386/kernel/dmi_scan.c~ia64-use-i386-dmi_scanc-fix	Fri Mar 17 15:49:37 2006
+++ 25-akpm/arch/i386/kernel/dmi_scan.c	Fri Mar 17 15:49:37 2006
@@ -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.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
@ 2006-01-19 14:19 Tolentino, Matthew E
  0 siblings, 0 replies; 17+ messages in thread
From: Tolentino, Matthew E @ 2006-01-19 14:19 UTC (permalink / raw)
  To: Andi Kleen, Bjorn Helgaas
  Cc: Matt Domsch, linux-ia64, openipmi-developer, akpm, linux-kernel

Andi Kleen <> wrote:
> Regarding physical vs virtual efi.smbios -- it sounds nasty to have
> such a difference in the EFI support for different architectures.
> Matthew, do you have a suggestion how this can be unified?

Yes, Bjorn and I talked about this offline.  I had a patch out there
a while back that converged this specifically for the acpi addresses.  
We should be able to convert them all to just one type.  

matt

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-01-18  0:17       ` Bjorn Helgaas
  2006-01-18  2:32         ` Andi Kleen
@ 2006-01-18 17:29         ` Bjorn Helgaas
  1 sibling, 0 replies; 17+ messages in thread
From: Bjorn Helgaas @ 2006-01-18 17:29 UTC (permalink / raw)
  To: Matt Domsch
  Cc: linux-ia64, ak, openipmi-developer, akpm, Tolentino, Matthew E,
	linux-kernel

On Tuesday 17 January 2006 17:17, Bjorn Helgaas wrote:
>> 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)) {

Ouch, ignore this patch if you haven't already.  The above is
parenthesized wrong.

Matt, what's your opinion on proceeding?  I know you want to get
the DMI stuff in distros.  I'm looking at reworking ioremap to
hide all this stuff, but that'll be a bigger change and may be
harder to get into distro releases.

For upstream, the ioremap rework sounds like the way to go, but
I don't know which the distros would prefer for updates.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-01-18  2:32         ` Andi Kleen
@ 2006-01-18 15:53           ` Bjorn Helgaas
  0 siblings, 0 replies; 17+ messages in thread
From: Bjorn Helgaas @ 2006-01-18 15:53 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Matt Domsch, linux-ia64, openipmi-developer, akpm, Tolentino,
	Matthew E, linux-kernel

On Tuesday 17 January 2006 19:32, Andi Kleen wrote:
> 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.

> > +	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;
> 
> I would expect your ioremap to already do such a lookup. That is at least
> how MTRRs on i386/x86-64 work.  If it does not how about you fix
> ioremap()?

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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-01-18  0:17       ` Bjorn Helgaas
@ 2006-01-18  2:32         ` Andi Kleen
  2006-01-18 15:53           ` Bjorn Helgaas
  2006-01-18 17:29         ` Bjorn Helgaas
  1 sibling, 1 reply; 17+ messages in thread
From: Andi Kleen @ 2006-01-18  2:32 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Matt Domsch, linux-ia64, openipmi-developer, akpm, Tolentino,
	Matthew E, linux-kernel

On Wednesday 18 January 2006 01:17, Bjorn Helgaas wrote:
> 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 iorem
> ap() does the wrong thing.

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.

> +	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;

I would expect your ioremap to already do such a lookup. That is at least
how MTRRs on i386/x86-64 work.  If it does not how about you fix
ioremap()? Or provide a suitable ia64 dmi_ioremap, but it's likely
better to do it generally.

Regarding physical vs virtual efi.smbios -- it sounds nasty to have such
a difference in the EFI support for different architectures. Matthew, do
you have a suggestion how this can be unified?

-Andi

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-01-14  0:24     ` Bjorn Helgaas
  2006-01-14  0:45       ` Alan Cox
  2006-01-14  1:19       ` Andi Kleen
@ 2006-01-18  0:17       ` Bjorn Helgaas
  2006-01-18  2:32         ` Andi Kleen
  2006-01-18 17:29         ` Bjorn Helgaas
  2 siblings, 2 replies; 17+ messages in thread
From: Bjorn Helgaas @ 2006-01-18  0:17 UTC (permalink / raw)
  To: Matt Domsch
  Cc: linux-ia64, ak, openipmi-developer, akpm, Tolentino, Matthew E,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-01-14  1:19       ` Andi Kleen
@ 2006-01-14  5:05         ` Bjorn Helgaas
  0 siblings, 0 replies; 17+ messages in thread
From: Bjorn Helgaas @ 2006-01-14  5:05 UTC (permalink / raw)
  To: Andi Kleen
  Cc: Matt Domsch, linux-ia64, openipmi-developer, akpm, Tolentino,
	Matthew E, linux-kernel

On Friday 13 January 2006 18:19, Andi Kleen wrote:
> On Saturday 14 January 2006 01:24, Bjorn Helgaas wrote:
> > On Friday 06 January 2006 15:39, Matt Domsch wrote:
> > > diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> > > ...
> > > +config DMI
> > > +       bool
> > > +       default y
> >
> > Should we have a way to turn this off?
> 
> 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?

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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-01-14  0:24     ` Bjorn Helgaas
  2006-01-14  0:45       ` Alan Cox
@ 2006-01-14  1:19       ` Andi Kleen
  2006-01-14  5:05         ` Bjorn Helgaas
  2006-01-18  0:17       ` Bjorn Helgaas
  2 siblings, 1 reply; 17+ messages in thread
From: Andi Kleen @ 2006-01-14  1:19 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Matt Domsch, linux-ia64, openipmi-developer, akpm, Tolentino,
	Matthew E, linux-kernel

On Saturday 14 January 2006 01:24, Bjorn Helgaas wrote:
> On Friday 06 January 2006 15:39, Matt Domsch wrote:
> > diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> > ...
> > +config DMI
> > +       bool
> > +       default y
>
> Should we have a way to turn this off?

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.

>
> > diff --git a/arch/ia64/kernel/Makefile b/arch/ia64/kernel/Makefile
> > ...
> > +dmi_scan-y                     += ../../i386/kernel/dmi_scan.o
>
> Ugh.  I really hate this sort of sharing.  Could dmi_scan.c go in
> drivers/firmware/ or something instead?

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

-Andi

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-01-14  0:24     ` Bjorn Helgaas
@ 2006-01-14  0:45       ` Alan Cox
  2006-01-14  1:19       ` Andi Kleen
  2006-01-18  0:17       ` Bjorn Helgaas
  2 siblings, 0 replies; 17+ messages in thread
From: Alan Cox @ 2006-01-14  0:45 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Matt Domsch, linux-ia64, ak, openipmi-developer, akpm, Tolentino,
	Matthew E, linux-kernel

> Ugh.  I really hate this sort of sharing.  Could dmi_scan.c go in
> drivers/firmware/ or something instead?

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-01-06 22:39   ` Matt Domsch
@ 2006-01-14  0:24     ` Bjorn Helgaas
  2006-01-14  0:45       ` Alan Cox
                         ` (2 more replies)
  2006-03-17 23:54     ` Andrew Morton
  1 sibling, 3 replies; 17+ messages in thread
From: Bjorn Helgaas @ 2006-01-14  0:24 UTC (permalink / raw)
  To: Matt Domsch
  Cc: linux-ia64, ak, openipmi-developer, akpm, Tolentino, Matthew E,
	linux-kernel

On Friday 06 January 2006 15:39, Matt Domsch wrote:
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> ...
> +config DMI
> +       bool
> +       default y

Should we have a way to turn this off?

> diff --git a/arch/ia64/kernel/Makefile b/arch/ia64/kernel/Makefile
> ...
> +dmi_scan-y                     += ../../i386/kernel/dmi_scan.o

Ugh.  I really hate this sort of sharing.  Could dmi_scan.c go in
drivers/firmware/ or something instead?

> diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
> ...
> +static int __init run_dmi_scan(void)
> +{
> +       dmi_scan_machine();
> +       return 0;
> +}
> +core_initcall(run_dmi_scan);

Shouldn't this be wrapped in "#ifdef CONFIG_DMI"?

Sorry this feedback is so late.  I only looked at it because 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 ;-)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-01-06 17:21 ` [PATCH 2.6.15] " Matt Domsch
@ 2006-01-06 22:39   ` Matt Domsch
  2006-01-14  0:24     ` Bjorn Helgaas
  2006-03-17 23:54     ` Andrew Morton
  0 siblings, 2 replies; 17+ messages in thread
From: Matt Domsch @ 2006-01-06 22:39 UTC (permalink / raw)
  To: linux-ia64, ak, openipmi-developer, akpm, Tolentino, Matthew E
  Cc: linux-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.

 
Signed-off-by: Matt Domsch <Matt_Domsch@dell.com>

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

diff --git a/arch/i386/kernel/dmi_scan.c b/arch/i386/kernel/dmi_scan.c
index 6a93d75..2764eab 100644
--- a/arch/i386/kernel/dmi_scan.c
+++ b/arch/i386/kernel/dmi_scan.c
@@ -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
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 199eeaf..51ac4e0 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -42,6 +42,10 @@ config TIME_INTERPOLATION
 	bool
 	default y
 
+config DMI
+	bool
+	default y
+
 config EFI
 	bool
 	default y
diff --git a/arch/ia64/kernel/Makefile b/arch/ia64/kernel/Makefile
index 307514f..6fc59ed 100644
--- a/arch/ia64/kernel/Makefile
+++ b/arch/ia64/kernel/Makefile
@@ -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
diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 5add0bc..03e0c60 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -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);
diff --git a/include/asm-ia64/io.h b/include/asm-ia64/io.h
index cf772a6..54f7457 100644
--- a/include/asm-ia64/io.h
+++ b/include/asm-ia64/io.h
@@ -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__
 
 /*


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2.6.15] ia64: use i386 dmi_scan.c
  2006-01-04 22:16 [PATCH 2.6.15 1/2] " Matt Domsch
@ 2006-01-06 17:21 ` Matt Domsch
  2006-01-06 22:39   ` Matt Domsch
  0 siblings, 1 reply; 17+ messages in thread
From: Matt Domsch @ 2006-01-06 17:21 UTC (permalink / raw)
  To: linux-ia64, ak, openipmi-developer, akpm; +Cc: linux-kernel

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.
 
Signed-off-by: Matt Domsch <Matt_Domsch@dell.com>

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

diff --git a/arch/i386/kernel/dmi_scan.c b/arch/i386/kernel/dmi_scan.c
index 6a93d75..87fd0a1 100644
--- a/arch/i386/kernel/dmi_scan.c
+++ b/arch/i386/kernel/dmi_scan.c
@@ -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 */
 
 
 /**
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 199eeaf..51ac4e0 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -42,6 +42,10 @@ config TIME_INTERPOLATION
 	bool
 	default y
 
+config DMI
+	bool
+	default y
+
 config EFI
 	bool
 	default y
diff --git a/arch/ia64/kernel/Makefile b/arch/ia64/kernel/Makefile
index 307514f..6fc59ed 100644
--- a/arch/ia64/kernel/Makefile
+++ b/arch/ia64/kernel/Makefile
@@ -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
diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 5add0bc..03e0c60 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -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);
diff --git a/include/asm-ia64/io.h b/include/asm-ia64/io.h
index cf772a6..54f7457 100644
--- a/include/asm-ia64/io.h
+++ b/include/asm-ia64/io.h
@@ -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__
 
 /*

^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2006-03-18 19:55 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-01-06 19:03 [PATCH 2.6.15] ia64: use i386 dmi_scan.c Tolentino, Matthew E
2006-01-06 22:36 ` Matt Domsch
  -- strict thread matches above, loose matches on Subject: below --
2006-01-19 14:19 Tolentino, Matthew E
2006-01-04 22:16 [PATCH 2.6.15 1/2] " Matt Domsch
2006-01-06 17:21 ` [PATCH 2.6.15] " Matt Domsch
2006-01-06 22:39   ` Matt Domsch
2006-01-14  0:24     ` Bjorn Helgaas
2006-01-14  0:45       ` Alan Cox
2006-01-14  1:19       ` Andi Kleen
2006-01-14  5:05         ` Bjorn Helgaas
2006-01-18  0:17       ` Bjorn Helgaas
2006-01-18  2:32         ` Andi Kleen
2006-01-18 15:53           ` Bjorn Helgaas
2006-01-18 17:29         ` Bjorn Helgaas
2006-03-17 23:54     ` Andrew Morton
2006-03-18 14:56       ` Matt Domsch
2006-03-18 15:43         ` Matt Domsch
2006-03-18 19:51           ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).