Re: [PATCH 4/7] target/ppc: Introduce ppc_radix64_xlate() for Radix tree translation

[David Gibson]

On Mon, Mar 30, 2020 at 05:34:40PM +0200, Cédric Le Goater wrote:
> >>> +        /* No valid pte or access denied due to protection */
> >>> +        if (cause_excp) {
> >>> +            ppc_radix64_raise_si(cpu, rwx, eaddr, fault_cause);
> >>> +        }
> >>> +        return 1;
> >>> +    }
> >>> +
> >>> +    ppc_radix64_set_rc(cpu, rwx, pte, pte_addr, g_prot);
> >>> +
> >>> +    return 0;
> >>> +}
> >>> +
> >>> +static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> >>> +                             uint64_t lpid, uint64_t pid, bool 
> >>> relocation,
> >>> +                             hwaddr *raddr, int *psizep, int *protp,
> >>> +                             bool cause_excp)
> >>> +{
> >>> +    ppc_v3_pate_t pate;
> >>> +    int psize, prot;
> >>> +    hwaddr g_raddr;
> >>> +
> >>> +    *psizep = INT_MAX;
> >>> +    *protp = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> >>> +
> >>> +    /* Get Process Table */
> >>> +    if (cpu->vhyp && lpid == 0) {
> >>
> >> Current code doesn't check lpid == 0. Not sure to see what it's for...
> > 
> > cpu->vhyp means a pseries machine, lpid == 0 means accessing quadrant3, 
> > so it's the kernel.
> 
> Sorry. I misread that. It would pid == 0 for the kernel. 
> 
> So yes, the test cpu->vhyp && lpid == 0 might be a bit overkill, given 
> that lpid is always 0 when running under a QEMU pseries machine.

Overkill and conceptually incorrect.  When in vhyp mode we're not
modelling the hypervisor part of the CPU, which means that really the
LPID doesn't exist, so we shouldn't be looking at it (though it will
always be 0 in practice).

> 
> 
> C.
> 
> > 
> >> especially env->spr[SPR_LPIDR] is always 0 with pseries machine types
> >> AFAICT... is it even possible to have lpid != 0 here ?
> > 
> > When under PowerNV, SPR_LPIDR can be set, but not under pseries.
> > 
> > C.
> > 
> >>
> >>
> >> Rest LGTM.
> >>
> >>> +        PPCVirtualHypervisorClass *vhc;
> >>> +        vhc = PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> >>> +        vhc->get_pate(cpu->vhyp, &pate);
> >>> +    } else {
> >>> +        if (!ppc64_v3_get_pate(cpu, lpid, &pate)) {
> >>> +            if (cause_excp) {
> >>> +                ppc_radix64_raise_si(cpu, rwx, eaddr, DSISR_NOPTE);
> >>> +            }
> >>> +            return 1;
> >>> +        }
> >>> +        if (!validate_pate(cpu, lpid, &pate)) {
> >>> +            if (cause_excp) {
> >>> +                ppc_radix64_raise_si(cpu, rwx, eaddr, DSISR_R_BADCONFIG);
> >>> +            }
> >>> +            return 1;
> >>> +        }
> >>> +        /* We don't support guest mode yet */
> >>> +        if (lpid != 0) {
> >>> +            error_report("PowerNV guest support Unimplemented");
> >>> +            exit(1);
> >>> +        }
> >>> +    }
> >>> +
> >>> +    /*
> >>> +     * Perform process-scoped translation if relocation enabled.
> >>> +     *
> >>> +     * - Translates an effective address to a host real address in
> >>> +     *   quadrants 0 and 3 when HV=1.
> >>> +     */
> >>> +    if (relocation) {
> >>> +        int ret = ppc_radix64_process_scoped_xlate(cpu, rwx, eaddr, 
> >>> lpid, pid,
> >>> +                                                   pate, &g_raddr, &prot,
> >>> +                                                   &psize, cause_excp);
> >>> +        if (ret) {
> >>> +            return ret;
> >>> +        }
> >>> +        *psizep = MIN(*psizep, psize);
> >>> +        *protp &= prot;
> >>> +    } else {
> >>> +        g_raddr = eaddr & R_EADDR_MASK;
> >>> +    }
> >>> +
> >>> +    *raddr = g_raddr;
> >>> +    return 0;
> >>> +}
> >>> +
> >>>  int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> >>>                                   int mmu_idx)
> >>>  {
> >>>      CPUState *cs = CPU(cpu);
> >>>      CPUPPCState *env = &cpu->env;
> >>> -    PPCVirtualHypervisorClass *vhc;
> >>> -    hwaddr raddr, pte_addr;
> >>> -    uint64_t lpid = 0, pid = 0, offset, size, prtbe0, pte;
> >>> -    int page_size, prot, fault_cause = 0;
> >>> -    ppc_v3_pate_t pate;
> >>> +    uint64_t lpid = 0, pid = 0;
> >>> +    int page_size, prot;
> >>>      bool relocation;
> >>> +    hwaddr raddr;
> >>>  
> >>>      assert(!(msr_hv && cpu->vhyp));
> >>>      assert((rwx == 0) || (rwx == 1) || (rwx == 2));
> >>> @@ -268,48 +370,11 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, 
> >>> vaddr eaddr, int rwx,
> >>>          return 1;
> >>>      }
> >>>  
> >>> -    /* Get Process Table */
> >>> -    if (cpu->vhyp) {
> >>> -        vhc = PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> >>> -        vhc->get_pate(cpu->vhyp, &pate);
> >>> -    } else {
> >>> -        if (!ppc64_v3_get_pate(cpu, lpid, &pate)) {
> >>> -            ppc_radix64_raise_si(cpu, rwx, eaddr, DSISR_NOPTE);
> >>> -            return 1;
> >>> -        }
> >>> -        if (!validate_pate(cpu, lpid, &pate)) {
> >>> -            ppc_radix64_raise_si(cpu, rwx, eaddr, DSISR_R_BADCONFIG);
> >>> -        }
> >>> -        /* We don't support guest mode yet */
> >>> -        if (lpid != 0) {
> >>> -            error_report("PowerNV guest support Unimplemented");
> >>> -            exit(1);
> >>> -       }
> >>> -    }
> >>> -
> >>> -    /* Index Process Table by PID to Find Corresponding Process Table 
> >>> Entry */
> >>> -    offset = pid * sizeof(struct prtb_entry);
> >>> -    size = 1ULL << ((pate.dw1 & PATE1_R_PRTS) + 12);
> >>> -    if (offset >= size) {
> >>> -        /* offset exceeds size of the process table */
> >>> -        ppc_radix64_raise_si(cpu, rwx, eaddr, DSISR_NOPTE);
> >>> +    /* Translate eaddr to raddr (where raddr is addr qemu needs for 
> >>> access) */
> >>> +    if (ppc_radix64_xlate(cpu, eaddr, rwx, lpid, pid, relocation, &raddr,
> >>> +                          &page_size, &prot, 1)) {
> >>>          return 1;
> >>>      }
> >>> -    prtbe0 = ldq_phys(cs->as, (pate.dw1 & PATE1_R_PRTB) + offset);
> >>> -
> >>> -    /* Walk Radix Tree from Process Table Entry to Convert EA to RA */
> >>> -    page_size = PRTBE_R_GET_RTS(prtbe0);
> >>> -    pte = ppc_radix64_walk_tree(cpu, eaddr & R_EADDR_MASK,
> >>> -                                prtbe0 & PRTBE_R_RPDB, prtbe0 & 
> >>> PRTBE_R_RPDS,
> >>> -                                &raddr, &page_size, &fault_cause, 
> >>> &pte_addr);
> >>> -    if (!pte || ppc_radix64_check_prot(cpu, rwx, pte, &fault_cause, 
> >>> &prot)) {
> >>> -        /* Couldn't get pte or access denied due to protection */
> >>> -        ppc_radix64_raise_si(cpu, rwx, eaddr, fault_cause);
> >>> -        return 1;
> >>> -    }
> >>> -
> >>> -    /* Update Reference and Change Bits */
> >>> -    ppc_radix64_set_rc(cpu, rwx, pte, pte_addr, &prot);
> >>>  
> >>>      tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> >>>                   prot, mmu_idx, 1UL << page_size);
> >>> @@ -318,16 +383,13 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, 
> >>> vaddr eaddr, int rwx,
> >>>  
> >>>  hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong 
> >>> eaddr)
> >>>  {
> >>> -    CPUState *cs = CPU(cpu);
> >>>      CPUPPCState *env = &cpu->env;
> >>> -    PPCVirtualHypervisorClass *vhc;
> >>> -    hwaddr raddr, pte_addr;
> >>> -    uint64_t lpid = 0, pid = 0, offset, size, prtbe0, pte;
> >>> -    int page_size, fault_cause = 0;
> >>> -    ppc_v3_pate_t pate;
> >>> +    uint64_t lpid = 0, pid = 0;
> >>> +    int psize, prot;
> >>> +    hwaddr raddr;
> >>>  
> >>>      /* Handle Real Mode */
> >>> -    if (msr_dr == 0) {
> >>> +    if ((msr_dr == 0) && (msr_hv || (cpu->vhyp && lpid == 0))) {
> >>>          /* In real mode top 4 effective addr bits (mostly) ignored */
> >>>          return eaddr & 0x0FFFFFFFFFFFFFFFULL;
> >>>      }
> >>> @@ -337,39 +399,8 @@ hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU 
> >>> *cpu, target_ulong eaddr)
> >>>          return -1;
> >>>      }
> >>>  
> >>> -    /* Get Process Table */
> >>> -    if (cpu->vhyp) {
> >>> -        vhc = PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> >>> -        vhc->get_pate(cpu->vhyp, &pate);
> >>> -    } else {
> >>> -        if (!ppc64_v3_get_pate(cpu, lpid, &pate)) {
> >>> -            return -1;
> >>> -        }
> >>> -        if (!validate_pate(cpu, lpid, &pate)) {
> >>> -            return -1;
> >>> -        }
> >>> -        /* We don't support guest mode yet */
> >>> -        if (lpid != 0) {
> >>> -            error_report("PowerNV guest support Unimplemented");
> >>> -            exit(1);
> >>> -       }
> >>> -    }
> >>> -
> >>> -    /* Index Process Table by PID to Find Corresponding Process Table 
> >>> Entry */
> >>> -    offset = pid * sizeof(struct prtb_entry);
> >>> -    size = 1ULL << ((pate.dw1 & PATE1_R_PRTS) + 12);
> >>> -    if (offset >= size) {
> >>> -        /* offset exceeds size of the process table */
> >>> -        return -1;
> >>> -    }
> >>> -    prtbe0 = ldq_phys(cs->as, (pate.dw1 & PATE1_R_PRTB) + offset);
> >>> -
> >>> -    /* Walk Radix Tree from Process Table Entry to Convert EA to RA */
> >>> -    page_size = PRTBE_R_GET_RTS(prtbe0);
> >>> -    pte = ppc_radix64_walk_tree(cpu, eaddr & R_EADDR_MASK,
> >>> -                                prtbe0 & PRTBE_R_RPDB, prtbe0 & 
> >>> PRTBE_R_RPDS,
> >>> -                                &raddr, &page_size, &fault_cause, 
> >>> &pte_addr);
> >>> -    if (!pte) {
> >>> +    if (ppc_radix64_xlate(cpu, eaddr, 0, lpid, pid, msr_dr, &raddr, 
> >>> &psize,
> >>> +                          &prot, 0)) {
> >>>          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