MPTable can not be high-memory on Linux
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
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
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
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
* 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
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;
* 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
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;
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
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
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
* 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
* 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
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
* 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
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
* 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
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.
participants (5)
-
Andi Kleen -
Ingo Molnar -
Kevin O'Connor -
Rafael J. Wysocki -
Yinghai Lu