On Wed, 2 Nov 2022 at 05:47, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> Reversed the sense of non-secure in get_phys_addr_lpae,
> and failed to initialize attrs.secure for ARMMMUIdx_Phys_S.
>
> Fixes: 48da29e4 ("target/arm: Add ptw_idx to S1Translate")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1293
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> target/arm/ptw.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 58a7bbda50..df3573f150 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -1357,7 +1357,7 @@ 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) {
> /*
> * Stage2_S -> Stage2 or Phys_S -> Phys_NS
> * Assert that the non-secure idx are even, and relative order.
> @@ -2671,6 +2671,13 @@ static bool get_phys_addr_with_struct(CPUARMState
> *env, S1Translate *ptw,
> bool is_secure = ptw->in_secure;
> ARMMMUIdx s1_mmu_idx;
>
> + /*
> + * The page table entries may downgrade secure to non-secure, but
> + * cannot upgrade an non-secure translation regime's attributes
> + * to secure.
> + */
> + result->f.attrs.secure = is_secure;
> +
> switch (mmu_idx) {
> case ARMMMUIdx_Phys_S:
> case ARMMMUIdx_Phys_NS:
> @@ -2712,12 +2719,6 @@ static bool get_phys_addr_with_struct(CPUARMState
> *env, S1Translate *ptw,
> break;
> }
>
> - /*
> - * The page table entries may downgrade secure to non-secure, but
> - * cannot upgrade an non-secure translation regime's attributes
> - * to secure.
> - */
> - result->f.attrs.secure = is_secure;
> result->f.attrs.user = regime_is_user(env, mmu_idx);
Do we also need to move this setting of attrs.user ?
get_phys_addr_disabled() doesn't set that either.
thanks
-- PMM
