diff mbox series

[13/22] target/arm: Handle Block and Page bits for security space

Message ID 20230124000027.3565716-14-richard.henderson@linaro.org
State New
Headers show
Series target/arm: Implement FEAT_RME | expand

Commit Message

Richard Henderson Jan. 24, 2023, midnight UTC
With Realm security state, bit 55 of a block or page descriptor during
the stage2 walk becomes the NS bit; during the stage1 walk the bit 5
NS bit is RES0.  With Root security state, bit 11 of the block or page
descriptor during the stage1 walk becomes the NSE bit.

Rather than collecting an NS bit and applying it later, compute the
output pa space from the input pa space and unconditionally assign.
This means that we no longer need to adjust the output space earlier
for the NSTable bit.

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

Comments

Peter Maydell Feb. 10, 2023, 11:53 a.m. UTC | #1
On Tue, 24 Jan 2023 at 00:06, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> With Realm security state, bit 55 of a block or page descriptor during
> the stage2 walk becomes the NS bit; during the stage1 walk the bit 5
> NS bit is RES0.  With Root security state, bit 11 of the block or page
> descriptor during the stage1 walk becomes the NSE bit.
>
> Rather than collecting an NS bit and applying it later, compute the
> output pa space from the input pa space and unconditionally assign.
> This means that we no longer need to adjust the output space earlier
> for the NSTable bit.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/ptw.c | 74 +++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 60 insertions(+), 14 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index ddafb1f329..849f5e89ca 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -1250,11 +1250,12 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
>      int32_t stride;
>      int addrsize, inputsize, outputsize;
>      uint64_t tcr = regime_tcr(env, mmu_idx);
> -    int ap, ns, xn, pxn;
> +    int ap, xn, pxn;
>      uint32_t el = regime_el(env, mmu_idx);
>      uint64_t descaddrmask;
>      bool aarch64 = arm_el_is_aa64(env, el);
>      uint64_t descriptor, new_descriptor;
> +    ARMSecuritySpace out_space;
>
>      /* TODO: This code does not support shareability levels. */
>      if (aarch64) {
> @@ -1434,8 +1435,6 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
>          ptw->in_ptw_idx += 1;
>          ptw->in_secure = false;
>          ptw->in_space = ARMSS_NonSecure;
> -        result->f.attrs.secure = false;
> -        result->f.attrs.space = ARMSS_NonSecure;
>      }
>
>      if (!S1_ptw_translate(env, ptw, descaddr, fi)) {
> @@ -1552,12 +1551,66 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
>      }
>
>      ap = extract32(attrs, 6, 2);
> +    out_space = ptw->in_space;
>      if (regime_is_stage2(mmu_idx)) {
> -        ns = mmu_idx == ARMMMUIdx_Stage2;
> +        /*
> +         * R_GYNXY: For stage2 in Realm security state, bit 55 is NS.
> +         * The bit remains ignored for other security states.
> +         */
> +        if (out_space == ARMSS_Realm && extract64(attrs, 55, 1)) {
> +            out_space = ARMSS_NonSecure;
> +        }
>          xn = extract64(attrs, 53, 2);
>          result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
>      } else {
> -        ns = extract32(attrs, 5, 1);
> +        int ns = extract32(attrs, 5, 1);
> +        switch (out_space) {
> +        case ARMSS_Root:
> +            /*
> +             * R_GVZML: Bit 11 becomes the NSE field in the EL3 regime.
> +             * R_XTYPW: NSE and NS together select the output pa space.
> +             */
> +            int nse = extract32(attrs, 11, 1);
> +            out_space = (nse << 1) | ns;
> +            if (out_space == ARMSS_Secure &&
> +                !cpu_isar_feature(aa64_sel2, cpu)) {
> +                out_space = ARMSS_NonSecure;
> +            }
> +            break;
> +        case ARMSS_Secure:
> +            if (ns) {
> +                out_space = ARMSS_NonSecure;
> +            }
> +            break;
> +        case ARMSS_Realm:
> +            switch (mmu_idx) {
> +            case ARMMMUIdx_Stage1_E0:
> +            case ARMMMUIdx_Stage1_E1:
> +            case ARMMMUIdx_Stage1_E1_PAN:
> +                /* I_CZPRF: For Realm EL1&0 stage1, NS bit is RES0. */
> +                break;
> +            case ARMMMUIdx_E2:
> +            case ARMMMUIdx_E20_0:
> +            case ARMMMUIdx_E20_2:
> +            case ARMMMUIdx_E20_2_PAN:
> +                /*
> +                 * R_LYKFZ, R_WGRZN: For Realm EL2 and EL2&1,
> +                 * NS changes the output to non-secure space.
> +                 */
> +                if (ns) {
> +                    out_space = ARMSS_NonSecure;
> +                }
> +                break;
> +            default:
> +                g_assert_not_reached();
> +            }
> +            break;
> +        case ARMSS_NonSecure:
> +            /* R_QRMFF: For NonSecure state, the NS bit is RES0. */
> +            break;
> +        default:
> +            g_assert_not_reached();
> +        }
>          xn = extract64(attrs, 54, 1);
>          pxn = extract64(attrs, 53, 1);
>          result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn);

