| Message ID | 20220822152741.1617527-32-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Implement FEAT_HAFDBS | expand |
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~
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;
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(-)