Subject: Re: [PATCH V2 36/68] powerpc/mm/radix: Add mmu context handling callback for radix

From: Balbir Singh <[email-masked]>
Date: Fri, 22 Apr 2016 16:19:22 +1000

On 09/04/16 16:13, Aneesh Kumar K.V wrote:
> Signed-off-by: Aneesh Kumar K.V <[email-masked]>
> ---
>  arch/powerpc/include/asm/mmu_context.h |  4 ++++
>  arch/powerpc/mm/mmu_context_hash64.c   | 42 +++++++++++++++++++++++++++-------
>  2 files changed, 38 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
> index 0dd522ff4559..8f16a065d33d 100644
> --- a/arch/powerpc/include/asm/mmu_context.h
> +++ b/arch/powerpc/include/asm/mmu_context.h
> @@ -37,10 +37,14 @@ extern void switch_slb(struct task_struct *tsk, struct mm_struct *mm);
>  extern void set_context(unsigned long id, pgd_t *pgd);
>  
>  #ifdef CONFIG_PPC_BOOK3S_64
> +extern void switch_radix_mmu_context(struct mm_struct *prev,
> +				     struct mm_struct *next);
>  static inline void switch_mmu_context(struct mm_struct *prev,
>  				      struct mm_struct *next,
>  				      struct task_struct *tsk)
>  {
> +	if (radix_enabled())
> +		return switch_radix_mmu_context(prev, next);
>  	return switch_slb(tsk, next);
>  }
>  
> diff --git a/arch/powerpc/mm/mmu_context_hash64.c b/arch/powerpc/mm/mmu_context_hash64.c
> index 4e4efbc2658e..08cc3182b744 100644
> --- a/arch/powerpc/mm/mmu_context_hash64.c
> +++ b/arch/powerpc/mm/mmu_context_hash64.c
> @@ -58,6 +58,17 @@ again:
>  	return index;
>  }
>  EXPORT_SYMBOL_GPL(__init_new_context);
> +static int rinit_new_context(struct mm_struct *mm, int index)
> +{
> +	unsigned long rts_field;
> +
> +	/*
> +	 * set the process table entry,
> +	 */
> +	rts_field = 3ull << PPC_BITLSHIFT(2);

rts_field1? The code needs to add some comments on what the size bits are as well. Do we set rts_field2?

> +	process_tb[index].prtb0 = cpu_to_be64(rts_field | __pa(mm->pgd) | R_PGD_INDEX_SIZE);
> +	return 0;
> +}
>  
>  int init_new_context(struct task_struct *tsk, struct mm_struct *mm)
>  {
> @@ -67,13 +78,18 @@ int init_new_context(struct task_struct *tsk, struct mm_struct *mm)
>  	if (index < 0)
>  		return index;
>  
> -	/* The old code would re-promote on fork, we don't do that
> -	 * when using slices as it could cause problem promoting slices
> -	 * that have been forced down to 4K
> -	 */
> -	if (slice_mm_new_context(mm))
> -		slice_set_user_psize(mm, mmu_virtual_psize);
> -	subpage_prot_init_new_context(mm);
> +	if (radix_enabled()) {
> +		rinit_new_context(mm, index);
> +	} else {
> +
> +		/* The old code would re-promote on fork, we don't do that
> +		 * when using slices as it could cause problem promoting slices
> +		 * that have been forced down to 4K
> +		 */
> +		if (slice_mm_new_context(mm))
> +			slice_set_user_psize(mm, mmu_virtual_psize);
> +		subpage_prot_init_new_context(mm);
> +	}
>  	mm->context.id = index;
>  #ifdef CONFIG_PPC_ICSWX
>  	mm->context.cop_lockp = kmalloc(sizeof(spinlock_t), GFP_KERNEL);
> @@ -145,8 +161,18 @@ void destroy_context(struct mm_struct *mm)
>  	mm->context.cop_lockp = NULL;
>  #endif /* CONFIG_PPC_ICSWX */
>  
> +	if (radix_enabled())
> +		process_tb[mm->context.id].prtb1 = 0;
> +	else
> +		subpage_prot_free(mm);
>  	destroy_pagetable_page(mm);
>  	__destroy_context(mm->context.id);
> -	subpage_prot_free(mm);
>  	mm->context.id = MMU_NO_CONTEXT;

Got to be careful w.r.t. setting it to 0

>  }
> +
> +void switch_radix_mmu_context(struct mm_struct *prev, struct mm_struct *next)
> +{
> +	mtspr(SPRN_PID, next->context.id);

Can we have next->context.id = 0, OS? Any additional checks?

> +	asm volatile("isync": : :"memory");
> +

Why do we need the prev again?

> +}