Re: [PATCH v5 12/13] x86, 64bit: Print init kernel lowmap correctly
On Fri, Dec 21, 2012 at 03:52:53PM -0800, Yinghai Lu wrote: > On Fri, Dec 21, 2012 at 3:39 PM, Konrad Rzeszutek Wilk > wrote: > > On Fri, Dec 21, 2012 at 02:44:39PM -0800, Yinghai Lu wrote: > >> > >> maybe we can change the subject of this patch to: > >> > >> Subject: [PATCH] x86, 64bit: Don't set max_pfn_mapped wrong on native boot > >> path > > > > Or the inverse. > > > > Set max_pfn_mapped correctly on non-native boot path? > > > > But this patch is not actually touching max_pfn_mapped - it is vaddr_end? > > No, > > it is 0 for native path > > > > So maybe: > > > > Subject: For platforms to set max_pfn_mapped, take that under advisement > > when blowing away __ka page entries. ^^ that > > hard to understand. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 12/13] x86, 64bit: Print init kernel lowmap correctly
On Fri, Dec 21, 2012 at 3:39 PM, Konrad Rzeszutek Wilk wrote: > On Fri, Dec 21, 2012 at 02:44:39PM -0800, Yinghai Lu wrote: >> >> maybe we can change the subject of this patch to: >> >> Subject: [PATCH] x86, 64bit: Don't set max_pfn_mapped wrong on native boot >> path > > Or the inverse. > > Set max_pfn_mapped correctly on non-native boot path? > > But this patch is not actually touching max_pfn_mapped - it is vaddr_end? No, it is 0 for native path > So maybe: > > Subject: For platforms to set max_pfn_mapped, take that under advisement when > blowing away __ka page entries. hard to understand. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 12/13] x86, 64bit: Print init kernel lowmap correctly
On Fri, Dec 21, 2012 at 02:44:39PM -0800, Yinghai Lu wrote: > On Fri, Dec 21, 2012 at 2:26 PM, Konrad Rzeszutek Wilk > wrote: > >> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c > >> index 4178530..30f6190 100644 > >> --- a/arch/x86/mm/init_64.c > >> +++ b/arch/x86/mm/init_64.c > >> @@ -304,10 +304,14 @@ void __init init_extra_mapping_uc(unsigned long > >> phys, unsigned long size) > >> void __init cleanup_highmap(void) > >> { > >> unsigned long vaddr = __START_KERNEL_map; > >> - unsigned long vaddr_end = __START_KERNEL_map + (max_pfn_mapped << > >> PAGE_SHIFT); > >> + unsigned long vaddr_end = __START_KERNEL_map + KERNEL_IMAGE_SIZE; > > > > Should you remove the line in head64.c that sets the > > max_pfn_mapped to KERNEL_IMAGE_SIZE >> PAGE_SHIFT? > > > >> unsigned long end = roundup((unsigned long)_brk_end, PMD_SIZE) - 1; > >> pmd_t *pmd = level2_kernel_pgt; > >> > >> + /* Xen has its own end somehow with abused max_pfn_mapped */ > > > > Could you clarify please? > > > > My recollection is that the max_pfn_mapped would point to the end of the > > RAMdisk. And yes (from mmu.c): > > > >1862 /* max_pfn_mapped is the last pfn mapped in the initial > > memory > >1863 * mappings. Considering that on Xen after the kernel > > mappings we > >1864 * have the mappings of some pages that don't exist in pfn > > space, we > >1865 * set max_pfn_mapped to the last real pfn mapped. */ > >1866 max_pfn_mapped = PFN_DOWN(__pa(xen_start_info->mfn_list)); > >1867 > > > > And if you follow xen_start_info, you get to include/xen/interface/xen.h > > which has: > > > > 406 * 4. This the order of bootstrap elements in the initial virtual > > region: > > 407 * a. relocated kernel image > > 408 * b. initial ram disk [mod_start, mod_len] > > 409 * c. list of allocated page frames [mfn_list, nr_pages] > > > > so per that code I believe max_pfn_mapped covers the kernel and the ramdisk > > - no more. > > > > for native path, in x86_64_start_kernel, we set max_pfn_mapped wrongly (my > fault > , I messed up low mapping and high mapping). > before this patchset, low_mapping end before end of x86_64_start_kernel is > 1G, and high mapping end is 512M. > > max_pfn_mapped is for low mapping. > > in this patch, for native patch, we keep max_pfn_mapped untouched, so > before clean_highmap, it will be 0. > > so we check !max_pfn_mapped to make xen still work. > OK. Might want to have a comment pointing to the xen/mmu.c and the max_pfn_mapped that is happening there. Thought if somebody is using 'cscope' or 'tags' they should be able to find it. Perhaps just have a comment and say: '/* Xen includes the RAMdisk as well - which is right after the kernel. */ > > > >> + if (max_pfn_mapped) > >> + vaddr_end = __START_KERNEL_map + (max_pfn_mapped << > >> PAGE_SHIFT); > >> + > >> for (; vaddr + PMD_SIZE - 1 < vaddr_end; pmd++, vaddr += PMD_SIZE) { > >> if (pmd_none(*pmd)) > >> continue; > >> -- > > > > This part of the patch does not seem to have much to do with the printk? > > Should it be seperate patch? > > maybe we can change the subject of this patch to: > > Subject: [PATCH] x86, 64bit: Don't set max_pfn_mapped wrong on native boot > path Or the inverse. Set max_pfn_mapped correctly on non-native boot path? But this patch is not actually touching max_pfn_mapped - it is vaddr_end? So maybe: Subject: For platforms to set max_pfn_mapped, take that under advisement when blowing away __ka page entries. > > ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 12/13] x86, 64bit: Print init kernel lowmap correctly
On Fri, Dec 21, 2012 at 2:26 PM, Konrad Rzeszutek Wilk wrote: >> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c >> index 4178530..30f6190 100644 >> --- a/arch/x86/mm/init_64.c >> +++ b/arch/x86/mm/init_64.c >> @@ -304,10 +304,14 @@ void __init init_extra_mapping_uc(unsigned long phys, >> unsigned long size) >> void __init cleanup_highmap(void) >> { >> unsigned long vaddr = __START_KERNEL_map; >> - unsigned long vaddr_end = __START_KERNEL_map + (max_pfn_mapped << >> PAGE_SHIFT); >> + unsigned long vaddr_end = __START_KERNEL_map + KERNEL_IMAGE_SIZE; > > Should you remove the line in head64.c that sets the > max_pfn_mapped to KERNEL_IMAGE_SIZE >> PAGE_SHIFT? > >> unsigned long end = roundup((unsigned long)_brk_end, PMD_SIZE) - 1; >> pmd_t *pmd = level2_kernel_pgt; >> >> + /* Xen has its own end somehow with abused max_pfn_mapped */ > > Could you clarify please? > > My recollection is that the max_pfn_mapped would point to the end of the > RAMdisk. And yes (from mmu.c): > >1862 /* max_pfn_mapped is the last pfn mapped in the initial memory >1863 * mappings. Considering that on Xen after the kernel > mappings we >1864 * have the mappings of some pages that don't exist in pfn > space, we >1865 * set max_pfn_mapped to the last real pfn mapped. */ >1866 max_pfn_mapped = PFN_DOWN(__pa(xen_start_info->mfn_list)); >1867 > > And if you follow xen_start_info, you get to include/xen/interface/xen.h > which has: > > 406 * 4. This the order of bootstrap elements in the initial virtual > region: > 407 * a. relocated kernel image > 408 * b. initial ram disk [mod_start, mod_len] > 409 * c. list of allocated page frames [mfn_list, nr_pages] > > so per that code I believe max_pfn_mapped covers the kernel and the ramdisk - > no more. > for native path, in x86_64_start_kernel, we set max_pfn_mapped wrongly (my fault , I messed up low mapping and high mapping). before this patchset, low_mapping end before end of x86_64_start_kernel is 1G, and high mapping end is 512M. max_pfn_mapped is for low mapping. in this patch, for native patch, we keep max_pfn_mapped untouched, so before clean_highmap, it will be 0. so we check !max_pfn_mapped to make xen still work. > >> + if (max_pfn_mapped) >> + vaddr_end = __START_KERNEL_map + (max_pfn_mapped << >> PAGE_SHIFT); >> + >> for (; vaddr + PMD_SIZE - 1 < vaddr_end; pmd++, vaddr += PMD_SIZE) { >> if (pmd_none(*pmd)) >> continue; >> -- > > This part of the patch does not seem to have much to do with the printk? > Should it be seperate patch? maybe we can change the subject of this patch to: Subject: [PATCH] x86, 64bit: Don't set max_pfn_mapped wrong on native boot path ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 12/13] x86, 64bit: Print init kernel lowmap correctly
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c > index 4178530..30f6190 100644 > --- a/arch/x86/mm/init_64.c > +++ b/arch/x86/mm/init_64.c > @@ -304,10 +304,14 @@ void __init init_extra_mapping_uc(unsigned long phys, > unsigned long size) > void __init cleanup_highmap(void) > { > unsigned long vaddr = __START_KERNEL_map; > - unsigned long vaddr_end = __START_KERNEL_map + (max_pfn_mapped << > PAGE_SHIFT); > + unsigned long vaddr_end = __START_KERNEL_map + KERNEL_IMAGE_SIZE; Should you remove the line in head64.c that sets the max_pfn_mapped to KERNEL_IMAGE_SIZE >> PAGE_SHIFT? > unsigned long end = roundup((unsigned long)_brk_end, PMD_SIZE) - 1; > pmd_t *pmd = level2_kernel_pgt; > > + /* Xen has its own end somehow with abused max_pfn_mapped */ Could you clarify please? My recollection is that the max_pfn_mapped would point to the end of the RAMdisk. And yes (from mmu.c): 1862 /* max_pfn_mapped is the last pfn mapped in the initial memory 1863 * mappings. Considering that on Xen after the kernel mappings we 1864 * have the mappings of some pages that don't exist in pfn space, we 1865 * set max_pfn_mapped to the last real pfn mapped. */ 1866 max_pfn_mapped = PFN_DOWN(__pa(xen_start_info->mfn_list)); 1867 And if you follow xen_start_info, you get to include/xen/interface/xen.h which has: 406 * 4. This the order of bootstrap elements in the initial virtual region: 407 * a. relocated kernel image 408 * b. initial ram disk [mod_start, mod_len] 409 * c. list of allocated page frames [mfn_list, nr_pages] so per that code I believe max_pfn_mapped covers the kernel and the ramdisk - no more. > + if (max_pfn_mapped) > + vaddr_end = __START_KERNEL_map + (max_pfn_mapped << PAGE_SHIFT); > + > for (; vaddr + PMD_SIZE - 1 < vaddr_end; pmd++, vaddr += PMD_SIZE) { > if (pmd_none(*pmd)) > continue; > -- This part of the patch does not seem to have much to do with the printk? Should it be seperate patch? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 12/13] x86, 64bit: Print init kernel lowmap correctly
When we get x86_64_start_kernel from arch/x86/kernel/head_64.S, We have 1. kernel highmap 512M (KERNEL_IMAGE_SIZE) from kernel loaded address. 2. kernel lowmap: [0, 1024M), and size (_end - _text) from kernel loaded address. for example, if the kernel bzImage is loaded high from 8G, will get: 1. kernel highmap: [8G, 8G+512M) 2. kernel lowmap: [0, 1024M), and [8G, 8G +_end - _text) So max_pfn_mapped that is for low map pfn recording is not that simple to 512M for 64 bit. Try to print out two ranges, when kernel is loaded high. Also need to use KERNEL_IMAGE_SIZE directly for highmap cleanup. Signed-off-by: Yinghai Lu --- arch/x86/kernel/head64.c |2 -- arch/x86/kernel/setup.c | 23 +-- arch/x86/mm/init_64.c|6 +- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c index 8ea1bc9..8d426b4 100644 --- a/arch/x86/kernel/head64.c +++ b/arch/x86/kernel/head64.c @@ -97,8 +97,6 @@ void __init x86_64_start_kernel(char * real_mode_data) /* Make NULL pointers segfault */ zap_identity_mappings(); - max_pfn_mapped = KERNEL_IMAGE_SIZE >> PAGE_SHIFT; - for (i = 0; i < NUM_EXCEPTION_VECTORS; i++) { #ifdef CONFIG_EARLY_PRINTK set_intr_gate(i, &early_idt_handlers[i]); diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index 2dbe2ce..87473fc 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -681,6 +681,26 @@ static int __init parse_reservelow(char *p) early_param("reservelow", parse_reservelow); +static __init void print_init_mem_mapped(void) +{ +#ifdef CONFIG_X86_32 + printk(KERN_DEBUG "initial memory mapped: [mem 0x-%#010lx]\n", + (max_pfn_mapped