Re: [Xen-devel] [PATCH 5/7] xen: support RAM at addresses 0 and 4096

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-09-12 at 14:25 +0100, Jan Beulich wrote:
> >>> On 12.09.13 at 14:42, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> > Currently the mapping from pages to zones causes the page at zero to go into
> > zone -1 and the page at 4096 to go into zone 0, which is the Xen zone
> > (confusing various assertions).
> 
> So that's a problem on ARM only, right? Because x86 avoids passing
> the low first Mb to the allocator. I wonder whether ARM shouldn't at
> least avoid making the page at 0 available for allocation too, which
> would address half of the problem. Avoiding MFN 1 would be less
> natural, I agree.

I went back and forth on this for a bit.

> 
> > Arrange instead for the mapping to be such that zone 0 is always reserved 
> > for
> > Xen and all other pages map to a zone >= 1.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Cc: keir@xxxxxxx 
> > ---
> >  xen/common/page_alloc.c |    6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> > index 41251b2..e2cd139 100644
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -257,11 +257,11 @@ unsigned long __init alloc_boot_pages(
> >   */
> >  
> >  #define MEMZONE_XEN 0
> > -#define NR_ZONES    (PADDR_BITS - PAGE_SHIFT)
> > +#define NR_ZONES    (PADDR_BITS - PAGE_SHIFT + 1 + 1)
> 
> So we've got a total delta of two between old and new handling
> here...
> 
> > -#define bits_to_zone(b) (((b) < (PAGE_SHIFT + 1)) ? 0 : ((b) - PAGE_SHIFT 
> > - 1))
> > +#define bits_to_zone(b) (((b) < PAGE_SHIFT) ? 1 : ((b) - PAGE_SHIFT + 1 + 
> > 1))
> 
> ... but a delta of 3 on the right side of the colon here (the delta being
> just one on the left side is the actual meat of your fix as I understand
> it). Or, taking a slightly different perspective, using < and <= in the
> condition should not alter the result (but does if I'm not mistaken).

Hrm, you are right. I had a debug patch which printed out the
bits_to_zone and page_to_zone values for a bunch of supposed-to-be
consistent values, and they looked correct.

I can check again thouse, since something is clearly not quite hanging
together.
> 
> >  #define page_to_zone(pg) (is_xen_heap_page(pg) ? MEMZONE_XEN :  \
> > -                          (fls(page_to_mfn(pg)) - 1))
> > +                          (fls(page_to_mfn(pg)) + 1))
> 
> And the total delta is 2 here again.
> 
> Overall I'm really uncertain whether it wouldn't be better for ARM to
> play by the x86 rules in this respect,

Leaving page 0 free would make this a bit less confusing I suspect, by
removing the "off by two".

>  or alternatively to further
> generalize what you try to do here by allowing x86 to specify a bias
> for the shift to skip all zones currently covering the low Mb, which on
> ARM would end up being 1.

That could be a good approach.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel