In handle_sys() we don't do the check for whether the register is
marked as needing an FPU/SVE/SME access check until after we've
handled the special cases covered by ARM_CP_SPECIAL_MASK.  This is
conceptually the wrong way around, because if for example we happen
to implement an FPU-access-checked register as ARM_CP_NOP, we should
do the access check first.

Move the access checks up so they are with all the other access
checks, not sandwiched between the special-case read/write handling
and the normal-case read/write handling. This doesn't change
behaviour at the moment, because we happen not to define any
cpregs with both ARM_CPU_{FPU,SVE,SME} and one of the cases
dealt with by ARM_CP_SPECIAL_MASK.

Moving this code also means we have the correct place to put the
FEAT_NV/FEAT_NV2 access handling, which should come after the access
checks and before we try to do any read/write action.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/tcg/translate-a64.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 00d12e148ca..2d26cb6210f 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -2189,6 +2189,14 @@ static void handle_sys(DisasContext *s, bool isread,
         gen_a64_update_pc(s, 0);
     }
 
+    if ((ri->type & ARM_CP_FPU) && !fp_access_check_only(s)) {
+        return;
+    } else if ((ri->type & ARM_CP_SVE) && !sve_access_check(s)) {
+        return;
+    } else if ((ri->type & ARM_CP_SME) && !sme_access_check(s)) {
+        return;
+    }
+
     /* Handle special cases first */
     switch (ri->type & ARM_CP_SPECIAL_MASK) {
     case 0:
@@ -2267,13 +2275,6 @@ static void handle_sys(DisasContext *s, bool isread,
     default:
         g_assert_not_reached();
     }
-    if ((ri->type & ARM_CP_FPU) && !fp_access_check_only(s)) {
-        return;
-    } else if ((ri->type & ARM_CP_SVE) && !sve_access_check(s)) {
-        return;
-    } else if ((ri->type & ARM_CP_SME) && !sme_access_check(s)) {
-        return;
-    }
 
     if (ri->type & ARM_CP_IO) {
         /* I/O operations must end the TB here (whether read or write) */
-- 
2.34.1

On 12/18/23 22:32, Peter Maydell wrote:
> In handle_sys() we don't do the check for whether the register is
> marked as needing an FPU/SVE/SME access check until after we've
> handled the special cases covered by ARM_CP_SPECIAL_MASK.  This is
> conceptually the wrong way around, because if for example we happen
> to implement an FPU-access-checked register as ARM_CP_NOP, we should
> do the access check first.
> 
> Move the access checks up so they are with all the other access
> checks, not sandwiched between the special-case read/write handling
> and the normal-case read/write handling. This doesn't change
> behaviour at the moment, because we happen not to define any
> cpregs with both ARM_CPU_{FPU,SVE,SME} and one of the cases
> dealt with by ARM_CP_SPECIAL_MASK.
> 
> Moving this code also means we have the correct place to put the
> FEAT_NV/FEAT_NV2 access handling, which should come after the access
> checks and before we try to do any read/write action.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/tcg/translate-a64.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)

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

r~