Re: [PATCH v2 06/28] target/ppc/mmu_common.c: Move calculation of a value closer to its usage

["Nicholas Piggin" <npiggin@gmail.com>]

On Thu May 2, 2024 at 9:43 AM AEST, BALATON Zoltan wrote:
> In mmubooke206_check_tlb() prot2 is calculated first but only used
> after an unrelated check that can return before tha value is used.
> Move the calculation after the check, closer to where it is used, to
> keep them together and avoid computing it when not needed.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>

Reviwed-by: Nicholas Piggin <npiggin@gmail.com>

> ---
>  target/ppc/mmu_common.c | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
> index 168ff842a5..b0aca8ec02 100644
> --- a/target/ppc/mmu_common.c
> +++ b/target/ppc/mmu_common.c
> @@ -828,6 +828,18 @@ static int mmubooke206_check_tlb(CPUPPCState *env, ppcmas_tlb_t *tlb,
>  
>  found_tlb:
>  
> +    /* Check the address space and permissions */
> +    if (access_type == MMU_INST_FETCH) {
> +        /* There is no way to fetch code using epid load */
> +        assert(!use_epid);
> +        as = FIELD_EX64(env->msr, MSR, IR);
> +    }
> +
> +    if (as != ((tlb->mas1 & MAS1_TS) >> MAS1_TS_SHIFT)) {
> +        qemu_log_mask(CPU_LOG_MMU, "%s: AS doesn't match\n", __func__);
> +        return -1;
> +    }
> +
>      if (pr) {
>          if (tlb->mas7_3 & MAS3_UR) {
>              prot2 |= PAGE_READ;
> @@ -849,19 +861,6 @@ found_tlb:
>              prot2 |= PAGE_EXEC;
>          }
>      }
> -
> -    /* Check the address space and permissions */
> -    if (access_type == MMU_INST_FETCH) {
> -        /* There is no way to fetch code using epid load */
> -        assert(!use_epid);
> -        as = FIELD_EX64(env->msr, MSR, IR);
> -    }
> -
> -    if (as != ((tlb->mas1 & MAS1_TS) >> MAS1_TS_SHIFT)) {
> -        qemu_log_mask(CPU_LOG_MMU, "%s: AS doesn't match\n", __func__);
> -        return -1;
> -    }
> -
>      *prot = prot2;
>      if (prot2 & prot_for_access_type(access_type)) {
>          qemu_log_mask(CPU_LOG_MMU, "%s: good TLB!\n", __func__);