Re: [Qemu-ppc] [RFC NO-MERGE 11/12] target/ppc: Implement ISA V3.00 radix page fault handler

[David Gibson]

On Fri, Feb 24, 2017 at 06:18:50PM +1100, Suraj Jitindar Singh wrote:
> On Mon, 2017-02-20 at 14:16 +1100, David Gibson wrote:
> > On Fri, Feb 17, 2017 at 04:08:11PM +1100, Suraj Jitindar Singh
> wrote:
[snip]
> > > +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 =
> > > +        PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> > > +    hwaddr raddr;
> > > +    uint64_t lpid = 0, pid = 0, offset, size, patbe, prtbe0;
> > > +    int page_size, prot, fault_cause = 0;
> > > +
> > > +    assert((rwx == 0) || (rwx == 1) || (rwx == 2));
> > > +
> > > +    /* Real Mode Access */
> > > +    if (((rwx == 2) && (msr_ir == 0)) || ((rwx != 2) && (msr_dr ==
> > > 0))) {
> > > +        /* In real mode top 4 effective addr bits (mostly) ignored
> > > */
> > > +        raddr = eaddr & 0x0FFFFFFFFFFFFFFFULL;
> > > +
> > > +        if (msr_hv) {
> > > +            /* if EA0 == 0, raddr = eaddr4:63 | HRMOR */
> > > +            if (!(eaddr >> 63)) {
> > > +                raddr |= env->spr[SPR_HRMOR];
> > > +            }
> > > +        } else {
> > > +            /* TODO */
> > > +            /* PowerNV ONLY */
> > Uh.. isn't this the wrong way around?  msr_hv is set in hypervisor
> > mode, isn't it?
> 
> Um, so on powernv we will have to do partition-scoped translation which
> we don't have to worry about yet but this is where it would go.
> Technically the HRMOR stuff is powernv as well and will never be
> reached. It just seemed so easy to add now...

Sure, but that doesn't actually address the point I'm making - for
pseries msr_hv will be always false, so you'll go down the PowerNV
partition translation path, which isn't right.

> > In fact, there are multiple places further down this function which
> > won't work for powernv anyway (e.g. getting the patbe, not
> > translating
> > guest-guest to guest-host addresses for reading the process table).
> > So you might as well just assert (cpu->vhyp) up the top and be done
> 
> True - I'll do that

-- 
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