[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1162565722.19677.68.camel@ustr-linux-shaha1.unisys.com>
Date: Fri, 03 Nov 2006 09:55:21 -0500
From: Amul Shah <amul.shah@...sys.com>
To: Andi Kleen <ak@...e.de>
Cc: LKML <linux-kernel@...r.kernel.org>,
fastboot <fastboot@...ts.osdl.org>,
Vivek Goyal <vgoyal@...ibm.com>
Subject: Re: [RFC] [PATCH 2.6.19-rc4] kdump panics early in boot when
reserving MP Tables located in high memory
On Fri, 2006-11-03 at 03:40 +0100, Andi Kleen wrote:
> On Thursday 02 November 2006 23:24, Amul Shah wrote:
>
> >
> > The ACPI tables and MP Tables reside higher in memory. When reserving
> > memory with reserve_bootmem_generic, the function has a BUG panic if the
> > memory location to reserve is above the top of memory. The MP table is
> > above the top of memory in a user defined memory map.
>
> I think it would be cleaner to add a check in reserve_bootmem_generic
> that just returns when pfn >= end_pfn && pfn < end_pfn_mapped
>
> How about this patch? Does it work?
>
> -Andi
>
> Handle reserve_bootmem_generic beyond end_pfn
>
> This can happen on kexec kernels with some configurations, in particularly
> on Unisys ES7000 systems.
>
> Analysis by Amul Shah
>
> Cc: Amul Shah <amul.shah@...sys.com>
>
> Signed-off-by: Andi Kleen <ak@...e.de>
>
> Index: linux/arch/x86_64/mm/init.c
> ===================================================================
> --- linux.orig/arch/x86_64/mm/init.c
> +++ linux/arch/x86_64/mm/init.c
> @@ -655,9 +655,22 @@ void free_initrd_mem(unsigned long start
>
> void __init reserve_bootmem_generic(unsigned long phys, unsigned len)
> {
> - /* Should check here against the e820 map to avoid double free */
> #ifdef CONFIG_NUMA
> int nid = phys_to_nid(phys);
> +#endif
> + unsigned long pfn = phys >> PAGE_SHIFT;
> + if (pfn >= end_pfn) {
> + /* This can happen with kdump kernels when accessing firmware
> + tables. */
> + if (pfn < end_pfn_map)
> + return;
> + printk(KERN_ERR "reserve_bootmem: illegal reserve %lx %u\n",
> + phys, len);
> + return;
> + }
> +
> + /* Should check here against the e820 map to avoid double free */
> +#ifdef CONFIG_NUMA
> reserve_bootmem_node(NODE_DATA(nid), phys, len);
> #else
> reserve_bootmem(phys, len);
Andi,
That won't worked because in arch/86_64/kernel/e820.c, the exactmap
parsing clobbers end_pfn_map.
static int __init parse_memmap_opt(char *p)
{
char *oldp;
unsigned long long start_at, mem_size;
if (!strcmp(p, "exactmap")) {
#ifdef CONFIG_CRASH_DUMP
/* If we are doing a crash dump, we
* still need to know the real mem
* size before original memory map is
* reset.
*/
saved_max_pfn = e820_end_of_ram();
#endif
end_pfn_map = 0;
e820.nr_map = 0;
userdef = 1;
return 0;
}
The following was my alternate patch with uses the saved_max_pfn
variable which avoids the MP config table reservation bug. I'll rewrite
it to go into reserve_bootmem_generic and submit that patch once I have
tested it.
I chose to use end_user_pfn because it is left unmodified unless the
user specifies an exactmap or a "mem=" value as a kernel boot parameter.
This might be a no-no, since I didn't check to see if I'm under the top
of memory. I'm making the assumption that since the user chose to
define memory the user knows what s/he is doing.
This patch is just for comment since I'll be using the same logic when I
update the patch that Andi sent.
thanks,
Amul
diff -Naur linux-2.6.19-rc4/arch/x86_64/kernel/mpparse.c linux-2.6.19-rc4-pfncheck/arch/x86_64/kernel/mpparse.c
--- linux-2.6.19-rc4/arch/x86_64/kernel/mpparse.c 2006-10-31 17:38:41.000000000 -0500
+++ linux-2.6.19-rc4-pfncheck/arch/x86_64/kernel/mpparse.c 2006-11-03 10:18:24.000000000 -0500
@@ -34,6 +34,7 @@
/* Have we found an MP table */
int smp_found_config;
unsigned int __initdata maxcpus = NR_CPUS;
+extern unsigned long end_user_pfn;
int acpi_found_madt;
@@ -528,6 +529,8 @@
extern void __bad_mpf_size(void);
unsigned int *bp = phys_to_virt(base);
struct intel_mp_floating *mpf;
+ int mpf_below_mem;
+ unsigned long mpf_pfn;
Dprintk("Scan SMP from %p for %ld bytes.\n", bp,length);
if (sizeof(*mpf) != 16)
@@ -542,8 +545,33 @@
|| (mpf->mpf_specification == 4)) ) {
smp_found_config = 1;
+ mpf_below_mem = 0;
reserve_bootmem_generic(virt_to_phys(mpf), PAGE_SIZE);
- if (mpf->mpf_physptr)
+ if (mpf->mpf_physptr) {
+ mpf_pfn = mpf->mpf_physptr>>PAGE_SHIFT;
+ mpf_below_mem = 1;
+#ifdef CONFIG_CRASH_DUMP
+ if (mpf_pfn > end_pfn &&
+ mpf_pfn < saved_max_pfn) {
+ printk(KERN_WARNING "WARNING: "
+ "mpf_physptr > end_pfn %lx in "
+ "user defined map\n", end_pfn);
+ printk(KERN_WARNING "WARNING: "
+ "Not reserving the MP Tables\n");
+ mpf_below_mem = 0;
+ }
+#endif
+ if (mpf_pfn > end_pfn &&
+ end_user_pfn != MAXMEM>>PAGE_SHIFT) {
+ printk(KERN_WARNING "WARNING: "
+ "mpf_physptr > end_pfn %lx. Try"
+ " a larger 'mem='\n", end_pfn);
+ printk(KERN_WARNING "WARNING: "
+ "Not reserving the MP Tables\n");
+ mpf_below_mem = 0;
+ }
+ }
+ if (mpf_below_mem)
reserve_bootmem_generic(mpf->mpf_physptr, PAGE_SIZE);
mpf_found = mpf;
return 1;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists