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.