On 07/01/2016 12:13 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2016-06-30 at 18:01 +0200, Cédric Le Goater wrote:
>> +static uint32_t ppc_hash64_pte_size_decode(PowerPCCPU *cpu, uint64_t
>> pte0,
>> +                                           uint64_t pte1, uint32_t
>> slb_pshift)
>>  {
>> -    switch (slb_pshift) {
>> -    case 12:
>> -        return 12;
>> -    case 16:
>> -        if ((pte1 & 0xf000) == 0x1000) {
>> -            return 16;
>> -        }
>> -        return 0;
>> -    case 24:
>> -        if ((pte1 & 0xff000) == 0) {
>> -            return 24;
>> -        }
>> -        return 0;
>> -    }
>> -    return 0;
>> +    unsigned spshift;
>> +
>> +    return ppc_hash64_hpte_page_shift_noslb(cpu, pte0, pte1,
>> &spshift);
>>  }
> 
> Why not call ppc_hash64_hpte_page_shift_noslb() directly from the call
> site ? That or rename it to ppc_hash64_pte_size_decode :-)

yes, clearly :) but that segment page shift is bothering me.  

David, 

Do you think I can remove that parameter as it is never used or do you
have some plans for it ? 

> Otherwise yes, your patch looks correct as in what
> doesppc_hash64_hpte_page_shift_noslb() is definitely more correct than
> what ppc_hash64_pte_size_decode() is doing.

Thanks,

C.
 


Reply via email to