Subject: Re: [PATCH] powerpc/mm: Move hpte encode routine to platform specific files

From: "Aneesh Kumar K.V" <[email-masked]>
Date: Fri, 11 Nov 2016 07:04:58 +0530

Balbir Singh <[email-masked]> writes:

> On 11/11/16 10:54, Balbir Singh wrote:
>> 
>> 
>> On 10/11/16 20:24, Aneesh Kumar K.V wrote:
>>> They differ between powernv and pseries. While we are at it copy
>>> a version for ps3 too. For now this is same as what we have for pSeries.
>>>
>>> Fixes: 50de596de8be6 ("powerpc/mm/hash: Add support for Power9 Hash")
>>> Signed-off-by: Aneesh Kumar K.V <[email-masked]>
>> 
>> 
>> I have not looked at the patch closely, but do we want all of this
>> duplication. I was wondering if instead of
>> 
>> cpu_has_feature() we could firmware_has_feature() for now
>> 
>
> Basically something like
>
> Not-yet-signed-off-by: Balbir Singh <[email-masked]>
>
>
> diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> index ac6d26e..2b23366 100644
> --- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> +++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> @@ -282,7 +282,8 @@ static inline unsigned long hpte_encode_avpn(unsigned long vpn, int psize,
>  	 */
>  	v = (vpn >> (23 - VPN_SHIFT)) & ~(mmu_psize_defs[psize].avpnm);
>  	v <<= HPTE_V_AVPN_SHIFT;
> -	if (!cpu_has_feature(CPU_FTR_ARCH_300))
> +	if (!cpu_has_feature(CPU_FTR_ARCH_300) ||
> +		firmware_has_feature(FW_FEATURE_LPAR))
>  		v |= ((unsigned long) ssize) << HPTE_V_SSIZE_SHIFT;

I looked at this before and in the end decided to go otherwise. A large
part of that patch is comment duplication. My thinking was if the
encoding needs to be different, then it is better to have seperate
functions to do the encoding.

>  	return v;
>  }
> @@ -310,7 +311,8 @@ static inline unsigned long hpte_encode_r(unsigned long pa, int base_psize,
>  					  int actual_psize, int ssize)
>  {
>
> -	if (cpu_has_feature(CPU_FTR_ARCH_300))
> +	if (cpu_has_feature(CPU_FTR_ARCH_300) &&
> +		!firmware_has_feature(FW_FEATURE_LPAR))
>  		pa |= ((unsigned long) ssize) << HPTE_R_3_0_SSIZE_SHIFT;
>
>  	/* A 4K page needs no special encoding */

-aneesh