On Wed, 19 Jul 2023 at 16:56, Jean-Philippe Brucker <jean-phili...@linaro.org> wrote: > > In realm state, stage-2 translation tables are fetched from the realm > physical address space (R_PGRQD). > > Signed-off-by: Jean-Philippe Brucker <jean-phili...@linaro.org> > --- > target/arm/ptw.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/target/arm/ptw.c b/target/arm/ptw.c > index d1de934702..6318e13b98 100644 > --- a/target/arm/ptw.c > +++ b/target/arm/ptw.c > @@ -164,7 +164,11 @@ static ARMMMUIdx ptw_idx_for_stage_2(CPUARMState *env, > ARMMMUIdx stage2idx) > * an NS stage 1+2 lookup while the NS bit is 0.) > */ > if (!arm_is_secure_below_el3(env) || !arm_el_is_aa64(env, 3)) { > - return ARMMMUIdx_Phys_NS; > + if (arm_security_space_below_el3(env) == ARMSS_Realm) { > + return ARMMMUIdx_Phys_Realm; > + } else { > + return ARMMMUIdx_Phys_NS; > + } > } > if (stage2idx == ARMMMUIdx_Stage2_S) { > s2walk_secure = !(env->cp15.vstcr_el2 & VSTCR_SW);
This isn't wrong, but arm_is_secure_below_el3() calls arm_security_space_below_el3(), so we kinda duplicate work there. I think we should instead have: if (!arm_el_is_aa64(env, 3)) { return ARMMMUIdx_Phys_NS; } switch (arm_security_space_below_el3(env)) { case ARMSS_NonSecure: return ARMMUIdx_Phys_NS; case ARMSS_Realm: return ARMMMUIdx_Phys_Realm; case ARMSS_Secure: [existing code to look at the SW/NSW bits] return s2walk_secure ? ...; default: g_assert_not_reached(); } The comment above the function also needs tweaking to say "SCR_EL3.NS or SCR_EL3.NSE bits" (we do already do the TLB flush in scr_write). thanks -- PMM