| Message ID | 20230714154648.327466-8-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm/ptw: Cleanups and a few bugfixes | expand |
On 7/14/23 16:46, Peter Maydell wrote: > When we do a translation in Secure state, the NSTable bits in table > descriptors may downgrade us to NonSecure; we update ptw->in_secure > and ptw->in_space accordingly. We guard that check correctly with a > conditional that means it's only applied for Secure stage 1 > translations. However, later on in get_phys_addr_lpae() we fold the > effects of the NSTable bits into the final descriptor attributes > bits, and there we do it unconditionally regardless of the CPU state. > That means that in Realm state (where in_secure is false) we will set > bit 5 in attrs, and later use it to decide to output to non-secure > space. > > We don't in fact need to do this folding in at all any more (since > commit 2f1ff4e7b9f30c): if an NSTable bit was set then we have > already set ptw->in_space to ARMSS_NonSecure, and in that situation > we don't look at attrs bit 5. The only thing we still need to deal > with is the real NS bit in the final descriptor word, so we can just > drop the code that ORed in the NSTable bit. > > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > target/arm/ptw.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 9e45160e1ba..c30d3fe69a0 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -1884,11 +1884,10 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, * Extract attributes from the (modified) descriptor, and apply * table descriptors. Stage 2 table descriptors do not include * any attribute fields. HPD disables all the table attributes - * except NSTable. + * except NSTable (which we have already handled). */ attrs = new_descriptor & (MAKE_64BIT_MASK(2, 10) | MAKE_64BIT_MASK(50, 14)); if (!regime_is_stage2(mmu_idx)) { - attrs |= !ptw->in_secure << 5; /* NS */ if (!param.hpd) { attrs |= extract64(tableattrs, 0, 2) << 53; /* XN, PXN */ /*
When we do a translation in Secure state, the NSTable bits in table descriptors may downgrade us to NonSecure; we update ptw->in_secure and ptw->in_space accordingly. We guard that check correctly with a conditional that means it's only applied for Secure stage 1 translations. However, later on in get_phys_addr_lpae() we fold the effects of the NSTable bits into the final descriptor attributes bits, and there we do it unconditionally regardless of the CPU state. That means that in Realm state (where in_secure is false) we will set bit 5 in attrs, and later use it to decide to output to non-secure space. We don't in fact need to do this folding in at all any more (since commit 2f1ff4e7b9f30c): if an NSTable bit was set then we have already set ptw->in_space to ARMSS_NonSecure, and in that situation we don't look at attrs bit 5. The only thing we still need to deal with is the real NS bit in the final descriptor word, so we can just drop the code that ORed in the NSTable bit. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/ptw.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)