* [Qemu-devel] [PATCH 1/5] target-arm/translate.c: Use arm_dc_feature() in ENABLE_ARCH_ macros
2014-10-28 19:23 [Qemu-devel] [PATCH 0/5] target-arm: Avoid passing CPUARMState around the decoder Peter Maydell
@ 2014-10-28 19:24 ` Peter Maydell
2014-10-31 13:09 ` Alex Bennée
2014-11-04 9:04 ` Claudio Fontana
2014-10-28 19:24 ` [Qemu-devel] [PATCH 2/5] target-arm/translate.c: Use arm_dc_feature() rather than arm_feature() Peter Maydell
` (3 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Peter Maydell @ 2014-10-28 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: patches
All the places where we use the ENABLE_ARCH_* and ARCH() macros have a
DisasContext* s, so switch them over to use arm_dc_feature() rather than
arm_feature() so we don't need to pass the CPUARMState* env around too.
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 1d52e47..f69e5ef 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -38,16 +38,16 @@
#include "trace-tcg.h"
-#define ENABLE_ARCH_4T arm_feature(env, ARM_FEATURE_V4T)
-#define ENABLE_ARCH_5 arm_feature(env, ARM_FEATURE_V5)
+#define ENABLE_ARCH_4T arm_dc_feature(s, ARM_FEATURE_V4T)
+#define ENABLE_ARCH_5 arm_dc_feature(s, ARM_FEATURE_V5)
/* currently all emulated v5 cores are also v5TE, so don't bother */
-#define ENABLE_ARCH_5TE arm_feature(env, ARM_FEATURE_V5)
+#define ENABLE_ARCH_5TE arm_dc_feature(s, ARM_FEATURE_V5)
#define ENABLE_ARCH_5J 0
-#define ENABLE_ARCH_6 arm_feature(env, ARM_FEATURE_V6)
-#define ENABLE_ARCH_6K arm_feature(env, ARM_FEATURE_V6K)
-#define ENABLE_ARCH_6T2 arm_feature(env, ARM_FEATURE_THUMB2)
-#define ENABLE_ARCH_7 arm_feature(env, ARM_FEATURE_V7)
-#define ENABLE_ARCH_8 arm_feature(env, ARM_FEATURE_V8)
+#define ENABLE_ARCH_6 arm_dc_feature(s, ARM_FEATURE_V6)
+#define ENABLE_ARCH_6K arm_dc_feature(s, ARM_FEATURE_V6K)
+#define ENABLE_ARCH_6T2 arm_dc_feature(s, ARM_FEATURE_THUMB2)
+#define ENABLE_ARCH_7 arm_dc_feature(s, ARM_FEATURE_V7)
+#define ENABLE_ARCH_8 arm_dc_feature(s, ARM_FEATURE_V8)
#define ARCH(x) do { if (!ENABLE_ARCH_##x) goto illegal_op; } while(0)
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PATCH 1/5] target-arm/translate.c: Use arm_dc_feature() in ENABLE_ARCH_ macros
2014-10-28 19:24 ` [Qemu-devel] [PATCH 1/5] target-arm/translate.c: Use arm_dc_feature() in ENABLE_ARCH_ macros Peter Maydell
@ 2014-10-31 13:09 ` Alex Bennée
2014-11-04 9:04 ` Claudio Fontana
1 sibling, 0 replies; 19+ messages in thread
From: Alex Bennée @ 2014-10-31 13:09 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> All the places where we use the ENABLE_ARCH_* and ARCH() macros have a
> DisasContext* s, so switch them over to use arm_dc_feature() rather than
> arm_feature() so we don't need to pass the CPUARMState* env around too.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@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 1d52e47..f69e5ef 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -38,16 +38,16 @@
> #include "trace-tcg.h"
>
>
> -#define ENABLE_ARCH_4T arm_feature(env, ARM_FEATURE_V4T)
> -#define ENABLE_ARCH_5 arm_feature(env, ARM_FEATURE_V5)
> +#define ENABLE_ARCH_4T arm_dc_feature(s, ARM_FEATURE_V4T)
> +#define ENABLE_ARCH_5 arm_dc_feature(s, ARM_FEATURE_V5)
> /* currently all emulated v5 cores are also v5TE, so don't bother */
> -#define ENABLE_ARCH_5TE arm_feature(env, ARM_FEATURE_V5)
> +#define ENABLE_ARCH_5TE arm_dc_feature(s, ARM_FEATURE_V5)
> #define ENABLE_ARCH_5J 0
> -#define ENABLE_ARCH_6 arm_feature(env, ARM_FEATURE_V6)
> -#define ENABLE_ARCH_6K arm_feature(env, ARM_FEATURE_V6K)
> -#define ENABLE_ARCH_6T2 arm_feature(env, ARM_FEATURE_THUMB2)
> -#define ENABLE_ARCH_7 arm_feature(env, ARM_FEATURE_V7)
> -#define ENABLE_ARCH_8 arm_feature(env, ARM_FEATURE_V8)
> +#define ENABLE_ARCH_6 arm_dc_feature(s, ARM_FEATURE_V6)
> +#define ENABLE_ARCH_6K arm_dc_feature(s, ARM_FEATURE_V6K)
> +#define ENABLE_ARCH_6T2 arm_dc_feature(s, ARM_FEATURE_THUMB2)
> +#define ENABLE_ARCH_7 arm_dc_feature(s, ARM_FEATURE_V7)
> +#define ENABLE_ARCH_8 arm_dc_feature(s, ARM_FEATURE_V8)
>
> #define ARCH(x) do { if (!ENABLE_ARCH_##x) goto illegal_op; } while(0)
--
Alex Bennée
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PATCH 1/5] target-arm/translate.c: Use arm_dc_feature() in ENABLE_ARCH_ macros
2014-10-28 19:24 ` [Qemu-devel] [PATCH 1/5] target-arm/translate.c: Use arm_dc_feature() in ENABLE_ARCH_ macros Peter Maydell
2014-10-31 13:09 ` Alex Bennée
@ 2014-11-04 9:04 ` Claudio Fontana
2014-11-04 12:04 ` Peter Maydell
1 sibling, 1 reply; 19+ messages in thread
From: Claudio Fontana @ 2014-11-04 9:04 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches
On 28.10.2014 20:24, Peter Maydell wrote:
> All the places where we use the ENABLE_ARCH_* and ARCH() macros have a
> DisasContext* s, so switch them over to use arm_dc_feature() rather than
> arm_feature() so we don't need to pass the CPUARMState* env around too.
>
> 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 1d52e47..f69e5ef 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -38,16 +38,16 @@
> #include "trace-tcg.h"
>
>
> -#define ENABLE_ARCH_4T arm_feature(env, ARM_FEATURE_V4T)
> -#define ENABLE_ARCH_5 arm_feature(env, ARM_FEATURE_V5)
> +#define ENABLE_ARCH_4T arm_dc_feature(s, ARM_FEATURE_V4T)
> +#define ENABLE_ARCH_5 arm_dc_feature(s, ARM_FEATURE_V5)
> /* currently all emulated v5 cores are also v5TE, so don't bother */
> -#define ENABLE_ARCH_5TE arm_feature(env, ARM_FEATURE_V5)
> +#define ENABLE_ARCH_5TE arm_dc_feature(s, ARM_FEATURE_V5)
> #define ENABLE_ARCH_5J 0
> -#define ENABLE_ARCH_6 arm_feature(env, ARM_FEATURE_V6)
> -#define ENABLE_ARCH_6K arm_feature(env, ARM_FEATURE_V6K)
> -#define ENABLE_ARCH_6T2 arm_feature(env, ARM_FEATURE_THUMB2)
> -#define ENABLE_ARCH_7 arm_feature(env, ARM_FEATURE_V7)
> -#define ENABLE_ARCH_8 arm_feature(env, ARM_FEATURE_V8)
> +#define ENABLE_ARCH_6 arm_dc_feature(s, ARM_FEATURE_V6)
> +#define ENABLE_ARCH_6K arm_dc_feature(s, ARM_FEATURE_V6K)
> +#define ENABLE_ARCH_6T2 arm_dc_feature(s, ARM_FEATURE_THUMB2)
> +#define ENABLE_ARCH_7 arm_dc_feature(s, ARM_FEATURE_V7)
> +#define ENABLE_ARCH_8 arm_dc_feature(s, ARM_FEATURE_V8)
>
> #define ARCH(x) do { if (!ENABLE_ARCH_##x) goto illegal_op; } while(0)
>
>
nit: the line for ENABLE_ARCH_6K seems to be differently aligned than the rest.
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
--
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München
office: +49 89 158834 4135
mobile: +49 15253060158
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PATCH 1/5] target-arm/translate.c: Use arm_dc_feature() in ENABLE_ARCH_ macros
2014-11-04 9:04 ` Claudio Fontana
@ 2014-11-04 12:04 ` Peter Maydell
0 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2014-11-04 12:04 UTC (permalink / raw)
To: Claudio Fontana; +Cc: QEMU Developers, Patch Tracking
On 4 November 2014 09:04, Claudio Fontana <claudio.fontana@huawei.com> wrote:
> On 28.10.2014 20:24, Peter Maydell wrote:
> nit: the line for ENABLE_ARCH_6K seems to be differently aligned than the rest.
>
> Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
Thanks; I fixed the missing space.
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 2/5] target-arm/translate.c: Use arm_dc_feature() rather than arm_feature()
2014-10-28 19:23 [Qemu-devel] [PATCH 0/5] target-arm: Avoid passing CPUARMState around the decoder Peter Maydell
2014-10-28 19:24 ` [Qemu-devel] [PATCH 1/5] target-arm/translate.c: Use arm_dc_feature() in ENABLE_ARCH_ macros Peter Maydell
@ 2014-10-28 19:24 ` Peter Maydell
2014-10-31 13:40 ` Alex Bennée
2014-11-04 9:02 ` Claudio Fontana
2014-10-28 19:24 ` [Qemu-devel] [PATCH 3/5] target-arm/translate.c: Don't use IS_M() Peter Maydell
` (2 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Peter Maydell @ 2014-10-28 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: patches
Use arm_dc_feature() rather than arm_feature() to avoid using
CPUARMState unnecessarily.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/translate.c | 140 ++++++++++++++++++++++++++++---------------------
1 file changed, 80 insertions(+), 60 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index f69e5ef..08ce5b0 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -2619,7 +2619,7 @@ static int disas_dsp_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
#define VFP_SREG(insn, bigbit, smallbit) \
((VFP_REG_SHR(insn, bigbit - 1) & 0x1e) | (((insn) >> (smallbit)) & 1))
#define VFP_DREG(reg, insn, bigbit, smallbit) do { \
- if (arm_feature(env, ARM_FEATURE_VFP3)) { \
+ if (arm_dc_feature(s, ARM_FEATURE_VFP3)) { \
reg = (((insn) >> (bigbit)) & 0x0f) \
| (((insn) >> ((smallbit) - 4)) & 0x10); \
} else { \
@@ -2970,7 +2970,7 @@ static int disas_vfp_v8_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
{
uint32_t rd, rn, rm, dp = extract32(insn, 8, 1);
- if (!arm_feature(env, ARM_FEATURE_V8)) {
+ if (!arm_dc_feature(s, ARM_FEATURE_V8)) {
return 1;
}
@@ -3010,8 +3010,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
TCGv_i32 tmp;
TCGv_i32 tmp2;
- if (!arm_feature(env, ARM_FEATURE_VFP))
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP)) {
return 1;
+ }
/* FIXME: this access check should not take precedence over UNDEF
* for invalid encodings; we will generate incorrect syndrome information
@@ -3055,8 +3056,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
if (insn & 0xf)
return 1;
if (insn & 0x00c00060
- && !arm_feature(env, ARM_FEATURE_NEON))
+ && !arm_dc_feature(s, ARM_FEATURE_NEON)) {
return 1;
+ }
pass = (insn >> 21) & 1;
if (insn & (1 << 22)) {
@@ -3151,8 +3153,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
VFP3 restricts all id registers to privileged
accesses. */
if (IS_USER(s)
- && arm_feature(env, ARM_FEATURE_VFP3))
+ && arm_dc_feature(s, ARM_FEATURE_VFP3)) {
return 1;
+ }
tmp = load_cpu_field(vfp.xregs[rn]);
break;
case ARM_VFP_FPEXC:
@@ -3164,8 +3167,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
case ARM_VFP_FPINST2:
/* Not present in VFP3. */
if (IS_USER(s)
- || arm_feature(env, ARM_FEATURE_VFP3))
+ || arm_dc_feature(s, ARM_FEATURE_VFP3)) {
return 1;
+ }
tmp = load_cpu_field(vfp.xregs[rn]);
break;
case ARM_VFP_FPSCR:
@@ -3178,15 +3182,16 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
}
break;
case ARM_VFP_MVFR2:
- if (!arm_feature(env, ARM_FEATURE_V8)) {
+ if (!arm_dc_feature(s, ARM_FEATURE_V8)) {
return 1;
}
/* fall through */
case ARM_VFP_MVFR0:
case ARM_VFP_MVFR1:
if (IS_USER(s)
- || !arm_feature(env, ARM_FEATURE_MVFR))
+ || !arm_dc_feature(s, ARM_FEATURE_MVFR)) {
return 1;
+ }
tmp = load_cpu_field(vfp.xregs[rn]);
break;
default:
@@ -3367,8 +3372,8 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
* UNPREDICTABLE if bit 8 is set prior to ARMv8
* (we choose to UNDEF)
*/
- if ((dp && !arm_feature(env, ARM_FEATURE_V8)) ||
- !arm_feature(env, ARM_FEATURE_VFP_FP16)) {
+ if ((dp && !arm_dc_feature(s, ARM_FEATURE_V8)) ||
+ !arm_dc_feature(s, ARM_FEATURE_VFP_FP16)) {
return 1;
}
if (!extract32(rn, 1, 1)) {
@@ -3447,7 +3452,7 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
* correct : an input NaN should come out with its sign bit
* flipped if it is a negated-input.
*/
- if (!arm_feature(env, ARM_FEATURE_VFP4)) {
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP4)) {
return 1;
}
if (dp) {
@@ -3488,8 +3493,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
}
break;
case 14: /* fconst */
- if (!arm_feature(env, ARM_FEATURE_VFP3))
- return 1;
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
+ return 1;
+ }
n = (insn << 12) & 0x80000000;
i = ((insn >> 12) & 0x70) | (insn & 0xf);
@@ -3644,23 +3650,27 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
gen_vfp_sito(dp, 0);
break;
case 20: /* fshto */
- if (!arm_feature(env, ARM_FEATURE_VFP3))
- return 1;
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
+ return 1;
+ }
gen_vfp_shto(dp, 16 - rm, 0);
break;
case 21: /* fslto */
- if (!arm_feature(env, ARM_FEATURE_VFP3))
- return 1;
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
+ return 1;
+ }
gen_vfp_slto(dp, 32 - rm, 0);
break;
case 22: /* fuhto */
- if (!arm_feature(env, ARM_FEATURE_VFP3))
- return 1;
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
+ return 1;
+ }
gen_vfp_uhto(dp, 16 - rm, 0);
break;
case 23: /* fulto */
- if (!arm_feature(env, ARM_FEATURE_VFP3))
- return 1;
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
+ return 1;
+ }
gen_vfp_ulto(dp, 32 - rm, 0);
break;
case 24: /* ftoui */
@@ -3676,23 +3686,27 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
gen_vfp_tosiz(dp, 0);
break;
case 28: /* ftosh */
- if (!arm_feature(env, ARM_FEATURE_VFP3))
- return 1;
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
+ return 1;
+ }
gen_vfp_tosh(dp, 16 - rm, 0);
break;
case 29: /* ftosl */
- if (!arm_feature(env, ARM_FEATURE_VFP3))
- return 1;
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
+ return 1;
+ }
gen_vfp_tosl(dp, 32 - rm, 0);
break;
case 30: /* ftouh */
- if (!arm_feature(env, ARM_FEATURE_VFP3))
- return 1;
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
+ return 1;
+ }
gen_vfp_touh(dp, 16 - rm, 0);
break;
case 31: /* ftoul */
- if (!arm_feature(env, ARM_FEATURE_VFP3))
- return 1;
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
+ return 1;
+ }
gen_vfp_toul(dp, 32 - rm, 0);
break;
default: /* undefined */
@@ -3963,14 +3977,18 @@ static uint32_t msr_mask(CPUARMState *env, DisasContext *s, int flags, int spsr)
/* Mask out undefined bits. */
mask &= ~CPSR_RESERVED;
- if (!arm_feature(env, ARM_FEATURE_V4T))
+ if (!arm_dc_feature(s, ARM_FEATURE_V4T)) {
mask &= ~CPSR_T;
- if (!arm_feature(env, ARM_FEATURE_V5))
+ }
+ if (!arm_dc_feature(s, ARM_FEATURE_V5)) {
mask &= ~CPSR_Q; /* V5TE in reality*/
- if (!arm_feature(env, ARM_FEATURE_V6))
+ }
+ if (!arm_dc_feature(s, ARM_FEATURE_V6)) {
mask &= ~(CPSR_E | CPSR_GE);
- if (!arm_feature(env, ARM_FEATURE_THUMB2))
+ }
+ if (!arm_dc_feature(s, ARM_FEATURE_THUMB2)) {
mask &= ~CPSR_IT;
+ }
/* Mask out execution state and reserved bits. */
if (!spsr) {
mask &= ~(CPSR_EXEC | CPSR_RESERVED);
@@ -5092,7 +5110,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
return 1;
}
if (!u) { /* SHA-1 */
- if (!arm_feature(env, ARM_FEATURE_V8_SHA1)) {
+ if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA1)) {
return 1;
}
tmp = tcg_const_i32(rd);
@@ -5102,7 +5120,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
gen_helper_crypto_sha1_3reg(cpu_env, tmp, tmp2, tmp3, tmp4);
tcg_temp_free_i32(tmp4);
} else { /* SHA-256 */
- if (!arm_feature(env, ARM_FEATURE_V8_SHA256) || size == 3) {
+ if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA256) || size == 3) {
return 1;
}
tmp = tcg_const_i32(rd);
@@ -5237,7 +5255,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
break;
case NEON_3R_FLOAT_MISC:
/* VMAXNM/VMINNM in ARMv8 */
- if (u && !arm_feature(env, ARM_FEATURE_V8)) {
+ if (u && !arm_dc_feature(s, ARM_FEATURE_V8)) {
return 1;
}
break;
@@ -5248,7 +5266,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
}
break;
case NEON_3R_VFM:
- if (!arm_feature(env, ARM_FEATURE_VFP4) || u) {
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP4) || u) {
return 1;
}
break;
@@ -6067,7 +6085,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
if (op == 14 && size == 2) {
TCGv_i64 tcg_rn, tcg_rm, tcg_rd;
- if (!arm_feature(env, ARM_FEATURE_V8_PMULL)) {
+ if (!arm_dc_feature(s, ARM_FEATURE_V8_PMULL)) {
return 1;
}
tcg_rn = tcg_temp_new_i64();
@@ -6555,7 +6573,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
}
break;
case NEON_2RM_VCVT_F16_F32:
- if (!arm_feature(env, ARM_FEATURE_VFP_FP16) ||
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP_FP16) ||
q || (rm & 1)) {
return 1;
}
@@ -6579,7 +6597,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
tcg_temp_free_i32(tmp);
break;
case NEON_2RM_VCVT_F32_F16:
- if (!arm_feature(env, ARM_FEATURE_VFP_FP16) ||
+ if (!arm_dc_feature(s, ARM_FEATURE_VFP_FP16) ||
q || (rd & 1)) {
return 1;
}
@@ -6603,7 +6621,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
tcg_temp_free_i32(tmp3);
break;
case NEON_2RM_AESE: case NEON_2RM_AESMC:
- if (!arm_feature(env, ARM_FEATURE_V8_AES)
+ if (!arm_dc_feature(s, ARM_FEATURE_V8_AES)
|| ((rm | rd) & 1)) {
return 1;
}
@@ -6625,7 +6643,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
tcg_temp_free_i32(tmp3);
break;
case NEON_2RM_SHA1H:
- if (!arm_feature(env, ARM_FEATURE_V8_SHA1)
+ if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA1)
|| ((rm | rd) & 1)) {
return 1;
}
@@ -6643,10 +6661,10 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
}
/* bit 6 (q): set -> SHA256SU0, cleared -> SHA1SU1 */
if (q) {
- if (!arm_feature(env, ARM_FEATURE_V8_SHA256)) {
+ if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA256)) {
return 1;
}
- } else if (!arm_feature(env, ARM_FEATURE_V8_SHA1)) {
+ } else if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA1)) {
return 1;
}
tmp = tcg_const_i32(rd);
@@ -7039,13 +7057,13 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
cpnum = (insn >> 8) & 0xf;
/* First check for coprocessor space used for XScale/iwMMXt insns */
- if (arm_feature(env, ARM_FEATURE_XSCALE) && (cpnum < 2)) {
+ if (arm_dc_feature(s, ARM_FEATURE_XSCALE) && (cpnum < 2)) {
if (extract32(s->c15_cpar, cpnum, 1) == 0) {
return 1;
}
- if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
+ if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) {
return disas_iwmmxt_insn(env, s, insn);
- } else if (arm_feature(env, ARM_FEATURE_XSCALE)) {
+ } else if (arm_dc_feature(s, ARM_FEATURE_XSCALE)) {
return disas_dsp_insn(env, s, insn);
}
return 1;
@@ -7082,7 +7100,7 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
}
if (ri->accessfn ||
- (arm_feature(env, ARM_FEATURE_XSCALE) && cpnum < 14)) {
+ (arm_dc_feature(s, ARM_FEATURE_XSCALE) && cpnum < 14)) {
/* Emit code to perform further access permissions checks at
* runtime; this may result in an exception.
* Note that on XScale all cp0..c13 registers do an access check
@@ -7125,7 +7143,7 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
* in which case the syndrome information won't actually be
* guest visible.
*/
- assert(!arm_feature(env, ARM_FEATURE_V8));
+ assert(!arm_dc_feature(s, ARM_FEATURE_V8));
syndrome = syn_uncategorized();
break;
}
@@ -7569,8 +7587,9 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
/* Unconditional instructions. */
if (((insn >> 25) & 7) == 1) {
/* NEON Data processing. */
- if (!arm_feature(env, ARM_FEATURE_NEON))
+ if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
goto illegal_op;
+ }
if (disas_neon_data_insn(env, s, insn))
goto illegal_op;
@@ -7578,8 +7597,9 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
}
if ((insn & 0x0f100000) == 0x04000000) {
/* NEON load/store. */
- if (!arm_feature(env, ARM_FEATURE_NEON))
+ if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
goto illegal_op;
+ }
if (disas_neon_ls_insn(env, s, insn))
goto illegal_op;
@@ -7596,7 +7616,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
((insn & 0x0f30f010) == 0x0710f000)) {
if ((insn & (1 << 22)) == 0) {
/* PLDW; v7MP */
- if (!arm_feature(env, ARM_FEATURE_V7MP)) {
+ if (!arm_dc_feature(s, ARM_FEATURE_V7MP)) {
goto illegal_op;
}
}
@@ -7611,7 +7631,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
}
if (((insn & 0x0f700000) == 0x04100000) ||
((insn & 0x0f700010) == 0x06100000)) {
- if (!arm_feature(env, ARM_FEATURE_V7MP)) {
+ if (!arm_dc_feature(s, ARM_FEATURE_V7MP)) {
goto illegal_op;
}
return; /* v7MP: Unallocated memory hint: must NOP */
@@ -7708,7 +7728,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
gen_bx_im(s, val);
return;
} else if ((insn & 0x0e000f00) == 0x0c000100) {
- if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
+ if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) {
/* iWMMXt register transfer. */
if (extract32(s->c15_cpar, 1, 1)) {
if (!disas_iwmmxt_insn(env, s, insn)) {
@@ -7864,7 +7884,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
* op1 == 3 is UNPREDICTABLE but handle as UNDEFINED.
* Bits 8, 10 and 11 should be zero.
*/
- if (!arm_feature(env, ARM_FEATURE_CRC) || op1 == 0x3 ||
+ if (!arm_dc_feature(s, ARM_FEATURE_CRC) || op1 == 0x3 ||
(c & 0xd) != 0) {
goto illegal_op;
}
@@ -8673,7 +8693,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
case 1:
case 3:
/* SDIV, UDIV */
- if (!arm_feature(env, ARM_FEATURE_ARM_DIV)) {
+ if (!arm_dc_feature(s, ARM_FEATURE_ARM_DIV)) {
goto illegal_op;
}
if (((insn >> 5) & 7) || (rd != 15)) {
@@ -9069,8 +9089,8 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
int conds;
int logic_cc;
- if (!(arm_feature(env, ARM_FEATURE_THUMB2)
- || arm_feature (env, ARM_FEATURE_M))) {
+ if (!(arm_dc_feature(s, ARM_FEATURE_THUMB2)
+ || arm_dc_feature(s, ARM_FEATURE_M))) {
/* Thumb-1 cores may need to treat bl and blx as a pair of
16-bit instructions to get correct prefetch abort behavior. */
insn = insn_hw1;
@@ -9523,7 +9543,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
uint32_t sz = op & 0x3;
uint32_t c = op & 0x8;
- if (!arm_feature(env, ARM_FEATURE_CRC)) {
+ if (!arm_dc_feature(s, ARM_FEATURE_CRC)) {
goto illegal_op;
}
@@ -9651,7 +9671,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
tmp2 = load_reg(s, rm);
if ((op & 0x50) == 0x10) {
/* sdiv, udiv */
- if (!arm_feature(env, ARM_FEATURE_THUMB_DIV)) {
+ if (!arm_dc_feature(s, ARM_FEATURE_THUMB_DIV)) {
goto illegal_op;
}
if (op & 0x20)
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PATCH 2/5] target-arm/translate.c: Use arm_dc_feature() rather than arm_feature()
2014-10-28 19:24 ` [Qemu-devel] [PATCH 2/5] target-arm/translate.c: Use arm_dc_feature() rather than arm_feature() Peter Maydell
@ 2014-10-31 13:40 ` Alex Bennée
2014-11-04 9:02 ` Claudio Fontana
1 sibling, 0 replies; 19+ messages in thread
From: Alex Bennée @ 2014-10-31 13:40 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> Use arm_dc_feature() rather than arm_feature() to avoid using
> CPUARMState unnecessarily.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> target-arm/translate.c | 140 ++++++++++++++++++++++++++++---------------------
> 1 file changed, 80 insertions(+), 60 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index f69e5ef..08ce5b0 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -2619,7 +2619,7 @@ static int disas_dsp_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
> #define VFP_SREG(insn, bigbit, smallbit) \
> ((VFP_REG_SHR(insn, bigbit - 1) & 0x1e) | (((insn) >> (smallbit)) & 1))
> #define VFP_DREG(reg, insn, bigbit, smallbit) do { \
> - if (arm_feature(env, ARM_FEATURE_VFP3)) { \
> + if (arm_dc_feature(s, ARM_FEATURE_VFP3)) { \
> reg = (((insn) >> (bigbit)) & 0x0f) \
> | (((insn) >> ((smallbit) - 4)) & 0x10); \
> } else { \
> @@ -2970,7 +2970,7 @@ static int disas_vfp_v8_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
> {
> uint32_t rd, rn, rm, dp = extract32(insn, 8, 1);
>
> - if (!arm_feature(env, ARM_FEATURE_V8)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V8)) {
> return 1;
> }
>
> @@ -3010,8 +3010,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> TCGv_i32 tmp;
> TCGv_i32 tmp2;
>
> - if (!arm_feature(env, ARM_FEATURE_VFP))
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP)) {
> return 1;
> + }
>
> /* FIXME: this access check should not take precedence over UNDEF
> * for invalid encodings; we will generate incorrect syndrome information
> @@ -3055,8 +3056,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> if (insn & 0xf)
> return 1;
> if (insn & 0x00c00060
> - && !arm_feature(env, ARM_FEATURE_NEON))
> + && !arm_dc_feature(s, ARM_FEATURE_NEON)) {
> return 1;
> + }
>
> pass = (insn >> 21) & 1;
> if (insn & (1 << 22)) {
> @@ -3151,8 +3153,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> VFP3 restricts all id registers to privileged
> accesses. */
> if (IS_USER(s)
> - && arm_feature(env, ARM_FEATURE_VFP3))
> + && arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> return 1;
> + }
> tmp = load_cpu_field(vfp.xregs[rn]);
> break;
> case ARM_VFP_FPEXC:
> @@ -3164,8 +3167,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> case ARM_VFP_FPINST2:
> /* Not present in VFP3. */
> if (IS_USER(s)
> - || arm_feature(env, ARM_FEATURE_VFP3))
> + || arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> return 1;
> + }
> tmp = load_cpu_field(vfp.xregs[rn]);
> break;
> case ARM_VFP_FPSCR:
> @@ -3178,15 +3182,16 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> }
> break;
> case ARM_VFP_MVFR2:
> - if (!arm_feature(env, ARM_FEATURE_V8)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V8)) {
> return 1;
> }
> /* fall through */
> case ARM_VFP_MVFR0:
> case ARM_VFP_MVFR1:
> if (IS_USER(s)
> - || !arm_feature(env, ARM_FEATURE_MVFR))
> + || !arm_dc_feature(s, ARM_FEATURE_MVFR)) {
> return 1;
> + }
> tmp = load_cpu_field(vfp.xregs[rn]);
> break;
> default:
> @@ -3367,8 +3372,8 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> * UNPREDICTABLE if bit 8 is set prior to ARMv8
> * (we choose to UNDEF)
> */
> - if ((dp && !arm_feature(env, ARM_FEATURE_V8)) ||
> - !arm_feature(env, ARM_FEATURE_VFP_FP16)) {
> + if ((dp && !arm_dc_feature(s, ARM_FEATURE_V8)) ||
> + !arm_dc_feature(s, ARM_FEATURE_VFP_FP16)) {
> return 1;
> }
> if (!extract32(rn, 1, 1)) {
> @@ -3447,7 +3452,7 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> * correct : an input NaN should come out with its sign bit
> * flipped if it is a negated-input.
> */
> - if (!arm_feature(env, ARM_FEATURE_VFP4)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP4)) {
> return 1;
> }
> if (dp) {
> @@ -3488,8 +3493,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> }
> break;
> case 14: /* fconst */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
>
> n = (insn << 12) & 0x80000000;
> i = ((insn >> 12) & 0x70) | (insn & 0xf);
> @@ -3644,23 +3650,27 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> gen_vfp_sito(dp, 0);
> break;
> case 20: /* fshto */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_shto(dp, 16 - rm, 0);
> break;
> case 21: /* fslto */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_slto(dp, 32 - rm, 0);
> break;
> case 22: /* fuhto */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_uhto(dp, 16 - rm, 0);
> break;
> case 23: /* fulto */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_ulto(dp, 32 - rm, 0);
> break;
> case 24: /* ftoui */
> @@ -3676,23 +3686,27 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> gen_vfp_tosiz(dp, 0);
> break;
> case 28: /* ftosh */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_tosh(dp, 16 - rm, 0);
> break;
> case 29: /* ftosl */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_tosl(dp, 32 - rm, 0);
> break;
> case 30: /* ftouh */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_touh(dp, 16 - rm, 0);
> break;
> case 31: /* ftoul */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_toul(dp, 32 - rm, 0);
> break;
> default: /* undefined */
> @@ -3963,14 +3977,18 @@ static uint32_t msr_mask(CPUARMState *env, DisasContext *s, int flags, int spsr)
>
> /* Mask out undefined bits. */
> mask &= ~CPSR_RESERVED;
> - if (!arm_feature(env, ARM_FEATURE_V4T))
> + if (!arm_dc_feature(s, ARM_FEATURE_V4T)) {
> mask &= ~CPSR_T;
> - if (!arm_feature(env, ARM_FEATURE_V5))
> + }
> + if (!arm_dc_feature(s, ARM_FEATURE_V5)) {
> mask &= ~CPSR_Q; /* V5TE in reality*/
> - if (!arm_feature(env, ARM_FEATURE_V6))
> + }
> + if (!arm_dc_feature(s, ARM_FEATURE_V6)) {
> mask &= ~(CPSR_E | CPSR_GE);
> - if (!arm_feature(env, ARM_FEATURE_THUMB2))
> + }
> + if (!arm_dc_feature(s, ARM_FEATURE_THUMB2)) {
> mask &= ~CPSR_IT;
> + }
> /* Mask out execution state and reserved bits. */
> if (!spsr) {
> mask &= ~(CPSR_EXEC | CPSR_RESERVED);
> @@ -5092,7 +5110,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> return 1;
> }
> if (!u) { /* SHA-1 */
> - if (!arm_feature(env, ARM_FEATURE_V8_SHA1)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA1)) {
> return 1;
> }
> tmp = tcg_const_i32(rd);
> @@ -5102,7 +5120,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> gen_helper_crypto_sha1_3reg(cpu_env, tmp, tmp2, tmp3, tmp4);
> tcg_temp_free_i32(tmp4);
> } else { /* SHA-256 */
> - if (!arm_feature(env, ARM_FEATURE_V8_SHA256) || size == 3) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA256) || size == 3) {
> return 1;
> }
> tmp = tcg_const_i32(rd);
> @@ -5237,7 +5255,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> break;
> case NEON_3R_FLOAT_MISC:
> /* VMAXNM/VMINNM in ARMv8 */
> - if (u && !arm_feature(env, ARM_FEATURE_V8)) {
> + if (u && !arm_dc_feature(s, ARM_FEATURE_V8)) {
> return 1;
> }
> break;
> @@ -5248,7 +5266,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> }
> break;
> case NEON_3R_VFM:
> - if (!arm_feature(env, ARM_FEATURE_VFP4) || u) {
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP4) || u) {
> return 1;
> }
> break;
> @@ -6067,7 +6085,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> if (op == 14 && size == 2) {
> TCGv_i64 tcg_rn, tcg_rm, tcg_rd;
>
> - if (!arm_feature(env, ARM_FEATURE_V8_PMULL)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V8_PMULL)) {
> return 1;
> }
> tcg_rn = tcg_temp_new_i64();
> @@ -6555,7 +6573,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> }
> break;
> case NEON_2RM_VCVT_F16_F32:
> - if (!arm_feature(env, ARM_FEATURE_VFP_FP16) ||
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP_FP16) ||
> q || (rm & 1)) {
> return 1;
> }
> @@ -6579,7 +6597,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> tcg_temp_free_i32(tmp);
> break;
> case NEON_2RM_VCVT_F32_F16:
> - if (!arm_feature(env, ARM_FEATURE_VFP_FP16) ||
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP_FP16) ||
> q || (rd & 1)) {
> return 1;
> }
> @@ -6603,7 +6621,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> tcg_temp_free_i32(tmp3);
> break;
> case NEON_2RM_AESE: case NEON_2RM_AESMC:
> - if (!arm_feature(env, ARM_FEATURE_V8_AES)
> + if (!arm_dc_feature(s, ARM_FEATURE_V8_AES)
> || ((rm | rd) & 1)) {
> return 1;
> }
> @@ -6625,7 +6643,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> tcg_temp_free_i32(tmp3);
> break;
> case NEON_2RM_SHA1H:
> - if (!arm_feature(env, ARM_FEATURE_V8_SHA1)
> + if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA1)
> || ((rm | rd) & 1)) {
> return 1;
> }
> @@ -6643,10 +6661,10 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> }
> /* bit 6 (q): set -> SHA256SU0, cleared -> SHA1SU1 */
> if (q) {
> - if (!arm_feature(env, ARM_FEATURE_V8_SHA256)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA256)) {
> return 1;
> }
> - } else if (!arm_feature(env, ARM_FEATURE_V8_SHA1)) {
> + } else if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA1)) {
> return 1;
> }
> tmp = tcg_const_i32(rd);
> @@ -7039,13 +7057,13 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> cpnum = (insn >> 8) & 0xf;
>
> /* First check for coprocessor space used for XScale/iwMMXt insns */
> - if (arm_feature(env, ARM_FEATURE_XSCALE) && (cpnum < 2)) {
> + if (arm_dc_feature(s, ARM_FEATURE_XSCALE) && (cpnum < 2)) {
> if (extract32(s->c15_cpar, cpnum, 1) == 0) {
> return 1;
> }
> - if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
> + if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) {
> return disas_iwmmxt_insn(env, s, insn);
> - } else if (arm_feature(env, ARM_FEATURE_XSCALE)) {
> + } else if (arm_dc_feature(s, ARM_FEATURE_XSCALE)) {
> return disas_dsp_insn(env, s, insn);
> }
> return 1;
> @@ -7082,7 +7100,7 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> }
>
> if (ri->accessfn ||
> - (arm_feature(env, ARM_FEATURE_XSCALE) && cpnum < 14)) {
> + (arm_dc_feature(s, ARM_FEATURE_XSCALE) && cpnum < 14)) {
> /* Emit code to perform further access permissions checks at
> * runtime; this may result in an exception.
> * Note that on XScale all cp0..c13 registers do an access check
> @@ -7125,7 +7143,7 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> * in which case the syndrome information won't actually be
> * guest visible.
> */
> - assert(!arm_feature(env, ARM_FEATURE_V8));
> + assert(!arm_dc_feature(s, ARM_FEATURE_V8));
> syndrome = syn_uncategorized();
> break;
> }
> @@ -7569,8 +7587,9 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> /* Unconditional instructions. */
> if (((insn >> 25) & 7) == 1) {
> /* NEON Data processing. */
> - if (!arm_feature(env, ARM_FEATURE_NEON))
> + if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
> goto illegal_op;
> + }
>
> if (disas_neon_data_insn(env, s, insn))
> goto illegal_op;
> @@ -7578,8 +7597,9 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> }
> if ((insn & 0x0f100000) == 0x04000000) {
> /* NEON load/store. */
> - if (!arm_feature(env, ARM_FEATURE_NEON))
> + if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
> goto illegal_op;
> + }
>
> if (disas_neon_ls_insn(env, s, insn))
> goto illegal_op;
> @@ -7596,7 +7616,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> ((insn & 0x0f30f010) == 0x0710f000)) {
> if ((insn & (1 << 22)) == 0) {
> /* PLDW; v7MP */
> - if (!arm_feature(env, ARM_FEATURE_V7MP)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V7MP)) {
> goto illegal_op;
> }
> }
> @@ -7611,7 +7631,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> }
> if (((insn & 0x0f700000) == 0x04100000) ||
> ((insn & 0x0f700010) == 0x06100000)) {
> - if (!arm_feature(env, ARM_FEATURE_V7MP)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V7MP)) {
> goto illegal_op;
> }
> return; /* v7MP: Unallocated memory hint: must NOP */
> @@ -7708,7 +7728,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> gen_bx_im(s, val);
> return;
> } else if ((insn & 0x0e000f00) == 0x0c000100) {
> - if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
> + if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) {
> /* iWMMXt register transfer. */
> if (extract32(s->c15_cpar, 1, 1)) {
> if (!disas_iwmmxt_insn(env, s, insn)) {
> @@ -7864,7 +7884,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> * op1 == 3 is UNPREDICTABLE but handle as UNDEFINED.
> * Bits 8, 10 and 11 should be zero.
> */
> - if (!arm_feature(env, ARM_FEATURE_CRC) || op1 == 0x3 ||
> + if (!arm_dc_feature(s, ARM_FEATURE_CRC) || op1 == 0x3 ||
> (c & 0xd) != 0) {
> goto illegal_op;
> }
> @@ -8673,7 +8693,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> case 1:
> case 3:
> /* SDIV, UDIV */
> - if (!arm_feature(env, ARM_FEATURE_ARM_DIV)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_ARM_DIV)) {
> goto illegal_op;
> }
> if (((insn >> 5) & 7) || (rd != 15)) {
> @@ -9069,8 +9089,8 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> int conds;
> int logic_cc;
>
> - if (!(arm_feature(env, ARM_FEATURE_THUMB2)
> - || arm_feature (env, ARM_FEATURE_M))) {
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB2)
> + || arm_dc_feature(s, ARM_FEATURE_M))) {
> /* Thumb-1 cores may need to treat bl and blx as a pair of
> 16-bit instructions to get correct prefetch abort behavior. */
> insn = insn_hw1;
> @@ -9523,7 +9543,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> uint32_t sz = op & 0x3;
> uint32_t c = op & 0x8;
>
> - if (!arm_feature(env, ARM_FEATURE_CRC)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_CRC)) {
> goto illegal_op;
> }
>
> @@ -9651,7 +9671,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> tmp2 = load_reg(s, rm);
> if ((op & 0x50) == 0x10) {
> /* sdiv, udiv */
> - if (!arm_feature(env, ARM_FEATURE_THUMB_DIV)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_THUMB_DIV)) {
> goto illegal_op;
> }
> if (op & 0x20)
--
Alex Bennée
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PATCH 2/5] target-arm/translate.c: Use arm_dc_feature() rather than arm_feature()
2014-10-28 19:24 ` [Qemu-devel] [PATCH 2/5] target-arm/translate.c: Use arm_dc_feature() rather than arm_feature() Peter Maydell
2014-10-31 13:40 ` Alex Bennée
@ 2014-11-04 9:02 ` Claudio Fontana
1 sibling, 0 replies; 19+ messages in thread
From: Claudio Fontana @ 2014-11-04 9:02 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
On 28.10.2014 20:24, Peter Maydell wrote:
> Use arm_dc_feature() rather than arm_feature() to avoid using
> CPUARMState unnecessarily.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target-arm/translate.c | 140 ++++++++++++++++++++++++++++---------------------
> 1 file changed, 80 insertions(+), 60 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index f69e5ef..08ce5b0 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -2619,7 +2619,7 @@ static int disas_dsp_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
> #define VFP_SREG(insn, bigbit, smallbit) \
> ((VFP_REG_SHR(insn, bigbit - 1) & 0x1e) | (((insn) >> (smallbit)) & 1))
> #define VFP_DREG(reg, insn, bigbit, smallbit) do { \
> - if (arm_feature(env, ARM_FEATURE_VFP3)) { \
> + if (arm_dc_feature(s, ARM_FEATURE_VFP3)) { \
> reg = (((insn) >> (bigbit)) & 0x0f) \
> | (((insn) >> ((smallbit) - 4)) & 0x10); \
> } else { \
> @@ -2970,7 +2970,7 @@ static int disas_vfp_v8_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
> {
> uint32_t rd, rn, rm, dp = extract32(insn, 8, 1);
>
> - if (!arm_feature(env, ARM_FEATURE_V8)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V8)) {
> return 1;
> }
>
> @@ -3010,8 +3010,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> TCGv_i32 tmp;
> TCGv_i32 tmp2;
>
> - if (!arm_feature(env, ARM_FEATURE_VFP))
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP)) {
> return 1;
> + }
>
> /* FIXME: this access check should not take precedence over UNDEF
> * for invalid encodings; we will generate incorrect syndrome information
> @@ -3055,8 +3056,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> if (insn & 0xf)
> return 1;
> if (insn & 0x00c00060
> - && !arm_feature(env, ARM_FEATURE_NEON))
> + && !arm_dc_feature(s, ARM_FEATURE_NEON)) {
> return 1;
> + }
>
> pass = (insn >> 21) & 1;
> if (insn & (1 << 22)) {
> @@ -3151,8 +3153,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> VFP3 restricts all id registers to privileged
> accesses. */
> if (IS_USER(s)
> - && arm_feature(env, ARM_FEATURE_VFP3))
> + && arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> return 1;
> + }
> tmp = load_cpu_field(vfp.xregs[rn]);
> break;
> case ARM_VFP_FPEXC:
> @@ -3164,8 +3167,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> case ARM_VFP_FPINST2:
> /* Not present in VFP3. */
> if (IS_USER(s)
> - || arm_feature(env, ARM_FEATURE_VFP3))
> + || arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> return 1;
> + }
> tmp = load_cpu_field(vfp.xregs[rn]);
> break;
> case ARM_VFP_FPSCR:
> @@ -3178,15 +3182,16 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> }
> break;
> case ARM_VFP_MVFR2:
> - if (!arm_feature(env, ARM_FEATURE_V8)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V8)) {
> return 1;
> }
> /* fall through */
> case ARM_VFP_MVFR0:
> case ARM_VFP_MVFR1:
> if (IS_USER(s)
> - || !arm_feature(env, ARM_FEATURE_MVFR))
> + || !arm_dc_feature(s, ARM_FEATURE_MVFR)) {
> return 1;
> + }
> tmp = load_cpu_field(vfp.xregs[rn]);
> break;
> default:
> @@ -3367,8 +3372,8 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> * UNPREDICTABLE if bit 8 is set prior to ARMv8
> * (we choose to UNDEF)
> */
> - if ((dp && !arm_feature(env, ARM_FEATURE_V8)) ||
> - !arm_feature(env, ARM_FEATURE_VFP_FP16)) {
> + if ((dp && !arm_dc_feature(s, ARM_FEATURE_V8)) ||
> + !arm_dc_feature(s, ARM_FEATURE_VFP_FP16)) {
> return 1;
> }
> if (!extract32(rn, 1, 1)) {
> @@ -3447,7 +3452,7 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> * correct : an input NaN should come out with its sign bit
> * flipped if it is a negated-input.
> */
> - if (!arm_feature(env, ARM_FEATURE_VFP4)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP4)) {
> return 1;
> }
> if (dp) {
> @@ -3488,8 +3493,9 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> }
> break;
> case 14: /* fconst */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
>
> n = (insn << 12) & 0x80000000;
> i = ((insn >> 12) & 0x70) | (insn & 0xf);
> @@ -3644,23 +3650,27 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> gen_vfp_sito(dp, 0);
> break;
> case 20: /* fshto */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_shto(dp, 16 - rm, 0);
> break;
> case 21: /* fslto */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_slto(dp, 32 - rm, 0);
> break;
> case 22: /* fuhto */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_uhto(dp, 16 - rm, 0);
> break;
> case 23: /* fulto */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_ulto(dp, 32 - rm, 0);
> break;
> case 24: /* ftoui */
> @@ -3676,23 +3686,27 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> gen_vfp_tosiz(dp, 0);
> break;
> case 28: /* ftosh */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_tosh(dp, 16 - rm, 0);
> break;
> case 29: /* ftosl */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_tosl(dp, 32 - rm, 0);
> break;
> case 30: /* ftouh */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_touh(dp, 16 - rm, 0);
> break;
> case 31: /* ftoul */
> - if (!arm_feature(env, ARM_FEATURE_VFP3))
> - return 1;
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP3)) {
> + return 1;
> + }
> gen_vfp_toul(dp, 32 - rm, 0);
> break;
> default: /* undefined */
> @@ -3963,14 +3977,18 @@ static uint32_t msr_mask(CPUARMState *env, DisasContext *s, int flags, int spsr)
>
> /* Mask out undefined bits. */
> mask &= ~CPSR_RESERVED;
> - if (!arm_feature(env, ARM_FEATURE_V4T))
> + if (!arm_dc_feature(s, ARM_FEATURE_V4T)) {
> mask &= ~CPSR_T;
> - if (!arm_feature(env, ARM_FEATURE_V5))
> + }
> + if (!arm_dc_feature(s, ARM_FEATURE_V5)) {
> mask &= ~CPSR_Q; /* V5TE in reality*/
> - if (!arm_feature(env, ARM_FEATURE_V6))
> + }
> + if (!arm_dc_feature(s, ARM_FEATURE_V6)) {
> mask &= ~(CPSR_E | CPSR_GE);
> - if (!arm_feature(env, ARM_FEATURE_THUMB2))
> + }
> + if (!arm_dc_feature(s, ARM_FEATURE_THUMB2)) {
> mask &= ~CPSR_IT;
> + }
> /* Mask out execution state and reserved bits. */
> if (!spsr) {
> mask &= ~(CPSR_EXEC | CPSR_RESERVED);
> @@ -5092,7 +5110,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> return 1;
> }
> if (!u) { /* SHA-1 */
> - if (!arm_feature(env, ARM_FEATURE_V8_SHA1)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA1)) {
> return 1;
> }
> tmp = tcg_const_i32(rd);
> @@ -5102,7 +5120,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> gen_helper_crypto_sha1_3reg(cpu_env, tmp, tmp2, tmp3, tmp4);
> tcg_temp_free_i32(tmp4);
> } else { /* SHA-256 */
> - if (!arm_feature(env, ARM_FEATURE_V8_SHA256) || size == 3) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA256) || size == 3) {
> return 1;
> }
> tmp = tcg_const_i32(rd);
> @@ -5237,7 +5255,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> break;
> case NEON_3R_FLOAT_MISC:
> /* VMAXNM/VMINNM in ARMv8 */
> - if (u && !arm_feature(env, ARM_FEATURE_V8)) {
> + if (u && !arm_dc_feature(s, ARM_FEATURE_V8)) {
> return 1;
> }
> break;
> @@ -5248,7 +5266,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> }
> break;
> case NEON_3R_VFM:
> - if (!arm_feature(env, ARM_FEATURE_VFP4) || u) {
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP4) || u) {
> return 1;
> }
> break;
> @@ -6067,7 +6085,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> if (op == 14 && size == 2) {
> TCGv_i64 tcg_rn, tcg_rm, tcg_rd;
>
> - if (!arm_feature(env, ARM_FEATURE_V8_PMULL)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V8_PMULL)) {
> return 1;
> }
> tcg_rn = tcg_temp_new_i64();
> @@ -6555,7 +6573,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> }
> break;
> case NEON_2RM_VCVT_F16_F32:
> - if (!arm_feature(env, ARM_FEATURE_VFP_FP16) ||
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP_FP16) ||
> q || (rm & 1)) {
> return 1;
> }
> @@ -6579,7 +6597,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> tcg_temp_free_i32(tmp);
> break;
> case NEON_2RM_VCVT_F32_F16:
> - if (!arm_feature(env, ARM_FEATURE_VFP_FP16) ||
> + if (!arm_dc_feature(s, ARM_FEATURE_VFP_FP16) ||
> q || (rd & 1)) {
> return 1;
> }
> @@ -6603,7 +6621,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> tcg_temp_free_i32(tmp3);
> break;
> case NEON_2RM_AESE: case NEON_2RM_AESMC:
> - if (!arm_feature(env, ARM_FEATURE_V8_AES)
> + if (!arm_dc_feature(s, ARM_FEATURE_V8_AES)
> || ((rm | rd) & 1)) {
> return 1;
> }
> @@ -6625,7 +6643,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> tcg_temp_free_i32(tmp3);
> break;
> case NEON_2RM_SHA1H:
> - if (!arm_feature(env, ARM_FEATURE_V8_SHA1)
> + if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA1)
> || ((rm | rd) & 1)) {
> return 1;
> }
> @@ -6643,10 +6661,10 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> }
> /* bit 6 (q): set -> SHA256SU0, cleared -> SHA1SU1 */
> if (q) {
> - if (!arm_feature(env, ARM_FEATURE_V8_SHA256)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA256)) {
> return 1;
> }
> - } else if (!arm_feature(env, ARM_FEATURE_V8_SHA1)) {
> + } else if (!arm_dc_feature(s, ARM_FEATURE_V8_SHA1)) {
> return 1;
> }
> tmp = tcg_const_i32(rd);
> @@ -7039,13 +7057,13 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> cpnum = (insn >> 8) & 0xf;
>
> /* First check for coprocessor space used for XScale/iwMMXt insns */
> - if (arm_feature(env, ARM_FEATURE_XSCALE) && (cpnum < 2)) {
> + if (arm_dc_feature(s, ARM_FEATURE_XSCALE) && (cpnum < 2)) {
> if (extract32(s->c15_cpar, cpnum, 1) == 0) {
> return 1;
> }
> - if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
> + if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) {
> return disas_iwmmxt_insn(env, s, insn);
> - } else if (arm_feature(env, ARM_FEATURE_XSCALE)) {
> + } else if (arm_dc_feature(s, ARM_FEATURE_XSCALE)) {
> return disas_dsp_insn(env, s, insn);
> }
> return 1;
> @@ -7082,7 +7100,7 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> }
>
> if (ri->accessfn ||
> - (arm_feature(env, ARM_FEATURE_XSCALE) && cpnum < 14)) {
> + (arm_dc_feature(s, ARM_FEATURE_XSCALE) && cpnum < 14)) {
> /* Emit code to perform further access permissions checks at
> * runtime; this may result in an exception.
> * Note that on XScale all cp0..c13 registers do an access check
> @@ -7125,7 +7143,7 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> * in which case the syndrome information won't actually be
> * guest visible.
> */
> - assert(!arm_feature(env, ARM_FEATURE_V8));
> + assert(!arm_dc_feature(s, ARM_FEATURE_V8));
> syndrome = syn_uncategorized();
> break;
> }
> @@ -7569,8 +7587,9 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> /* Unconditional instructions. */
> if (((insn >> 25) & 7) == 1) {
> /* NEON Data processing. */
> - if (!arm_feature(env, ARM_FEATURE_NEON))
> + if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
> goto illegal_op;
> + }
>
> if (disas_neon_data_insn(env, s, insn))
> goto illegal_op;
> @@ -7578,8 +7597,9 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> }
> if ((insn & 0x0f100000) == 0x04000000) {
> /* NEON load/store. */
> - if (!arm_feature(env, ARM_FEATURE_NEON))
> + if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
> goto illegal_op;
> + }
>
> if (disas_neon_ls_insn(env, s, insn))
> goto illegal_op;
> @@ -7596,7 +7616,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> ((insn & 0x0f30f010) == 0x0710f000)) {
> if ((insn & (1 << 22)) == 0) {
> /* PLDW; v7MP */
> - if (!arm_feature(env, ARM_FEATURE_V7MP)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V7MP)) {
> goto illegal_op;
> }
> }
> @@ -7611,7 +7631,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> }
> if (((insn & 0x0f700000) == 0x04100000) ||
> ((insn & 0x0f700010) == 0x06100000)) {
> - if (!arm_feature(env, ARM_FEATURE_V7MP)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_V7MP)) {
> goto illegal_op;
> }
> return; /* v7MP: Unallocated memory hint: must NOP */
> @@ -7708,7 +7728,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> gen_bx_im(s, val);
> return;
> } else if ((insn & 0x0e000f00) == 0x0c000100) {
> - if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
> + if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) {
> /* iWMMXt register transfer. */
> if (extract32(s->c15_cpar, 1, 1)) {
> if (!disas_iwmmxt_insn(env, s, insn)) {
> @@ -7864,7 +7884,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> * op1 == 3 is UNPREDICTABLE but handle as UNDEFINED.
> * Bits 8, 10 and 11 should be zero.
> */
> - if (!arm_feature(env, ARM_FEATURE_CRC) || op1 == 0x3 ||
> + if (!arm_dc_feature(s, ARM_FEATURE_CRC) || op1 == 0x3 ||
> (c & 0xd) != 0) {
> goto illegal_op;
> }
> @@ -8673,7 +8693,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> case 1:
> case 3:
> /* SDIV, UDIV */
> - if (!arm_feature(env, ARM_FEATURE_ARM_DIV)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_ARM_DIV)) {
> goto illegal_op;
> }
> if (((insn >> 5) & 7) || (rd != 15)) {
> @@ -9069,8 +9089,8 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> int conds;
> int logic_cc;
>
> - if (!(arm_feature(env, ARM_FEATURE_THUMB2)
> - || arm_feature (env, ARM_FEATURE_M))) {
> + if (!(arm_dc_feature(s, ARM_FEATURE_THUMB2)
> + || arm_dc_feature(s, ARM_FEATURE_M))) {
> /* Thumb-1 cores may need to treat bl and blx as a pair of
> 16-bit instructions to get correct prefetch abort behavior. */
> insn = insn_hw1;
> @@ -9523,7 +9543,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> uint32_t sz = op & 0x3;
> uint32_t c = op & 0x8;
>
> - if (!arm_feature(env, ARM_FEATURE_CRC)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_CRC)) {
> goto illegal_op;
> }
>
> @@ -9651,7 +9671,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> tmp2 = load_reg(s, rm);
> if ((op & 0x50) == 0x10) {
> /* sdiv, udiv */
> - if (!arm_feature(env, ARM_FEATURE_THUMB_DIV)) {
> + if (!arm_dc_feature(s, ARM_FEATURE_THUMB_DIV)) {
> goto illegal_op;
> }
> if (op & 0x20)
>
--
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München
office: +49 89 158834 4135
mobile: +49 15253060158
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 3/5] target-arm/translate.c: Don't use IS_M()
2014-10-28 19:23 [Qemu-devel] [PATCH 0/5] target-arm: Avoid passing CPUARMState around the decoder Peter Maydell
2014-10-28 19:24 ` [Qemu-devel] [PATCH 1/5] target-arm/translate.c: Use arm_dc_feature() in ENABLE_ARCH_ macros Peter Maydell
2014-10-28 19:24 ` [Qemu-devel] [PATCH 2/5] target-arm/translate.c: Use arm_dc_feature() rather than arm_feature() Peter Maydell
@ 2014-10-28 19:24 ` Peter Maydell
2014-10-31 13:42 ` Alex Bennée
2014-11-04 8:59 ` Claudio Fontana
2014-10-28 19:24 ` [Qemu-devel] [PATCH 4/5] target-arm/translate.c: Don't pass CPUARMState around in the decoder Peter Maydell
2014-10-28 19:24 ` [Qemu-devel] [PATCH 5/5] target-arm/translate.c: Don't pass CPUARMState * to disas_arm_insn() Peter Maydell
4 siblings, 2 replies; 19+ messages in thread
From: Peter Maydell @ 2014-10-28 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: patches
Instead of using IS_M(), use arm_dc_feature(s, ARM_FEATURE_M), so we
don't need to pass CPUARMState pointers around the decoder.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/translate.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 08ce5b0..5119fb9 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7574,8 +7574,9 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
s->pc += 4;
/* M variants do not implement ARM mode. */
- if (IS_M(env))
+ if (arm_dc_feature(s, ARM_FEATURE_M)) {
goto illegal_op;
+ }
cond = insn >> 28;
if (cond == 0xf){
/* In ARMv3 and v4 the NV condition is UNPREDICTABLE; we
@@ -9300,7 +9301,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
/* Load/store multiple, RFE, SRS. */
if (((insn >> 23) & 1) == ((insn >> 24) & 1)) {
/* RFE, SRS: not available in user mode or on M profile */
- if (IS_USER(s) || IS_M(env)) {
+ if (IS_USER(s) || arm_dc_feature(s, ARM_FEATURE_M)) {
goto illegal_op;
}
if (insn & (1 << 20)) {
@@ -9804,7 +9805,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
op = (insn >> 20) & 7;
switch (op) {
case 0: /* msr cpsr. */
- if (IS_M(env)) {
+ if (arm_dc_feature(s, ARM_FEATURE_M)) {
tmp = load_reg(s, rn);
addr = tcg_const_i32(insn & 0xff);
gen_helper_v7m_msr(cpu_env, addr, tmp);
@@ -9815,8 +9816,9 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
}
/* fall through */
case 1: /* msr spsr. */
- if (IS_M(env))
+ if (arm_dc_feature(s, ARM_FEATURE_M)) {
goto illegal_op;
+ }
tmp = load_reg(s, rn);
if (gen_set_psr(s,
msr_mask(env, s, (insn >> 8) & 0xf, op == 1),
@@ -9884,7 +9886,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
break;
case 6: /* mrs cpsr. */
tmp = tcg_temp_new_i32();
- if (IS_M(env)) {
+ if (arm_dc_feature(s, ARM_FEATURE_M)) {
addr = tcg_const_i32(insn & 0xff);
gen_helper_v7m_mrs(tmp, cpu_env, addr);
tcg_temp_free_i32(addr);
@@ -9895,8 +9897,9 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
break;
case 7: /* mrs spsr. */
/* Not accessible in user mode. */
- if (IS_USER(s) || IS_M(env))
+ if (IS_USER(s) || arm_dc_feature(s, ARM_FEATURE_M)) {
goto illegal_op;
+ }
tmp = load_cpu_field(spsr);
store_reg(s, rd, tmp);
break;
@@ -10851,7 +10854,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
if (IS_USER(s)) {
break;
}
- if (IS_M(env)) {
+ if (arm_dc_feature(s, ARM_FEATURE_M)) {
tmp = tcg_const_i32((insn & (1 << 4)) != 0);
/* FAULTMASK */
if (insn & 1) {
@@ -11123,7 +11126,7 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
break;
}
#else
- if (dc->pc >= 0xfffffff0 && IS_M(env)) {
+ if (dc->pc >= 0xfffffff0 && arm_dc_feature(dc, ARM_FEATURE_M)) {
/* We always get here via a jump, so know we are not in a
conditional execution block. */
gen_exception_internal(EXCP_EXCEPTION_EXIT);
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PATCH 3/5] target-arm/translate.c: Don't use IS_M()
2014-10-28 19:24 ` [Qemu-devel] [PATCH 3/5] target-arm/translate.c: Don't use IS_M() Peter Maydell
@ 2014-10-31 13:42 ` Alex Bennée
2014-11-04 0:38 ` Peter Maydell
2014-11-04 8:59 ` Claudio Fontana
1 sibling, 1 reply; 19+ messages in thread
From: Alex Bennée @ 2014-10-31 13:42 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> Instead of using IS_M(), use arm_dc_feature(s, ARM_FEATURE_M), so we
> don't need to pass CPUARMState pointers around the decoder.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
I almost wondered if it was killing of the IS_M macro and making direct
calls in cpu.h and helper.c.
Anyway:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> target-arm/translate.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 08ce5b0..5119fb9 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7574,8 +7574,9 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> s->pc += 4;
>
> /* M variants do not implement ARM mode. */
> - if (IS_M(env))
> + if (arm_dc_feature(s, ARM_FEATURE_M)) {
> goto illegal_op;
> + }
> cond = insn >> 28;
> if (cond == 0xf){
> /* In ARMv3 and v4 the NV condition is UNPREDICTABLE; we
> @@ -9300,7 +9301,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> /* Load/store multiple, RFE, SRS. */
> if (((insn >> 23) & 1) == ((insn >> 24) & 1)) {
> /* RFE, SRS: not available in user mode or on M profile */
> - if (IS_USER(s) || IS_M(env)) {
> + if (IS_USER(s) || arm_dc_feature(s, ARM_FEATURE_M)) {
> goto illegal_op;
> }
> if (insn & (1 << 20)) {
> @@ -9804,7 +9805,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> op = (insn >> 20) & 7;
> switch (op) {
> case 0: /* msr cpsr. */
> - if (IS_M(env)) {
> + if (arm_dc_feature(s, ARM_FEATURE_M)) {
> tmp = load_reg(s, rn);
> addr = tcg_const_i32(insn & 0xff);
> gen_helper_v7m_msr(cpu_env, addr, tmp);
> @@ -9815,8 +9816,9 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> }
> /* fall through */
> case 1: /* msr spsr. */
> - if (IS_M(env))
> + if (arm_dc_feature(s, ARM_FEATURE_M)) {
> goto illegal_op;
> + }
> tmp = load_reg(s, rn);
> if (gen_set_psr(s,
> msr_mask(env, s, (insn >> 8) & 0xf, op == 1),
> @@ -9884,7 +9886,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> break;
> case 6: /* mrs cpsr. */
> tmp = tcg_temp_new_i32();
> - if (IS_M(env)) {
> + if (arm_dc_feature(s, ARM_FEATURE_M)) {
> addr = tcg_const_i32(insn & 0xff);
> gen_helper_v7m_mrs(tmp, cpu_env, addr);
> tcg_temp_free_i32(addr);
> @@ -9895,8 +9897,9 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> break;
> case 7: /* mrs spsr. */
> /* Not accessible in user mode. */
> - if (IS_USER(s) || IS_M(env))
> + if (IS_USER(s) || arm_dc_feature(s, ARM_FEATURE_M)) {
> goto illegal_op;
> + }
> tmp = load_cpu_field(spsr);
> store_reg(s, rd, tmp);
> break;
> @@ -10851,7 +10854,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
> if (IS_USER(s)) {
> break;
> }
> - if (IS_M(env)) {
> + if (arm_dc_feature(s, ARM_FEATURE_M)) {
> tmp = tcg_const_i32((insn & (1 << 4)) != 0);
> /* FAULTMASK */
> if (insn & 1) {
> @@ -11123,7 +11126,7 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
> break;
> }
> #else
> - if (dc->pc >= 0xfffffff0 && IS_M(env)) {
> + if (dc->pc >= 0xfffffff0 && arm_dc_feature(dc, ARM_FEATURE_M)) {
> /* We always get here via a jump, so know we are not in a
> conditional execution block. */
> gen_exception_internal(EXCP_EXCEPTION_EXIT);
--
Alex Bennée
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PATCH 3/5] target-arm/translate.c: Don't use IS_M()
2014-10-31 13:42 ` Alex Bennée
@ 2014-11-04 0:38 ` Peter Maydell
0 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2014-11-04 0:38 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers, Patch Tracking
On 31 October 2014 13:42, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> Instead of using IS_M(), use arm_dc_feature(s, ARM_FEATURE_M), so we
>> don't need to pass CPUARMState pointers around the decoder.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> I almost wondered if it was killing of the IS_M macro and making direct
> calls in cpu.h and helper.c.
Yes, eventually, but one of the remaining uses is in
the code that one of the other series on list reworks,
so trying to do that here would have clashed, so I
deliberately didn't drop IS_M altogether yet.
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] target-arm/translate.c: Don't use IS_M()
2014-10-28 19:24 ` [Qemu-devel] [PATCH 3/5] target-arm/translate.c: Don't use IS_M() Peter Maydell
2014-10-31 13:42 ` Alex Bennée
@ 2014-11-04 8:59 ` Claudio Fontana
1 sibling, 0 replies; 19+ messages in thread
From: Claudio Fontana @ 2014-11-04 8:59 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
On 28.10.2014 20:24, Peter Maydell wrote:
> Instead of using IS_M(), use arm_dc_feature(s, ARM_FEATURE_M), so we
> don't need to pass CPUARMState pointers around the decoder.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target-arm/translate.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 08ce5b0..5119fb9 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7574,8 +7574,9 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> s->pc += 4;
>
> /* M variants do not implement ARM mode. */
> - if (IS_M(env))
> + if (arm_dc_feature(s, ARM_FEATURE_M)) {
> goto illegal_op;
> + }
> cond = insn >> 28;
> if (cond == 0xf){
> /* In ARMv3 and v4 the NV condition is UNPREDICTABLE; we
> @@ -9300,7 +9301,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> /* Load/store multiple, RFE, SRS. */
> if (((insn >> 23) & 1) == ((insn >> 24) & 1)) {
> /* RFE, SRS: not available in user mode or on M profile */
> - if (IS_USER(s) || IS_M(env)) {
> + if (IS_USER(s) || arm_dc_feature(s, ARM_FEATURE_M)) {
> goto illegal_op;
> }
> if (insn & (1 << 20)) {
> @@ -9804,7 +9805,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> op = (insn >> 20) & 7;
> switch (op) {
> case 0: /* msr cpsr. */
> - if (IS_M(env)) {
> + if (arm_dc_feature(s, ARM_FEATURE_M)) {
> tmp = load_reg(s, rn);
> addr = tcg_const_i32(insn & 0xff);
> gen_helper_v7m_msr(cpu_env, addr, tmp);
> @@ -9815,8 +9816,9 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> }
> /* fall through */
> case 1: /* msr spsr. */
> - if (IS_M(env))
> + if (arm_dc_feature(s, ARM_FEATURE_M)) {
> goto illegal_op;
> + }
> tmp = load_reg(s, rn);
> if (gen_set_psr(s,
> msr_mask(env, s, (insn >> 8) & 0xf, op == 1),
> @@ -9884,7 +9886,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> break;
> case 6: /* mrs cpsr. */
> tmp = tcg_temp_new_i32();
> - if (IS_M(env)) {
> + if (arm_dc_feature(s, ARM_FEATURE_M)) {
> addr = tcg_const_i32(insn & 0xff);
> gen_helper_v7m_mrs(tmp, cpu_env, addr);
> tcg_temp_free_i32(addr);
> @@ -9895,8 +9897,9 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> break;
> case 7: /* mrs spsr. */
> /* Not accessible in user mode. */
> - if (IS_USER(s) || IS_M(env))
> + if (IS_USER(s) || arm_dc_feature(s, ARM_FEATURE_M)) {
> goto illegal_op;
> + }
> tmp = load_cpu_field(spsr);
> store_reg(s, rd, tmp);
> break;
> @@ -10851,7 +10854,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
> if (IS_USER(s)) {
> break;
> }
> - if (IS_M(env)) {
> + if (arm_dc_feature(s, ARM_FEATURE_M)) {
> tmp = tcg_const_i32((insn & (1 << 4)) != 0);
> /* FAULTMASK */
> if (insn & 1) {
> @@ -11123,7 +11126,7 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
> break;
> }
> #else
> - if (dc->pc >= 0xfffffff0 && IS_M(env)) {
> + if (dc->pc >= 0xfffffff0 && arm_dc_feature(dc, ARM_FEATURE_M)) {
> /* We always get here via a jump, so know we are not in a
> conditional execution block. */
> gen_exception_internal(EXCP_EXCEPTION_EXIT);
>
--
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München
office: +49 89 158834 4135
mobile: +49 15253060158
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 4/5] target-arm/translate.c: Don't pass CPUARMState around in the decoder
2014-10-28 19:23 [Qemu-devel] [PATCH 0/5] target-arm: Avoid passing CPUARMState around the decoder Peter Maydell
` (2 preceding siblings ...)
2014-10-28 19:24 ` [Qemu-devel] [PATCH 3/5] target-arm/translate.c: Don't use IS_M() Peter Maydell
@ 2014-10-28 19:24 ` Peter Maydell
2014-10-31 13:43 ` Alex Bennée
2014-11-04 8:57 ` Claudio Fontana
2014-10-28 19:24 ` [Qemu-devel] [PATCH 5/5] target-arm/translate.c: Don't pass CPUARMState * to disas_arm_insn() Peter Maydell
4 siblings, 2 replies; 19+ messages in thread
From: Peter Maydell @ 2014-10-28 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: patches
Passing the CPUARMState around in the decoder is a recipe for
bugs where we accidentally generate code that depends on CPU
state which isn't reflected in the TB flags. Stop doing this
and instead use DisasContext as a way to pass around those
bits of CPU state which are known to be safe to use.
This commit simply removes initial "CPUARMState *env" parameters
from various function definitions, and removes the initial "env"
argument from the places where those functions are called.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/translate.c | 94 +++++++++++++++++++++++++++-----------------------
1 file changed, 50 insertions(+), 44 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 5119fb9..9e2dda2 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -834,8 +834,7 @@ static inline void gen_bx(DisasContext *s, TCGv_i32 var)
/* Variant of store_reg which uses branch&exchange logic when storing
to r15 in ARM architecture v7 and above. The source must be a temporary
and will be marked as dead. */
-static inline void store_reg_bx(CPUARMState *env, DisasContext *s,
- int reg, TCGv_i32 var)
+static inline void store_reg_bx(DisasContext *s, int reg, TCGv_i32 var)
{
if (reg == 15 && ENABLE_ARCH_7) {
gen_bx(s, var);
@@ -848,8 +847,7 @@ static inline void store_reg_bx(CPUARMState *env, DisasContext *s,
* to r15 in ARM architecture v5T and above. This is used for storing
* the results of a LDR/LDM/POP into r15, and corresponds to the cases
* in the ARM ARM which use the LoadWritePC() pseudocode function. */
-static inline void store_reg_from_load(CPUARMState *env, DisasContext *s,
- int reg, TCGv_i32 var)
+static inline void store_reg_from_load(DisasContext *s, int reg, TCGv_i32 var)
{
if (reg == 15 && ENABLE_ARCH_5) {
gen_bx(s, var);
@@ -1541,7 +1539,7 @@ static inline int gen_iwmmxt_shift(uint32_t insn, uint32_t mask, TCGv_i32 dest)
/* Disassemble an iwMMXt instruction. Returns nonzero if an error occurred
(ie. an undefined instruction). */
-static int disas_iwmmxt_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
+static int disas_iwmmxt_insn(DisasContext *s, uint32_t insn)
{
int rd, wrd;
int rdhi, rdlo, rd0, rd1, i;
@@ -2547,7 +2545,7 @@ static int disas_iwmmxt_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
/* Disassemble an XScale DSP instruction. Returns nonzero if an error occurred
(ie. an undefined instruction). */
-static int disas_dsp_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
+static int disas_dsp_insn(DisasContext *s, uint32_t insn)
{
int acc, rd0, rd1, rdhi, rdlo;
TCGv_i32 tmp, tmp2;
@@ -2966,7 +2964,7 @@ static const uint8_t fp_decode_rm[] = {
FPROUNDING_NEGINF,
};
-static int disas_vfp_v8_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
+static int disas_vfp_v8_insn(DisasContext *s, uint32_t insn)
{
uint32_t rd, rn, rm, dp = extract32(insn, 8, 1);
@@ -3002,7 +3000,7 @@ static int disas_vfp_v8_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
/* Disassemble a VFP instruction. Returns nonzero if an error occurred
(ie. an undefined instruction). */
-static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
+static int disas_vfp_insn(DisasContext *s, uint32_t insn)
{
uint32_t rd, rn, rm, op, i, n, offset, delta_d, delta_m, bank_mask;
int dp, veclen;
@@ -3039,7 +3037,7 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
/* Encodings with T=1 (Thumb) or unconditional (ARM):
* only used in v8 and above.
*/
- return disas_vfp_v8_insn(env, s, insn);
+ return disas_vfp_v8_insn(s, insn);
}
dp = ((insn & 0xf00) == 0xb00);
@@ -3962,7 +3960,8 @@ static inline void gen_mulxy(TCGv_i32 t0, TCGv_i32 t1, int x, int y)
}
/* Return the mask of PSR bits set by a MSR instruction. */
-static uint32_t msr_mask(CPUARMState *env, DisasContext *s, int flags, int spsr) {
+static uint32_t msr_mask(DisasContext *s, int flags, int spsr)
+{
uint32_t mask;
mask = 0;
@@ -4312,7 +4311,7 @@ static struct {
/* Translate a NEON load/store element instruction. Return nonzero if the
instruction is invalid. */
-static int disas_neon_ls_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
+static int disas_neon_ls_insn(DisasContext *s, uint32_t insn)
{
int rd, rn, rm;
int op;
@@ -5054,7 +5053,7 @@ static const uint8_t neon_2rm_sizes[] = {
We process data in a mixture of 32-bit and 64-bit chunks.
Mostly we use 32-bit chunks so we can use normal scalar instructions. */
-static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
+static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
{
int op;
int q;
@@ -7049,7 +7048,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
return 0;
}
-static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
+static int disas_coproc_insn(DisasContext *s, uint32_t insn)
{
int cpnum, is64, crn, crm, opc1, opc2, isread, rt, rt2;
const ARMCPRegInfo *ri;
@@ -7062,9 +7061,9 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
return 1;
}
if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) {
- return disas_iwmmxt_insn(env, s, insn);
+ return disas_iwmmxt_insn(s, insn);
} else if (arm_dc_feature(s, ARM_FEATURE_XSCALE)) {
- return disas_dsp_insn(env, s, insn);
+ return disas_dsp_insn(s, insn);
}
return 1;
}
@@ -7592,8 +7591,9 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
goto illegal_op;
}
- if (disas_neon_data_insn(env, s, insn))
+ if (disas_neon_data_insn(s, insn)) {
goto illegal_op;
+ }
return;
}
if ((insn & 0x0f100000) == 0x04000000) {
@@ -7602,13 +7602,14 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
goto illegal_op;
}
- if (disas_neon_ls_insn(env, s, insn))
+ if (disas_neon_ls_insn(s, insn)) {
goto illegal_op;
+ }
return;
}
if ((insn & 0x0f000e10) == 0x0e000a00) {
/* VFP. */
- if (disas_vfp_insn(env, s, insn)) {
+ if (disas_vfp_insn(s, insn)) {
goto illegal_op;
}
return;
@@ -7732,7 +7733,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) {
/* iWMMXt register transfer. */
if (extract32(s->c15_cpar, 1, 1)) {
- if (!disas_iwmmxt_insn(env, s, insn)) {
+ if (!disas_iwmmxt_insn(s, insn)) {
return;
}
}
@@ -7805,8 +7806,10 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
if (shift)
val = (val >> shift) | (val << (32 - shift));
i = ((insn & (1 << 22)) != 0);
- if (gen_set_psr_im(s, msr_mask(env, s, (insn >> 16) & 0xf, i), i, val))
+ if (gen_set_psr_im(s, msr_mask(s, (insn >> 16) & 0xf, i),
+ i, val)) {
goto illegal_op;
+ }
}
}
} else if ((insn & 0x0f900000) == 0x01000000
@@ -7821,7 +7824,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
/* PSR = reg */
tmp = load_reg(s, rm);
i = ((op1 & 2) != 0);
- if (gen_set_psr(s, msr_mask(env, s, (insn >> 16) & 0xf, i), i, tmp))
+ if (gen_set_psr(s, msr_mask(s, (insn >> 16) & 0xf, i), i, tmp))
goto illegal_op;
} else {
/* reg = PSR */
@@ -8059,14 +8062,14 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
if (logic_cc) {
gen_logic_CC(tmp);
}
- store_reg_bx(env, s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x01:
tcg_gen_xor_i32(tmp, tmp, tmp2);
if (logic_cc) {
gen_logic_CC(tmp);
}
- store_reg_bx(env, s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x02:
if (set_cc && rd == 15) {
@@ -8082,7 +8085,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
} else {
tcg_gen_sub_i32(tmp, tmp, tmp2);
}
- store_reg_bx(env, s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
}
break;
case 0x03:
@@ -8091,7 +8094,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
} else {
tcg_gen_sub_i32(tmp, tmp2, tmp);
}
- store_reg_bx(env, s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x04:
if (set_cc) {
@@ -8099,7 +8102,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
} else {
tcg_gen_add_i32(tmp, tmp, tmp2);
}
- store_reg_bx(env, s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x05:
if (set_cc) {
@@ -8107,7 +8110,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
} else {
gen_add_carry(tmp, tmp, tmp2);
}
- store_reg_bx(env, s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x06:
if (set_cc) {
@@ -8115,7 +8118,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
} else {
gen_sub_carry(tmp, tmp, tmp2);
}
- store_reg_bx(env, s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x07:
if (set_cc) {
@@ -8123,7 +8126,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
} else {
gen_sub_carry(tmp, tmp2, tmp);
}
- store_reg_bx(env, s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x08:
if (set_cc) {
@@ -8156,7 +8159,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
if (logic_cc) {
gen_logic_CC(tmp);
}
- store_reg_bx(env, s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 0x0d:
if (logic_cc && rd == 15) {
@@ -8169,7 +8172,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
if (logic_cc) {
gen_logic_CC(tmp2);
}
- store_reg_bx(env, s, rd, tmp2);
+ store_reg_bx(s, rd, tmp2);
}
break;
case 0x0e:
@@ -8177,7 +8180,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
if (logic_cc) {
gen_logic_CC(tmp);
}
- store_reg_bx(env, s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
default:
case 0x0f:
@@ -8185,7 +8188,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
if (logic_cc) {
gen_logic_CC(tmp2);
}
- store_reg_bx(env, s, rd, tmp2);
+ store_reg_bx(s, rd, tmp2);
break;
}
if (op1 != 0x0f && op1 != 0x0d) {
@@ -8823,7 +8826,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
}
if (insn & (1 << 20)) {
/* Complete the load. */
- store_reg_from_load(env, s, rd, tmp);
+ store_reg_from_load(s, rd, tmp);
}
break;
case 0x08:
@@ -8886,7 +8889,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
loaded_var = tmp;
loaded_base = 1;
} else {
- store_reg_from_load(env, s, i, tmp);
+ store_reg_from_load(s, i, tmp);
}
} else {
/* store */
@@ -8969,10 +8972,10 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
case 0xe:
if (((insn >> 8) & 0xe) == 10) {
/* VFP. */
- if (disas_vfp_insn(env, s, insn)) {
+ if (disas_vfp_insn(s, insn)) {
goto illegal_op;
}
- } else if (disas_coproc_insn(env, s, insn)) {
+ } else if (disas_coproc_insn(s, insn)) {
/* Coprocessor. */
goto illegal_op;
}
@@ -9453,7 +9456,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
gen_arm_shift_reg(tmp, op, tmp2, logic_cc);
if (logic_cc)
gen_logic_CC(tmp);
- store_reg_bx(env, s, rd, tmp);
+ store_reg_bx(s, rd, tmp);
break;
case 1: /* Sign/zero extend. */
tmp = load_reg(s, rm);
@@ -9735,17 +9738,19 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
if (((insn >> 24) & 3) == 3) {
/* Translate into the equivalent ARM encoding. */
insn = (insn & 0xe2ffffff) | ((insn & (1 << 28)) >> 4) | (1 << 28);
- if (disas_neon_data_insn(env, s, insn))
+ if (disas_neon_data_insn(s, insn)) {
goto illegal_op;
+ }
} else if (((insn >> 8) & 0xe) == 10) {
- if (disas_vfp_insn(env, s, insn)) {
+ if (disas_vfp_insn(s, insn)) {
goto illegal_op;
}
} else {
if (insn & (1 << 28))
goto illegal_op;
- if (disas_coproc_insn (env, s, insn))
+ if (disas_coproc_insn(s, insn)) {
goto illegal_op;
+ }
}
break;
case 8: case 9: case 10: case 11:
@@ -9821,7 +9826,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
}
tmp = load_reg(s, rn);
if (gen_set_psr(s,
- msr_mask(env, s, (insn >> 8) & 0xf, op == 1),
+ msr_mask(s, (insn >> 8) & 0xf, op == 1),
op == 1, tmp))
goto illegal_op;
break;
@@ -10087,8 +10092,9 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
int writeback = 0;
int memidx;
if ((insn & 0x01100000) == 0x01000000) {
- if (disas_neon_ls_insn(env, s, insn))
+ if (disas_neon_ls_insn(s, insn)) {
goto illegal_op;
+ }
break;
}
op = ((insn >> 21) & 3) | ((insn >> 22) & 4);
@@ -10784,7 +10790,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
store_reg(s, 13, addr);
/* set the new PC value */
if ((insn & 0x0900) == 0x0900) {
- store_reg_from_load(env, s, 15, tmp);
+ store_reg_from_load(s, 15, tmp);
}
break;
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PATCH 4/5] target-arm/translate.c: Don't pass CPUARMState around in the decoder
2014-10-28 19:24 ` [Qemu-devel] [PATCH 4/5] target-arm/translate.c: Don't pass CPUARMState around in the decoder Peter Maydell
@ 2014-10-31 13:43 ` Alex Bennée
2014-11-04 8:57 ` Claudio Fontana
1 sibling, 0 replies; 19+ messages in thread
From: Alex Bennée @ 2014-10-31 13:43 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> Passing the CPUARMState around in the decoder is a recipe for
> bugs where we accidentally generate code that depends on CPU
> state which isn't reflected in the TB flags. Stop doing this
> and instead use DisasContext as a way to pass around those
> bits of CPU state which are known to be safe to use.
>
> This commit simply removes initial "CPUARMState *env" parameters
> from various function definitions, and removes the initial "env"
> argument from the places where those functions are called.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> target-arm/translate.c | 94 +++++++++++++++++++++++++++-----------------------
> 1 file changed, 50 insertions(+), 44 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 5119fb9..9e2dda2 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -834,8 +834,7 @@ static inline void gen_bx(DisasContext *s, TCGv_i32 var)
> /* Variant of store_reg which uses branch&exchange logic when storing
> to r15 in ARM architecture v7 and above. The source must be a temporary
> and will be marked as dead. */
> -static inline void store_reg_bx(CPUARMState *env, DisasContext *s,
> - int reg, TCGv_i32 var)
> +static inline void store_reg_bx(DisasContext *s, int reg, TCGv_i32 var)
> {
> if (reg == 15 && ENABLE_ARCH_7) {
> gen_bx(s, var);
> @@ -848,8 +847,7 @@ static inline void store_reg_bx(CPUARMState *env, DisasContext *s,
> * to r15 in ARM architecture v5T and above. This is used for storing
> * the results of a LDR/LDM/POP into r15, and corresponds to the cases
> * in the ARM ARM which use the LoadWritePC() pseudocode function. */
> -static inline void store_reg_from_load(CPUARMState *env, DisasContext *s,
> - int reg, TCGv_i32 var)
> +static inline void store_reg_from_load(DisasContext *s, int reg, TCGv_i32 var)
> {
> if (reg == 15 && ENABLE_ARCH_5) {
> gen_bx(s, var);
> @@ -1541,7 +1539,7 @@ static inline int gen_iwmmxt_shift(uint32_t insn, uint32_t mask, TCGv_i32 dest)
>
> /* Disassemble an iwMMXt instruction. Returns nonzero if an error occurred
> (ie. an undefined instruction). */
> -static int disas_iwmmxt_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
> +static int disas_iwmmxt_insn(DisasContext *s, uint32_t insn)
> {
> int rd, wrd;
> int rdhi, rdlo, rd0, rd1, i;
> @@ -2547,7 +2545,7 @@ static int disas_iwmmxt_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
>
> /* Disassemble an XScale DSP instruction. Returns nonzero if an error occurred
> (ie. an undefined instruction). */
> -static int disas_dsp_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
> +static int disas_dsp_insn(DisasContext *s, uint32_t insn)
> {
> int acc, rd0, rd1, rdhi, rdlo;
> TCGv_i32 tmp, tmp2;
> @@ -2966,7 +2964,7 @@ static const uint8_t fp_decode_rm[] = {
> FPROUNDING_NEGINF,
> };
>
> -static int disas_vfp_v8_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
> +static int disas_vfp_v8_insn(DisasContext *s, uint32_t insn)
> {
> uint32_t rd, rn, rm, dp = extract32(insn, 8, 1);
>
> @@ -3002,7 +3000,7 @@ static int disas_vfp_v8_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
>
> /* Disassemble a VFP instruction. Returns nonzero if an error occurred
> (ie. an undefined instruction). */
> -static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> +static int disas_vfp_insn(DisasContext *s, uint32_t insn)
> {
> uint32_t rd, rn, rm, op, i, n, offset, delta_d, delta_m, bank_mask;
> int dp, veclen;
> @@ -3039,7 +3037,7 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> /* Encodings with T=1 (Thumb) or unconditional (ARM):
> * only used in v8 and above.
> */
> - return disas_vfp_v8_insn(env, s, insn);
> + return disas_vfp_v8_insn(s, insn);
> }
>
> dp = ((insn & 0xf00) == 0xb00);
> @@ -3962,7 +3960,8 @@ static inline void gen_mulxy(TCGv_i32 t0, TCGv_i32 t1, int x, int y)
> }
>
> /* Return the mask of PSR bits set by a MSR instruction. */
> -static uint32_t msr_mask(CPUARMState *env, DisasContext *s, int flags, int spsr) {
> +static uint32_t msr_mask(DisasContext *s, int flags, int spsr)
> +{
> uint32_t mask;
>
> mask = 0;
> @@ -4312,7 +4311,7 @@ static struct {
>
> /* Translate a NEON load/store element instruction. Return nonzero if the
> instruction is invalid. */
> -static int disas_neon_ls_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> +static int disas_neon_ls_insn(DisasContext *s, uint32_t insn)
> {
> int rd, rn, rm;
> int op;
> @@ -5054,7 +5053,7 @@ static const uint8_t neon_2rm_sizes[] = {
> We process data in a mixture of 32-bit and 64-bit chunks.
> Mostly we use 32-bit chunks so we can use normal scalar instructions. */
>
> -static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> +static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
> {
> int op;
> int q;
> @@ -7049,7 +7048,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> return 0;
> }
>
> -static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> +static int disas_coproc_insn(DisasContext *s, uint32_t insn)
> {
> int cpnum, is64, crn, crm, opc1, opc2, isread, rt, rt2;
> const ARMCPRegInfo *ri;
> @@ -7062,9 +7061,9 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> return 1;
> }
> if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) {
> - return disas_iwmmxt_insn(env, s, insn);
> + return disas_iwmmxt_insn(s, insn);
> } else if (arm_dc_feature(s, ARM_FEATURE_XSCALE)) {
> - return disas_dsp_insn(env, s, insn);
> + return disas_dsp_insn(s, insn);
> }
> return 1;
> }
> @@ -7592,8 +7591,9 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> goto illegal_op;
> }
>
> - if (disas_neon_data_insn(env, s, insn))
> + if (disas_neon_data_insn(s, insn)) {
> goto illegal_op;
> + }
> return;
> }
> if ((insn & 0x0f100000) == 0x04000000) {
> @@ -7602,13 +7602,14 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> goto illegal_op;
> }
>
> - if (disas_neon_ls_insn(env, s, insn))
> + if (disas_neon_ls_insn(s, insn)) {
> goto illegal_op;
> + }
> return;
> }
> if ((insn & 0x0f000e10) == 0x0e000a00) {
> /* VFP. */
> - if (disas_vfp_insn(env, s, insn)) {
> + if (disas_vfp_insn(s, insn)) {
> goto illegal_op;
> }
> return;
> @@ -7732,7 +7733,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) {
> /* iWMMXt register transfer. */
> if (extract32(s->c15_cpar, 1, 1)) {
> - if (!disas_iwmmxt_insn(env, s, insn)) {
> + if (!disas_iwmmxt_insn(s, insn)) {
> return;
> }
> }
> @@ -7805,8 +7806,10 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (shift)
> val = (val >> shift) | (val << (32 - shift));
> i = ((insn & (1 << 22)) != 0);
> - if (gen_set_psr_im(s, msr_mask(env, s, (insn >> 16) & 0xf, i), i, val))
> + if (gen_set_psr_im(s, msr_mask(s, (insn >> 16) & 0xf, i),
> + i, val)) {
> goto illegal_op;
> + }
> }
> }
> } else if ((insn & 0x0f900000) == 0x01000000
> @@ -7821,7 +7824,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> /* PSR = reg */
> tmp = load_reg(s, rm);
> i = ((op1 & 2) != 0);
> - if (gen_set_psr(s, msr_mask(env, s, (insn >> 16) & 0xf, i), i, tmp))
> + if (gen_set_psr(s, msr_mask(s, (insn >> 16) & 0xf, i), i, tmp))
> goto illegal_op;
> } else {
> /* reg = PSR */
> @@ -8059,14 +8062,14 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (logic_cc) {
> gen_logic_CC(tmp);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x01:
> tcg_gen_xor_i32(tmp, tmp, tmp2);
> if (logic_cc) {
> gen_logic_CC(tmp);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x02:
> if (set_cc && rd == 15) {
> @@ -8082,7 +8085,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> } else {
> tcg_gen_sub_i32(tmp, tmp, tmp2);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> }
> break;
> case 0x03:
> @@ -8091,7 +8094,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> } else {
> tcg_gen_sub_i32(tmp, tmp2, tmp);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x04:
> if (set_cc) {
> @@ -8099,7 +8102,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> } else {
> tcg_gen_add_i32(tmp, tmp, tmp2);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x05:
> if (set_cc) {
> @@ -8107,7 +8110,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> } else {
> gen_add_carry(tmp, tmp, tmp2);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x06:
> if (set_cc) {
> @@ -8115,7 +8118,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> } else {
> gen_sub_carry(tmp, tmp, tmp2);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x07:
> if (set_cc) {
> @@ -8123,7 +8126,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> } else {
> gen_sub_carry(tmp, tmp2, tmp);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x08:
> if (set_cc) {
> @@ -8156,7 +8159,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (logic_cc) {
> gen_logic_CC(tmp);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x0d:
> if (logic_cc && rd == 15) {
> @@ -8169,7 +8172,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (logic_cc) {
> gen_logic_CC(tmp2);
> }
> - store_reg_bx(env, s, rd, tmp2);
> + store_reg_bx(s, rd, tmp2);
> }
> break;
> case 0x0e:
> @@ -8177,7 +8180,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (logic_cc) {
> gen_logic_CC(tmp);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> default:
> case 0x0f:
> @@ -8185,7 +8188,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (logic_cc) {
> gen_logic_CC(tmp2);
> }
> - store_reg_bx(env, s, rd, tmp2);
> + store_reg_bx(s, rd, tmp2);
> break;
> }
> if (op1 != 0x0f && op1 != 0x0d) {
> @@ -8823,7 +8826,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> }
> if (insn & (1 << 20)) {
> /* Complete the load. */
> - store_reg_from_load(env, s, rd, tmp);
> + store_reg_from_load(s, rd, tmp);
> }
> break;
> case 0x08:
> @@ -8886,7 +8889,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> loaded_var = tmp;
> loaded_base = 1;
> } else {
> - store_reg_from_load(env, s, i, tmp);
> + store_reg_from_load(s, i, tmp);
> }
> } else {
> /* store */
> @@ -8969,10 +8972,10 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> case 0xe:
> if (((insn >> 8) & 0xe) == 10) {
> /* VFP. */
> - if (disas_vfp_insn(env, s, insn)) {
> + if (disas_vfp_insn(s, insn)) {
> goto illegal_op;
> }
> - } else if (disas_coproc_insn(env, s, insn)) {
> + } else if (disas_coproc_insn(s, insn)) {
> /* Coprocessor. */
> goto illegal_op;
> }
> @@ -9453,7 +9456,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> gen_arm_shift_reg(tmp, op, tmp2, logic_cc);
> if (logic_cc)
> gen_logic_CC(tmp);
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 1: /* Sign/zero extend. */
> tmp = load_reg(s, rm);
> @@ -9735,17 +9738,19 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> if (((insn >> 24) & 3) == 3) {
> /* Translate into the equivalent ARM encoding. */
> insn = (insn & 0xe2ffffff) | ((insn & (1 << 28)) >> 4) | (1 << 28);
> - if (disas_neon_data_insn(env, s, insn))
> + if (disas_neon_data_insn(s, insn)) {
> goto illegal_op;
> + }
> } else if (((insn >> 8) & 0xe) == 10) {
> - if (disas_vfp_insn(env, s, insn)) {
> + if (disas_vfp_insn(s, insn)) {
> goto illegal_op;
> }
> } else {
> if (insn & (1 << 28))
> goto illegal_op;
> - if (disas_coproc_insn (env, s, insn))
> + if (disas_coproc_insn(s, insn)) {
> goto illegal_op;
> + }
> }
> break;
> case 8: case 9: case 10: case 11:
> @@ -9821,7 +9826,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> }
> tmp = load_reg(s, rn);
> if (gen_set_psr(s,
> - msr_mask(env, s, (insn >> 8) & 0xf, op == 1),
> + msr_mask(s, (insn >> 8) & 0xf, op == 1),
> op == 1, tmp))
> goto illegal_op;
> break;
> @@ -10087,8 +10092,9 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> int writeback = 0;
> int memidx;
> if ((insn & 0x01100000) == 0x01000000) {
> - if (disas_neon_ls_insn(env, s, insn))
> + if (disas_neon_ls_insn(s, insn)) {
> goto illegal_op;
> + }
> break;
> }
> op = ((insn >> 21) & 3) | ((insn >> 22) & 4);
> @@ -10784,7 +10790,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
> store_reg(s, 13, addr);
> /* set the new PC value */
> if ((insn & 0x0900) == 0x0900) {
> - store_reg_from_load(env, s, 15, tmp);
> + store_reg_from_load(s, 15, tmp);
> }
> break;
--
Alex Bennée
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PATCH 4/5] target-arm/translate.c: Don't pass CPUARMState around in the decoder
2014-10-28 19:24 ` [Qemu-devel] [PATCH 4/5] target-arm/translate.c: Don't pass CPUARMState around in the decoder Peter Maydell
2014-10-31 13:43 ` Alex Bennée
@ 2014-11-04 8:57 ` Claudio Fontana
1 sibling, 0 replies; 19+ messages in thread
From: Claudio Fontana @ 2014-11-04 8:57 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
On 28.10.2014 20:24, Peter Maydell wrote:
> Passing the CPUARMState around in the decoder is a recipe for
> bugs where we accidentally generate code that depends on CPU
> state which isn't reflected in the TB flags. Stop doing this
> and instead use DisasContext as a way to pass around those
> bits of CPU state which are known to be safe to use.
>
> This commit simply removes initial "CPUARMState *env" parameters
> from various function definitions, and removes the initial "env"
> argument from the places where those functions are called.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target-arm/translate.c | 94 +++++++++++++++++++++++++++-----------------------
> 1 file changed, 50 insertions(+), 44 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 5119fb9..9e2dda2 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -834,8 +834,7 @@ static inline void gen_bx(DisasContext *s, TCGv_i32 var)
> /* Variant of store_reg which uses branch&exchange logic when storing
> to r15 in ARM architecture v7 and above. The source must be a temporary
> and will be marked as dead. */
> -static inline void store_reg_bx(CPUARMState *env, DisasContext *s,
> - int reg, TCGv_i32 var)
> +static inline void store_reg_bx(DisasContext *s, int reg, TCGv_i32 var)
> {
> if (reg == 15 && ENABLE_ARCH_7) {
> gen_bx(s, var);
> @@ -848,8 +847,7 @@ static inline void store_reg_bx(CPUARMState *env, DisasContext *s,
> * to r15 in ARM architecture v5T and above. This is used for storing
> * the results of a LDR/LDM/POP into r15, and corresponds to the cases
> * in the ARM ARM which use the LoadWritePC() pseudocode function. */
> -static inline void store_reg_from_load(CPUARMState *env, DisasContext *s,
> - int reg, TCGv_i32 var)
> +static inline void store_reg_from_load(DisasContext *s, int reg, TCGv_i32 var)
> {
> if (reg == 15 && ENABLE_ARCH_5) {
> gen_bx(s, var);
> @@ -1541,7 +1539,7 @@ static inline int gen_iwmmxt_shift(uint32_t insn, uint32_t mask, TCGv_i32 dest)
>
> /* Disassemble an iwMMXt instruction. Returns nonzero if an error occurred
> (ie. an undefined instruction). */
> -static int disas_iwmmxt_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
> +static int disas_iwmmxt_insn(DisasContext *s, uint32_t insn)
> {
> int rd, wrd;
> int rdhi, rdlo, rd0, rd1, i;
> @@ -2547,7 +2545,7 @@ static int disas_iwmmxt_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
>
> /* Disassemble an XScale DSP instruction. Returns nonzero if an error occurred
> (ie. an undefined instruction). */
> -static int disas_dsp_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
> +static int disas_dsp_insn(DisasContext *s, uint32_t insn)
> {
> int acc, rd0, rd1, rdhi, rdlo;
> TCGv_i32 tmp, tmp2;
> @@ -2966,7 +2964,7 @@ static const uint8_t fp_decode_rm[] = {
> FPROUNDING_NEGINF,
> };
>
> -static int disas_vfp_v8_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
> +static int disas_vfp_v8_insn(DisasContext *s, uint32_t insn)
> {
> uint32_t rd, rn, rm, dp = extract32(insn, 8, 1);
>
> @@ -3002,7 +3000,7 @@ static int disas_vfp_v8_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
>
> /* Disassemble a VFP instruction. Returns nonzero if an error occurred
> (ie. an undefined instruction). */
> -static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> +static int disas_vfp_insn(DisasContext *s, uint32_t insn)
> {
> uint32_t rd, rn, rm, op, i, n, offset, delta_d, delta_m, bank_mask;
> int dp, veclen;
> @@ -3039,7 +3037,7 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> /* Encodings with T=1 (Thumb) or unconditional (ARM):
> * only used in v8 and above.
> */
> - return disas_vfp_v8_insn(env, s, insn);
> + return disas_vfp_v8_insn(s, insn);
> }
>
> dp = ((insn & 0xf00) == 0xb00);
> @@ -3962,7 +3960,8 @@ static inline void gen_mulxy(TCGv_i32 t0, TCGv_i32 t1, int x, int y)
> }
>
> /* Return the mask of PSR bits set by a MSR instruction. */
> -static uint32_t msr_mask(CPUARMState *env, DisasContext *s, int flags, int spsr) {
> +static uint32_t msr_mask(DisasContext *s, int flags, int spsr)
> +{
> uint32_t mask;
>
> mask = 0;
> @@ -4312,7 +4311,7 @@ static struct {
>
> /* Translate a NEON load/store element instruction. Return nonzero if the
> instruction is invalid. */
> -static int disas_neon_ls_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> +static int disas_neon_ls_insn(DisasContext *s, uint32_t insn)
> {
> int rd, rn, rm;
> int op;
> @@ -5054,7 +5053,7 @@ static const uint8_t neon_2rm_sizes[] = {
> We process data in a mixture of 32-bit and 64-bit chunks.
> Mostly we use 32-bit chunks so we can use normal scalar instructions. */
>
> -static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> +static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
> {
> int op;
> int q;
> @@ -7049,7 +7048,7 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
> return 0;
> }
>
> -static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> +static int disas_coproc_insn(DisasContext *s, uint32_t insn)
> {
> int cpnum, is64, crn, crm, opc1, opc2, isread, rt, rt2;
> const ARMCPRegInfo *ri;
> @@ -7062,9 +7061,9 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
> return 1;
> }
> if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) {
> - return disas_iwmmxt_insn(env, s, insn);
> + return disas_iwmmxt_insn(s, insn);
> } else if (arm_dc_feature(s, ARM_FEATURE_XSCALE)) {
> - return disas_dsp_insn(env, s, insn);
> + return disas_dsp_insn(s, insn);
> }
> return 1;
> }
> @@ -7592,8 +7591,9 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> goto illegal_op;
> }
>
> - if (disas_neon_data_insn(env, s, insn))
> + if (disas_neon_data_insn(s, insn)) {
> goto illegal_op;
> + }
> return;
> }
> if ((insn & 0x0f100000) == 0x04000000) {
> @@ -7602,13 +7602,14 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> goto illegal_op;
> }
>
> - if (disas_neon_ls_insn(env, s, insn))
> + if (disas_neon_ls_insn(s, insn)) {
> goto illegal_op;
> + }
> return;
> }
> if ((insn & 0x0f000e10) == 0x0e000a00) {
> /* VFP. */
> - if (disas_vfp_insn(env, s, insn)) {
> + if (disas_vfp_insn(s, insn)) {
> goto illegal_op;
> }
> return;
> @@ -7732,7 +7733,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) {
> /* iWMMXt register transfer. */
> if (extract32(s->c15_cpar, 1, 1)) {
> - if (!disas_iwmmxt_insn(env, s, insn)) {
> + if (!disas_iwmmxt_insn(s, insn)) {
> return;
> }
> }
> @@ -7805,8 +7806,10 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (shift)
> val = (val >> shift) | (val << (32 - shift));
> i = ((insn & (1 << 22)) != 0);
> - if (gen_set_psr_im(s, msr_mask(env, s, (insn >> 16) & 0xf, i), i, val))
> + if (gen_set_psr_im(s, msr_mask(s, (insn >> 16) & 0xf, i),
> + i, val)) {
> goto illegal_op;
> + }
> }
> }
> } else if ((insn & 0x0f900000) == 0x01000000
> @@ -7821,7 +7824,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> /* PSR = reg */
> tmp = load_reg(s, rm);
> i = ((op1 & 2) != 0);
> - if (gen_set_psr(s, msr_mask(env, s, (insn >> 16) & 0xf, i), i, tmp))
> + if (gen_set_psr(s, msr_mask(s, (insn >> 16) & 0xf, i), i, tmp))
> goto illegal_op;
> } else {
> /* reg = PSR */
> @@ -8059,14 +8062,14 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (logic_cc) {
> gen_logic_CC(tmp);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x01:
> tcg_gen_xor_i32(tmp, tmp, tmp2);
> if (logic_cc) {
> gen_logic_CC(tmp);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x02:
> if (set_cc && rd == 15) {
> @@ -8082,7 +8085,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> } else {
> tcg_gen_sub_i32(tmp, tmp, tmp2);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> }
> break;
> case 0x03:
> @@ -8091,7 +8094,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> } else {
> tcg_gen_sub_i32(tmp, tmp2, tmp);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x04:
> if (set_cc) {
> @@ -8099,7 +8102,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> } else {
> tcg_gen_add_i32(tmp, tmp, tmp2);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x05:
> if (set_cc) {
> @@ -8107,7 +8110,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> } else {
> gen_add_carry(tmp, tmp, tmp2);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x06:
> if (set_cc) {
> @@ -8115,7 +8118,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> } else {
> gen_sub_carry(tmp, tmp, tmp2);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x07:
> if (set_cc) {
> @@ -8123,7 +8126,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> } else {
> gen_sub_carry(tmp, tmp2, tmp);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x08:
> if (set_cc) {
> @@ -8156,7 +8159,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (logic_cc) {
> gen_logic_CC(tmp);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 0x0d:
> if (logic_cc && rd == 15) {
> @@ -8169,7 +8172,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (logic_cc) {
> gen_logic_CC(tmp2);
> }
> - store_reg_bx(env, s, rd, tmp2);
> + store_reg_bx(s, rd, tmp2);
> }
> break;
> case 0x0e:
> @@ -8177,7 +8180,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (logic_cc) {
> gen_logic_CC(tmp);
> }
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> default:
> case 0x0f:
> @@ -8185,7 +8188,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> if (logic_cc) {
> gen_logic_CC(tmp2);
> }
> - store_reg_bx(env, s, rd, tmp2);
> + store_reg_bx(s, rd, tmp2);
> break;
> }
> if (op1 != 0x0f && op1 != 0x0d) {
> @@ -8823,7 +8826,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> }
> if (insn & (1 << 20)) {
> /* Complete the load. */
> - store_reg_from_load(env, s, rd, tmp);
> + store_reg_from_load(s, rd, tmp);
> }
> break;
> case 0x08:
> @@ -8886,7 +8889,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> loaded_var = tmp;
> loaded_base = 1;
> } else {
> - store_reg_from_load(env, s, i, tmp);
> + store_reg_from_load(s, i, tmp);
> }
> } else {
> /* store */
> @@ -8969,10 +8972,10 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> case 0xe:
> if (((insn >> 8) & 0xe) == 10) {
> /* VFP. */
> - if (disas_vfp_insn(env, s, insn)) {
> + if (disas_vfp_insn(s, insn)) {
> goto illegal_op;
> }
> - } else if (disas_coproc_insn(env, s, insn)) {
> + } else if (disas_coproc_insn(s, insn)) {
> /* Coprocessor. */
> goto illegal_op;
> }
> @@ -9453,7 +9456,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> gen_arm_shift_reg(tmp, op, tmp2, logic_cc);
> if (logic_cc)
> gen_logic_CC(tmp);
> - store_reg_bx(env, s, rd, tmp);
> + store_reg_bx(s, rd, tmp);
> break;
> case 1: /* Sign/zero extend. */
> tmp = load_reg(s, rm);
> @@ -9735,17 +9738,19 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> if (((insn >> 24) & 3) == 3) {
> /* Translate into the equivalent ARM encoding. */
> insn = (insn & 0xe2ffffff) | ((insn & (1 << 28)) >> 4) | (1 << 28);
> - if (disas_neon_data_insn(env, s, insn))
> + if (disas_neon_data_insn(s, insn)) {
> goto illegal_op;
> + }
> } else if (((insn >> 8) & 0xe) == 10) {
> - if (disas_vfp_insn(env, s, insn)) {
> + if (disas_vfp_insn(s, insn)) {
> goto illegal_op;
> }
> } else {
> if (insn & (1 << 28))
> goto illegal_op;
> - if (disas_coproc_insn (env, s, insn))
> + if (disas_coproc_insn(s, insn)) {
> goto illegal_op;
> + }
> }
> break;
> case 8: case 9: case 10: case 11:
> @@ -9821,7 +9826,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> }
> tmp = load_reg(s, rn);
> if (gen_set_psr(s,
> - msr_mask(env, s, (insn >> 8) & 0xf, op == 1),
> + msr_mask(s, (insn >> 8) & 0xf, op == 1),
> op == 1, tmp))
> goto illegal_op;
> break;
> @@ -10087,8 +10092,9 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
> int writeback = 0;
> int memidx;
> if ((insn & 0x01100000) == 0x01000000) {
> - if (disas_neon_ls_insn(env, s, insn))
> + if (disas_neon_ls_insn(s, insn)) {
> goto illegal_op;
> + }
> break;
> }
> op = ((insn >> 21) & 3) | ((insn >> 22) & 4);
> @@ -10784,7 +10790,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
> store_reg(s, 13, addr);
> /* set the new PC value */
> if ((insn & 0x0900) == 0x0900) {
> - store_reg_from_load(env, s, 15, tmp);
> + store_reg_from_load(s, 15, tmp);
> }
> break;
>
>
--
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München
office: +49 89 158834 4135
mobile: +49 15253060158
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 5/5] target-arm/translate.c: Don't pass CPUARMState * to disas_arm_insn()
2014-10-28 19:23 [Qemu-devel] [PATCH 0/5] target-arm: Avoid passing CPUARMState around the decoder Peter Maydell
` (3 preceding siblings ...)
2014-10-28 19:24 ` [Qemu-devel] [PATCH 4/5] target-arm/translate.c: Don't pass CPUARMState around in the decoder Peter Maydell
@ 2014-10-28 19:24 ` Peter Maydell
2014-10-31 13:47 ` Alex Bennée
2014-11-04 8:55 ` Claudio Fontana
4 siblings, 2 replies; 19+ messages in thread
From: Peter Maydell @ 2014-10-28 19:24 UTC (permalink / raw)
To: qemu-devel; +Cc: patches
Refactor to avoid passing a CPUARMState * to disas_arm_insn(). To do this
we move the "read insn from memory" code to the callsite and pass the
insn to the function instead.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target-arm/translate.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 9e2dda2..932fa03 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7560,18 +7560,15 @@ static void gen_srs(DisasContext *s,
tcg_temp_free_i32(addr);
}
-static void disas_arm_insn(CPUARMState * env, DisasContext *s)
+static void disas_arm_insn(DisasContext *s, unsigned int insn)
{
- unsigned int cond, insn, val, op1, i, shift, rm, rs, rn, rd, sh;
+ unsigned int cond, val, op1, i, shift, rm, rs, rn, rd, sh;
TCGv_i32 tmp;
TCGv_i32 tmp2;
TCGv_i32 tmp3;
TCGv_i32 addr;
TCGv_i64 tmp64;
- insn = arm_ldl_code(env, s->pc, s->bswap_code);
- s->pc += 4;
-
/* M variants do not implement ARM mode. */
if (arm_dc_feature(s, ARM_FEATURE_M)) {
goto illegal_op;
@@ -11199,7 +11196,9 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
}
}
} else {
- disas_arm_insn(env, dc);
+ unsigned int insn = arm_ldl_code(env, dc->pc, dc->bswap_code);
+ dc->pc += 4;
+ disas_arm_insn(dc, insn);
}
if (dc->condjmp && !dc->is_jmp) {
--
1.9.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PATCH 5/5] target-arm/translate.c: Don't pass CPUARMState * to disas_arm_insn()
2014-10-28 19:24 ` [Qemu-devel] [PATCH 5/5] target-arm/translate.c: Don't pass CPUARMState * to disas_arm_insn() Peter Maydell
@ 2014-10-31 13:47 ` Alex Bennée
2014-11-04 0:40 ` Peter Maydell
2014-11-04 8:55 ` Claudio Fontana
1 sibling, 1 reply; 19+ messages in thread
From: Alex Bennée @ 2014-10-31 13:47 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, patches
Peter Maydell <peter.maydell@linaro.org> writes:
> Refactor to avoid passing a CPUARMState * to disas_arm_insn(). To do this
> we move the "read insn from memory" code to the callsite and pass the
> insn to the function instead.
>
<snip>
>
> -static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> +static void disas_arm_insn(DisasContext *s, unsigned int insn)
I note that in the aarch64 code we used the unambiguous uint32_t for the
insn type. I'm hard pressed to imagine it actually breaking anything
though.
> {
> - unsigned int cond, insn, val, op1, i, shift, rm, rs, rn, rd, sh;
> + unsigned int cond, val, op1, i, shift, rm, rs, rn, rd, sh;
> TCGv_i32 tmp;
> TCGv_i32 tmp2;
> TCGv_i32 tmp3;
> TCGv_i32 addr;
> TCGv_i64 tmp64;
>
> - insn = arm_ldl_code(env, s->pc, s->bswap_code);
> - s->pc += 4;
> -
> /* M variants do not implement ARM mode. */
> if (arm_dc_feature(s, ARM_FEATURE_M)) {
> goto illegal_op;
> @@ -11199,7 +11196,9 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
> }
> }
> } else {
> - disas_arm_insn(env, dc);
> + unsigned int insn = arm_ldl_code(env, dc->pc, dc->bswap_code);
> + dc->pc += 4;
> + disas_arm_insn(dc, insn);
> }
>
> if (dc->condjmp && !dc->is_jmp) {
Anyway looks fine:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PATCH 5/5] target-arm/translate.c: Don't pass CPUARMState * to disas_arm_insn()
2014-10-31 13:47 ` Alex Bennée
@ 2014-11-04 0:40 ` Peter Maydell
0 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2014-11-04 0:40 UTC (permalink / raw)
To: Alex Bennée; +Cc: QEMU Developers, Patch Tracking
On 31 October 2014 13:47, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> Refactor to avoid passing a CPUARMState * to disas_arm_insn(). To do this
>> we move the "read insn from memory" code to the callsite and pass the
>> insn to the function instead.
>>
> <snip>
>>
>> -static void disas_arm_insn(CPUARMState * env, DisasContext *s)
>> +static void disas_arm_insn(DisasContext *s, unsigned int insn)
>
> I note that in the aarch64 code we used the unambiguous uint32_t for the
> insn type. I'm hard pressed to imagine it actually breaking anything
> though.
Yeah, we already strongly assume int to be 32 bits, and I wanted
to make this patch not mess with the type of the existing variable.
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 5/5] target-arm/translate.c: Don't pass CPUARMState * to disas_arm_insn()
2014-10-28 19:24 ` [Qemu-devel] [PATCH 5/5] target-arm/translate.c: Don't pass CPUARMState * to disas_arm_insn() Peter Maydell
2014-10-31 13:47 ` Alex Bennée
@ 2014-11-04 8:55 ` Claudio Fontana
1 sibling, 0 replies; 19+ messages in thread
From: Claudio Fontana @ 2014-11-04 8:55 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: patches
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
On 28.10.2014 20:24, Peter Maydell wrote:
> Refactor to avoid passing a CPUARMState * to disas_arm_insn(). To do this
> we move the "read insn from memory" code to the callsite and pass the
> insn to the function instead.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target-arm/translate.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 9e2dda2..932fa03 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7560,18 +7560,15 @@ static void gen_srs(DisasContext *s,
> tcg_temp_free_i32(addr);
> }
>
> -static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> +static void disas_arm_insn(DisasContext *s, unsigned int insn)
> {
> - unsigned int cond, insn, val, op1, i, shift, rm, rs, rn, rd, sh;
> + unsigned int cond, val, op1, i, shift, rm, rs, rn, rd, sh;
> TCGv_i32 tmp;
> TCGv_i32 tmp2;
> TCGv_i32 tmp3;
> TCGv_i32 addr;
> TCGv_i64 tmp64;
>
> - insn = arm_ldl_code(env, s->pc, s->bswap_code);
> - s->pc += 4;
> -
> /* M variants do not implement ARM mode. */
> if (arm_dc_feature(s, ARM_FEATURE_M)) {
> goto illegal_op;
> @@ -11199,7 +11196,9 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
> }
> }
> } else {
> - disas_arm_insn(env, dc);
> + unsigned int insn = arm_ldl_code(env, dc->pc, dc->bswap_code);
> + dc->pc += 4;
> + disas_arm_insn(dc, insn);
> }
>
> if (dc->condjmp && !dc->is_jmp) {
>
--
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München
office: +49 89 158834 4135
mobile: +49 15253060158
^ permalink raw reply [flat|nested] 19+ messages in thread