diff mbox series

[02/36] target/arm: Don't allow Thumb Neon insns without FEATURE_NEON

Message ID 20200430181003.21682-3-peter.maydell@linaro.org
State New
Headers show
Series target/arm: Convert Neon to decodetree (part 1) | expand

Commit Message

Peter Maydell April 30, 2020, 6:09 p.m. UTC 
We were accidentally permitting decode of Thumb Neon insns even if
the CPU didn't have the FEATURE_NEON bit set, because the feature
check was being done before the call to disas_neon_data_insn() and
disas_neon_ls_insn() in the Arm decoder but was omitted from the
Thumb decoder.  Push the feature bit check down into the called
functions so it is done for both Arm and Thumb encodings.

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

Comments

Richard Henderson April 30, 2020, 6:22 p.m. UTC | #1 
On 4/30/20 11:09 AM, Peter Maydell wrote:
> We were accidentally permitting decode of Thumb Neon insns even if
> the CPU didn't have the FEATURE_NEON bit set, because the feature
> check was being done before the call to disas_neon_data_insn() and
> disas_neon_ls_insn() in the Arm decoder but was omitted from the
> Thumb decoder.  Push the feature bit check down into the called
> functions so it is done for both Arm and Thumb encodings.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/translate.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

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

r~
Philippe Mathieu-Daudé May 1, 2020, 4:56 p.m. UTC | #2 
On 4/30/20 8:09 PM, Peter Maydell wrote:
> We were accidentally permitting decode of Thumb Neon insns even if
> the CPU didn't have the FEATURE_NEON bit set, because the feature
> check was being done before the call to disas_neon_data_insn() and
> disas_neon_ls_insn() in the Arm decoder but was omitted from the
> Thumb decoder.  Push the feature bit check down into the called
> functions so it is done for both Arm and Thumb encodings.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/translate.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index d4ad2028f12..ab5324a5aaa 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -3258,6 +3258,10 @@ static int disas_neon_ls_insn(DisasContext *s, uint32_t insn)
>       TCGv_i32 tmp2;
>       TCGv_i64 tmp64;
>   
> +    if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
> +        return 1;
> +    }
> +
>       /* FIXME: this access check should not take precedence over UNDEF
>        * for invalid encodings; we will generate incorrect syndrome information
>        * for attempts to execute invalid vfp/neon encodings with FP disabled.
> @@ -5002,6 +5006,10 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
>       TCGv_ptr ptr1, ptr2, ptr3;
>       TCGv_i64 tmp64;
>   
> +    if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
> +        return 1;
> +    }
> +
>       /* FIXME: this access check should not take precedence over UNDEF
>        * for invalid encodings; we will generate incorrect syndrome information
>        * for attempts to execute invalid vfp/neon encodings with FP disabled.
> @@ -10948,10 +10956,6 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
>   
>           if (((insn >> 25) & 7) == 1) {
>               /* NEON Data processing.  */
> -            if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
> -                goto illegal_op;
> -            }
> -
>               if (disas_neon_data_insn(s, insn)) {
>                   goto illegal_op;
>               }
> @@ -10959,10 +10963,6 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
>           }
>           if ((insn & 0x0f100000) == 0x04000000) {
>               /* NEON load/store.  */
> -            if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
> -                goto illegal_op;
> -            }
> -
>               if (disas_neon_ls_insn(s, insn)) {
>                   goto illegal_op;
>               }
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
diff mbox series

Patch

diff --git a/target/arm/translate.c b/target/arm/translate.c
index d4ad2028f12..ab5324a5aaa 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -3258,6 +3258,10 @@  static int disas_neon_ls_insn(DisasContext *s, uint32_t insn)
     TCGv_i32 tmp2;
     TCGv_i64 tmp64;
 
+    if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
+        return 1;
+    }
+
     /* FIXME: this access check should not take precedence over UNDEF
      * for invalid encodings; we will generate incorrect syndrome information
      * for attempts to execute invalid vfp/neon encodings with FP disabled.
@@ -5002,6 +5006,10 @@  static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
     TCGv_ptr ptr1, ptr2, ptr3;
     TCGv_i64 tmp64;
 
+    if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
+        return 1;
+    }
+
     /* FIXME: this access check should not take precedence over UNDEF
      * for invalid encodings; we will generate incorrect syndrome information
      * for attempts to execute invalid vfp/neon encodings with FP disabled.
@@ -10948,10 +10956,6 @@  static void disas_arm_insn(DisasContext *s, unsigned int insn)
 
         if (((insn >> 25) & 7) == 1) {
             /* NEON Data processing.  */
-            if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
-                goto illegal_op;
-            }
-
             if (disas_neon_data_insn(s, insn)) {
                 goto illegal_op;
             }
@@ -10959,10 +10963,6 @@  static void disas_arm_insn(DisasContext *s, unsigned int insn)
         }
         if ((insn & 0x0f100000) == 0x04000000) {
             /* NEON load/store.  */
-            if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
-                goto illegal_op;
-            }
-
             if (disas_neon_ls_insn(s, insn)) {
                 goto illegal_op;
             }