[Qemu-devel] [PATCH v2] target/arm: Implement NSACR gating of floating point

Peter Maydell posted 1 patch 4 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190510110357.18825-1-peter.maydell@linaro.org
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Maintainers: Peter Maydell <peter.maydell@linaro.org>
target/arm/helper.c | 75 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 73 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH v2] target/arm: Implement NSACR gating of floating point
Posted by Peter Maydell 4 years, 11 months ago
The NSACR register allows secure code to configure the FPU
to be inaccessible to non-secure code. If the NSACR.CP10
bit is set then:
 * NS accesses to the FPU trap as UNDEF (ie to NS EL1 or EL2)
 * CPACR.{CP10,CP11} behave as if RAZ/WI
 * HCPTR.{TCP11,TCP10} behave as if RAO/WI

Note that we do not implement the NSACR.NSASEDIS bit which
gates only access to Advanced SIMD, in the same way that
we don't implement the equivalent CPACR.ASEDIS and HCPTR.TASE.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Changes v1->v2:
 * fixed bug in cptr_el2_read() that meant we were forcing
   HCPTR.{TCP11,TCP10} to 0 when they should be 1
---
 target/arm/helper.c | 75 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 73 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 1e6eb0d0f36..f1fcce0313b 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -928,9 +928,36 @@ static void cpacr_write(CPUARMState *env, const ARMCPRegInfo *ri,
         }
         value &= mask;
     }
+
+    /*
+     * For A-profile AArch32 EL3 (but not M-profile secure mode), if NSACR.CP10
+     * is 0 then CPACR.{CP11,CP10} ignore writes and read as 0b00.
+     */
+    if (arm_feature(env, ARM_FEATURE_EL3) && !arm_el_is_aa64(env, 3) &&
+        !arm_is_secure(env) && !extract32(env->cp15.nsacr, 10, 1)) {
+        value &= ~(0xf << 20);
+        value |= env->cp15.cpacr_el1 & (0xf << 20);
+    }
+
     env->cp15.cpacr_el1 = value;
 }
 
+static uint64_t cpacr_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    /*
+     * For A-profile AArch32 EL3 (but not M-profile secure mode), if NSACR.CP10
+     * is 0 then CPACR.{CP11,CP10} ignore writes and read as 0b00.
+     */
+    uint64_t value = env->cp15.cpacr_el1;
+
+    if (arm_feature(env, ARM_FEATURE_EL3) && !arm_el_is_aa64(env, 3) &&
+        !arm_is_secure(env) && !extract32(env->cp15.nsacr, 10, 1)) {
+        value &= ~(0xf << 20);
+    }
+    return value;
+}
+
+
 static void cpacr_reset(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     /* Call cpacr_write() so that we reset with the correct RAO bits set
@@ -996,7 +1023,7 @@ static const ARMCPRegInfo v6_cp_reginfo[] = {
     { .name = "CPACR", .state = ARM_CP_STATE_BOTH, .opc0 = 3,
       .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 2, .accessfn = cpacr_access,
       .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.cpacr_el1),
-      .resetfn = cpacr_reset, .writefn = cpacr_write },
+      .resetfn = cpacr_reset, .writefn = cpacr_write, .readfn = cpacr_read },
     REGINFO_SENTINEL
 };
 
@@ -4681,6 +4708,36 @@ uint64_t arm_hcr_el2_eff(CPUARMState *env)
     return ret;
 }
 
+static void cptr_el2_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                           uint64_t value)
+{
+    /*
+     * For A-profile AArch32 EL3, if NSACR.CP10
+     * is 0 then HCPTR.{TCP11,TCP10} ignore writes and read as 1.
+     */
+    if (arm_feature(env, ARM_FEATURE_EL3) && !arm_el_is_aa64(env, 3) &&
+        !arm_is_secure(env) && !extract32(env->cp15.nsacr, 10, 1)) {
+        value &= ~(0x3 << 10);
+        value |= env->cp15.cptr_el[2] & (0x3 << 10);
+    }
+    env->cp15.cptr_el[2] = value;
+}
+
+static uint64_t cptr_el2_read(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+    /*
+     * For A-profile AArch32 EL3, if NSACR.CP10
+     * is 0 then HCPTR.{TCP11,TCP10} ignore writes and read as 1.
+     */
+    uint64_t value = env->cp15.cptr_el[2];
+
+    if (arm_feature(env, ARM_FEATURE_EL3) && !arm_el_is_aa64(env, 3) &&
+        !arm_is_secure(env) && !extract32(env->cp15.nsacr, 10, 1)) {
+        value |= 0x3 << 10;
+    }
+    return value;
+}
+
 static const ARMCPRegInfo el2_cp_reginfo[] = {
     { .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
       .type = ARM_CP_IO,
@@ -4728,7 +4785,8 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
     { .name = "CPTR_EL2", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 2,
       .access = PL2_RW, .accessfn = cptr_access, .resetvalue = 0,
-      .fieldoffset = offsetof(CPUARMState, cp15.cptr_el[2]) },
+      .fieldoffset = offsetof(CPUARMState, cp15.cptr_el[2]),
+      .readfn = cptr_el2_read, .writefn = cptr_el2_write },
     { .name = "MAIR_EL2", .state = ARM_CP_STATE_BOTH,
       .opc0 = 3, .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 0,
       .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.mair_el[2]),
@@ -13527,6 +13585,19 @@ int fp_exception_el(CPUARMState *env, int cur_el)
         break;
     }
 
