Re: [PATCH] 8xx: map_page() skip pinned region and tlbie debugging aid

[Dan Malek <dan@embeddededge.com>]

On Jun 25, 2005, at 10:53 AM, Marcelo Tosatti wrote:

> Dan: I dont think ioremap() is an issue because it never works inside 
> the
> kernel's static virtual address space (which is the only one we're 
> interested
> in having pinned at the moment).

Take a close look at the initialization code.  I believe it also
pins the IMMR space, which is subject to ioremap().

>  source "drivers/Kconfig"
> diff --git a/arch/ppc/kernel/misc.S b/arch/ppc/kernel/misc.S
> --- a/arch/ppc/kernel/misc.S
> +++ b/arch/ppc/kernel/misc.S
> @@ -565,6 +565,19 @@ _GLOBAL(_tlbie)
>  	SYNC_601
>  	isync
>  #else /* CONFIG_SMP */
> +#ifdef CONFIG_DEBUG_PIN_TLBIE
> +/* check if the address being invalidated overlaps with the pinned 
> region */
> +	lis	r4,(pin_area_start)@ha
> +	lwz	r5,(pin_area_start)@l(4)
> +	cmplw	r3, r5
> +	blt	11f
> +	lis	r4,(pin_area_end)@ha
> +	lwz	r5,(pin_area_end)@l(4)
> +	cmplw	r3, r5
> +	bge	11f
> +	trap
> +#endif
> +11:
>  	tlbie	r3
>  	sync

We don't need this kind of assembly code on the 8xx.  Just define
_tlbie as a macro (which has always been done) and write this debug
stuff as C code.

> +#ifdef CONFIG_PIN_TLB
> +unsigned long pin_area_start = KERNELBASE;
> +unsigned long pin_area_end = KERNELBASE + 0x00800000;
> +#endif

This only covers the kernel instruction space.  We pin 24M bytes
of data plus 8M bytes of IMMR.

> +#ifdef CONFIG_PIN_TLB
> +			if (va < pin_area_start || va >= pin_area_end)
> +#endif
> +				flush_HPTE(0, va, pmd_val(*pd));

We really want to see this generate an error.  We shouldn't be
calling this on any of the pinned spaces.  In the case of initially
mapping the kernel space, we should set up the page tables but
not call this far down that we get here.

Thanks.

	-- Dan