When an AArch32 Neon or VFP insn is trapped to AArch64 EL2, bits
[19:0] of the syndrome in ESR_EL2 are RES0.  However, when it is
trapped to AArch32 EL2, the HSR syndrome information defines some
extra fields:
 [5] : TA
 [3:0] : coproc

where the TA bit is 1 for a trapped Neon insn and 0 for a trapped
VFP insn, and the coproc field is 0b1010 when TA is 0, and 0 when
TA is 1.

We attempted to address this in commit fa33eead ("target/arm: Add
coproc parameter to syn_fp_access_trap"), but got it wrong: we
thought the RES0 condition was "is v8A" rather than "is EL2 AArch32",
and we made all insns be TA=0 coproc = 0b1010 rather than only the
VFP ones.  Correct the condition we use to decide the coproc and TA
fields.  We set these fields unconditionally; later on in
arm_cpu_do_interrupt_aarch64() we will squash them to zero if we are
taking the exception to AArch64.

NB: there is some disagreement between different revisions of the
Arm ARM about the exact handling of 'coproc':
 * the v8A Arm ARM text says coproc is 0b1010 when TA is 1
 * the v8A Arm ARM pseudocode in AArch32_CheckFPAdvSIMDTrap()
   sets coproc to 0b1010 when TA is 0
 * the v7A Arm ARM text says coproc is 0b1010 when TA is 0
 * the v7A Arm ARM pseudocode sets coproc to 0b1010 when TA is 0

The v7A Arm ARM pseudocode also disagrees with the v7A text, v8A text
and v8A pseudocode in only setting TA to 1 for traps caused by
HCPTR.TASE; the others set Ta for all trapped AdvSIMD insns
(i.e. including traps caused by HCPTR.TCP10).

We assume that the v8A pseudocode is incorrect about coproc (as it is
the odd one out) and that the v7A pseudocode is incorrect about when
TA is set (again, as it is the odd one out).

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/1153
Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Richard Henderson <[email protected]>
Message-id: [email protected]
---
 target/arm/tcg/translate-vfp.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/target/arm/tcg/translate-vfp.c b/target/arm/tcg/translate-vfp.c
index 7474973952..6e944a0322 100644
--- a/target/arm/tcg/translate-vfp.c
+++ b/target/arm/tcg/translate-vfp.c
@@ -216,19 +216,20 @@ static void gen_update_fp_context(DisasContext *s)
  * whether VFP is enabled via FPEXC.EN: this should be true for FMXR/FMRX
  * accesses to FPSID, FPEXC, MVFR0, MVFR1, MVFR2, and false for all other 
insns.
  */
-static bool vfp_access_check_a(DisasContext *s, bool ignore_vfp_enabled)
+static bool vfp_access_check_a(DisasContext *s, bool ignore_vfp_enabled,
+                               bool is_neon)
 {
     if (s->fp_excp_el) {
         /*
-         * The full syndrome is only used for HSR when HCPTR traps:
-         * For v8, when TA==0, coproc is RES0.
-         * For v7, any use of a Floating-point instruction or access
-         * to a Floating-point Extension register that is trapped to
-         * Hyp mode because of a trap configured in the HCPTR sets
-         * this field to 0xA.
+         * The full syndrome is only used for HSR when HCPTR traps.
+         * When trapping to AArch64, the TA and coproc fields are RES0
+         * (we will squash them in arm_cpu_do_interrupt_aarch64()).
+         * When trapping to AArch32:
+         *  - for VFP insns, TA=0 and coproc = 0b1010
+         *  - for Neon insns, TA=1 and coproc = 0
          */
-        int coproc = arm_dc_feature(s, ARM_FEATURE_V8) ? 0 : 0xa;
-        uint32_t syn = syn_a32_fp_access_trap(1, 0xe, 0, coproc);
+        int coproc = is_neon ? 0 : 0xa;
+        uint32_t syn = syn_a32_fp_access_trap(1, 0xe, is_neon, coproc);
 
         gen_exception_insn_el(s, 0, EXCP_UDEF, syn, s->fp_excp_el);
         return false;
@@ -299,7 +300,7 @@ bool vfp_access_check(DisasContext *s)
     if (arm_dc_feature(s, ARM_FEATURE_M)) {
         return vfp_access_check_m(s, false);
     } else {
-        return vfp_access_check_a(s, false);
+        return vfp_access_check_a(s, false, false);
     }
 }
 
@@ -311,7 +312,11 @@ bool vfp_access_check(DisasContext *s)
  */
 bool neon_access_check(DisasContext *s)
 {
-    return vfp_access_check(s);
+    if (arm_dc_feature(s, ARM_FEATURE_M)) {
+        return vfp_access_check_m(s, false);
+    } else {
+        return vfp_access_check_a(s, false, true);
+    }
 }
 
 static bool trans_VSEL(DisasContext *s, arg_VSEL *a)
@@ -822,7 +827,7 @@ static bool trans_VMSR_VMRS(DisasContext *s, arg_VMSR_VMRS 
*a)
      * Call vfp_access_check_a() directly, because we need to tell
      * it to ignore FPEXC.EN for some register accesses.
      */
-    if (!vfp_access_check_a(s, ignore_vfp_enabled)) {
+    if (!vfp_access_check_a(s, ignore_vfp_enabled, false)) {
         return true;
     }
 
-- 
2.43.0


Reply via email to