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

[Suraj Jitindar Singh]

On Tue, 2017-02-28 at 13:29 +1100, David Gibson wrote:
> 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.

So on that path we'll need a PowerNV check. If PowerNV we'll do
partition scoped translation, otherwise do nothing.

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