Pass the correct stage2 mmu_idx to regime_translation_disabled,
which we computed afterward.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/ptw.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index dbe5852af6..680139b478 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -211,11 +211,10 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
                                ARMMMUFaultInfo *fi)
 {
     bool is_secure = *is_secure_ptr;
+    ARMMMUIdx s2_mmu_idx = is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
 
     if (arm_mmu_idx_is_stage1_of_2(mmu_idx) &&
-        !regime_translation_disabled(env, ARMMMUIdx_Stage2, is_secure)) {
-        ARMMMUIdx s2_mmu_idx = is_secure ? ARMMMUIdx_Stage2_S
-                                         : ARMMMUIdx_Stage2;
+        !regime_translation_disabled(env, s2_mmu_idx, is_secure)) {
         GetPhysAddrResult s2 = {};
         int ret;
 
-- 
2.34.1

On Mon, 22 Aug 2022 at 17:23, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Pass the correct stage2 mmu_idx to regime_translation_disabled,
> which we computed afterward.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/ptw.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index dbe5852af6..680139b478 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -211,11 +211,10 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
>                                 ARMMMUFaultInfo *fi)
>  {
>      bool is_secure = *is_secure_ptr;
> +    ARMMMUIdx s2_mmu_idx = is_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
>
>      if (arm_mmu_idx_is_stage1_of_2(mmu_idx) &&
> -        !regime_translation_disabled(env, ARMMMUIdx_Stage2, is_secure)) {
> -        ARMMMUIdx s2_mmu_idx = is_secure ? ARMMMUIdx_Stage2_S
> -                                         : ARMMMUIdx_Stage2;
> +        !regime_translation_disabled(env, s2_mmu_idx, is_secure)) {
>          GetPhysAddrResult s2 = {};
>          int ret;


This doesn't actually change the behaviour, though, right?
regime_translation_disabled() at this point in the patchset doesn't
do anything that makes a distinction between Stage2_S and Stage2 AFAICT.
So this is just making the code clearer; we fixed the actual bug in patch 19.

With a tweaked commit message,
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Alternatively, pull this patch earlier so it's before patch 19 and
is the one fixing the bug; then patch 19 is only adding the
is_secure argument to regime_translation_disabled() and doesn't
fix the bug in passing. That would be neater but maybe the
patchset reshuffle is too painful so I don't insist on it.

thanks
-- PMM

On 9/20/22 09:01, Peter Maydell wrote:
> This doesn't actually change the behaviour, though, right?
> regime_translation_disabled() at this point in the patchset doesn't
> do anything that makes a distinction between Stage2_S and Stage2 AFAICT.
> So this is just making the code clearer; we fixed the actual bug in patch 19.
> 
> With a tweaked commit message,
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> Alternatively, pull this patch earlier so it's before patch 19 and
> is the one fixing the bug; then patch 19 is only adding the
> is_secure argument to regime_translation_disabled() and doesn't
> fix the bug in passing. That would be neater but maybe the
> patchset reshuffle is too painful so I don't insist on it.

It wasn't too bad to reshuffle.

r~