Various of these cases say "NS is RES0", but the code is
still extracting it from the attrs bit 5 and passing it
to get_S1prot(), which relies on being passed 1 for any case
where the translation table indicates that the memory is
Non-secure, whether the NS bit in the final block or page descriptor
was 1 or not (it uses it to implement the SCR_EL3.SIF bit).
This used to happen automatically because we set tableattrs to
1 << 4 if the initial access was non-secure, and then that bit
would always end up in attrs bit 5 regardless of what the various
NSTable and NS bits in the descriptors were, but the refactoring
in the previous patch changes that and so we need to do something
else I think.

thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index ddafb1f329..849f5e89ca 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1250,11 +1250,12 @@  static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
     int32_t stride;
     int addrsize, inputsize, outputsize;
     uint64_t tcr = regime_tcr(env, mmu_idx);
-    int ap, ns, xn, pxn;
+    int ap, xn, pxn;
     uint32_t el = regime_el(env, mmu_idx);
     uint64_t descaddrmask;
     bool aarch64 = arm_el_is_aa64(env, el);
     uint64_t descriptor, new_descriptor;
+    ARMSecuritySpace out_space;
 
     /* TODO: This code does not support shareability levels. */
     if (aarch64) {
@@ -1434,8 +1435,6 @@  static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
         ptw->in_ptw_idx += 1;
         ptw->in_secure = false;
         ptw->in_space = ARMSS_NonSecure;
-        result->f.attrs.secure = false;
-        result->f.attrs.space = ARMSS_NonSecure;
     }
 
     if (!S1_ptw_translate(env, ptw, descaddr, fi)) {
@@ -1552,12 +1551,66 @@  static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
     }
 
     ap = extract32(attrs, 6, 2);
+    out_space = ptw->in_space;
     if (regime_is_stage2(mmu_idx)) {
-        ns = mmu_idx == ARMMMUIdx_Stage2;
+        /*
+         * R_GYNXY: For stage2 in Realm security state, bit 55 is NS.
+         * The bit remains ignored for other security states.
+         */
+        if (out_space == ARMSS_Realm && extract64(attrs, 55, 1)) {
+            out_space = ARMSS_NonSecure;
+        }
         xn = extract64(attrs, 53, 2);
         result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
     } else {
-        ns = extract32(attrs, 5, 1);
+        int ns = extract32(attrs, 5, 1);
+        switch (out_space) {
+        case ARMSS_Root:
+            /*
+             * R_GVZML: Bit 11 becomes the NSE field in the EL3 regime.
+             * R_XTYPW: NSE and NS together select the output pa space.
+             */
+            int nse = extract32(attrs, 11, 1);
+            out_space = (nse << 1) | ns;
+            if (out_space == ARMSS_Secure &&
+                !cpu_isar_feature(aa64_sel2, cpu)) {
+                out_space = ARMSS_NonSecure;
+            }
+            break;
+        case ARMSS_Secure:
+            if (ns) {
+                out_space = ARMSS_NonSecure;
+            }
+            break;
+        case ARMSS_Realm:
+            switch (mmu_idx) {
+            case ARMMMUIdx_Stage1_E0:
+            case ARMMMUIdx_Stage1_E1:
+            case ARMMMUIdx_Stage1_E1_PAN:
+                /* I_CZPRF: For Realm EL1&0 stage1, NS bit is RES0. */
+                break;
+            case ARMMMUIdx_E2:
+            case ARMMMUIdx_E20_0:
+            case ARMMMUIdx_E20_2:
+            case ARMMMUIdx_E20_2_PAN:
+                /*
+                 * R_LYKFZ, R_WGRZN: For Realm EL2 and EL2&1,
+                 * NS changes the output to non-secure space.
+                 */
+                if (ns) {
+                    out_space = ARMSS_NonSecure;
+                }
+                break;
+            default:
+                g_assert_not_reached();
+            }
+            break;
+        case ARMSS_NonSecure:
+            /* R_QRMFF: For NonSecure state, the NS bit is RES0. */
+            break;
+        default:
+            g_assert_not_reached();
+        }
         xn = extract64(attrs, 54, 1);
         pxn = extract64(attrs, 53, 1);
         result->f.prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn);
@@ -1587,15 +1640,8 @@  static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
         }
     }
 
-    if (ns) {
-        /*
-         * The NS bit will (as required by the architecture) have no effect if
-         * the CPU doesn't support TZ or this is a non-secure translation
-         * regime, because the attribute will already be non-secure.
-         */
-        result->f.attrs.secure = false;
-        result->f.attrs.space = ARMSS_NonSecure;
-    }
+    result->f.attrs.space = out_space;
+    result->f.attrs.secure = arm_space_is_secure(out_space);
 
     /* When in aarch64 mode, and BTI is enabled, remember GP in the TLB.  */
     if (aarch64 && cpu_isar_feature(aa64_bti, cpu)) {