Messages in this thread | | | Date | Wed, 13 Oct 2010 17:24:36 -0700 | From | Jeremy Fitzhardinge <> | Subject | Re: [PATCH 0/4] memblock related fixes for -tip |
| |
On 10/13/2010 05:08 PM, Yinghai wrote: > > On Oct 13, 2010, at 4:34 PM, Jeremy Fitzhardinge <jeremy@goop.org> wrote: > >> On 10/13/2010 04:18 PM, H. Peter Anvin wrote: >>> On 10/13/2010 04:14 PM, Jeremy Fitzhardinge wrote: >>>> The Xen code is setting the max_pfn_mapped correctly, but it is just >>>> being overridden by: >>>> >>>> max_pfn_mapped = KERNEL_IMAGE_SIZE >> PAGE_SHIFT; >>>> >>>> in setup_arch() - and KERNEL_IMAGE_SIZE is hard-coded to 512MB... >>>> >>>> How is this correct? Does kernel/head_64.S map everything up to 512MB >>>> or something? >>>> >>> Yes: >>> /* >>> * 512 MB kernel mapping. We spend a full page on this pagetable >>> * anyway. >>> * >>> * The kernel code+data+bss must not be bigger than that. >>> * >>> * (NOTE: at +512MB starts the module area, see MODULES_VADDR. >>> * If you want to increase this then increase MODULES_VADDR >>> * too.) >>> */ >>> PMDS(0, __PAGE_KERNEL_LARGE_EXEC, >>> KERNEL_IMAGE_SIZE/PMD_SIZE) >>> >>> This is, however, wrong in the sense that it obviously shouldn't be >>> getting executed on the Xen codepath. >> I see. head_32.S sets max_pfn_mapped for itself, so head_64.S should >> probably do that too. Patch below, but I haven't checked to see if it >> fixes the original problem (but I don't see why the pagetable memory >> allocation should be limited to the mapped region, especially if you >> want to put it as high as possible, especially since the mm/init_64.c >> code doesn't expect it to be mapped). >> >> J >> >> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com> >> Date: Wed, 13 Oct 2010 16:31:11 -0700 >> Subject: [PATCH] x86-64: don't force max_pfn_mapped to 512MB >> >> head_64.S maps up to 512MB, but the Xen entry path doesn't necessarily >> map that high. Set max_pfn_mapped in head_64.S accordingly. (The Xen >> code already sets max_pfn_mapped correctly.) >> >> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com> >> >> diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S >> index 239046b..b75c6f4 100644 >> --- a/arch/x86/kernel/head_64.S >> +++ b/arch/x86/kernel/head_64.S >> @@ -141,6 +141,8 @@ ident_complete: >> addq %rbp, trampoline_level4_pgt + (511*8)(%rip) >> #endif >> >> + movq $KERNEL_IMAGE_SIZE / PAGE_SIZE, max_pfn_mapped(%rip) >> + >> /* Due to ENTRY(), sometimes the empty space gets filled with >> * zeros. Better take a jmp than relying on empty space being >> * filled with 0x90 (nop) >> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c >> index b11a238..c3cebfe 100644 >> --- a/arch/x86/kernel/setup.c >> +++ b/arch/x86/kernel/setup.c >> @@ -932,7 +932,6 @@ void __init setup_arch(char **cmdline_p) >> max_low_pfn = max_pfn; >> >> high_memory = (void *)__va(max_pfn * PAGE_SIZE - 1) + 1; >> - max_pfn_mapped = KERNEL_IMAGE_SIZE >> PAGE_SHIFT; >> #endif >> >> /* >> > I like this one, can you check if only this one is needed ?
It is sufficient, but I'd prefer to have both. I don't think limiting the pagetable allocation to mapped memory is necessarily correct, and there's no harm in making the Xen code more robust (plus it cleans up a couple of #ifdefs).
J
| |