On Tue, 24 Jan 2023 at 00:01, Richard Henderson <richard.hender...@linaro.org> wrote: > > Test in_space instead of in_secure so that we don't switch > out of Root space. Handle the output space change immediately, > rather than try and combine the NSTable and NS bits later. > > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > --- > target/arm/ptw.c | 31 ++++++++++++++----------------- > 1 file changed, 14 insertions(+), 17 deletions(-) > > diff --git a/target/arm/ptw.c b/target/arm/ptw.c > index c1b0b8e610..ddafb1f329 100644 > --- a/target/arm/ptw.c > +++ b/target/arm/ptw.c > @@ -1240,7 +1240,6 @@ static bool get_phys_addr_lpae(CPUARMState *env, > S1Translate *ptw, > { > ARMCPU *cpu = env_archcpu(env); > ARMMMUIdx mmu_idx = ptw->in_mmu_idx; > - bool is_secure = ptw->in_secure; > int32_t level; > ARMVAParameters param; > uint64_t ttbr; > @@ -1256,7 +1255,6 @@ static bool get_phys_addr_lpae(CPUARMState *env, > S1Translate *ptw, > uint64_t descaddrmask; > bool aarch64 = arm_el_is_aa64(env, el); > uint64_t descriptor, new_descriptor; > - bool nstable; > > /* TODO: This code does not support shareability levels. */ > if (aarch64) { > @@ -1417,29 +1415,29 @@ static bool get_phys_addr_lpae(CPUARMState *env, > S1Translate *ptw, > descaddrmask = MAKE_64BIT_MASK(0, 40); > } > descaddrmask &= ~indexmask_grainsize; > - > - /* > - * Secure accesses start with the page table in secure memory and > - * can be downgraded to non-secure at any step. Non-secure accesses > - * remain non-secure. We implement this by just ORing in the NSTable/NS > - * bits at each step. > - */ > - tableattrs = is_secure ? 0 : (1 << 4); > + tableattrs = 0; > > next_level: > descaddr |= (address >> (stride * (4 - level))) & indexmask; > descaddr &= ~7ULL; > - nstable = extract32(tableattrs, 4, 1); > - if (nstable && ptw->in_secure) { > - /* > - * Stage2_S -> Stage2 or Phys_S -> Phys_NS > - * Assert that the non-secure idx are even, and relative order. > - */ > + > + /* > + * Process the NSTable bit from the previous level. This changes > + * the table address space and the output space from Secure to > + * NonSecure. With RME, the EL3 translation regime does not change > + * from Root to NonSecure. > + */
To check my understanding, this means that the bit that the spec describes as FEAT_RME changing the behaviour of NSTable in the EL3 stage 1 translation regime is implemented by us by having the in_space for EL3 be different for FEAT_RME and not-FEAT_RME ? > + if (extract32(tableattrs, 4, 1) && ptw->in_space == ARMSS_Secure) { > + /* Stage2_S -> Stage2 or Phys_S -> Phys_NS */ > QEMU_BUILD_BUG_ON(ARMMMUIdx_Phys_S + 1 != ARMMMUIdx_Phys_NS); > QEMU_BUILD_BUG_ON(ARMMMUIdx_Stage2_S + 1 != ARMMMUIdx_Stage2); > ptw->in_ptw_idx += 1; > ptw->in_secure = false; > + ptw->in_space = ARMSS_NonSecure; > + result->f.attrs.secure = false; > + result->f.attrs.space = ARMSS_NonSecure; > } > + > if (!S1_ptw_translate(env, ptw, descaddr, fi)) { > goto do_fault; > } > @@ -1542,7 +1540,6 @@ static bool get_phys_addr_lpae(CPUARMState *env, > S1Translate *ptw, > */ > attrs = new_descriptor & (MAKE_64BIT_MASK(2, 10) | MAKE_64BIT_MASK(50, > 14)); > if (!regime_is_stage2(mmu_idx)) { > - attrs |= nstable << 5; /* NS */ This removes the code where we copy the NSTable bit across to attrs, but there's still code below here that assumes it can get the combined NS bit from bit 5 of attrs, isn't there? (It passes it to get_S1prot().) thanks -- PMM