| Message ID | 20230124000027.3565716-10-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Implement FEAT_RME | expand |
On Tue, 24 Jan 2023 at 00:02, Richard Henderson <richard.henderson@linaro.org> wrote: > > It will be helpful to have ARMMMUIdx_Phys_* to be in the same > relative order as ARMSecuritySpace enumerators. This requires > the adjustment to the nstable check. While there, check for being > in secure state rather than rely on clearing the low bit making > no change to non-secure state. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/cpu.h | 12 ++++++------ > target/arm/ptw.c | 10 ++++------ > 2 files changed, 10 insertions(+), 12 deletions(-) > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index cfc62d60b0..0114e1ed87 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -3057,18 +3057,18 @@ typedef enum ARMMMUIdx { > ARMMMUIdx_E2 = 6 | ARM_MMU_IDX_A, > ARMMMUIdx_E3 = 7 | ARM_MMU_IDX_A, > > - /* TLBs with 1-1 mapping to the physical address spaces. */ > - ARMMMUIdx_Phys_NS = 8 | ARM_MMU_IDX_A, > - ARMMMUIdx_Phys_S = 9 | ARM_MMU_IDX_A, > - > /* > * Used for second stage of an S12 page table walk, or for descriptor > * loads during first stage of an S1 page table walk. Note that both > * are in use simultaneously for SecureEL2: the security state for > * the S2 ptw is selected by the NS bit from the S1 ptw. > */ > - ARMMMUIdx_Stage2 = 10 | ARM_MMU_IDX_A, > - ARMMMUIdx_Stage2_S = 11 | ARM_MMU_IDX_A, > + ARMMMUIdx_Stage2_S = 8 | ARM_MMU_IDX_A, > + ARMMMUIdx_Stage2 = 9 | ARM_MMU_IDX_A, > + > + /* TLBs with 1-1 mapping to the physical address spaces. */ > + ARMMMUIdx_Phys_S = 10 | ARM_MMU_IDX_A, > + ARMMMUIdx_Phys_NS = 11 | ARM_MMU_IDX_A, > > /* > * These are not allocated TLBs and are used only for AT system > diff --git a/target/arm/ptw.c b/target/arm/ptw.c > index 437f6fefa9..59cf64d0a6 100644 > --- a/target/arm/ptw.c > +++ b/target/arm/ptw.c > @@ -1410,16 +1410,14 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, > descaddr |= (address >> (stride * (4 - level))) & indexmask; > descaddr &= ~7ULL; > nstable = extract32(tableattrs, 4, 1); > - if (nstable) { > + if (nstable && ptw->in_secure) { > /* > * Stage2_S -> Stage2 or Phys_S -> Phys_NS > * Assert that the non-secure idx are even, and relative order. > */ Comment needs updating to match the code change (we're no longer asserting that the NS indexes are even). > - QEMU_BUILD_BUG_ON((ARMMMUIdx_Phys_NS & 1) != 0); > - QEMU_BUILD_BUG_ON((ARMMMUIdx_Stage2 & 1) != 0); > - QEMU_BUILD_BUG_ON(ARMMMUIdx_Phys_NS + 1 != ARMMMUIdx_Phys_S); > - QEMU_BUILD_BUG_ON(ARMMMUIdx_Stage2 + 1 != ARMMMUIdx_Stage2_S); > - ptw->in_ptw_idx &= ~1; > + 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; > } > if (!S1_ptw_translate(env, ptw, descaddr, fi)) { Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
diff --git a/target/arm/cpu.h b/target/arm/cpu.h index cfc62d60b0..0114e1ed87 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -3057,18 +3057,18 @@ typedef enum ARMMMUIdx { ARMMMUIdx_E2 = 6 | ARM_MMU_IDX_A, ARMMMUIdx_E3 = 7 | ARM_MMU_IDX_A, - /* TLBs with 1-1 mapping to the physical address spaces. */ - ARMMMUIdx_Phys_NS = 8 | ARM_MMU_IDX_A, - ARMMMUIdx_Phys_S = 9 | ARM_MMU_IDX_A, - /* * Used for second stage of an S12 page table walk, or for descriptor * loads during first stage of an S1 page table walk. Note that both * are in use simultaneously for SecureEL2: the security state for * the S2 ptw is selected by the NS bit from the S1 ptw. */ - ARMMMUIdx_Stage2 = 10 | ARM_MMU_IDX_A, - ARMMMUIdx_Stage2_S = 11 | ARM_MMU_IDX_A, + ARMMMUIdx_Stage2_S = 8 | ARM_MMU_IDX_A, + ARMMMUIdx_Stage2 = 9 | ARM_MMU_IDX_A, + + /* TLBs with 1-1 mapping to the physical address spaces. */ + ARMMMUIdx_Phys_S = 10 | ARM_MMU_IDX_A, + ARMMMUIdx_Phys_NS = 11 | ARM_MMU_IDX_A, /* * These are not allocated TLBs and are used only for AT system diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 437f6fefa9..59cf64d0a6 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -1410,16 +1410,14 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw, descaddr |= (address >> (stride * (4 - level))) & indexmask; descaddr &= ~7ULL; nstable = extract32(tableattrs, 4, 1); - if (nstable) { + if (nstable && ptw->in_secure) { /* * Stage2_S -> Stage2 or Phys_S -> Phys_NS * Assert that the non-secure idx are even, and relative order. */ - QEMU_BUILD_BUG_ON((ARMMMUIdx_Phys_NS & 1) != 0); - QEMU_BUILD_BUG_ON((ARMMMUIdx_Stage2 & 1) != 0); - QEMU_BUILD_BUG_ON(ARMMMUIdx_Phys_NS + 1 != ARMMMUIdx_Phys_S); - QEMU_BUILD_BUG_ON(ARMMMUIdx_Stage2 + 1 != ARMMMUIdx_Stage2_S); - ptw->in_ptw_idx &= ~1; + 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; } if (!S1_ptw_translate(env, ptw, descaddr, fi)) {
It will be helpful to have ARMMMUIdx_Phys_* to be in the same relative order as ARMSecuritySpace enumerators. This requires the adjustment to the nstable check. While there, check for being in secure state rather than rely on clearing the low bit making no change to non-secure state. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/cpu.h | 12 ++++++------ target/arm/ptw.c | 10 ++++------ 2 files changed, 10 insertions(+), 12 deletions(-)