[Xen-devel,v2,18/24] xen/arm: mm: Rename and clarify AP[1] in the stage-1 page table

Message ID 20170912100330.2168-19-julien.grall@arm.com
State Superseded
Headers show
Series
  • xen/arm: Memory subsystem clean-up
Related show

Commit Message

Julien Grall Sept. 12, 2017, 10:03 a.m.
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).

Update the comment and also rename the field to match the description in
the ARM ARM.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>

---

    Changes in v2:
        - Add Andre's reviewed-by
---
 xen/arch/arm/mm.c          | 10 +++++-----
 xen/include/asm-arm/lpae.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

Comments

Stefano Stabellini Sept. 19, 2017, 11:01 p.m. | #1
On Tue, 12 Sep 2017, 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).
> 
> Update the comment and also rename the field to match the description in
> the ARM ARM.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> 
>     Changes in v2:
>         - Add Andre's reviewed-by
> ---
>  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 fc76f03526..b3286b4a89 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 118ee5ae1a..b30853e79d 100644
> --- a/xen/include/asm-arm/lpae.h
> +++ b/xen/include/asm-arm/lpae.h
> @@ -35,7 +35,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 */
> -- 
> 2.11.0
>

Patch

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index fc76f03526..b3286b4a89 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 118ee5ae1a..b30853e79d 100644
--- a/xen/include/asm-arm/lpae.h
+++ b/xen/include/asm-arm/lpae.h
@@ -35,7 +35,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 */