Re: [Xen-devel] [PATCH 21/27] xen/arm: mm: Rename and clarify AP[1] in the stage-1 page table

[Andre Przywara <andre.przywara@xxxxxxx>]

Hi,

On 14/08/17 15:24, Julien Grall wrote:
> The description of AP[1] in Xen is based on testing rather than the ARM
> ARM.
> 
> Per the ARM ARM, on EL2 stage-1 page table, AP[1] is RES1 as the
> translation regime applies to only one exception level (see D4.4.4 and
> G4.6.1 in ARM DDI 0487B.a).

Indeed.

> 
> Update the comment and also rename the field to match the description in
> the ARM ARM.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>

Reviewed-by: Andre Przywara <andre.przywara@xxxxxxx>

Cheers,
Andre.

> ---
>  xen/arch/arm/mm.c          | 10 +++++-----
>  xen/include/asm-arm/lpae.h |  2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index ce1858fbf3..c0d5fda269 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -273,7 +273,7 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned 
> attr)
>              .table = 0,           /* Set to 1 for links and 4k maps */
>              .ai = attr,
>              .ns = 1,              /* Hyp mode is in the non-secure world */
> -            .user = 1,            /* See below */
> +            .up = 1,              /* See below */
>              .ro = 0,              /* Assume read-write */
>              .af = 1,              /* No need for access tracking */
>              .ng = 1,              /* Makes TLB flushes easier */
> @@ -282,10 +282,10 @@ static inline lpae_t mfn_to_xen_entry(mfn_t mfn, 
> unsigned attr)
>              .avail = 0,           /* Reference count for domheap mapping */
>          }};
>      /*
> -     * Setting the User bit is strange, but the ATS1H[RW] instructions
> -     * don't seem to work otherwise, and since we never run on Xen
> -     * pagetables in User mode it's OK.  If this changes, remember
> -     * to update the hard-coded values in head.S too.
> +     * For EL2 stage-1 page table, up (aka AP[1]) is RES1 as the translation
> +     * regime applies to only one exception level (see D4.4.4 and G4.6.1
> +     * in ARM DDI 0487B.a). If this changes, remember to update the
> +     * hard-coded values in head.S too.
>       */
>  
>      switch ( attr )
> diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
> index a62b118630..9402434c1e 100644
> --- a/xen/include/asm-arm/lpae.h
> +++ b/xen/include/asm-arm/lpae.h
> @@ -33,7 +33,7 @@ typedef struct __packed {
>       */
>      unsigned long ai:3;         /* Attribute Index */
>      unsigned long ns:1;         /* Not-Secure */
> -    unsigned long user:1;       /* User-visible */
> +    unsigned long up:1;         /* Unpriviledged access */
>      unsigned long ro:1;         /* Read-Only */
>      unsigned long sh:2;         /* Shareability */
>      unsigned long af:1;         /* Access Flag */
> 

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