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

[Cédric Le Goater]

On 3/31/20 3:13 AM, David Gibson wrote:
> 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).

I will remove the extra check. This has the nice effect of cleaning up 
a couple of weird changes in patch 7.

Thanks,

C.