Re: [PATCH v2 03/10] target/ppc: Push real-mode handling into ppc_radix64_xlate

[David Gibson]

On Mon, Jun 21, 2021 at 09:51:08AM -0300, Bruno Larsen (billionai) wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> This removes some incomplete duplication between
> ppc_radix64_handle_mmu_fault and ppc_radix64_get_phys_page_debug.
> The former was correct wrt SPR_HRMOR and the latter was not.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Applied to ppc-for-6.1, thanks.

> ---
>  target/ppc/mmu-radix64.c | 77 ++++++++++++++++++----------------------
>  1 file changed, 34 insertions(+), 43 deletions(-)
> 
> diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> index 1c707d387d..dd5ae69052 100644
> --- a/target/ppc/mmu-radix64.c
> +++ b/target/ppc/mmu-radix64.c
> @@ -465,7 +465,6 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU 
> *cpu,
>   */
>  static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
>                               MMUAccessType access_type,
> -                             bool relocation,
>                               hwaddr *raddr, int *psizep, int *protp,
>                               bool guest_visible)
>  {
> @@ -474,6 +473,37 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr 
> eaddr,
>      ppc_v3_pate_t pate;
>      int psize, prot;
>      hwaddr g_raddr;
> +    bool relocation;
> +
> +    assert(!(msr_hv && cpu->vhyp));
> +
> +    relocation = (access_type == MMU_INST_FETCH ? msr_ir : msr_dr);
> +
> +    /* HV or virtual hypervisor Real Mode Access */
> +    if (!relocation && (msr_hv || cpu->vhyp)) {
> +        /* In real mode top 4 effective addr bits (mostly) ignored */
> +        *raddr = eaddr & 0x0FFFFFFFFFFFFFFFULL;
> +
> +        /* In HV mode, add HRMOR if top EA bit is clear */
> +        if (msr_hv || !env->has_hv_mode) {

Not in scope, because this is a code motion, but that test looks
bogus.  If we don't have an HV mode, we won't have an HRMOR either.

> +            if (!(eaddr >> 63)) {
> +                *raddr |= env->spr[SPR_HRMOR];
> +           }
> +        }
> +        *protp = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> +        *psizep = TARGET_PAGE_BITS;
> +        return 0;
> +    }
> +
> +    /*
> +     * Check UPRT (we avoid the check in real mode to deal with
> +     * transitional states during kexec.
> +     */
> +    if (guest_visible && !ppc64_use_proc_tbl(cpu)) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "LPCR:UPRT not set in radix mode ! LPCR="
> +                      TARGET_FMT_lx "\n", env->spr[SPR_LPCR]);
> +    }
>  
>      /* Virtual Mode Access - get the fully qualified address */
>      if (!ppc_radix64_get_fully_qualified_addr(&cpu->env, eaddr, &lpid, 
> &pid)) {
> @@ -559,43 +589,11 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr 
> eaddr,
>                                   MMUAccessType access_type, int mmu_idx)
>  {
>      CPUState *cs = CPU(cpu);
> -    CPUPPCState *env = &cpu->env;
>      int page_size, prot;
> -    bool relocation;
>      hwaddr raddr;
>  
> -    assert(!(msr_hv && cpu->vhyp));
> -
> -    relocation = (access_type == MMU_INST_FETCH ? msr_ir : msr_dr);
> -    /* HV or virtual hypervisor Real Mode Access */
> -    if (!relocation && (msr_hv || cpu->vhyp)) {
> -        /* In real mode top 4 effective addr bits (mostly) ignored */
> -        raddr = eaddr & 0x0FFFFFFFFFFFFFFFULL;
> -
> -        /* In HV mode, add HRMOR if top EA bit is clear */
> -        if (msr_hv || !env->has_hv_mode) {
> -            if (!(eaddr >> 63)) {
> -                raddr |= env->spr[SPR_HRMOR];
> -           }
> -        }
> -        tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> -                     PAGE_READ | PAGE_WRITE | PAGE_EXEC, mmu_idx,
> -                     TARGET_PAGE_SIZE);
> -        return 0;
> -    }
> -
> -    /*
> -     * Check UPRT (we avoid the check in real mode to deal with
> -     * transitional states during kexec.
> -     */
> -    if (!ppc64_use_proc_tbl(cpu)) {
> -        qemu_log_mask(LOG_GUEST_ERROR,
> -                      "LPCR:UPRT not set in radix mode ! LPCR="
> -                      TARGET_FMT_lx "\n", env->spr[SPR_LPCR]);
> -    }
> -
>      /* Translate eaddr to raddr (where raddr is addr qemu needs for access) 
> */
> -    if (ppc_radix64_xlate(cpu, eaddr, access_type, relocation, &raddr,
> +    if (ppc_radix64_xlate(cpu, eaddr, access_type, &raddr,
>                            &page_size, &prot, true)) {
>          return 1;
>      }
> @@ -607,18 +605,11 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr 
> eaddr,
>  
>  hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
>  {
> -    CPUPPCState *env = &cpu->env;
>      int psize, prot;
>      hwaddr raddr;
>  
> -    /* Handle Real Mode */
> -    if ((msr_dr == 0) && (msr_hv || cpu->vhyp)) {
> -        /* In real mode top 4 effective addr bits (mostly) ignored */
> -        return eaddr & 0x0FFFFFFFFFFFFFFFULL;
> -    }
> -
> -    if (ppc_radix64_xlate(cpu, eaddr, 0, msr_dr, &raddr, &psize,
> -                          &prot, false)) {
> +    if (ppc_radix64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
> +                          &psize, &prot, false)) {
>          return -1;
>      }
>  

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature