| Message ID | 20200203144716.32204-15-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 PAN bit is preserved, or set as per SCTLR_ELx.SPAN, > plus several other conditions listed in the ARM ARM. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > v2: Tidy preservation of CPSR_PAN in take_aarch32_exception (pmm). > --- > target/arm/helper.c | 40 +++++++++++++++++++++++++++++++++++++--- > 1 file changed, 37 insertions(+), 3 deletions(-) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index 18e4cbb63c..4c0eb7e7d9 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -8772,8 +8772,12 @@ static void take_aarch32_exception(CPUARMState *env, int new_mode, > uint32_t mask, uint32_t offset, > uint32_t newpc) > { > + int new_el; > + > /* Change the CPU state so as to actually take the exception. */ > switch_mode(env, new_mode); > + new_el = arm_current_el(env); > + > /* > * For exceptions taken to AArch32 we must clear the SS bit in both > * PSTATE and in the old-state value we save to SPSR_<mode>, so zero it now. > @@ -8786,7 +8790,7 @@ static void take_aarch32_exception(CPUARMState *env, int new_mode, > env->uncached_cpsr = (env->uncached_cpsr & ~CPSR_M) | new_mode; > /* Set new mode endianness */ > env->uncached_cpsr &= ~CPSR_E; > - if (env->cp15.sctlr_el[arm_current_el(env)] & SCTLR_EE) { > + if (env->cp15.sctlr_el[new_el] & SCTLR_EE) { > env->uncached_cpsr |= CPSR_E; > } > /* J and IL must always be cleared for exception entry */ > @@ -8797,6 +8801,12 @@ static void take_aarch32_exception(CPUARMState *env, int new_mode, > env->thumb = (env->cp15.sctlr_el[2] & SCTLR_TE) != 0; > env->elr_el[2] = env->regs[15]; > } else { > + /* CPSR.PAN is preserved unless target is EL1 and SCTLR.SPAN == 0. */ > + if (cpu_isar_feature(aa64_pan, env_archcpu(env)) > + && new_el == 1 > + && !(env->cp15.sctlr_el[1] & SCTLR_SPAN)) { > + env->uncached_cpsr |= CPSR_PAN; > + } This doesn't catch the "taking exception to EL3 and AArch32 is EL3" case, which is also supposed to honour SCTLR.SPAN. Given where this code is, we know we're taking an exception to AArch32 and that we're not going to Hyp mode, so in fact every case where we get here is one where we should honour SCTLR.SPAN and I think we can just drop the "new_el == 1" part of the condition. Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 2/7/20 6:01 PM, Peter Maydell wrote: >> + /* CPSR.PAN is preserved unless target is EL1 and SCTLR.SPAN == 0. */ >> + if (cpu_isar_feature(aa64_pan, env_archcpu(env)) >> + && new_el == 1 >> + && !(env->cp15.sctlr_el[1] & SCTLR_SPAN)) { >> + env->uncached_cpsr |= CPSR_PAN; >> + } > This doesn't catch the "taking exception to EL3 and AArch32 is EL3" > case, which is also supposed to honour SCTLR.SPAN. > > Given where this code is, we know we're taking an exception to > AArch32 and that we're not going to Hyp mode, so in fact every > case where we get here is one where we should honour SCTLR.SPAN > and I think we can just drop the "new_el == 1" part of the condition. Presumably that becomes env->cp15.sctlr_el[new_el] as well, so that we get the secure version of the sctlr. r~
On 2/8/20 8:45 AM, Richard Henderson wrote: > On 2/7/20 6:01 PM, Peter Maydell wrote: >>> + /* CPSR.PAN is preserved unless target is EL1 and SCTLR.SPAN == 0. */ >>> + if (cpu_isar_feature(aa64_pan, env_archcpu(env)) >>> + && new_el == 1 >>> + && !(env->cp15.sctlr_el[1] & SCTLR_SPAN)) { >>> + env->uncached_cpsr |= CPSR_PAN; >>> + } >> This doesn't catch the "taking exception to EL3 and AArch32 is EL3" >> case, which is also supposed to honour SCTLR.SPAN. >> >> Given where this code is, we know we're taking an exception to >> AArch32 and that we're not going to Hyp mode, so in fact every >> case where we get here is one where we should honour SCTLR.SPAN >> and I think we can just drop the "new_el == 1" part of the condition. > > Presumably that becomes env->cp15.sctlr_el[new_el] as well, so that we get the > secure version of the sctlr. Actually, there's another clause that I missed before: # When the target of the exception is EL3, from Non-secure # state, this bit is set to 0 regardless # of the value of the Secure SCTLR.SPAN bit. See G8.2.33. Will fix for v4. r~
diff --git a/target/arm/helper.c b/target/arm/helper.c index 18e4cbb63c..4c0eb7e7d9 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -8772,8 +8772,12 @@ static void take_aarch32_exception(CPUARMState *env, int new_mode, uint32_t mask, uint32_t offset, uint32_t newpc) { + int new_el; + /* Change the CPU state so as to actually take the exception. */ switch_mode(env, new_mode); + new_el = arm_current_el(env); + /* * For exceptions taken to AArch32 we must clear the SS bit in both * PSTATE and in the old-state value we save to SPSR_<mode>, so zero it now. @@ -8786,7 +8790,7 @@ static void take_aarch32_exception(CPUARMState *env, int new_mode, env->uncached_cpsr = (env->uncached_cpsr & ~CPSR_M) | new_mode; /* Set new mode endianness */ env->uncached_cpsr &= ~CPSR_E; - if (env->cp15.sctlr_el[arm_current_el(env)] & SCTLR_EE) { + if (env->cp15.sctlr_el[new_el] & SCTLR_EE) { env->uncached_cpsr |= CPSR_E; } /* J and IL must always be cleared for exception entry */ @@ -8797,6 +8801,12 @@ static void take_aarch32_exception(CPUARMState *env, int new_mode, env->thumb = (env->cp15.sctlr_el[2] & SCTLR_TE) != 0; env->elr_el[2] = env->regs[15]; } else { + /* CPSR.PAN is preserved unless target is EL1 and SCTLR.SPAN == 0. */ + if (cpu_isar_feature(aa64_pan, env_archcpu(env)) + && new_el == 1 + && !(env->cp15.sctlr_el[1] & SCTLR_SPAN)) { + env->uncached_cpsr |= CPSR_PAN; + } /* * this is a lie, as there was no c1_sys on V4T/V5, but who cares * and we should just guard the thumb mode on V4 @@ -9059,6 +9069,7 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs) unsigned int new_el = env->exception.target_el; target_ulong addr = env->cp15.vbar_el[new_el]; unsigned int new_mode = aarch64_pstate_mode(new_el, true); + unsigned int old_mode; unsigned int cur_el = arm_current_el(env); /* @@ -9138,20 +9149,43 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs) } if (is_a64(env)) { - env->banked_spsr[aarch64_banked_spsr_index(new_el)] = pstate_read(env); + old_mode = pstate_read(env); aarch64_save_sp(env, arm_current_el(env)); env->elr_el[new_el] = env->pc; } else { - env->banked_spsr[aarch64_banked_spsr_index(new_el)] = cpsr_read(env); + old_mode = cpsr_read(env); env->elr_el[new_el] = env->regs[15]; aarch64_sync_32_to_64(env); env->condexec_bits = 0; } + env->banked_spsr[aarch64_banked_spsr_index(new_el)] = old_mode; + qemu_log_mask(CPU_LOG_INT, "...with ELR 0x%" PRIx64 "\n", env->elr_el[new_el]); + if (cpu_isar_feature(aa64_pan, cpu)) { + /* The value of PSTATE.PAN is normally preserved, except when ... */ + new_mode |= old_mode & PSTATE_PAN; + switch (new_el) { + case 2: + /* ... the target is EL2 with HCR_EL2.{E2H,TGE} == '11' ... */ + if ((arm_hcr_el2_eff(env) & (HCR_E2H | HCR_TGE)) + != (HCR_E2H | HCR_TGE)) { + break; + } + /* fall through */ + case 1: + /* ... the target is EL1 ... */ + /* ... and SCTLR_ELx.SPAN == 0, then set to 1. */ + if ((env->cp15.sctlr_el[new_el] & SCTLR_SPAN) == 0) { + new_mode |= PSTATE_PAN; + } + break; + } + } + pstate_write(env, PSTATE_DAIF | new_mode); env->aarch64 = 1; aarch64_restore_sp(env, new_el);
The PAN bit is preserved, or set as per SCTLR_ELx.SPAN, plus several other conditions listed in the ARM ARM. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- v2: Tidy preservation of CPSR_PAN in take_aarch32_exception (pmm). --- target/arm/helper.c | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-)