Re: [PATCH] powerpc/book3s/mm: Clear MMU_FTR_HPTE_TABLE when radix is enabled.

[Nicholas Piggin <npiggin@gmail.com>]

Aneesh Kumar K.V's on May 14, 2019 4:02 pm:
> Avoids confusion when printing Oops message like below
> 
>  Faulting instruction address: 0xc00000000008bdb4
>  Oops: Kernel access of bad area, sig: 11 [#1]
>  LE PAGE_SIZE=64K MMU=Radix MMU=Hash SMP NR_CPUS=2048 NUMA PowerNV
> 
> Either ibm,pa-features or ibm,powerpc-cpu-features can be used to enable the
> MMU features. We don't clear related MMU feature bits there. We use the kernel
> commandline to determine what translation mode we want to use and clear the
> HPTE or radix bit accordingly. On LPAR we do have to renable HASH bit if the
> hypervisor can't do radix.

Well we have the HPTE feature: the CPU supports hash MMU mode. It's
just the the kernel is booted in radix mode.

Could make a difference for KVM, if it will support an HPT guest or
not.

That's all highly theoretical and we have other inconsistencies
already in this stuff, I'd just like to try make things a bit better
in the long term.

Can we just add an early_radix_enabled() in the oops printing code
to select radix or hash MMU?

> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
> index b97aee03924f..0fa6cac3fe82 100644
> --- a/arch/powerpc/mm/pgtable.c
> +++ b/arch/powerpc/mm/pgtable.c
> @@ -77,9 +77,6 @@ static struct page *maybe_pte_to_page(pte_t pte)
>  
>  static pte_t set_pte_filter_hash(pte_t pte)
>  {
> -	if (radix_enabled())
> -		return pte;
> -
>  	pte = __pte(pte_val(pte) & ~_PAGE_HPTEFLAGS);
>  	if (pte_looks_normal(pte) && !(cpu_has_feature(CPU_FTR_COHERENT_ICACHE) ||
>  				       cpu_has_feature(CPU_FTR_NOEXECUTE))) {
> @@ -110,6 +107,8 @@ static pte_t set_pte_filter(pte_t pte)
>  
>  	if (mmu_has_feature(MMU_FTR_HPTE_TABLE))
>  		return set_pte_filter_hash(pte);
> +	else if (radix_enabled())
> +		return pte;
>  
>  	/* No exec permission in the first place, move on */
>  	if (!pte_exec(pte) || !pte_looks_normal(pte))
> @@ -140,7 +139,7 @@ static pte_t set_access_flags_filter(pte_t pte, struct vm_area_struct *vma,
>  {
>  	struct page *pg;
>  
> -	if (mmu_has_feature(MMU_FTR_HPTE_TABLE))
> +	if (mmu_has_feature(MMU_FTR_HPTE_TABLE) || radix_enabled())
>  		return pte;
>  
>  	/* So here, we only care about exec faults, as we use them

These would still be good cleanup to make the HPTE_TABLE feature
independent from radix.

Thanks,
Nick