Re: [PATCH v4 4/4] target/ppc: Add support for Radix partition-scoped translation

[Cédric Le Goater]

>>> -    *raddr = g_raddr;
>>> +    /*
>>> +     * Perform partition-scoped translation if !HV or HV access to
>>> +     * quadrants 1 or 2. Translates a guest real address to a host
>>> +     * real address.
>>> +     */
>>> +    if ((lpid != 0) || (!cpu->vhyp && !msr_hv)) {
>>
>> This check is too complex for my taste. Also it doesn't seem right
>> to look at lpid if the machine is pseries, even if it would happen
>> to work because pseries cannot have lpid != 0. I think we should
>> have distinct paths for powernv and pseries.
>>
>> A bit like with the following squashed in:
>>
>> =======================================
>> --- a/target/ppc/mmu-radix64.c
>> +++ b/target/ppc/mmu-radix64.c
>> @@ -489,22 +489,28 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr 
>> eaddr, int rwx,
>>          g_raddr = eaddr & R_EADDR_MASK;
>>      }
>>  
>> -    /*
>> -     * Perform partition-scoped translation if !HV or HV access to
>> -     * quadrants 1 or 2. Translates a guest real address to a host
>> -     * real address.
>> -     */
>> -    if ((lpid != 0) || (!cpu->vhyp && !msr_hv)) {
>> -        int ret = ppc_radix64_partition_scoped_xlate(cpu, rwx, eaddr, 
>> g_raddr,
>> +    if (cpu->vhyp) {
>> +        *raddr = g_raddr;
>> +    } else {
>> +        /*
>> +         * Perform partition-scoped translation if !HV or HV access to
>> +         * quadrants 1 or 2. Translates a guest real address to a host
>> +         * real address.
>> +         */
>> +        if (lpid || !msr_hv) {
>> +            int ret;
>> +
>> +            ret = ppc_radix64_partition_scoped_xlate(cpu, rwx, eaddr, 
>> g_raddr,
>>                                                       pate, raddr, &prot, 
>> &psize,
>>                                                       0, cause_excp);
>> -        if (ret) {
>> -            return ret;
>> +            if (ret) {
>> +                return ret;
>> +            }
>> +            *psizep = MIN(*psizep, psize);
>> +            *protp &= prot;
>> +        } else {
>> +            *raddr = g_raddr;
>>          }
>> -        *psizep = MIN(*psizep, psize);
>> -        *protp &= prot;
>> -    } else {
>> -        *raddr = g_raddr;
>>      }
>>  
>>      return 0;
>> =======================================
>>
>> David,
>>
>> If my comment makes sense to you, can you squash the above fix into
>> Cedric's patch ?
> 
> Yes.  I also think we shouldn't be looking at lpid for the vhyp case.
> I've applied the rest of the series to ppc-for-5.1, and folded in this
> correction as suggested.


I explored a solution with two ppc_radix64_xlate() routines, one simple 
for pseries, a second more complex for powernv but it didn't look very
good. May be it will be easier now that the first patches are merged. 

Thanks,

C.