Re: [PATCH v2 2/2] target/arm: Fix handling of SW and NSW bits for stage 2 walks

[Richard Henderson <richard.henderson@linaro.org>]

On 5/4/23 14:54, Peter Maydell wrote:
> We currently don't correctly handle the VSTCR_EL2.SW and VTCR_EL2.NSW
> configuration bits.  These allow configuration of whether the stage 2
> page table walks for Secure IPA and NonSecure IPA should do their
> descriptor reads from Secure or NonSecure physical addresses. (This
> is separate from how the translation table base address and other
> parameters are set: an NS IPA always uses VTTBR_EL2 and VTCR_EL2
> for its base address and walk parameters, regardless of the NSW bit,
> and similarly for Secure.)
> 
> Provide a new function ptw_idx_for_stage_2() which returns the
> MMU index to use for descriptor reads, and use it to set up
> the .in_ptw_idx wherever we call get_phys_addr_lpae().
> 
> For a stage 2 walk, wherever we call get_phys_addr_lpae():
>   * .in_ptw_idx should be ptw_idx_for_stage_2() of the .in_mmu_idx
>   * .in_secure should be true if .in_mmu_idx is Stage2_S
> 
> This allows us to correct S1_ptw_translate() so that it consistently
> always sets its (out_secure, out_phys) to the result it gets from the
> S2 walk (either by calling get_phys_addr_lpae() or by TLB lookup).
> This makes better conceptual sense because the S2 walk should return
> us an (address space, address) tuple, not an address that we then
> randomly assign to S or NS.
> 
> Our previous handling of SW and NSW was broken, so guest code
> trying to use these bits to put the s2 page tables in the "other"
> address space wouldn't work correctly.
> 
> Cc:qemu-stable@nongnu.org
> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1600
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/ptw.c | 76 ++++++++++++++++++++++++++++++++----------------
>   1 file changed, 51 insertions(+), 25 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~