Re: [PATCH v2] POWERPC: Allow 32-bit pgtable code to support 36-bit physical

[Scott Wood <scottwood@freescale.com>]

Becky Bruce wrote:
>  #if _PAGE_HASHPTE != 0
> +#ifndef CONFIG_PTE_64BIT
>  	pte_update(ptep, ~_PAGE_HASHPTE, pte_val(pte) & ~_PAGE_HASHPTE);
>  #else
> +	/*
> +	 * We have to do the write of the 64b pte as 2 stores.  This
> +	 * code assumes that the entry we're storing to is currently
> +	 * not valid and that all callers have the page table lock.
> +	 * Having the entry be not valid protects readers who might read
> +	 * between the first and second stores.
> +	 */
> +	unsigned int tmp;
> +
> +	__asm__ __volatile__("\
> +1:	lwarx	%0,0,%4\n\
> +	rlwimi	%L2,%0,0,30,30\n\
> +	stw	%2,0(%3)\n\
> +	eieio\n\
> +	stwcx.  %L2,0,%4\n\
> +	bne-	1b"
> +	: "=&r" (tmp), "=m" (*ptep)
> +	: "r" (pte), "r" (ptep), "r" ((unsigned long)(ptep) + 4), "m" (*ptep)
> +	: "cc");
> +#endif	/* CONFIG_PTE_64BIT */

Could the stw to the same reservation granule as the stwcx cancel the 
reservation on some implementations?  Plus, if you're assuming that the 
entry is currently invalid and all callers have the page table lock, do 
we need the lwarx/stwcx at all?  At the least, it should check 
PTE_ATOMIC_UPDATES.

%2 should be "+&r", not "r".

> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index 7f65127..ca5b58b 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -128,18 +128,22 @@ config FSL_EMB_PERFMON
>  
>  config PTE_64BIT
>  	bool
> -	depends on 44x || E500
> +	depends on 44x || E500 || 6xx
>  	default y if 44x
> -	default y if E500 && PHYS_64BIT
> +	default y if PHYS_64BIT

How is this different from PHYS_64BIT?

>  config PHYS_64BIT
> -	bool 'Large physical address support' if E500
> -	depends on 44x || E500
> +	bool 'Large physical address support' if E500 || 6xx

Maybe "if !44x", or have 44x "select" this, rather than listing all 
arches where it's optional.

> +	depends on 44x || E500 || 6xx
>  	select RESOURCES_64BIT
>  	default y if 44x
>  	---help---
>  	  This option enables kernel support for larger than 32-bit physical
> -	  addresses.  This features is not be available on all e500 cores.
> +	  addresses.  This features is not be available on all cores.

"This features is not be"?

-Scott