Make the x86_{phys,virt}_bits common for 32- and 64-bits, and use the
former in ioremap's phys_addr_valid() check also on 32bit/PAE.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

---
 arch/x86/kernel/cpu/common.c |   17 +++++++++++++++++
 arch/x86/mm/ioremap.c        |   15 ++++++---------
 include/asm-x86/processor.h  |    4 ++--
 3 files changed, 25 insertions(+), 11 deletions(-)

--- linux-2.6.27-rc5/arch/x86/kernel/cpu/common.c	2008-08-21 14:37:29.000000000 +0200
+++ 2.6.27-rc5-x86-phys-virt-bits/arch/x86/kernel/cpu/common.c	2008-08-22 15:24:33.000000000 +0200
@@ -305,6 +305,18 @@ static void __cpuinit early_get_cap(stru
 				c->x86_capability[1] = cpuid_edx(0x80000001);
 				c->x86_capability[6] = cpuid_ecx(0x80000001);
 			}
+			if (xlvl >= 0x80000008) {
+				u32 eax = cpuid_eax(0x80000008);
+
+				c->x86_phys_bits = eax & 0xff;
+				c->x86_virt_bits = (eax >> 8) & 0xff;
+				/* CPUID workaround for Intel 0F33/0F34 CPU */
+				if (c->x86_vendor == X86_VENDOR_INTEL
+				    && c->x86 == 0xF && c->x86_model == 0x3
+				    && (c->x86_mask == 0x3
+					|| c->x86_mask == 0x4))
+					c->x86_phys_bits = 36;
+			}
 		}
 
 	}
