MPTable can not be high-memory on Linux

MPTable can not be high-memory on Linux

[Kevin O'Connor]

Hi Stefan,

I'm CC'ing lkml and coreboot mailing lists.

On Thu, Feb 19, 2009 at 10:26:17PM +0100, Stefan Reinauer wrote:
> OK, I played some more, and I finally found out it's earlyprintk and not
> early_printk. Plus there is bootmem_debug:
> 
> Initializing cgroup subsys cpuset
> Initializing cgroup subsys cpu
> Linux version 2.6.27.7-9-default (geeko@buildhost) (gcc version 4.3.2
> [gcc-4_3-branch re0
> PAT WC disabled due to known CPU erratum.
> BIOS-provided physical RAM map:
>  BIOS-e820: 0000000000000000 - 0000000000001000 (reserved)
>  BIOS-e820: 0000000000001000 - 0000000000090000 (usable)
>  BIOS-e820: 0000000000090000 - 0000000000100000 (reserved)
>  BIOS-e820: 0000000000100000 - 000000003f7f0000 (usable)
>  BIOS-e820: 000000003f7f0000 - 0000000040000000 (reserved)
>  BIOS-e820: 00000000f0000000 - 00000000f4000000 (reserved)
[...]
> ACPI: no DMI BIOS year, acpi=force is required to enable ACPI
> ACPI: Disabling ACPI support
[...]
> found SMP MP-table at [c00f9fc0] 000f9fc0
> bootmem::mark_bootmem_node nid=0 start=f9 end=fb reserve=1 flags=0
> bootmem::__reserve nid=0 start=f9 end=fb flags=0
> bootmem::__reserve silent double reserve of PFN f9
> bootmem::__reserve silent double reserve of PFN fa
> BUG: Int 6: CR2 00000000
>      EDI c00f9fd0  ESI 3f7f5410  EBP 0003f7f5  ESP c0535f20
>      EBX c056c00c  EDX 00000006  ECX 00000001  EAX c056c03c
>      err 00000000  EIP c054f8f0   CS 00000060  flg 00010046
> Stack: 00000000 00000000 00000001 00038000 0003f7f5 f880abf0 3f7f5410
> c00f9fd0
>        00000001 c054f91d 00000000 c054571a c0413e7e c00f9fc0 000f9fc0
> 00000001
>        3f7bf64d 00000000 3e2c5000 c054578a 00000000 c053f3b6 3f7bf64d
> 00000000
> 
> So it dies because of the way it tries to reserve the MP-table.
> 
> Here's the stack trace.
> 
>  [<c054f8f0>] mark_bootmem+0x9b/0xab
>  [<c054f91d>] reserve_bootmem+0x1d/0x1f
>  [<c054571a>] smp_scan_config+0xd9/0xfa
>  [<c054578a>] __find_smp_config+0x4f/0x6e
>  [<c053f3b6>] setup_arch+0x576/0x639
>  [<c054ebc4>] cgroup_init_subsys+0x29/0xc9
>  [<c053a6ac>] start_kernel+0x6b/0x31f
>  =======================

It looks like the problem is that the MPTable is located in the last
64K of memory (instead of the first few megabytes).  There is a
comment about this in arch/x86/kernel/mpparse.c:

   /*
    * We cannot access to MPC table to compute
    * table size yet, as only few megabytes from
    * the bottom is mapped now.
    * PC-9800's MPC table places on the very last
    * of physical memory; so that simply reserving
    * PAGE_SIZE from mpg->mpf_physptr yields BUG()
    * in reserve_bootmem.
    */

However, that comment is in an #ifdef specific to 32bit kernels.
(Though, it's not clear to me how that code would help as it sets size
to be a negative number.)

The easiest way to fix this is to change SeaBIOS to copy the whole
mptable to the first megabyte.  Also, we need to fix your SMBIOS so
that ACPI is used instead of the mptable.

-Kevin

Re: MPTable can not be high-memory on Linux

[Rafael J. Wysocki]

On Sunday 22 February 2009, Kevin O'Connor wrote:
> Hi Stefan,
> 
> I'm CC'ing lkml and coreboot mailing lists.

(added more CCs)

> On Thu, Feb 19, 2009 at 10:26:17PM +0100, Stefan Reinauer wrote:
> > OK, I played some more, and I finally found out it's earlyprintk and not
> > early_printk. Plus there is bootmem_debug:
> > 
> > Initializing cgroup subsys cpuset
> > Initializing cgroup subsys cpu
> > Linux version 2.6.27.7-9-default (geeko@buildhost) (gcc version 4.3.2
> > [gcc-4_3-branch re0
> > PAT WC disabled due to known CPU erratum.
> > BIOS-provided physical RAM map:
> >  BIOS-e820: 0000000000000000 - 0000000000001000 (reserved)
> >  BIOS-e820: 0000000000001000 - 0000000000090000 (usable)
> >  BIOS-e820: 0000000000090000 - 0000000000100000 (reserved)
> >  BIOS-e820: 0000000000100000 - 000000003f7f0000 (usable)
> >  BIOS-e820: 000000003f7f0000 - 0000000040000000 (reserved)
> >  BIOS-e820: 00000000f0000000 - 00000000f4000000 (reserved)
> [...]
> > ACPI: no DMI BIOS year, acpi=force is required to enable ACPI
> > ACPI: Disabling ACPI support
> [...]
> > found SMP MP-table at [c00f9fc0] 000f9fc0
> > bootmem::mark_bootmem_node nid=0 start=f9 end=fb reserve=1 flags=0
> > bootmem::__reserve nid=0 start=f9 end=fb flags=0
> > bootmem::__reserve silent double reserve of PFN f9
> > bootmem::__reserve silent double reserve of PFN fa
> > BUG: Int 6: CR2 00000000
> >      EDI c00f9fd0  ESI 3f7f5410  EBP 0003f7f5  ESP c0535f20
> >      EBX c056c00c  EDX 00000006  ECX 00000001  EAX c056c03c
> >      err 00000000  EIP c054f8f0   CS 00000060  flg 00010046
> > Stack: 00000000 00000000 00000001 00038000 0003f7f5 f880abf0 3f7f5410
> > c00f9fd0
> >        00000001 c054f91d 00000000 c054571a c0413e7e c00f9fc0 000f9fc0
> > 00000001
> >        3f7bf64d 00000000 3e2c5000 c054578a 00000000 c053f3b6 3f7bf64d
> > 00000000
> > 
> > So it dies because of the way it tries to reserve the MP-table.
> > 
> > Here's the stack trace.
> > 
> >  [<c054f8f0>] mark_bootmem+0x9b/0xab
> >  [<c054f91d>] reserve_bootmem+0x1d/0x1f
> >  [<c054571a>] smp_scan_config+0xd9/0xfa
> >  [<c054578a>] __find_smp_config+0x4f/0x6e
> >  [<c053f3b6>] setup_arch+0x576/0x639
> >  [<c054ebc4>] cgroup_init_subsys+0x29/0xc9
> >  [<c053a6ac>] start_kernel+0x6b/0x31f
> >  =======================
> 
> It looks like the problem is that the MPTable is located in the last
> 64K of memory (instead of the first few megabytes).  There is a
> comment about this in arch/x86/kernel/mpparse.c:
> 
>    /*
>     * We cannot access to MPC table to compute
>     * table size yet, as only few megabytes from
>     * the bottom is mapped now.
>     * PC-9800's MPC table places on the very last
>     * of physical memory; so that simply reserving
>     * PAGE_SIZE from mpg->mpf_physptr yields BUG()
>     * in reserve_bootmem.
>     */
> 
> However, that comment is in an #ifdef specific to 32bit kernels.
> (Though, it's not clear to me how that code would help as it sets size
> to be a negative number.)
> 
> The easiest way to fix this is to change SeaBIOS to copy the whole
> mptable to the first megabyte.  Also, we need to fix your SMBIOS so
> that ACPI is used instead of the mptable.
> 
> -Kevin

Re: MPTable can not be high-memory on Linux

[Andi Kleen]

Kevin O'Connor <kevin@koconnor.net> writes:
>
> It looks like the problem is that the MPTable is located in the last
> 64K of memory (instead of the first few megabytes).  There is a
> comment about this in arch/x86/kernel/mpparse.c:
>
>    /*
>     * We cannot access to MPC table to compute
>     * table size yet, as only few megabytes from
>     * the bottom is mapped now.
>     * PC-9800's MPC table places on the very last
>     * of physical memory; so that simply reserving
>     * PAGE_SIZE from mpg->mpf_physptr yields BUG()
>     * in reserve_bootmem.
>     */
>
> However, that comment is in an #ifdef specific to 32bit kernels.
> (Though, it's not clear to me how that code would help as it sets size
> to be a negative number.)
>
> The easiest way to fix this is to change SeaBIOS to copy the whole
> mptable to the first megabyte.  Also, we need to fix your SMBIOS so
> that ACPI is used instead of the mptable.

The kernel should just use early_ioremap to access it I guess.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: MPTable can not be high-memory on Linux

[Yinghai Lu]

Rafael J. Wysocki wrote:
> On Sunday 22 February 2009, Kevin O'Connor wrote:
>> Hi Stefan,
>>
>> I'm CC'ing lkml and coreboot mailing lists.
> 
> (added more CCs)
> 
>> On Thu, Feb 19, 2009 at 10:26:17PM +0100, Stefan Reinauer wrote:
>>> OK, I played some more, and I finally found out it's earlyprintk and not
>>> early_printk. Plus there is bootmem_debug:
>>>
>>> Initializing cgroup subsys cpuset
>>> Initializing cgroup subsys cpu
>>> Linux version 2.6.27.7-9-default (geeko@buildhost) (gcc version 4.3.2
>>> [gcc-4_3-branch re0
>>> PAT WC disabled due to known CPU erratum.
>>> BIOS-provided physical RAM map:
>>>  BIOS-e820: 0000000000000000 - 0000000000001000 (reserved)
>>>  BIOS-e820: 0000000000001000 - 0000000000090000 (usable)
>>>  BIOS-e820: 0000000000090000 - 0000000000100000 (reserved)
>>>  BIOS-e820: 0000000000100000 - 000000003f7f0000 (usable)
>>>  BIOS-e820: 000000003f7f0000 - 0000000040000000 (reserved)
>>>  BIOS-e820: 00000000f0000000 - 00000000f4000000 (reserved)
>> [...]
>>> ACPI: no DMI BIOS year, acpi=force is required to enable ACPI
>>> ACPI: Disabling ACPI support
>> [...]
>>> found SMP MP-table at [c00f9fc0] 000f9fc0
>>> bootmem::mark_bootmem_node nid=0 start=f9 end=fb reserve=1 flags=0
>>> bootmem::__reserve nid=0 start=f9 end=fb flags=0
>>> bootmem::__reserve silent double reserve of PFN f9
>>> bootmem::__reserve silent double reserve of PFN fa
>>> BUG: Int 6: CR2 00000000
>>>      EDI c00f9fd0  ESI 3f7f5410  EBP 0003f7f5  ESP c0535f20
>>>      EBX c056c00c  EDX 00000006  ECX 00000001  EAX c056c03c
>>>      err 00000000  EIP c054f8f0   CS 00000060  flg 00010046
>>> Stack: 00000000 00000000 00000001 00038000 0003f7f5 f880abf0 3f7f5410
>>> c00f9fd0
>>>        00000001 c054f91d 00000000 c054571a c0413e7e c00f9fc0 000f9fc0
>>> 00000001
>>>        3f7bf64d 00000000 3e2c5000 c054578a 00000000 c053f3b6 3f7bf64d
>>> 00000000
>>>
>>> So it dies because of the way it tries to reserve the MP-table.
>>>
>>> Here's the stack trace.
>>>
>>>  [<c054f8f0>] mark_bootmem+0x9b/0xab
>>>  [<c054f91d>] reserve_bootmem+0x1d/0x1f
>>>  [<c054571a>] smp_scan_config+0xd9/0xfa
>>>  [<c054578a>] __find_smp_config+0x4f/0x6e
>>>  [<c053f3b6>] setup_arch+0x576/0x639
>>>  [<c054ebc4>] cgroup_init_subsys+0x29/0xc9
>>>  [<c053a6ac>] start_kernel+0x6b/0x31f
>>>  =======================
>> It looks like the problem is that the MPTable is located in the last
>> 64K of memory (instead of the first few megabytes).  There is a
>> comment about this in arch/x86/kernel/mpparse.c:
>>
>>    /*
>>     * We cannot access to MPC table to compute
>>     * table size yet, as only few megabytes from
>>     * the bottom is mapped now.
>>     * PC-9800's MPC table places on the very last
>>     * of physical memory; so that simply reserving
>>     * PAGE_SIZE from mpg->mpf_physptr yields BUG()
>>     * in reserve_bootmem.
>>     */
>>
>> However, that comment is in an #ifdef specific to 32bit kernels.
>> (Though, it's not clear to me how that code would help as it sets size
>> to be a negative number.)

that should work for a long time.

0xf9fc0 < 1M is quite < max_low_pfn, so wonder why bootmem could panic.

YH

Re: MPTable can not be high-memory on Linux

[Kevin O'Connor]

On Sun, Feb 22, 2009 at 01:33:35PM -0800, Yinghai Lu wrote:
[...]
> >>>  BIOS-e820: 0000000000090000 - 0000000000100000 (reserved)
> >>>  BIOS-e820: 0000000000100000 - 000000003f7f0000 (usable)
> >>>  BIOS-e820: 000000003f7f0000 - 0000000040000000 (reserved)
[...]
> >>> found SMP MP-table at [c00f9fc0] 000f9fc0
[...]
> 
> that should work for a long time.
> 
> 0xf9fc0 < 1M is quite < max_low_pfn, so wonder why bootmem could panic.

On this machine the mptable "floating" structure is at 0xf9fc0.  It
points to the rest of the table which is in the 0x3f7f0000 area.

Note, that this is on a Coreboot+SeaBIOS machine - so we can change
the bios.  However, the mptable spec does allow for part of the table
to be high memory.

-Kevin

Re: MPTable can not be high-memory on Linux

[Ingo Molnar]

* Kevin O'Connor <kevin@koconnor.net> wrote:

> On Sun, Feb 22, 2009 at 01:33:35PM -0800, Yinghai Lu wrote:
> [...]
> > >>>  BIOS-e820: 0000000000090000 - 0000000000100000 (reserved)
> > >>>  BIOS-e820: 0000000000100000 - 000000003f7f0000 (usable)
> > >>>  BIOS-e820: 000000003f7f0000 - 0000000040000000 (reserved)
> [...]
> > >>> found SMP MP-table at [c00f9fc0] 000f9fc0
> [...]
> > 
> > that should work for a long time.
> > 
> > 0xf9fc0 < 1M is quite < max_low_pfn, so wonder why bootmem could panic.
> 
> On this machine the mptable "floating" structure is at 
> 0xf9fc0.  It points to the rest of the table which is in the 
> 0x3f7f0000 area.
> 
> Note, that this is on a Coreboot+SeaBIOS machine - so we can 
> change the bios.  However, the mptable spec does allow for 
> part of the table to be high memory.

yes, and i'd prefer if it worked fine even if it's that high.

	Ingo

Re: MPTable can not be high-memory on Linux

[Yinghai Lu]

Ingo Molnar wrote:
> * Kevin O'Connor <kevin@koconnor.net> wrote:
> 
>> On Sun, Feb 22, 2009 at 01:33:35PM -0800, Yinghai Lu wrote:
>> [...]
>>>>>>  BIOS-e820: 0000000000090000 - 0000000000100000 (reserved)
>>>>>>  BIOS-e820: 0000000000100000 - 000000003f7f0000 (usable)
>>>>>>  BIOS-e820: 000000003f7f0000 - 0000000040000000 (reserved)
>> [...]
>>>>>> found SMP MP-table at [c00f9fc0] 000f9fc0
>> [...]
>>> that should work for a long time.
>>>
>>> 0xf9fc0 < 1M is quite < max_low_pfn, so wonder why bootmem could panic.
>> On this machine the mptable "floating" structure is at 
>> 0xf9fc0.  It points to the rest of the table which is in the 
>> 0x3f7f0000 area.
>>
>> Note, that this is on a Coreboot+SeaBIOS machine - so we can 
>> change the bios.  However, the mptable spec does allow for 
>> part of the table to be high memory.
> 
> yes, and i'd prefer if it worked fine even if it's that high.
> 

please check

[PATCH] x86: check physptr with max_low_pfn on 32bit

Impact: fix bug

coreboot aka LinuxBIOS try to put mptable on somewhere much high than
max_low_pfn, it cause panic

so need to check physptr with max_low_pfn * PAGE_SIZE.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/kernel/mpparse.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Index: linux-2.6/arch/x86/kernel/mpparse.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/mpparse.c
+++ linux-2.6/arch/x86/kernel/mpparse.c
@@ -710,13 +710,22 @@ static int __init smp_scan_config(unsign
 				 * of physical memory; so that simply reserving
 				 * PAGE_SIZE from mpf->physptr yields BUG()
 				 * in reserve_bootmem.
+				 * also need to make sure physptr is below than
+				 * max_low_pfn
+				 * we don't need reserve the area above max_low_pfn
 				 */
 				unsigned long end = max_low_pfn * PAGE_SIZE;
-				if (mpf->physptr + size > end)
-					size = end - mpf->physptr;
-#endif
+
+				if (mpf->physptr < end) {
+					if (mpf->physptr + size > end)
+						size = end - mpf->physptr;
+					reserve_bootmem_generic(mpf->physptr, size,
+							BOOTMEM_DEFAULT);
+				}
+#else
 				reserve_bootmem_generic(mpf->physptr, size,
 						BOOTMEM_DEFAULT);
+#endif
 			}
 
 			return 1;

Re: MPTable can not be high-memory on Linux

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> Ingo Molnar wrote:
> > * Kevin O'Connor <kevin@koconnor.net> wrote:
> > 
> >> On Sun, Feb 22, 2009 at 01:33:35PM -0800, Yinghai Lu wrote:
> >> [...]
> >>>>>>  BIOS-e820: 0000000000090000 - 0000000000100000 (reserved)
> >>>>>>  BIOS-e820: 0000000000100000 - 000000003f7f0000 (usable)
> >>>>>>  BIOS-e820: 000000003f7f0000 - 0000000040000000 (reserved)
> >> [...]
> >>>>>> found SMP MP-table at [c00f9fc0] 000f9fc0
> >> [...]
> >>> that should work for a long time.
> >>>
> >>> 0xf9fc0 < 1M is quite < max_low_pfn, so wonder why bootmem could panic.
> >> On this machine the mptable "floating" structure is at 
> >> 0xf9fc0.  It points to the rest of the table which is in the 
> >> 0x3f7f0000 area.
> >>
> >> Note, that this is on a Coreboot+SeaBIOS machine - so we can 
> >> change the bios.  However, the mptable spec does allow for 
> >> part of the table to be high memory.
> > 
> > yes, and i'd prefer if it worked fine even if it's that high.
> > 
> 
> please check
> 
> [PATCH] x86: check physptr with max_low_pfn on 32bit

Applied to tip:x86/apic, thanks Yinghai!

This will show up in v2.6.30. I suspect Coreboot will be changed 
anyway (older Linux kernels may crash), but it's worth having 
this bug fixed nevertheless.

	Ingo

Re: MPTable can not be high-memory on Linux

[Kevin O'Connor]

On Sun, Feb 22, 2009 at 10:14:56PM -0800, Yinghai Lu wrote:
> please check
> 
> [PATCH] x86: check physptr with max_low_pfn on 32bit

Thanks for looking at this issue.

Unfortunately, the kernel still does not work with the applied patch.
Neither 32bit nor 64bit kernels boot.  (Note, I applied the patch to
Linus' git, and I had to replace mpf->physptr with mpf->mpf_physptr.)

The error messages from before and after patching appear to be the
same.


In order to aid in testing, I modified SeaBIOS to force the mptable
into high memory even in qemu mode.  A modified bios.bin file can be
found at:

http://linuxtogo.org/~kevin/SeaBIOS/bios.bin-high-mptable

To use it with qemu, the steps are roughly:

mkdir testbios
cp /usr/share/qemu/*.bin testbios/
cp bios.bin-high-mptable testbios/bios.bin
qemu -L testbios/ -hda mytestlinux.img -serial file:mylog


The modified bios.bin can also be built manually by running:

git clone git://linuxtogo.org/home/kevin/seabios.git
cd seabios
patch -p1 < patch-from-below
make
cp out/bios.bin ../bios.bin-high-mptable

-Kevin


diff --git a/src/config.h b/src/config.h
index 56e5302..1ecc86f 100644
--- a/src/config.h
+++ b/src/config.h
@@ -21,7 +21,7 @@
 #define CONFIG_DEBUG_LEVEL 1
 
 // Send debugging information to serial port
-#define CONFIG_DEBUG_SERIAL 0
+#define CONFIG_DEBUG_SERIAL 1
 
 // Support for int13 floppy drive access
 #define CONFIG_FLOPPY_SUPPORT 1
@@ -77,7 +77,7 @@
 // Support finding a UUID (for smbios) via "magic" outl sequence.
 #define CONFIG_UUID_BACKDOOR 1
 // Support generation of ACPI tables (for emulators)
-#define CONFIG_ACPI 1
+#define CONFIG_ACPI 0
 // Support bios callbacks specific to via vgabios.
 #define CONFIG_VGAHOOKS 0
 // Support S3 resume handler.
diff --git a/src/mptable.c b/src/mptable.c
index 9e030fe..79cc7ba 100644
--- a/src/mptable.c
+++ b/src/mptable.c
@@ -19,7 +19,7 @@ mptable_init(void)
     dprintf(3, "init MPTable\n");
 
     int smp_cpus = smp_probe();
-    if (smp_cpus <= 1)
+    if (0 && smp_cpus <= 1)
         // Building an mptable on uniprocessor machines confuses some OSes.
         return;
 
@@ -39,7 +39,9 @@ mptable_init(void)
     /* floating pointer structure */
     struct mptable_floating_s *floating = (void*)start;
     memset(floating, 0, sizeof(*floating));
-    struct mptable_config_s *config = (void*)&floating[1];
+//    struct mptable_config_s *config = (void*)&floating[1];
+    struct mptable_config_s *config = (void*)(RamSize - 64*1024);
+    add_e820((u32)config, 64*1024, E820_RESERVED);
     floating->signature = MPTABLE_SIGNATURE;
     floating->physaddr = (u32)config;
     floating->length = 1;

Re: MPTable can not be high-memory on Linux

[Yinghai Lu]

Kevin O'Connor wrote:
> On Sun, Feb 22, 2009 at 10:14:56PM -0800, Yinghai Lu wrote:
>> please check
>>
>> [PATCH] x86: check physptr with max_low_pfn on 32bit
> 
> Thanks for looking at this issue.
> 
> Unfortunately, the kernel still does not work with the applied patch.
> Neither 32bit nor 64bit kernels boot.  (Note, I applied the patch to
> Linus' git, and I had to replace mpf->physptr with mpf->mpf_physptr.)
> 
> The error messages from before and after patching appear to be the
> same.
> 
> 

please add this patch too

[PATCH] x86: ioremap mptable

Impact: fix boot with mptable above max_low_mapped

try to use early_ioremap it.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/kernel/mpparse.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Index: linux-2.6/arch/x86/kernel/mpparse.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/mpparse.c
+++ linux-2.6/arch/x86/kernel/mpparse.c
@@ -611,12 +611,20 @@ static void __init __get_smp_config(unsi
 		construct_default_ISA_mptable(mpf->feature1);
 
 	} else if (mpf->physptr) {
+		struct mpc_table *mpc;
+		unsigned int length;
 
+		mpc = early_ioremap(mpf->physptr, PAGE_SIZE);
+		length = mpc->length;
+		apic_printk(APIC_VERBOSE, "  mpc: %x-%x\n", mpf->physptr,
+			 mpf->physptr + length);
+		early_iounmap(mpc, PAGE_SIZE);
+		mpc = early_ioremap(mpf->physptr, length);
 		/*
 		 * Read the physical hardware table.  Anything here will
 		 * override the defaults.
 		 */
-		if (!smp_read_mpc(phys_to_virt(mpf->physptr), early)) {
+		if (!smp_read_mpc(mpc, early)) {
 #ifdef CONFIG_X86_LOCAL_APIC
 			smp_found_config = 0;
 #endif
@@ -624,9 +632,12 @@ static void __init __get_smp_config(unsi
 			       "BIOS bug, MP table errors detected!...\n");
 			printk(KERN_ERR "... disabling SMP support. "
 			       "(tell your hw vendor)\n");
+			early_iounmap(mpc, length);
 			return;
 		}
 
+		early_iounmap(mpc, length);
+
 		if (early)
 			return;
 #ifdef CONFIG_X86_IO_APIC

Re: MPTable can not be high-memory on Linux

[Kevin O'Connor]

On Sat, Feb 28, 2009 at 07:10:18PM -0800, Yinghai Lu wrote:
> Kevin O'Connor wrote:
> > On Sun, Feb 22, 2009 at 10:14:56PM -0800, Yinghai Lu wrote:
> >> please check
> >>
> >> [PATCH] x86: check physptr with max_low_pfn on 32bit
> > 
> > Thanks for looking at this issue.
> > 
> > Unfortunately, the kernel still does not work with the applied patch.
> > Neither 32bit nor 64bit kernels boot.  (Note, I applied the patch to
> > Linus' git, and I had to replace mpf->physptr with mpf->mpf_physptr.)
> 
> please add this patch too
> 
> [PATCH] x86: ioremap mptable

Thanks.  Both 32bit and 64bit kernels are working now.

-Kevin

[PATCH] x86: ioremap mptable -v2

[Yinghai Lu]

Impact: fix boot with mptable above max_low_mapped

try to use early_ioremap it.

v2: also get the exact size for reserve_bootmem in case we got big size than 4k

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Reported-and-tested-by: Kevin O'Connor <kevin@koconnor.net>

---
 arch/x86/kernel/mpparse.c |   20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

Index: linux-2.6/arch/x86/kernel/mpparse.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/mpparse.c
+++ linux-2.6/arch/x86/kernel/mpparse.c
@@ -611,12 +611,20 @@ static void __init __get_smp_config(unsi
 		construct_default_ISA_mptable(mpf->feature1);
 
 	} else if (mpf->physptr) {
+		struct mpc_table *mpc;
+		unsigned long size;
 
+		mpc = early_ioremap(mpf->physptr, PAGE_SIZE);
+		size = mpc->length;
+		apic_printk(APIC_VERBOSE, "  mpc: %lx-%lx\n", (unsigned long)mpf->physptr,
+			 mpf->physptr + size);
+		early_iounmap(mpc, PAGE_SIZE);
+		mpc = early_ioremap(mpf->physptr, size);
 		/*
 		 * Read the physical hardware table.  Anything here will
 		 * override the defaults.
 		 */
-		if (!smp_read_mpc(phys_to_virt(mpf->physptr), early)) {
+		if (!smp_read_mpc(mpc, early)) {
 #ifdef CONFIG_X86_LOCAL_APIC
 			smp_found_config = 0;
 #endif
@@ -624,9 +632,12 @@ static void __init __get_smp_config(unsi
 			       "BIOS bug, MP table errors detected!...\n");
 			printk(KERN_ERR "... disabling SMP support. "
 			       "(tell your hw vendor)\n");
+			early_iounmap(mpc, size);
 			return;
 		}
 
+		early_iounmap(mpc, size);
+
 		if (early)
 			return;
 #ifdef CONFIG_X86_IO_APIC
@@ -700,7 +711,12 @@ static int __init smp_scan_config(unsign
 			reserve_bootmem_generic(virt_to_phys(mpf), PAGE_SIZE,
 					BOOTMEM_DEFAULT);
 			if (mpf->physptr) {
-				unsigned long size = PAGE_SIZE;
+				struct mpc_table *mpc;
+				unsigned long size;
+
+				mpc = early_ioremap(mpf->physptr, PAGE_SIZE);
+				size = mpc->length;
+				early_iounmap(mpc, PAGE_SIZE);
 #ifdef CONFIG_X86_32
 				/*
 				 * We cannot access to MPC table to compute[PATCH] x86: ioremap mptable -v2

Impact: fix boot with mptable above max_low_mapped

try to use early_ioremap it.

v2: also get the exact size for reserve_bootmem in case we got big size than 4k

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Reported-and-tested-by: Kevin O'Connor <kevin@koconnor.net>

---
 arch/x86/kernel/mpparse.c |   20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

Index: linux-2.6/arch/x86/kernel/mpparse.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/mpparse.c
+++ linux-2.6/arch/x86/kernel/mpparse.c
@@ -611,12 +611,20 @@ static void __init __get_smp_config(unsi
 		construct_default_ISA_mptable(mpf->feature1);
 
 	} else if (mpf->physptr) {
+		struct mpc_table *mpc;
+		unsigned long size;
 
+		mpc = early_ioremap(mpf->physptr, PAGE_SIZE);
+		size = mpc->length;
+		apic_printk(APIC_VERBOSE, "  mpc: %lx-%lx\n", (unsigned long)mpf->physptr,
+			 mpf->physptr + size);
+		early_iounmap(mpc, PAGE_SIZE);
+		mpc = early_ioremap(mpf->physptr, size);
 		/*
 		 * Read the physical hardware table.  Anything here will
 		 * override the defaults.
 		 */
-		if (!smp_read_mpc(phys_to_virt(mpf->physptr), early)) {
+		if (!smp_read_mpc(mpc, early)) {
 #ifdef CONFIG_X86_LOCAL_APIC
 			smp_found_config = 0;
 #endif
@@ -624,9 +632,12 @@ static void __init __get_smp_config(unsi
 			       "BIOS bug, MP table errors detected!...\n");
 			printk(KERN_ERR "... disabling SMP support. "
 			       "(tell your hw vendor)\n");
+			early_iounmap(mpc, size);
 			return;
 		}
 
+		early_iounmap(mpc, size);
+
 		if (early)
 			return;
 #ifdef CONFIG_X86_IO_APIC
@@ -700,7 +711,12 @@ static int __init smp_scan_config(unsign
 			reserve_bootmem_generic(virt_to_phys(mpf), PAGE_SIZE,
 					BOOTMEM_DEFAULT);
 			if (mpf->physptr) {
-				unsigned long size = PAGE_SIZE;
+				struct mpc_table *mpc;
+				unsigned long size;
+
+				mpc = early_ioremap(mpf->physptr, PAGE_SIZE);
+				size = mpc->length;
+				early_iounmap(mpc, PAGE_SIZE);
 #ifdef CONFIG_X86_32
 				/*
 				 * We cannot access to MPC table to compute

Re: [PATCH] x86: ioremap mptable -v2

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> Impact: fix boot with mptable above max_low_mapped
> 
> try to use early_ioremap it.
> 
> v2: also get the exact size for reserve_bootmem in case we got big size than 4k
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Reported-and-tested-by: Kevin O'Connor <kevin@koconnor.net>
> 
> ---
>  arch/x86/kernel/mpparse.c |   20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/arch/x86/kernel/mpparse.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/mpparse.c
> +++ linux-2.6/arch/x86/kernel/mpparse.c
> @@ -611,12 +611,20 @@ static void __init __get_smp_config(unsi
>  		construct_default_ISA_mptable(mpf->feature1);
>  
>  	} else if (mpf->physptr) {
> +		struct mpc_table *mpc;
> +		unsigned long size;
>  
> +		mpc = early_ioremap(mpf->physptr, PAGE_SIZE);
> +		size = mpc->length;
> +		apic_printk(APIC_VERBOSE, "  mpc: %lx-%lx\n", (unsigned long)mpf->physptr,
> +			 mpf->physptr + size);
> +		early_iounmap(mpc, PAGE_SIZE);
> +		mpc = early_ioremap(mpf->physptr, size);

no objections, but this bit of __get_smp_config() needs to be 
done cleaner - the whole mpf->physptr != 0 bit should probably 
go into a helper function.

	Ingo

Re: [PATCH] x86: ioremap mptable -v2

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> >  	} else if (mpf->physptr) {
> > +		struct mpc_table *mpc;
> > +		unsigned long size;
> >  
> > +		mpc = early_ioremap(mpf->physptr, PAGE_SIZE);
> > +		size = mpc->length;
> > +		apic_printk(APIC_VERBOSE, "  mpc: %lx-%lx\n", (unsigned long)mpf->physptr,
> > +			 mpf->physptr + size);
> > +		early_iounmap(mpc, PAGE_SIZE);
> > +		mpc = early_ioremap(mpf->physptr, size);
> 
> no objections, but this bit of __get_smp_config() needs to be 
> done cleaner - the whole mpf->physptr != 0 bit should probably 
> go into a helper function.

and if you do that it should be done via two patches, in two 
steps: first patch is a pure cleanup that moves this bit of 
__get_smp_config() into a helper function. The second patch then 
adds the early_ioremap().

	Ingo

Re: [PATCH] x86: ioremap mptable -v2

[Yinghai Lu]

Ingo Molnar wrote:
> * Yinghai Lu <yinghai@kernel.org> wrote:
> 
>> Impact: fix boot with mptable above max_low_mapped
>>
>> try to use early_ioremap it.
>>
>> v2: also get the exact size for reserve_bootmem in case we got big size than 4k
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> Reported-and-tested-by: Kevin O'Connor <kevin@koconnor.net>
>>
>> ---
>>  arch/x86/kernel/mpparse.c |   20 ++++++++++++++++++--
>>  1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> Index: linux-2.6/arch/x86/kernel/mpparse.c
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/kernel/mpparse.c
>> +++ linux-2.6/arch/x86/kernel/mpparse.c
>> @@ -611,12 +611,20 @@ static void __init __get_smp_config(unsi
>>  		construct_default_ISA_mptable(mpf->feature1);
>>  
>>  	} else if (mpf->physptr) {
>> +		struct mpc_table *mpc;
>> +		unsigned long size;
>>  
>> +		mpc = early_ioremap(mpf->physptr, PAGE_SIZE);
>> +		size = mpc->length;
>> +		apic_printk(APIC_VERBOSE, "  mpc: %lx-%lx\n", (unsigned long)mpf->physptr,
>> +			 mpf->physptr + size);
>> +		early_iounmap(mpc, PAGE_SIZE);
>> +		mpc = early_ioremap(mpf->physptr, size);
> 
> no objections, but this bit of __get_smp_config() needs to be 
> done cleaner - the whole mpf->physptr != 0 bit should probably 
> go into a helper function.
> 

please check

[PATCH] x86: ioremap mptable -v3

Impact: fix boot with mptable above max_low_mapped

try to use early_ioremap it.

v2: also get the exact size for reserve_bootmem in case we got big size than 4k
V3: according to Ingo, seperate get_mpc_size()

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Reported-and-tested-by: Kevin O'Connor <kevin@koconnor.net>

---
 arch/x86/kernel/mpparse.c |   24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

Index: linux-2.6/arch/x86/kernel/mpparse.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/mpparse.c
+++ linux-2.6/arch/x86/kernel/mpparse.c
@@ -558,6 +558,19 @@ static inline void __init construct_defa
 
 static struct mpf_intel *mpf_found;
 
+static unsigned long __init get_mpc_size(unsigned long physptr)
+{
+	struct mpc_table *mpc;
+	unsigned long size;
+
+	mpc = early_ioremap(physptr, PAGE_SIZE);
+	size = mpc->length;
+	early_iounmap(mpc, PAGE_SIZE);
+	apic_printk(APIC_VERBOSE, "  mpc: %lx-%lx\n", physptr, physptr + size);
+
+	return size;
+}
+
 /*
  * Scan the memory blocks for an SMP configuration block.
  */
@@ -611,12 +624,16 @@ static void __init __get_smp_config(unsi
 		construct_default_ISA_mptable(mpf->feature1);
 
 	} else if (mpf->physptr) {
+		struct mpc_table *mpc;
+		unsigned long size;
 
+		size = get_mpc_size(mpf->physptr);
+		mpc = early_ioremap(mpf->physptr, size);
 		/*
 		 * Read the physical hardware table.  Anything here will
 		 * override the defaults.
 		 */
-		if (!smp_read_mpc(phys_to_virt(mpf->physptr), early)) {
+		if (!smp_read_mpc(mpc, early)) {
 #ifdef CONFIG_X86_LOCAL_APIC
 			smp_found_config = 0;
 #endif
@@ -624,9 +641,12 @@ static void __init __get_smp_config(unsi
 			       "BIOS bug, MP table errors detected!...\n");
 			printk(KERN_ERR "... disabling SMP support. "
 			       "(tell your hw vendor)\n");
+			early_iounmap(mpc, size);
 			return;
 		}
 
+		early_iounmap(mpc, size);
+
 		if (early)
 			return;
 #ifdef CONFIG_X86_IO_APIC
@@ -700,7 +720,7 @@ static int __init smp_scan_config(unsign
 			reserve_bootmem_generic(virt_to_phys(mpf), PAGE_SIZE,
 					BOOTMEM_DEFAULT);
 			if (mpf->physptr) {
-				unsigned long size = PAGE_SIZE;
+				unsigned long size = get_mpc_size(mpf->physptr);
 #ifdef CONFIG_X86_32
 				/*
 				 * We cannot access to MPC table to compute

Re: [PATCH] x86: ioremap mptable -v2

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> V3: according to Ingo, seperate get_mpc_size()

No, that was not my suggestion. My suggestion was to separate 
this whole 'else if' branch:

>  	} else if (mpf->physptr) {
> +		struct mpc_table *mpc;
> +		unsigned long size;
>  
> +		size = get_mpc_size(mpf->physptr);
> +		mpc = early_ioremap(mpf->physptr, size);
>  		/*
>  		 * Read the physical hardware table.  Anything here will
>  		 * override the defaults.
>  		 */
> -		if (!smp_read_mpc(phys_to_virt(mpf->physptr), early)) {
> +		if (!smp_read_mpc(mpc, early)) {
>  #ifdef CONFIG_X86_LOCAL_APIC
>  			smp_found_config = 0;
>  #endif

... into a helper function - if that improves the code. Your 
patch does early_ioremap, iounmap then ioremap and iounmap - 
quite pointlessly.

You should resist cleanup suggestions that make the code worse, 
even if it comes from a maintainer :-)

	Ingo

Re: [PATCH] x86: ioremap mptable -v2

[Yinghai Lu]

Ingo Molnar wrote:
> * Yinghai Lu <yinghai@kernel.org> wrote:
> 
>> V3: according to Ingo, seperate get_mpc_size()
> 
> No, that was not my suggestion. My suggestion was to separate 
> this whole 'else if' branch:
> 
>>  	} else if (mpf->physptr) {
>> +		struct mpc_table *mpc;
>> +		unsigned long size;
>>  
>> +		size = get_mpc_size(mpf->physptr);
>> +		mpc = early_ioremap(mpf->physptr, size);
>>  		/*
>>  		 * Read the physical hardware table.  Anything here will
>>  		 * override the defaults.
>>  		 */
>> -		if (!smp_read_mpc(phys_to_virt(mpf->physptr), early)) {
>> +		if (!smp_read_mpc(mpc, early)) {
>>  #ifdef CONFIG_X86_LOCAL_APIC
>>  			smp_found_config = 0;
>>  #endif
> 
> ... into a helper function - if that improves the code.
oh, i missed it
> Your patch does early_ioremap, iounmap then ioremap and iounmap - 
> quite pointlessly.
try to get exact mpc size.
> 
> You should resist cleanup suggestions that make the code worse, 
> even if it comes from a maintainer :-)

we could do that later. to make __get_smp_config smaller and readable.

YH

Re: [PATCH] x86: ioremap mptable -v2

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> Ingo Molnar wrote:
> > * Yinghai Lu <yinghai@kernel.org> wrote:
> > 
> >> V3: according to Ingo, seperate get_mpc_size()
> > 
> > No, that was not my suggestion. My suggestion was to separate 
> > this whole 'else if' branch:
> > 
> >>  	} else if (mpf->physptr) {
> >> +		struct mpc_table *mpc;
> >> +		unsigned long size;
> >>  
> >> +		size = get_mpc_size(mpf->physptr);
> >> +		mpc = early_ioremap(mpf->physptr, size);
> >>  		/*
> >>  		 * Read the physical hardware table.  Anything here will
> >>  		 * override the defaults.
> >>  		 */
> >> -		if (!smp_read_mpc(phys_to_virt(mpf->physptr), early)) {
> >> +		if (!smp_read_mpc(mpc, early)) {
> >>  #ifdef CONFIG_X86_LOCAL_APIC
> >>  			smp_found_config = 0;
> >>  #endif
> > 
> > ... into a helper function - if that improves the code.
> oh, i missed it
> > Your patch does early_ioremap, iounmap then ioremap and iounmap - 
> > quite pointlessly.
> try to get exact mpc size.
> > 
> > You should resist cleanup suggestions that make the code worse, 
> > even if it comes from a maintainer :-)
> 
> we could do that later. to make __get_smp_config smaller and readable.

No, do it in two separate patches please: _first_ do the whole 
cleanup of these functions - on the assumption and expectation 
that it wont break anything. Then add the early_ioremap() change 
in a second patch - on top of the cleanup patch.

If we do a cleanup _after_ a functional change then we make the 
feature patch harder to revert and harder to fix as well. We'd 
always have to 'see through' the cleanup patch when considering 
breakages caused by the functional patch.

Like i suggested in my first reply ;-)

	Ingo