Subject: Re: [PATCH] powerpc/mm: Fix tlbie to add AVAL fields for 64K pages

From: "Aneesh Kumar K.V" <[email-masked]>
Date: Mon, 21 Apr 2014 13:32:30 +0530

Benjamin Herrenschmidt <[email-masked]> writes:

> On Mon, 2014-04-21 at 10:37 +0530, Aneesh Kumar K.V wrote:
>> The if condition check was based on a draft ISA doc. Remove the same.
>
> Is this a real bug ? Do we need to send that back to stable ?

We haven't found any issues without the patch. I found that by code
reading when implementing THP support for powerkvm guest.  Also that if condition
issue is only applicable for 64K base page size. ie, when base page size and
actual page size both are 64K. Now for 64K bits we have bits 58..61
already in AVA field of tlbie instruction. But ISA requires that we have
them also in AVAL field. Since they are duplicate bits may be we are
ok ?

-aneesh

>
> Ben.
>
>> Signed-off-by: Aneesh Kumar K.V <[email-masked]>
>> ---
>>  arch/powerpc/mm/hash_native_64.c | 38 ++++++++++++++++----------------------
>>  1 file changed, 16 insertions(+), 22 deletions(-)
>> 
>> diff --git a/arch/powerpc/mm/hash_native_64.c b/arch/powerpc/mm/hash_native_64.c
>> index 3ea26c25590b..cf1d325eae8b 100644
>> --- a/arch/powerpc/mm/hash_native_64.c
>> +++ b/arch/powerpc/mm/hash_native_64.c
>> @@ -82,17 +82,14 @@ static inline void __tlbie(unsigned long vpn, int psize, int apsize, int ssize)
>>  		va &= ~((1ul << mmu_psize_defs[apsize].shift) - 1);
>>  		va |= penc << 12;
>>  		va |= ssize << 8;
>> -		/* Add AVAL part */
>> -		if (psize != apsize) {
>> -			/*
>> -			 * MPSS, 64K base page size and 16MB parge page size
>> -			 * We don't need all the bits, but rest of the bits
>> -			 * must be ignored by the processor.
>> -			 * vpn cover upto 65 bits of va. (0...65) and we need
>> -			 * 58..64 bits of va.
>> -			 */
>> -			va |= (vpn & 0xfe);
>> -		}
>> +		/*
>> +		 * AVAL bits:
>> +		 * We don't need all the bits, but rest of the bits
>> +		 * must be ignored by the processor.
>> +		 * vpn cover upto 65 bits of va. (0...65) and we need
>> +		 * 58..64 bits of va.
>> +		 */
>> +		va |= (vpn & 0xfe); /* AVAL */
>>  		va |= 1; /* L */
>>  		asm volatile(ASM_FTR_IFCLR("tlbie %0,1", PPC_TLBIE(%1,%0), %2)
>>  			     : : "r" (va), "r"(0), "i" (CPU_FTR_ARCH_206)
>> @@ -133,17 +130,14 @@ static inline void __tlbiel(unsigned long vpn, int psize, int apsize, int ssize)
>>  		va &= ~((1ul << mmu_psize_defs[apsize].shift) - 1);
>>  		va |= penc << 12;
>>  		va |= ssize << 8;
>> -		/* Add AVAL part */
>> -		if (psize != apsize) {
>> -			/*
>> -			 * MPSS, 64K base page size and 16MB parge page size
>> -			 * We don't need all the bits, but rest of the bits
>> -			 * must be ignored by the processor.
>> -			 * vpn cover upto 65 bits of va. (0...65) and we need
>> -			 * 58..64 bits of va.
>> -			 */
>> -			va |= (vpn & 0xfe);
>> -		}
>> +		/*
>> +		 * AVAL bits:
>> +		 * We don't need all the bits, but rest of the bits
>> +		 * must be ignored by the processor.
>> +		 * vpn cover upto 65 bits of va. (0...65) and we need
>> +		 * 58..64 bits of va.
>> +		 */
>> +		va |= (vpn & 0xfe);
>>  		va |= 1; /* L */
>>  		asm volatile(".long 0x7c000224 | (%0 << 11) | (1 << 21)"
>>  			     : : "r"(va) : "memory");