Re: [PATCH 01/22] target/arm: Fix pmsav8 stage2 secure parameter

[Peter Maydell]

On Tue, 24 Jan 2023 at 00:01, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We have computed the security state for the stage2 lookup
> into s2walk_secure -- use it.

We've computed *something* into s2walk_secure, but it
doesn't make much sense for PMSAv8 because the VSTCR_EL2
and VTCR_EL2 registers don't exist there. s2walk_secure
also doesn't sound like something we should be using,
because in PMSAv8 there is no "stage 2 walk" being done (there
are no page tables to walk) -- we just use the information in
the EL2 MPU registers.

I think what we want to be passing to get_phys_addr_pmsav8()
is ipa_secure. It's a bit moot, of course, since PMSAv8 doesn't
have TrustZone and for PMSAv8-32 the security state is always
NonSecure. (For PMSAv8-64 it is always Secure, though...)
This means that ipa == address and ipa_secure == is_secure,
since the stage 1 MPU can't change either the address or the
security state. Passing both ipa and ipa_secure to the stage2
call means we're being consistent, I guess.

> Fixes: fca45e3467f ("target/arm: Add PMSAv8r functionality")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/ptw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 57f3615a66..b0f8c59767 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -2727,7 +2727,7 @@ static bool get_phys_addr_twostage(CPUARMState *env, 
> S1Translate *ptw,
>
>      if (arm_feature(env, ARM_FEATURE_PMSA)) {
>          ret = get_phys_addr_pmsav8(env, ipa, access_type,
> -                                   ptw->in_mmu_idx, is_secure, result, fi);
> +                                   ptw->in_mmu_idx, s2walk_secure, result, 
> fi);
>      } else {
>          ret = get_phys_addr_lpae(env, ptw, ipa, access_type,
>                                   is_el0, result, fi);
> --
> 2.34.1

thanks
-- PMM