@@ -326,12 +338,17 @@ static void __init early_cpu_detect(void
 
 	c->x86_cache_alignment = 32;
 	c->x86_clflush_size = 32;
+	c->x86_phys_bits = 32;
+	c->x86_virt_bits = 32;
 
 	if (!have_cpuid_p())
 		return;
 
 	cpu_detect(c);
 
+	if (cpu_has(c, X86_FEATURE_PAE) || cpu_has(c, X86_FEATURE_PSE36))
+		c->x86_phys_bits = 36;
+
 	get_cpu_vendor(c, 1);
 
 	if (c->x86_vendor != X86_VENDOR_UNKNOWN &&
--- linux-2.6.27-rc5/arch/x86/mm/ioremap.c	2008-08-29 10:55:13.000000000 +0200
+++ 2.6.27-rc5-x86-phys-virt-bits/arch/x86/mm/ioremap.c	2008-08-22 15:24:33.000000000 +0200
@@ -32,19 +32,16 @@ unsigned long __phys_addr(unsigned long 
 }
 EXPORT_SYMBOL(__phys_addr);
 
-static inline int phys_addr_valid(unsigned long addr)
-{
-	return addr < (1UL << boot_cpu_data.x86_phys_bits);
-}
-
-#else
+#endif
 
-static inline int ...
[ message continues ]

hm, the cpu/common.c bits just got reworked massively in tip/master. 
I've tried a blind merge (see the patch below) but at least the first 
hunk looks wrong. Mind looking at how to merge this?

	Ingo

--------------->
Subject: x86: x86_{phys,virt}_bits field also for i386 (v2)
From: "Jan Beulich" <jbeulich@novell.com>
Date: Fri, 05 Sep 2008 13:07:24 +0100

Make the x86_{phys,virt}_bits common for 32- and 64-bits, and use the
former in ioremap's phys_addr_valid() check also on 32bit/PAE.

Signed-off-by: Jan Beulich <jbeulich@novell.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>

---
 arch/x86/kernel/cpu/common.c |   17 +++++++++++++++++
 arch/x86/mm/ioremap.c        |   15 ++++++---------
 include/asm-x86/processor.h  |    4 ++--
 3 files changed, 25 insertions(+), 11 deletions(-)

Index: tip/arch/x86/kernel/cpu/common.c
===================================================================
--- tip.orig/arch/x86/kernel/cpu/common.c
+++ tip/arch/x86/kernel/cpu/common.c
@@ -464,6 +464,18 @@ static void __cpuinit get_cpu_cap(struct
 			c->x86_capability[1] = cpuid_edx(0x80000001);
 			c->x86_capability[6] = cpuid_ecx(0x80000001);
 		}
+			if (xlvl >= 0x80000008) {
+				u32 eax = cpuid_eax(0x80000008);
+
+				c->x86_phys_bits = eax & 0xff;
+				c->x86_virt_bits = (eax >> 8) & 0xff;
+				/* CPUID workaround for Intel 0F33/0F34 CPU */
+				if (c->x86_vendor == X86_VENDOR_INTEL
+				    && c->x86 == 0xF && c->x86_model == 0x3
+				    && (c->x86_mask == 0x3
+					|| c->x86_mask == 0x4))
+					c->x86_phys_bits = 36;
+			}
 	}
 
 #ifdef CONFIG_X86_64
@@ -503,6 +515,8 @@ static void __init early_identify_cpu(st
 	c->x86_clflush_size = 32;
 #endif
 	c->x86_cache_alignment = c->x86_clflush_size;
+	c->x86_phys_bits = 32;
+	c->x86_virt_bits = 32;
 
 	if (!have_cpuid_p())
 		return;
@@ -513,6 +527,9 @@ static void __init early_identify_cpu(st
 
 	cpu_detect(c);
 
+	if (cpu_has(c, X86_FEATURE_PAE) || cpu_has(c, X86_FEATURE_PSE36))
+		c->x86_phys_bits = 36;
+
 ...
[ message continues ]

there is the same code in the same func for 64 bit.

also could move the intel workaround code to early_init_intel and init_intel of
intel.c/intel_64.c

YH
--

[ message continues ]

This is my take at it:

Make the x86_{phys,virt}_bits common for 32- and 64-bits, and use the
former in ioremap's phys_addr_valid() check also on 32bit/PAE.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

---
 arch/x86/kernel/cpu/common.c |   22 +++++++++++++++++++++-
 arch/x86/mm/ioremap.c        |   13 ++++++-------
 include/asm-x86/processor.h  |    4 ++--
 3 files changed, 29 insertions(+), 10 deletions(-)

--- linux-x86.orig/arch/x86/kernel/cpu/common.c
+++ linux-x86/arch/x86/kernel/cpu/common.c
@@ -472,14 +472,20 @@ static void __cpuinit get_cpu_cap(struct
 		if (xlvl >= 0x80860001)
 			c->x86_capability[2] = cpuid_edx(0x80860001);
 	}
+#endif
 
 	if (c->extended_cpuid_level >= 0x80000008) {
 		u32 eax = cpuid_eax(0x80000008);
 
 		c->x86_virt_bits = (eax >> 8) & 0xff;
 		c->x86_phys_bits = eax & 0xff;
+		/* CPUID workaround for Intel 0F33/0F34 CPU */
+		if (c->x86_vendor == X86_VENDOR_INTEL
+		    && c->x86 == 0xF && c->x86_model == 0x3
+		    && (c->x86_mask == 0x3
+			|| c->x86_mask == 0x4))
+			c->x86_phys_bits = 36;
 	}
-#endif
 
 	if (c->extended_cpuid_level >= 0x80000007)
 		c->x86_power = cpuid_edx(0x80000007);
@@ -500,6 +506,8 @@ static void __init early_identify_cpu(st
 	c->x86_clflush_size = 64;
 #else
 	c->x86_clflush_size = 32;
+	c->x86_phys_bits = 32;
+	c->x86_virt_bits = 32;
 #endif
 	c->x86_cache_alignment = c->x86_clflush_size;
 
@@ -512,6 +520,11 @@ static void __init early_identify_cpu(st
 
 	cpu_detect(c);
 
+#ifdef CONFIG_X86_32
+	if (cpu_has(c, X86_FEATURE_PAE) || cpu_has(c, X86_FEATURE_PSE36))
+		c->x86_phys_bits = 36;
+#endif
+
 	get_cpu_vendor(c);
 
 	get_cpu_cap(c);
@@ -635,6 +648,8 @@ static void __cpuinit identify_cpu(struc
 #else
 	c->cpuid_level = -1;	/* CPUID not detected */
 	c->x86_clflush_size = 32;
+	c->x86_phys_bits = 32;
+	c->x86_virt_bits = 32;
 #endif
 	c->x86_cache_alignment = c->x86_clflush_size;
 	memset(&c->x86_capability, 0, sizeof c->x86_capability);
@@ -652,6 +667,11 @@ static void ...
[ message continues ]

applied to tip/x86/mm-debug - thanks Jan!

	Ingo
--

[ message continues ]

-tip testing found various kernel crashes on 32-bit testboxes and i've 
bisected it down to:

| 3766e71257859cecb73d929c4974c729efeae51f is first bad commit
| commit 3766e71257859cecb73d929c4974c729efeae51f
| Author: Jan Beulich <jbeulich@novell.com>
| Date:   Mon Sep 8 11:50:21 2008 +0100
|
|     x86: x86_{phys,virt}_bits field also for i386 (v2)

 # bad:  [6b822d60] manual merge of x86/xen
 # good: [bce7f793] Linux 2.6.26
 # good: [6069fb2e] Re-delete zombie 'drivers/usb/serial/airprime.c' f
 # good: [f97017cd] net-sched: Fix actions flushing
 # good: [36fd71d2] devcgroup: fix race against rmdir()
 # good: [21534a92] Merge branch 'sched/rt'
 # good: [6d97b826] Merge branch 'timers/urgent'
 # good: [9c43834d] Merge branch 'out-of-tree'
 # good: [4ef47608] Merge branch 'x86/unify-cpu-detect'
 # good: [b3800c14] Merge branch 'kmemcheck'
 # good: [b8eb91b4] Merge branch 'x86/iommu'
 # good: [cc643d46] x86: adjust vmalloc_sync_all() for Xen (2nd try)
 # bad:  [4d485d0e] Merge branch 'x86/unify-cpu-detect'
 # bad:  [3766e712] x86: x86_{phys,virt}_bits field also for i386 (v2)
 # good: [5f5ddc2f] Merge branch 'x86/core' into x86/mm-debug

a typical crash is like the one attached below. It's due to the ioremap 
failing. The drivers/char/rio/rio_linux.c driver probes these addresses:

   static int rio_probe_addrs[] = { 0xc0000, 0xd0000, 0xe0000 };

which is questionable ...

for now i've reverted it from current tip/master, see commit 
e3fdd129901. (you can reinstate the commit by doing "git revert 
e3fdd1299"

Even if we decided to fail these ioremaps it would be better to emit a 
warning instead of crashing the box.

	Ingo

[   19.902718] calling  rio_init+0x0/0xd43
[   19.908049] device: 'rioctl': device_add
[   19.912116] PM: Adding info for No Bus:rioctl
[   19.918452] ioremap: invalid physical address c0000
[   19.920060] BUG: unable to handle kernel paging request at 00007c00
[   19.926814] IP: [<c08f3418>] rio_init+0x8d5/0xd43
[   19.931608] *pde = 00000000 ...
[ message continues ]

We shouldn't fail them, they're valid. What the crash means is that even
addresses below 1Mb are considered out of range, which I can only take
as x86_phys_bits being zero (or a bogus very small number) on
secondary (or all) CPUs. However, looking at the call tree I can't see how
that could happen (provided CPUID doesn't produce garbage output):

- smp_store_cpu_info(), as it always did, pre-initializes the new CPU's
  info with boot_cpu_data, and calls identify_secondary_cpu()
- identify_secondary_cpu() calls identify_cpu()
- identify_cpu() pre-sets x86_phys_bits to 32, and since the field didn't
  exist for 32-bits before, nothing should be able to clear or otherwise
  alter it

Jan

--

[ message continues ]

yeah - questionable in the sense of assuming that it's all non-RAM. But 

there's nothing weird about this testbox (it's a usual whitebox) - and 
two other testboxes failed as well after some time (no crashlog 
available from them). A 64-bit testbox didnt fail so it seems 32-bit 
only.

	Ingo
--

[ message continues ]

I just spotted it - it's a mismerge on my part. Will send v3 in a minute.

Jan

--

[ message continues ]

Make the x86_{phys,virt}_bits common for 32- and 64-bits, and use the
former in ioremap's phys_addr_valid() check also on 32bit/PAE.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

---
 arch/x86/kernel/cpu/common.c |   17 ++++++++++++++++-
 arch/x86/mm/ioremap.c        |   15 +++++++--------
 include/asm-x86/processor.h  |    4 ++--
 3 files changed, 25 insertions(+), 11 deletions(-)

--- linux-x86.orig/arch/x86/kernel/cpu/common.c
+++ linux-x86/arch/x86/kernel/cpu/common.c
@@ -439,6 +439,11 @@ void __cpuinit cpu_detect(struct cpuinfo
 			c->x86_cache_alignment = c->x86_clflush_size;
 		}
 	}
+
+#ifdef CONFIG_X86_32
+	if (cpu_has(c, X86_FEATURE_PAE) || cpu_has(c, X86_FEATURE_PSE36))
+		c->x86_phys_bits = 36;
+#endif
 }
 
 static void __cpuinit get_cpu_cap(struct cpuinfo_x86 *c)
@@ -472,14 +477,20 @@ static void __cpuinit get_cpu_cap(struct
 		if (xlvl >= 0x80860001)
 			c->x86_capability[2] = cpuid_edx(0x80860001);
 	}
+#endif
 
 	if (c->extended_cpuid_level >= 0x80000008) {
 		u32 eax = cpuid_eax(0x80000008);
 
 		c->x86_virt_bits = (eax >> 8) & 0xff;
 		c->x86_phys_bits = eax & 0xff;
+		/* CPUID workaround for Intel 0F33/0F34 CPU */
+		if (c->x86_vendor == X86_VENDOR_INTEL
+		    && c->x86 == 0xF && c->x86_model == 0x3
+		    && (c->x86_mask == 0x3
+			|| c->x86_mask == 0x4))
+			c->x86_phys_bits = 36;
 	}
-#endif
 
 	if (c->extended_cpuid_level >= 0x80000007)
 		c->x86_power = cpuid_edx(0x80000007);
@@ -500,6 +511,8 @@ static void __init early_identify_cpu(st
 	c->x86_clflush_size = 64;
 #else
 	c->x86_clflush_size = 32;
+	c->x86_phys_bits = 32;
+	c->x86_virt_bits = 32;
 #endif
 	c->x86_cache_alignment = c->x86_clflush_size;
 
@@ -635,6 +648,8 @@ static void __cpuinit identify_cpu(struc
 #else
 	c->cpuid_level = -1;	/* CPUID not detected */
 	c->x86_clflush_size = 32;
+	c->x86_phys_bits = 32;
+	c->x86_virt_bits = 32;
 #endif
 	c->x86_cache_alignment = c->x86_clflush_size;
 	memset(&c->x86_capability, 0, sizeof c->x86_capability);
--- ...
[ message continues ]

thanks, i've applied the delta below.

	Ingo

---------->
From b9397fe215791b4f83bda23b9f3c0d5200e94558 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Tue, 9 Sep 2008 09:45:32 +0200
Subject: [PATCH] x86: x86_{phys,virt}_bits field also for i386, fix

fix mismerge.

Signed-off-by: Jan Beulich <jbeulich@novell.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/cpu/common.c |   15 +++++----------
 arch/x86/mm/ioremap.c        |    2 +-
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index df6c387..b26c09e 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -439,6 +439,11 @@ void __cpuinit cpu_detect(struct cpuinfo_x86 *c)
 			c->x86_cache_alignment = c->x86_clflush_size;
 		}
 	}
+
+#ifdef CONFIG_X86_32
+	if (cpu_has(c, X86_FEATURE_PAE) || cpu_has(c, X86_FEATURE_PSE36))
+		c->x86_phys_bits = 36;
+#endif
 }
 
 static void __cpuinit get_cpu_cap(struct cpuinfo_x86 *c)
@@ -520,11 +525,6 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
 
 	cpu_detect(c);
 
-#ifdef CONFIG_X86_32
-	if (cpu_has(c, X86_FEATURE_PAE) || cpu_has(c, X86_FEATURE_PSE36))
-		c->x86_phys_bits = 36;
-#endif
-
 	get_cpu_vendor(c);
 
 	get_cpu_cap(c);
@@ -667,11 +667,6 @@ static void __cpuinit identify_cpu(struct cpuinfo_x86 *c)
 
 	generic_identify(c);
 
-#ifdef CONFIG_X86_32
-	if (cpu_has(c, X86_FEATURE_PAE) || cpu_has(c, X86_FEATURE_PSE36))
-		c->x86_phys_bits = 36;
-#endif
-
 	if (this_cpu->c_identify)
 		this_cpu->c_identify(c);
 
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 74d16f2..ce207ba 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -25,7 +25,7 @@
 static inline int phys_addr_valid(unsigned long addr)
 {
 #ifdef CONFIG_RESOURCES_64BIT
-	return addr < (1UL << boot_cpu_data.x86_phys_bits);
+	return !(addr >> boot_cpu_data.x86_phys_bits);
 #else
 	return 1;
 #endif
--

[ message continues ]

build failure:

arch/x86/kernel/cpu/common.c: In function 'cpu_detect':
arch/x86/kernel/cpu/common.c:445: error: 'struct cpuinfo_x86' has no member named 'x86_phys_bits'

	Ingo
--

[ message continues ]

Not for me - the original patch made the field common to 32/64-bits, so the
delta patch (with the original one re-enabled [or the revert removed])
should be able to use it. It's also suspicious that you get this only in one
place, while the field is being used several times...

Jan

--

[ message continues ]

these lines should be in early_init_intel/init_intel.
also need to be checked if it could be overwriten by others functions later..

BTW, did you address Peter's concern?

YH
--

[ message continues ]

Sorry, I don't recall anything else I needed to address.

Jan

--

[ message continues ]