[PATCH v2 0/2] target/arm: Fix handling of VSTCR_EL2.SW and VTCR_EL2.NSW

Peter Maydell posted 2 patches 12 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230504135425.2748672-1-peter.maydell@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
target/arm/ptw.c | 81 ++++++++++++++++++++++++++++++++----------------
1 file changed, 54 insertions(+), 27 deletions(-)
[PATCH v2 0/2] target/arm: Fix handling of VSTCR_EL2.SW and VTCR_EL2.NSW
Posted by Peter Maydell 12 months ago
When FEAT_SEL2 (secure EL2) is implemented, the bits
VSTCR_EL2.SW and VTCR_EL2.NSW allow the guest to set things up
so that the stage 2 walk for an IPA is done to the other
address space, eg
 * a stage 2 walk for an NS IPA done to secure physical memory
   (where the translation table base address and other parameters
   for the walk come from the NS control registers VTTBR_EL2
   and VTCR_EL2)
 * a stage 2 walk for an S IPA done to non-secure physical memory
   (where the parameters from the walk come from the S control
   registers VSTTBR_EL2 and VSTCR_EL2)
We tried to implement this, but didn't get it right. In particular
the code is somewhat confused about whether it should handle
SW/NSW before doing a stage 2 walk (it does this for the s2
walk on the result of the s1 walk) or after doing a stage 2
walk (it does this for the s2 walks it does for s1 ptw loads).

Version 1 of this patchseries seemed to fix the reported bug,
but after more thought about this area of the code I think
it wasn't really completely addressing the issue. In particular
I suspect that in cases where we cache the result in an S2 TLB
we might not DTRT when we hit in the cache later.

So in v2 I've addressed the problem in a somewhat different way:

(1) when we call get_phys_addr_lpae() to do a stage 2 walk we
need to consistently get the ptw parameters right:
 * .in_ptw_idx should be ptw_idx_for_stage_2() of the .in_mmu_idx
   (where ptw_idx_for_stage_2() is a new function that determines
   whether we should be loading descriptors from S or NS, based
   on among other things the SW and NSW bits)
 * .in_secure should be true if .in_mmu_idx is Stage2_S

(2) S1_ptw_translate() should not do anything with the SW/NSW bits;
instead it just says "do an S2 walk" and trusts that the
(security state, address) tuple it effectively gets back from
that walk is the correct one.

This fixes https://gitlab.com/qemu-project/qemu/-/issues/1600 .

Changes v1->v2:
 * patch 1 is the same (and has been reviewed)
 * patch 2 is entirely different

Peter Maydell (2):
  target/arm: Don't allow stage 2 page table walks to downgrade to NS
  target/arm: Fix handling of SW and NSW bits for stage 2 walks

 target/arm/ptw.c | 81 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 54 insertions(+), 27 deletions(-)

-- 
2.34.1