Re: [PATCH v3 25/33] target/ppc/mmu_common.c: Remove BookE handling from get_physical_address_wtlb()

[Nicholas Piggin]

On Wed May 8, 2024 at 10:15 AM AEST, BALATON Zoltan wrote:
> This function is no longer called for BookE MMU model so remove parts
> related to it. This has uncovered a few may be used uninitialised
> warnings that are also fixed.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  target/ppc/mmu_common.c | 25 +++++--------------------
>  1 file changed, 5 insertions(+), 20 deletions(-)
>
> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
> index 83dc041a77..788e2bebd5 100644
> --- a/target/ppc/mmu_common.c
> +++ b/target/ppc/mmu_common.c
> @@ -629,12 +629,10 @@ static int mmubooke_get_physical_address(CPUPPCState 
> *env, mmu_ctx_t *ctx,
>          ret = mmubooke_check_tlb(env, tlb, &raddr, &ctx->prot, address,
>                                   access_type, i);
>          if (ret != -1) {
> -            if (ret >= 0) {
> -                ctx->raddr = raddr;
> -            }
>              break;
>          }
>      }
> +    ctx->raddr = raddr;

Not sure if this is quite the right thing to do or if it's just masking
used uninits.

>      qemu_log_mask(CPU_LOG_MMU,
>                    "%s: access %s " TARGET_FMT_lx " => " HWADDR_FMT_plx
>                    " %d %d\n", __func__, ret < 0 ? "refused" : "granted",
> @@ -841,9 +839,6 @@ static int mmubooke206_get_physical_address(CPUPPCState 
> *env, mmu_ctx_t *ctx,
>              ret = mmubooke206_check_tlb(env, tlb, &raddr, &ctx->prot, 
> address,
>                                          access_type, mmu_idx);
>              if (ret != -1) {
> -                if (ret >= 0) {
> -                    ctx->raddr = raddr;
> -                }
>                  goto found_tlb;
>              }
>          }
> @@ -851,6 +846,7 @@ static int mmubooke206_get_physical_address(CPUPPCState 
> *env, mmu_ctx_t *ctx,
>  
>  found_tlb:
>  
> +    ctx->raddr = raddr;
>      qemu_log_mask(CPU_LOG_MMU, "%s: access %s " TARGET_FMT_lx " => "
>                    HWADDR_FMT_plx " %d %d\n", __func__,
>                    ret < 0 ? "refused" : "granted", address, raddr,
> @@ -1107,20 +1103,9 @@ static int get_physical_address_wtlb(CPUPPCState *env, 
> mmu_ctx_t *ctx,
>                                       MMUAccessType access_type, int type,
>                                       int mmu_idx)
>  {
> -    bool real_mode;
> -
> -    if (env->mmu_model == POWERPC_MMU_BOOKE) {
> -        return mmubooke_get_physical_address(env, ctx, eaddr, access_type);
> -    } else if (env->mmu_model == POWERPC_MMU_BOOKE206) {
> -        return mmubooke206_get_physical_address(env, ctx, eaddr, access_type,
> -                                                mmu_idx);
> -    }

Also still don't like how this gets removed here rather than when it's
stopped being called. I know it's a tangle but it would be better to
try squash the would-be warnings first, then put this in the same as
the previous patch.

Add /* quiet used uninit */ warnings if they can't be fixed properly.

Thanks,
Nick

> -
> -    real_mode = (type == ACCESS_CODE) ? !FIELD_EX64(env->msr, MSR, IR)
> -                                      : !FIELD_EX64(env->msr, MSR, DR);
> -    if (real_mode && (env->mmu_model == POWERPC_MMU_SOFT_6xx ||
> -                      env->mmu_model == POWERPC_MMU_SOFT_4xx ||
> -                      env->mmu_model == POWERPC_MMU_REAL)) {
> +    bool real_mode = (type == ACCESS_CODE) ? !FIELD_EX64(env->msr, MSR, IR)
> +                                           : !FIELD_EX64(env->msr, MSR, DR);
> +    if (real_mode) {
>          memset(ctx, 0, sizeof(*ctx));
>          ctx->raddr = eaddr;
>          ctx->prot = PAGE_RWX;