diff mbox series

[14/22] target/arm: Handle no-execute for Realm and Root regimes

Message ID 20230124000027.3565716-15-richard.henderson@linaro.org (mailing list archive)
State New, archived
Headers show
Series target/arm: Implement FEAT_RME | expand

Commit Message

Richard Henderson Jan. 24, 2023, midnight UTC
While Root and Realm may read and write data from other spaces,
neither may execute from other pa spaces.

This happens for Stage1 EL3, EL2, EL2&0, but stage2 EL1&0.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/ptw.c | 66 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 58 insertions(+), 8 deletions(-)

Comments

Peter Maydell Feb. 10, 2023, 11:59 a.m. UTC | #1
On Tue, 24 Jan 2023 at 00:02, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> While Root and Realm may read and write data from other spaces,
> neither may execute from other pa spaces.
>
> This happens for Stage1 EL3, EL2, EL2&0, but stage2 EL1&0.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/ptw.c | 66 ++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 58 insertions(+), 8 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 849f5e89ca..6b6f8195eb 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -909,7 +909,7 @@ do_fault:
>   * @xn:      XN (execute-never) bits
>   * @s1_is_el0: true if this is S2 of an S1+2 walk for EL0
>   */
> -static int get_S2prot(CPUARMState *env, int s2ap, int xn, bool s1_is_el0)
> +static int get_S2prot_noexecute(int s2ap)
>  {
>      int prot = 0;
>
> @@ -919,6 +919,12 @@ static int get_S2prot(CPUARMState *env, int s2ap, int xn, bool s1_is_el0)
>      if (s2ap & 2) {
>          prot |= PAGE_WRITE;
>      }
> +    return prot;
> +}
> +
> +static int get_S2prot(CPUARMState *env, int s2ap, int xn, bool s1_is_el0)
> +{
> +    int prot = get_S2prot_noexecute(s2ap);
>
>      if (cpu_isar_feature(any_tts2uxn, env_archcpu(env))) {
>          switch (xn) {
> @@ -956,12 +962,14 @@ static int get_S2prot(CPUARMState *env, int s2ap, int xn, bool s1_is_el0)
>   * @mmu_idx: MMU index indicating required translation regime
>   * @is_aa64: TRUE if AArch64
>   * @ap:      The 2-bit simple AP (AP[2:1])
> - * @ns:      NS (non-secure) bit
>   * @xn:      XN (execute-never) bit
>   * @pxn:     PXN (privileged execute-never) bit
> + * @in_pa:   The original input pa space
> + * @out_pa:  The output pa space, modified by NSTable, NS, and NSE
>   */
>  static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
> -                      int ap, int ns, int xn, int pxn)
> +                      int ap, int xn, int pxn,
> +                      ARMSecuritySpace in_pa, ARMSecuritySpace out_pa)
>  {
>      bool is_user = regime_is_user(env, mmu_idx);
>      int prot_rw, user_rw;
> @@ -982,8 +990,39 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
>          }
>      }
>
> -    if (ns && arm_is_secure(env) && (env->cp15.scr_el3 & SCR_SIF)) {
> -        return prot_rw;

Ah, this looks like it fixes the bug introduced in patch 12;
I guess some of this needs rearranging between patches.

> +    if (in_pa != out_pa) {
> +        switch (in_pa) {
> +        case ARMSS_Root:
> +            /*
> +             * R_ZWRVD: permission fault for insn fetched from non-Root,
> +             * I_WWBFB: SIF has no effect in EL3.
> +             */
> +            return prot_rw;
> +        case ARMSS_Realm:
> +            /*
> +             * R_PKTDS: permission fault for insn fetched from non-Realm,
> +             * for Realm EL2 or EL2&0.  The corresponding fault for EL1&0
> +             * happens during any stage2 translation.
> +             */
> +            switch (mmu_idx) {
> +            case ARMMMUIdx_E2:
> +            case ARMMMUIdx_E20_0:
> +            case ARMMMUIdx_E20_2:
> +            case ARMMMUIdx_E20_2_PAN:
> +                return prot_rw;
> +            default:
> +                break;
> +            }
> +            break;
> +        case ARMSS_Secure:
> +            if (env->cp15.scr_el3 & SCR_SIF) {
> +                return prot_rw;
> +            }
> +            break;
> +        default:
> +            /* Input NonSecure must have output NonSecure. */
> +            g_assert_not_reached();
> +        }
>      }
>
>      /* TODO have_wxn should be replaced with
> @@ -1556,12 +1595,16 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
>          /*
>           * R_GYNXY: For stage2 in Realm security state, bit 55 is NS.
>           * The bit remains ignored for other security states.
> +         * R_YMCSL: Executing an insn fetched from non-Realm causes
> +         * a stage2 permission fault.
>           */
>          if (out_space == ARMSS_Realm && extract64(attrs, 55, 1)) {
>              out_space = ARMSS_NonSecure;
> +            result->f.prot = get_S2prot_noexecute(ap);
> +        } else {
> +            xn = extract64(attrs, 53, 2);
> +            result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
>          }
> -        xn = extract64(attrs, 53, 2);
> -        result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
>      } else {
>          int ns = extract32(attrs, 5, 1);
>          switch (out_space) {
> @@ -1613,7 +1656,14 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
>          }
>          xn = extract64(attrs, 54, 1);
>          pxn = extract64(attrs, 53, 1);
> -        result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn);
> +
> +        /*
> +         * Note that we modified ptw->in_space earlier for NSTable,
> +         * and result->f.attrs was initialized by get_phys_addr, so
> +         * that retains a copy of the original security space.
> +         */
> +        result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, xn, pxn,
> +                                    result->f.attrs.space, out_space);
>      }
>
>      if (!(result->f.prot & (1 << access_type))) {
> --
> 2.34.1

thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 849f5e89ca..6b6f8195eb 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -909,7 +909,7 @@  do_fault:
  * @xn:      XN (execute-never) bits
  * @s1_is_el0: true if this is S2 of an S1+2 walk for EL0
  */
-static int get_S2prot(CPUARMState *env, int s2ap, int xn, bool s1_is_el0)
+static int get_S2prot_noexecute(int s2ap)
 {
     int prot = 0;
 
@@ -919,6 +919,12 @@  static int get_S2prot(CPUARMState *env, int s2ap, int xn, bool s1_is_el0)
     if (s2ap & 2) {
         prot |= PAGE_WRITE;
     }
+    return prot;
+}
+
+static int get_S2prot(CPUARMState *env, int s2ap, int xn, bool s1_is_el0)
+{
+    int prot = get_S2prot_noexecute(s2ap);
 
     if (cpu_isar_feature(any_tts2uxn, env_archcpu(env))) {
         switch (xn) {
@@ -956,12 +962,14 @@  static int get_S2prot(CPUARMState *env, int s2ap, int xn, bool s1_is_el0)
  * @mmu_idx: MMU index indicating required translation regime
  * @is_aa64: TRUE if AArch64
  * @ap:      The 2-bit simple AP (AP[2:1])
- * @ns:      NS (non-secure) bit
  * @xn:      XN (execute-never) bit
  * @pxn:     PXN (privileged execute-never) bit
+ * @in_pa:   The original input pa space
+ * @out_pa:  The output pa space, modified by NSTable, NS, and NSE
  */
 static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
-                      int ap, int ns, int xn, int pxn)
+                      int ap, int xn, int pxn,
+                      ARMSecuritySpace in_pa, ARMSecuritySpace out_pa)
 {
     bool is_user = regime_is_user(env, mmu_idx);
     int prot_rw, user_rw;
@@ -982,8 +990,39 @@  static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
         }
     }
 
-    if (ns && arm_is_secure(env) && (env->cp15.scr_el3 & SCR_SIF)) {
-        return prot_rw;
+    if (in_pa != out_pa) {
+        switch (in_pa) {
+        case ARMSS_Root:
+            /*
+             * R_ZWRVD: permission fault for insn fetched from non-Root,
+             * I_WWBFB: SIF has no effect in EL3.
+             */
+            return prot_rw;
+        case ARMSS_Realm:
+            /*
+             * R_PKTDS: permission fault for insn fetched from non-Realm,
+             * for Realm EL2 or EL2&0.  The corresponding fault for EL1&0
+             * happens during any stage2 translation.
+             */
+            switch (mmu_idx) {
+            case ARMMMUIdx_E2:
+            case ARMMMUIdx_E20_0:
+            case ARMMMUIdx_E20_2:
+            case ARMMMUIdx_E20_2_PAN:
+                return prot_rw;
+            default:
+                break;
+            }
+            break;
+        case ARMSS_Secure:
+            if (env->cp15.scr_el3 & SCR_SIF) {
+                return prot_rw;
+            }
+            break;
+        default:
+            /* Input NonSecure must have output NonSecure. */
+            g_assert_not_reached();
+        }
     }
 
     /* TODO have_wxn should be replaced with
@@ -1556,12 +1595,16 @@  static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
         /*
          * R_GYNXY: For stage2 in Realm security state, bit 55 is NS.
          * The bit remains ignored for other security states.
+         * R_YMCSL: Executing an insn fetched from non-Realm causes
+         * a stage2 permission fault.
          */
         if (out_space == ARMSS_Realm && extract64(attrs, 55, 1)) {
             out_space = ARMSS_NonSecure;
+            result->f.prot = get_S2prot_noexecute(ap);
+        } else {
+            xn = extract64(attrs, 53, 2);
+            result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
         }
-        xn = extract64(attrs, 53, 2);
-        result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
     } else {
         int ns = extract32(attrs, 5, 1);
         switch (out_space) {
@@ -1613,7 +1656,14 @@  static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
         }
         xn = extract64(attrs, 54, 1);
         pxn = extract64(attrs, 53, 1);
-        result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn);
+
+        /*
+         * Note that we modified ptw->in_space earlier for NSTable,
+         * and result->f.attrs was initialized by get_phys_addr, so
+         * that retains a copy of the original security space.
+         */
+        result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, xn, pxn,
+                                    result->f.attrs.space, out_space);
     }
 
     if (!(result->f.prot & (1 << access_type))) {