Re: [PATCH 2/2 v2] kvm: powerpc: set cache coherency only for kernel managed pages

[Scott Wood <scottwood@freescale.com>]

On 07/18/2013 08:19:03 AM, Bharat Bhushan wrote:
> If there is a struct page for the requested mapping then it's
> normal RAM and the mapping is set to "M" bit (coherent, cacheable)
> otherwise this is treated as I/O and we set  "I + G"  (cache  
> inhibited, guarded)
> 
> This helps setting proper TLB mapping for direct assigned device
> 
> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> ---
> v2: some cleanup and added comment
>  -
>  arch/powerpc/kvm/e500_mmu_host.c |   23 ++++++++++++++++++-----
>  1 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/e500_mmu_host.c  
> b/arch/powerpc/kvm/e500_mmu_host.c
> index 1c6a9d7..02eb973 100644
> --- a/arch/powerpc/kvm/e500_mmu_host.c
> +++ b/arch/powerpc/kvm/e500_mmu_host.c
> @@ -64,13 +64,26 @@ static inline u32 e500_shadow_mas3_attrib(u32  
> mas3, int usermode)
>  	return mas3;
>  }
> 
> -static inline u32 e500_shadow_mas2_attrib(u32 mas2, int usermode)
> +static inline u32 e500_shadow_mas2_attrib(u32 mas2, pfn_t pfn)
>  {
> +	u32 mas2_attr;
> +
> +	mas2_attr = mas2 & MAS2_ATTRIB_MASK;
> +
> +	/*
> +	 * RAM is always mappable on e500 systems, so this is identical
> +	 * to kvm_is_mmio_pfn(), just without its overhead.
> +	 */
> +	if (!pfn_valid(pfn)) {

Please use page_is_ram(), which is what gets used when setting the WIMG  
for the host userspace mapping.  We want to make sure the two are  
consistent.

> +		/* Pages not managed by Kernel are treated as I/O, set  
> I + G */
> +		mas2_attr |= MAS2_I | MAS2_G;
>  #ifdef CONFIG_SMP
> -	return (mas2 & MAS2_ATTRIB_MASK) | MAS2_M;
> -#else
> -	return mas2 & MAS2_ATTRIB_MASK;
> +	} else {
> +		/* Kernel managed pages are actually RAM so set  "M" */
> +		mas2_attr |= MAS2_M;
>  #endif

Likewise, we want to make sure this matches the host entry.   
Unfortunately, this is a bit of a mess already.  64-bit booke appears  
to always set MAS2_M for TLB0 mappings.  The initial KERNELBASE mapping  
on boot uses M_IF_SMP, and the settlbcam() that (IIRC) replaces it uses  
_PAGE_COHERENT.  32-bit always uses _PAGE_COHERENT, except that initial  
KERNELBASE mapping.  _PAGE_COHERENT appears to be set based on  
CONFIG_SMP || CONFIG_PPC_STD_MMU (the latter config clears  
_PAGE_COHERENT in the non-CPU_FTR_NEED_COHERENT case).

As for what we actually want to happen, there are cases when we want M  
to be set for non-SMP.  One such case is AMP, where CPUs may be sharing  
memory even if the Linux instance only runs on one CPU (this is not  
hypothetical, BTW).  It's also possible that we encounter a hardware  
bug that requires MAS2_M, similar to what some of our non-booke chips  
require.

-Scott