+    /*
+     * The NSACR allows A-profile AArch32 EL3 and M-profile secure mode
+     * to control non-secure access to the FPU. It doesn't have any
+     * effect if EL3 is AArch64 or if EL3 doesn't exist at all.
+     */
+    if ((arm_feature(env, ARM_FEATURE_EL3) && !arm_el_is_aa64(env, 3) &&
+         cur_el <= 2 && !arm_is_secure_below_el3(env))) {
+        if (!extract32(env->cp15.nsacr, 10, 1)) {
+            /* FP insns act as UNDEF */
+            return cur_el == 2 ? 2 : 1;
+        }
+    }
+
     /* For the CPTR registers we don't need to guard with an ARM_FEATURE
      * check because zero bits in the registers mean "don't trap".
      */
-- 
2.20.1


Re: [Qemu-devel] [PATCH v2] target/arm: Implement NSACR gating of floating point
Posted by Peter Maydell 4 years, 10 months ago
Ping for code review, please?

thanks
-- PMM

On Fri, 10 May 2019 at 12:03, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The NSACR register allows secure code to configure the FPU
> to be inaccessible to non-secure code. If the NSACR.CP10
> bit is set then:
>  * NS accesses to the FPU trap as UNDEF (ie to NS EL1 or EL2)
>  * CPACR.{CP10,CP11} behave as if RAZ/WI
>  * HCPTR.{TCP11,TCP10} behave as if RAO/WI
>
> Note that we do not implement the NSACR.NSASEDIS bit which
> gates only access to Advanced SIMD, in the same way that
> we don't implement the equivalent CPACR.ASEDIS and HCPTR.TASE.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Changes v1->v2:
>  * fixed bug in cptr_el2_read() that meant we were forcing
>    HCPTR.{TCP11,TCP10} to 0 when they should be 1
> ---
>  target/arm/helper.c | 75 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 1e6eb0d0f36..f1fcce0313b 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -928,9 +928,36 @@ static void cpacr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>          }
>          value &= mask;
>      }
> +
> +    /*
> +     * For A-profile AArch32 EL3 (but not M-profile secure mode), if NSACR.CP10
> +     * is 0 then CPACR.{CP11,CP10} ignore writes and read as 0b00.
> +     */
> +    if (arm_feature(env, ARM_FEATURE_EL3) && !arm_el_is_aa64(env, 3) &&
> +        !arm_is_secure(env) && !extract32(env->cp15.nsacr, 10, 1)) {
> +        value &= ~(0xf << 20);
> +        value |= env->cp15.cpacr_el1 & (0xf << 20);
> +    }
> +
>      env->cp15.cpacr_el1 = value;
>  }
>
> +static uint64_t cpacr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    /*
> +     * For A-profile AArch32 EL3 (but not M-profile secure mode), if NSACR.CP10
> +     * is 0 then CPACR.{CP11,CP10} ignore writes and read as 0b00.
> +     */
> +    uint64_t value = env->cp15.cpacr_el1;
> +
> +    if (arm_feature(env, ARM_FEATURE_EL3) && !arm_el_is_aa64(env, 3) &&
> +        !arm_is_secure(env) && !extract32(env->cp15.nsacr, 10, 1)) {
> +        value &= ~(0xf << 20);
> +    }
> +    return value;
> +}
> +
> +
>  static void cpacr_reset(CPUARMState *env, const ARMCPRegInfo *ri)
>  {
>      /* Call cpacr_write() so that we reset with the correct RAO bits set
> @@ -996,7 +1023,7 @@ static const ARMCPRegInfo v6_cp_reginfo[] = {
>      { .name = "CPACR", .state = ARM_CP_STATE_BOTH, .opc0 = 3,
>        .crn = 1, .crm = 0, .opc1 = 0, .opc2 = 2, .accessfn = cpacr_access,
>        .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.cpacr_el1),
> -      .resetfn = cpacr_reset, .writefn = cpacr_write },
> +      .resetfn = cpacr_reset, .writefn = cpacr_write, .readfn = cpacr_read },
>      REGINFO_SENTINEL
>  };
>
> @@ -4681,6 +4708,36 @@ uint64_t arm_hcr_el2_eff(CPUARMState *env)
>      return ret;
>  }
>
> +static void cptr_el2_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +                           uint64_t value)
> +{
> +    /*
> +     * For A-profile AArch32 EL3, if NSACR.CP10
> +     * is 0 then HCPTR.{TCP11,TCP10} ignore writes and read as 1.
> +     */
> +    if (arm_feature(env, ARM_FEATURE_EL3) && !arm_el_is_aa64(env, 3) &&
> +        !arm_is_secure(env) && !extract32(env->cp15.nsacr, 10, 1)) {
> +        value &= ~(0x3 << 10);
> +        value |= env->cp15.cptr_el[2] & (0x3 << 10);
> +    }
> +    env->cp15.cptr_el[2] = value;
> +}
> +
> +static uint64_t cptr_el2_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> +    /*
> +     * For A-profile AArch32 EL3, if NSACR.CP10
> +     * is 0 then HCPTR.{TCP11,TCP10} ignore writes and read as 1.
> +     */
> +    uint64_t value = env->cp15.cptr_el[2];
> +
> +    if (arm_feature(env, ARM_FEATURE_EL3) && !arm_el_is_aa64(env, 3) &&
> +        !arm_is_secure(env) && !extract32(env->cp15.nsacr, 10, 1)) {
> +        value |= 0x3 << 10;
> +    }
> +    return value;
> +}
> +
>  static const ARMCPRegInfo el2_cp_reginfo[] = {
>      { .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
>        .type = ARM_CP_IO,
> @@ -4728,7 +4785,8 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>      { .name = "CPTR_EL2", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 2,
>        .access = PL2_RW, .accessfn = cptr_access, .resetvalue = 0,
> -      .fieldoffset = offsetof(CPUARMState, cp15.cptr_el[2]) },
> +      .fieldoffset = offsetof(CPUARMState, cp15.cptr_el[2]),
> +      .readfn = cptr_el2_read, .writefn = cptr_el2_write },
>      { .name = "MAIR_EL2", .state = ARM_CP_STATE_BOTH,
>        .opc0 = 3, .opc1 = 4, .crn = 10, .crm = 2, .opc2 = 0,
>        .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.mair_el[2]),
> @@ -13527,6 +13585,19 @@ int fp_exception_el(CPUARMState *env, int cur_el)
>          break;
>      }
>
> +    /*
> +     * The NSACR allows A-profile AArch32 EL3 and M-profile secure mode
> +     * to control non-secure access to the FPU. It doesn't have any
> +     * effect if EL3 is AArch64 or if EL3 doesn't exist at all.
> +     */
> +    if ((arm_feature(env, ARM_FEATURE_EL3) && !arm_el_is_aa64(env, 3) &&
> +         cur_el <= 2 && !arm_is_secure_below_el3(env))) {
> +        if (!extract32(env->cp15.nsacr, 10, 1)) {
> +            /* FP insns act as UNDEF */
> +            return cur_el == 2 ? 2 : 1;
> +        }
> +    }
> +
>      /* For the CPTR registers we don't need to guard with an ARM_FEATURE
>       * check because zero bits in the registers mean "don't trap".
>       */
> --
> 2.20.1

Re: [Qemu-devel] [PATCH v2] target/arm: Implement NSACR gating of floating point
Posted by Richard Henderson 4 years, 10 months ago
On 6/7/19 8:06 AM, Peter Maydell wrote:
> The NSACR register allows secure code to configure the FPU
> to be inaccessible to non-secure code. If the NSACR.CP10
> bit is set then:
>  * NS accesses to the FPU trap as UNDEF (ie to NS EL1 or EL2)
>  * CPACR.{CP10,CP11} behave as if RAZ/WI
>  * HCPTR.{TCP11,TCP10} behave as if RAO/WI
> 
> Note that we do not implement the NSACR.NSASEDIS bit which
> gates only access to Advanced SIMD, in the same way that
> we don't implement the equivalent CPACR.ASEDIS and HCPTR.TASE.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Changes v1->v2:
>  * fixed bug in cptr_el2_read() that meant we were forcing
>    HCPTR.{TCP11,TCP10} to 0 when they should be 1
> ---
>  target/arm/helper.c | 75 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 73 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~