On Thu May 2, 2024 at 9:43 AM AEST, BALATON Zoltan wrote:
> Repurpose get_segment_6xx_tlb() to do the whole address translation
> for POWERPC_MMU_SOFT_6xx MMU model by moving the BAT check there and
> renaming it to match other similar functions. These are only called
> once together so no need to keep these separate functions and
> combining them simplifies the caller allowing further restructuring.
Looks good...
>
> Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
> ---
> target/ppc/mmu_common.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
> index 98730035b1..ef1669b01d 100644
> --- a/target/ppc/mmu_common.c
> +++ b/target/ppc/mmu_common.c
> @@ -359,19 +359,25 @@ static int get_bat_6xx_tlb(CPUPPCState *env, mmu_ctx_t
> *ctx,
> return ret;
> }
>
> -/* Perform segment based translation */
> -static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
> - target_ulong eaddr, MMUAccessType access_type,
> - int type)
> +static int mmu6xx_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
> + target_ulong eaddr,
> + MMUAccessType access_type, int type)
> {
> PowerPCCPU *cpu = env_archcpu(env);
> hwaddr hash;
> - target_ulong vsid;
> - int ds, target_page_bits;
> + target_ulong vsid, sr, pgidx;
> bool pr;
> - int ret;
> - target_ulong sr, pgidx;
> + int ds, target_page_bits, ret = -1;
>
> + /* First try to find a BAT entry if there are any */
> + if (env->nb_BATs != 0) {
> + ret = get_bat_6xx_tlb(env, ctx, eaddr, access_type);
> + }
> + if (ret >= 0) {
> + return ret;
> + }
Would you consider not doing any rearranging of local variables there
and change this as:
/* First try to find a BAT entry if there are any */
if (env->nb_BATs != 0) {
int ret = get_bat_6xx_tlb(env, ctx, eaddr, access_type);
if (ret >= 0) {
return ret;
}
}
Otherwise,
Reviewed-by: Nicholas Piggin <npig...@gmail.com>
> + /* Perform segment based translation when no BATs matched */
> pr = FIELD_EX64(env->msr, MSR, PR);
> ctx->eaddr = eaddr;
>
> @@ -1193,14 +1199,8 @@ int get_physical_address_wtlb(CPUPPCState *env,
> mmu_ctx_t *ctx,
> if (real_mode) {
> ret = check_physical(env, ctx, eaddr, access_type);
> } else {
> - /* Try to find a BAT */
> - if (env->nb_BATs != 0) {
> - ret = get_bat_6xx_tlb(env, ctx, eaddr, access_type);
> - }
> - if (ret < 0) {
> - /* We didn't match any BAT entry or don't have BATs */
> - ret = get_segment_6xx_tlb(env, ctx, eaddr, access_type,
> type);
> - }
> + ret = mmu6xx_get_physical_address(env, ctx, eaddr, access_type,
> + type);
> }
> break;
>
