| Message ID | 20240405180232.3570066-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-9.0] target/arm: Use correct SecuritySpace for AArch64 AT ops at EL3 | expand |
On 4/5/24 08:02, Peter Maydell wrote: > When we do an AT address translation operation, the page table walk > is supposed to be performed in the context of the EL we're doing the > walk for, so for instance an AT S1E2R walk is done for EL2. In the > pseudocode an EL is passed to AArch64.AT(), which calls > SecurityStateAtEL() to find the security state that we should be > doing the walk with. > > In ats_write64() we get this wrong, instead using the current > security space always. This is fine for AT operations performed from > EL1 and EL2, because there the current security state and the > security state for the lower EL are the same. But for AT operations > performed from EL3, the current security state is always either > Secure or Root, whereas we want to use the security state defined by > SCR_EL3.{NS,NSE} for the walk. This affects not just guests using > FEAT_RME but also ones where EL3 is Secure state and the EL3 code > is trying to do an AT for a NonSecure EL2 or EL1. > > Use arm_security_space_below_el3() to get the SecuritySpace to > pass to do_ats_write() for all AT operations except the > AT S1E3* operations. > > Cc:qemu-stable@nongnu.org > Fixes: e1ee56ec2383 ("target/arm: Pass security space rather than flag for AT instructions") > Resolves:https://gitlab.com/qemu-project/qemu/-/issues/2250 > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > I guess most people don't run guest code at EL3 that does AT ops... > > target/arm/helper.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/arm/helper.c b/target/arm/helper.c index 3f3a5b55d4a..0af4ce2e8a7 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -3878,6 +3878,8 @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri, ARMMMUIdx mmu_idx; uint64_t hcr_el2 = arm_hcr_el2_eff(env); bool regime_e20 = (hcr_el2 & (HCR_E2H | HCR_TGE)) == (HCR_E2H | HCR_TGE); + bool for_el3 = false; + ARMSecuritySpace ss; switch (ri->opc2 & 6) { case 0: @@ -3895,6 +3897,7 @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri, break; case 6: /* AT S1E3R, AT S1E3W */ mmu_idx = ARMMMUIdx_E3; + for_el3 = true; break; default: g_assert_not_reached(); @@ -3913,8 +3916,8 @@ static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri, g_assert_not_reached(); } - env->cp15.par_el[1] = do_ats_write(env, value, access_type, - mmu_idx, arm_security_space(env)); + ss = for_el3 ? arm_security_space(env) : arm_security_space_below_el3(env); + env->cp15.par_el[1] = do_ats_write(env, value, access_type, mmu_idx, ss); #else /* Handled by hardware accelerator. */ g_assert_not_reached();
When we do an AT address translation operation, the page table walk is supposed to be performed in the context of the EL we're doing the walk for, so for instance an AT S1E2R walk is done for EL2. In the pseudocode an EL is passed to AArch64.AT(), which calls SecurityStateAtEL() to find the security state that we should be doing the walk with. In ats_write64() we get this wrong, instead using the current security space always. This is fine for AT operations performed from EL1 and EL2, because there the current security state and the security state for the lower EL are the same. But for AT operations performed from EL3, the current security state is always either Secure or Root, whereas we want to use the security state defined by SCR_EL3.{NS,NSE} for the walk. This affects not just guests using FEAT_RME but also ones where EL3 is Secure state and the EL3 code is trying to do an AT for a NonSecure EL2 or EL1. Use arm_security_space_below_el3() to get the SecuritySpace to pass to do_ats_write() for all AT operations except the AT S1E3* operations. Cc: qemu-stable@nongnu.org Fixes: e1ee56ec2383 ("target/arm: Pass security space rather than flag for AT instructions") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2250 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- I guess most people don't run guest code at EL3 that does AT ops... target/arm/helper.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)