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>