lkml.org 
[lkml]   [2012]   [Dec]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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
> <konrad.wilk@oracle.com> 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.

>
> ?


\
 
 \ /
  Last update: 2012-12-22 01:21    [W:0.071 / U:1.344 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site