Re: [Xen-devel] [RFC PATCH] xen/arm: Add 4-level page table for stage 2 translation

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-04-30 at 13:39 +0100, Julien Grall wrote:
> Hello Vijaya,
> 
> On 04/30/2014 01:15 PM, vijay.kilari@xxxxxxxxx wrote:
> > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > index d151724..c0e0362 100644
> > --- a/xen/arch/arm/arm64/head.S
> > +++ b/xen/arch/arm/arm64/head.S
> > @@ -224,13 +224,16 @@ skip_bss:
> >          ldr   x0, =MAIRVAL
> >          msr   mair_el2, x0
> >  
> > +        mrs x1, ID_AA64MMFR0_EL1
> > +
> >          /* Set up the HTCR:
> > -         * PASize -- 40 bits / 1TB
> > +         * PASize -- based on ID_AA64MMFR0_EL1.PARange value
> >           * Top byte is used
> >           * PT walks use Outer-Shareable accesses,
> >           * PT walks are write-back, write-allocate in both cache levels,
> >           * Full 64-bit address space goes through this table. */
> > -        ldr   x0, =0x80822500
> > +        ldr x0, =TCR_VAL_40PA
> 
> If you define TCR_VAL_40PA in another header, I would also move the
> comments explaining the different bits in this header.

Yeah, I'm increasingly not sure what was so wrong with this extensively
commented number TBH.


> > +#ifdef CONFIG_ARM_64
> > +/* Zeroeth level is of 1 page size */
> > +#define P2M_FIRST_ORDER 0
> 
> From your comment, should not it be P2M_ZEROETH_ORDER?

Actually it should be P2M_ROOT_ORDER.

> > diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> > index 905beb8..8477206 100644
> > --- a/xen/include/asm-arm/page.h
> > +++ b/xen/include/asm-arm/page.h
> > @@ -6,12 +6,13 @@
> >  #include <public/xen.h>
> >  #include <asm/processor.h>
> >  
> > +#ifdef CONFIG_ARM_64
> > +#define PADDR_BITS              48
> > +#else
> >  #define PADDR_BITS              40
> > +#endif
> >  #define PADDR_MASK              ((1ULL << PADDR_BITS)-1)
> >  
> > -#define VADDR_BITS              32
> > -#define VADDR_MASK              (~0UL)
> > -
> 
> Why did you remove VADDR_{BITS,MASK}?

These were unused (and wrong on arm64) I think removing them is OK, if a
little bit tangential to this patch.

> > +/* 40 bit VA to 32 bit PA */
> 
> Did you mean IPA instead of VA?
> 
> Also, on ARM, the PA is encoded with 40 bits.

Do you mean arm32?

Ian.



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