WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] Re: xen: memory initialization/balloon fixes (#3)

On Wed, Sep 28, 2011 at 11:45:02AM +0100, David Vrabel wrote:
> On 28/09/11 00:10, Konrad Rzeszutek Wilk wrote:
> >>> (XEN) Xen-e820 RAM map:
> >>> (XEN)  0000000000000000 - 000000000009d800 (usable)
> >>
> >> It's because it's not correctly handling the half-page of RAM at the end
> >> of this region.
> >>
> >> I don't have access to any test boxes with a dodgy BIOS like this so can
> >> you test this patch?  If it works I'll fold it in and post an updated
> >> series.
> > 
> > It works. Albeit I think we are going to hit a problem with dmidecode
> > if the DMI data is right in the reserved region
> > 
> > (http://lists.xensource.com/archives/html/xen-devel/2011-09/msg01299.html)
> > 
> > As in, if it starts in 9D800 - we consider 0->9d as RAM PFN, and 9e->100 as 
> > 1-1
> > mapping.
> > 
> > I am thinking that perhaps the call to xen_set_phys_identity, where
> > we call PFN_UP(x) should be replaced with PFN_DOWN(x). That way
> > we would consider 0>9c as RAM PFN and 9D->100 as 1-1 mapping.
> 
> I almost did an equivalent change (see below) but discarded it as it
> would have resulting in overlapping regions and attempting to
> release/map some pages twice.
> 
> I think we will have to move the release/map until after the final e820
> map has been sanitized so there are no overlapping regions.

<nods>Fortunatly for us, the overlap does not happen - they are just
next to each other. BTW, I think Xen hypervisor does the E820 sanitisation
so there shouldn't be any funny entries.

> 
> I'll prepare another patch for this.

OK.
> 
> > That would imply a new patch to your series naturally.
> >>
> >> Can you remember why this page alignment was required?  I'd like to
> > 
> > The e820_* calls define how the memory subsystem will use it.
> > It ended at some point assuming that the full page is RAM even thought
> > it was only half-RAM and tried to use it and blew the machine up.
> > 
> > The fix was to make the calls to the e820_* with size and regions
> > that were page-aligned.
> > 
> > Anyhow, here is what the bootup looks now:
> > 
> > [    0.000000] Freeing  9e-a0 pfn range: 2 pages freed
> > [    0.000000] 1-1 mapping on 9e->a0
> > [    0.000000] Freeing  a0-100 pfn range: 96 pages freed
> > [    0.000000] 1-1 mapping on a0->100
> > [    0.000000] Freeing  7fff0-80000 pfn range: 16 pages freed
> > [    0.000000] 1-1 mapping on 7fff0->80000
> > [    0.000000] Freeing  cfef0-cfef5 pfn range: 5 pages freed
> > [    0.000000] 1-1 mapping on cfef0->cfef5
> > [    0.000000] Freeing  cfef5-cff7f pfn range: 138 pages freed
> > [    0.000000] 1-1 mapping on cfef5->cff7f
> > [    0.000000] Freeing  cff7f-d0000 pfn range: 129 pages freed
> > [    0.000000] 1-1 mapping on cff7f->d0000
> > [    0.000000] Freeing  d0000-f0000 pfn range: 131072 pages freed
> > [    0.000000] 1-1 mapping on d0000->f0000
> > [    0.000000] Freeing  f0000-f4b58 pfn range: 19288 pages freed
> > [    0.000000] 1-1 mapping on f0000->fec10
> > [    0.000000] 1-1 mapping on fec10->fee01
> > [    0.000000] 1-1 mapping on fee01->100000
> > [    0.000000] Released 150746 pages of unused memory
> > [    0.000000] Set 196994 page(s) to 1-1 mapping
> > [    0.000000] BIOS-provided physical RAM map:
> > [    0.000000]  Xen: 0000000000000000 - 000000000009d000 (usable)
> > [    0.000000]  Xen: 000000000009d800 - 0000000000100000 (reserved)
> > [    0.000000]  Xen: 0000000000100000 - 000000007fff0000 (usable)
> > [    0.000000]  Xen: 000000007fff0000 - 0000000080000000 (reserved)
> > 
> > 
> >> update the comment with the reason because the bare-metal x86 memory
> >> init code doesn't appear to fixup the memory map in this way.
> >>
> >> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> >> index 986661b..e473c4c 100644
> >> --- a/arch/x86/xen/setup.c
> >> +++ b/arch/x86/xen/setup.c
> >> @@ -178,6 +178,19 @@ static unsigned long __init xen_get_max_pages(void)
> >>    return min(max_pages, MAX_DOMAIN_PAGES);
> >>  }
> >>
> >> +static void xen_e820_add_region(u64 start, u64 size, int type)

Might as well call this function "xen_align_and_add_e820_region"

> >> +{
> >> +  u64 end = start + size;
> >> +
> >> +  /* Align RAM regions to page boundaries. */
> >> +  if (type == E820_RAM || type == E820_UNUSABLE) {
> > 
> > Hm, do we care about E820_UNUSABLE to be page aligned?
> > If so, please comment why.
> 
> Er. We don't really but I think this if needs to be:
> 
>     /*
>      * Page align regions.
>      *
>      * Reduce RAM regions and expand other (reserved) regions.
>      */
>     if (type == E820_RAM || type == E820_UNUSABLE) {
>       start = PAGE_ALIGN(start);
>         end  &= ~((u64)PAGE_SIZE - 1);
>     } else {
>         start &= ~((u64)PAGE_SIZE - 1);
>         end = PAGE_ALIGN(start);
>     }
> 
> So reserved regions also become page aligned (which is part of the fix
> for the dmidecode bug).

<nods> That should be part of a seperate patch (well, the dmidecde
patch). Instead of the "infinite loop, won't boot on Konrad's
machines with non-standard E820".


> 
> >> +          start = PAGE_ALIGN(start);
> > 
> > Is that actually safe? Say it starts a 9ffff? We would
> > end up using 9f000 which is not right.
> 
> PAGE_ALIGN() (and ALIGN()) round upwards.

<smacks his head> Right.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel