[Cbe-oss-dev] [PATCH] 64K page support for kexec

[Benjamin Herrenschmidt]

On Mon, 2007-04-23 at 14:53 -0300, Luke Browning wrote:
> This patch fixes a couple of kexec problems related to 64K page 
> support in the kernel.  kexec issues a tlbie for each pte.  The 
> parameters for the tlbie are the page size and the virtual address.
> Support was missing for the computation of these two parameters
> for 64K pages.  This patch adds that support.  
> 
> Signed-off-by: Luke Browning <lukebrowning at us.ibm.com>

Acked-by: Benjamin Herrenschmidt <benh at kernel.crashing.org>

BTW. Your mailer is still mangling patches.... (wrapping them). Can't
you disable word wrap ? If you're using evolution, make sure to use the
"preformat" style, and do an "insert text file", that should keep
patches intact.

A couple of minor things that shouldn't block accepting the patch:

> -static unsigned long slot2va(unsigned long hpte_v, unsigned long slot)
> +static unsigned long slot2va(unsigned long hpte_v, unsigned long slot,
> int psize)
>  {
>  	unsigned long avpn = HPTE_V_AVPN_VAL(hpte_v);
>  	unsigned long va;
>  
>  	va = avpn << 23;
>  
> -	if (! (hpte_v & HPTE_V_LARGE)) {
> +	if (psize != MMU_PAGE_16M) {
>  		unsigned long vpi, pteg;

Maybe should be a test wether page shift > 23 ? One day we might
actually use the 16G pages option too :-) Though that would mean using
1T segment, which means a different hashing too... that's something we
haven't looked properly into yet. (We have had some patches for some
times but there are issues we haven't fully solved yet).

> +	if (pa & LP_MASK(0)) {
> +		for (i = 0; i < LP_BITS; i++) {
> +			if ((pa & LP_MASK(i+1)) == LP_MASK(i+1))
> +				break;
> +		}
> +		penc = LP_MASK(i+1) >> LP_SHIFT;
> +		for (i = MMU_PAGE_COUNT - 1; i > 0; i--) {
> +			if (!mmu_psize_defs[i].shift)
> +				continue;
> +			if (penc == mmu_psize_defs[i].penc)
> +				break;
> +		}
> +	}
> +	else {
> +		i = MMU_PAGE_16M;
> +	}
> +
> +	return i;
> +}

So you didn't like my idea of using a single loop ? Oh well, that should
do I suppose. One minor coding style nit, I would have preferred
something like

	if (!(pa & LP_MASK(0)))
		return MMU_PAGE_16M;

	for (...)

To limit the right shift of the whole function, but that's a detail.

>  		if (hpte_v & HPTE_V_VALID) {
> +			if (!(hpte_v & HPTE_V_LARGE))
> +				psize = MMU_PAGE_4K;
> +			else
> +				psize = hpte_decode_lpsize(hptep->r);

Another minor coding style nit, but maybe it would have been nicer to
have the 4K page also dealt within  hpte_decode_lpsize() ? Since the
function is likely to inline it, that will make no difference in
generated code but keeps the logic for getting the psize completely
within a single function...
 
> +			vpn = slot2va(hpte_v, slot, psize);
>  			hptep->v = 0;
> -			__tlbie(slot2va(hpte_v, slot), MMU_PAGE_4K);
> +			__tlbie(vpn, psize);
>  		}
>  	}

Cheers,
Ben.