| Message ID | 20200203144716.32204-9-richard.henderson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/arm: Implement PAN, ATS1E1, UAO | expand |
On Mon, 3 Feb 2020 at 14:47, Richard Henderson <richard.henderson@linaro.org> wrote: > > The only remaining use was in op_helper.c. Use PSTATE_SS > directly, and move the commentary so that it is more obvious > what is going on. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/cpu.h | 6 ------ > target/arm/op_helper.c | 9 ++++++++- > 2 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index 694b074298..c6dff1d55b 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -1186,12 +1186,6 @@ void pmu_init(ARMCPU *cpu); > #define CPSR_IT_2_7 (0xfc00U) > #define CPSR_GE (0xfU << 16) > #define CPSR_IL (1U << 20) > -/* Note that the RESERVED bits include bit 21, which is PSTATE_SS in > - * an AArch64 SPSR but RES0 in AArch32 SPSR and CPSR. In QEMU we use > - * env->uncached_cpsr bit 21 to store PSTATE.SS when executing in AArch32, > - * where it is live state but not accessible to the AArch32 code. > - */ > -#define CPSR_RESERVED (0x7U << 21) > #define CPSR_J (1U << 24) > #define CPSR_IT_0_1 (3U << 25) > #define CPSR_Q (1U << 27) > diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c > index acf1815ea3..af3020b78f 100644 > --- a/target/arm/op_helper.c > +++ b/target/arm/op_helper.c > @@ -387,7 +387,14 @@ void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome) > > uint32_t HELPER(cpsr_read)(CPUARMState *env) > { > - return cpsr_read(env) & ~(CPSR_EXEC | CPSR_RESERVED); > + /* > + * We store the ARMv8 PSTATE.SS bit in env->uncached_cpsr. > + * This is convenient for populating SPSR_ELx, but must be > + * hidden from aarch32 mode, where it is not visible. > + * > + * TODO: ARMv8.4-DIT -- need to move SS somewhere else. > + */ > + return cpsr_read(env) & ~(CPSR_EXEC | PSTATE_SS); So previously we were masking out [23:21], and now we only mask out [21]. Is this OK because we've now masked everywhere that might have been able to write non-zero to [23:22] ? (regarding the TODO comment, I guess the obvious place would be env->pstate.) > } > > void HELPER(cpsr_write)(CPUARMState *env, uint32_t val, uint32_t mask) > -- > 2.20.1 thanks -- PMM
On 2/7/20 5:36 PM, Peter Maydell wrote: >> - return cpsr_read(env) & ~(CPSR_EXEC | CPSR_RESERVED); >> + /* >> + * We store the ARMv8 PSTATE.SS bit in env->uncached_cpsr. >> + * This is convenient for populating SPSR_ELx, but must be >> + * hidden from aarch32 mode, where it is not visible. >> + * >> + * TODO: ARMv8.4-DIT -- need to move SS somewhere else. >> + */ >> + return cpsr_read(env) & ~(CPSR_EXEC | PSTATE_SS); > > So previously we were masking out [23:21], and now we only mask > out [21]. Is this OK because we've now masked everywhere that > might have been able to write non-zero to [23:22] ? Yes. On the chance that I've missed one, we'll now call anything that fails to do so a bug there, and not here. ;-) > (regarding the TODO comment, I guess the obvious place would > be env->pstate.) That was my thought too. That env->pstate & PSTATE_SS would be where we leave that bit all of the time, even when the rest of pstate is inactive in aa32 mode. r~
diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 694b074298..c6dff1d55b 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1186,12 +1186,6 @@ void pmu_init(ARMCPU *cpu); #define CPSR_IT_2_7 (0xfc00U) #define CPSR_GE (0xfU << 16) #define CPSR_IL (1U << 20) -/* Note that the RESERVED bits include bit 21, which is PSTATE_SS in - * an AArch64 SPSR but RES0 in AArch32 SPSR and CPSR. In QEMU we use - * env->uncached_cpsr bit 21 to store PSTATE.SS when executing in AArch32, - * where it is live state but not accessible to the AArch32 code. - */ -#define CPSR_RESERVED (0x7U << 21) #define CPSR_J (1U << 24) #define CPSR_IT_0_1 (3U << 25) #define CPSR_Q (1U << 27) diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index acf1815ea3..af3020b78f 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -387,7 +387,14 @@ void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome) uint32_t HELPER(cpsr_read)(CPUARMState *env) { - return cpsr_read(env) & ~(CPSR_EXEC | CPSR_RESERVED); + /* + * We store the ARMv8 PSTATE.SS bit in env->uncached_cpsr. + * This is convenient for populating SPSR_ELx, but must be + * hidden from aarch32 mode, where it is not visible. + * + * TODO: ARMv8.4-DIT -- need to move SS somewhere else. + */ + return cpsr_read(env) & ~(CPSR_EXEC | PSTATE_SS); } void HELPER(cpsr_write)(CPUARMState *env, uint32_t val, uint32_t mask)
The only remaining use was in op_helper.c. Use PSTATE_SS directly, and move the commentary so that it is more obvious what is going on. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/cpu.h | 6 ------ target/arm/op_helper.c | 9 ++++++++- 2 files changed, 8 insertions(+), 7 deletions(-)