On 2/10/23 01:36, Peter Maydell wrote:
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 ?
Correct -- space is Secure for non-RME EL3, and Root for RME EL3.
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().)
Oops. This gets fixed in patch 14. Some reordering needed... r~
