[RFC/PATCH 2/2] ppc64: Seperate usage of KERNELBASE and PAGE_OFFSET

David Gibson david at gibson.dropbear.id.au
Thu Aug 18 11:36:33 EST 2005 

On Wed, Aug 17, 2005 at 05:08:13PM +1000, Michael Ellerman wrote:
> This patch *tries* to seperate usage of KERNELBASE and PAGE_OFFSET.
> 
> PAGE_OFFSET == 0xC00..00 and always will. It's the quantity you subtract
> from a virtual kernel address to get a physical one.
> 
> KERNELBASE == 0xC00..00 *currently*, but might one day be something else,
> hold onto your hats. It points to the start of the kernel text + data in
> virtual memory.
> 
> I'd really appreciate it if people can cast an eye over this, as I'm sure
> I've got some wrong. There's still a few more users of KERNELBASE that could
> be converted to __pa() or __va() but I'll get to those later.
> 
> This actually boots on my P5 LPAR, but there might be some
> subtleties.

> @@ -248,12 +248,10 @@ void stabs_alloc(void)
>  			panic("Unable to allocate segment table for CPU %d.\n",
>  			      cpu);
>  
> -		newstab += KERNELBASE;
> +		memset(__va(newstab), 0, PAGE_SIZE);
>  
> -		memset((void *)newstab, 0, PAGE_SIZE);
> -
> -		paca[cpu].stab_addr = newstab;
> -		paca[cpu].stab_real = virt_to_abs(newstab);
> +		paca[cpu].stab_addr = (u64)__va(newstab);
> +		paca[cpu].stab_real = newstab;
>  		printk(KERN_DEBUG "Segment table for CPU %d at 0x%lx virtual, 0x%lx absolute\n", cpu, paca[cpu].stab_addr, paca[cpu].stab_real);
>  	}
>  }

Mistake here, which will break iSeries - you can't get rid of the
virt_to_abs().

> @@ -265,13 +263,13 @@ void stabs_alloc(void)
>   */
>  void stab_initialize(unsigned long stab)
>  {
> -	unsigned long vsid = get_kernel_vsid(KERNELBASE);
> +	unsigned long vsid = get_kernel_vsid(PAGE_OFFSET);
>  
>  	if (cpu_has_feature(CPU_FTR_SLB)) {
>  		slb_initialize();
>  	} else {
>  		asm volatile("isync; slbia; isync":::"memory");
> -		make_ste(stab, GET_ESID(KERNELBASE), vsid);
> +		make_ste(stab, GET_ESID(PAGE_OFFSET), vsid);
>  
>  		/* Order update */
>  		asm volatile("sync":::"memory");

Hrm.. it's a bit unclear what the right choice here is.  This is
bolting a stab entry covering the first segment.  Various things
assume that all the kernel text and static data is bolted, so you want
KERNELBASE.  But on the other hand things probably assume the vectors
are bolted, too.  It doesn't really matter as long as KERNELBASE is
within 256M of PAGE_OFFSET.

> Index: work/arch/ppc64/kernel/head.S
> ===================================================================
> --- work.orig/arch/ppc64/kernel/head.S
> +++ work/arch/ppc64/kernel/head.S
> @@ -100,7 +100,7 @@ END_FTR_SECTION(0, 1)
>  	 * This is required by the hypervisor
>  	 */
>  	. = 0x20
> -	.llong hvReleaseData-KERNELBASE
> +	.llong hvReleaseData-PAGE_OFFSET
>  
>  	/*
>  	 * At offset 0x28 and 0x30 are offsets to the msChunks
> @@ -108,8 +108,8 @@ END_FTR_SECTION(0, 1)
>  	 * between physical addresses and absolute addresses) and
>  	 * to the pidhash table (also used by the debugger)
>  	 */
> -	.llong msChunks-KERNELBASE
> -	.llong 0	/* pidhash-KERNELBASE SFRXXX */
> +	.llong msChunks-PAGE_OFFSET
> +	.llong 0	/* pidhash-PAGE_OFFSET SFRXXX */

Um.. I *think* these ones want to be KERNELBASE, not PAGE_OFFSET:
they're offsets within the load area.  Or even (symbol - _start) might
be better.  

> Index: work/arch/ppc64/kernel/pmac_smp.c
> ===================================================================
> --- work.orig/arch/ppc64/kernel/pmac_smp.c
> +++ work/arch/ppc64/kernel/pmac_smp.c
> @@ -241,7 +241,7 @@ static void __init smp_core99_kick_cpu(i
>  	unsigned long new_vector;
>  	unsigned long flags;
>  	volatile unsigned int *vector
> -		 = ((volatile unsigned int *)(KERNELBASE+0x100));
> +		 = ((volatile unsigned int *)(PAGE_OFFSET + 0x100));
>  
>  	if (nr < 1 || nr > 3)
>  		return;

Should probably use __va() here, too.

> @@ -253,8 +253,8 @@ static void __init smp_core99_kick_cpu(i
>  	/* Save reset vector */
>  	save_vector = *vector;
>  
> -	/* Setup fake reset vector that does	
> -	 *   b .pmac_secondary_start - KERNELBASE
> +	/* Setup fake reset vector that does
> +	 *   b .pmac_secondary_start - PAGE_OFFSET
>  	 */
>  	switch(nr) {
>  	case 1:
> @@ -268,7 +268,7 @@ static void __init smp_core99_kick_cpu(i
>  		new_vector = (unsigned long)pmac_secondary_start_3;
>  		break;
>  	}
> -	*vector = 0x48000002 + (new_vector - KERNELBASE);
> +	*vector = 0x48000002 + (new_vector - PAGE_OFFSET);
>  

And in these two.

> @@ -145,7 +145,7 @@ void slb_initialize(void)
>   	asm volatile("isync":::"memory");
>   	asm volatile("slbmte  %0,%0"::"r" (0) : "memory");
>  	asm volatile("isync; slbia; isync":::"memory");
> -	create_slbe(KERNELBASE, flags, 0);
> +	create_slbe(PAGE_OFFSET, flags, 0);
>  	create_slbe(VMALLOC_OFFSET, SLB_VSID_KERNEL, 1);
>  	/* We don't bolt the stack for the time being - we're in boot,
>  	 * so the stack is in the bolted segment.  By the time it goes

As with the stab code, it's unclear which we want here.

> Index: work/include/asm-ppc64/mmu.h
> ===================================================================
> --- work.orig/include/asm-ppc64/mmu.h
> +++ work/include/asm-ppc64/mmu.h
> @@ -29,8 +29,8 @@
>  
>  /* Location of cpu0's segment table */
>  #define STAB0_PAGE	0x9
> -#define STAB0_PHYS_ADDR	(STAB0_PAGE<<PAGE_SHIFT)
> -#define STAB0_VIRT_ADDR	(KERNELBASE+STAB0_PHYS_ADDR)
> +#define STAB0_PHYS_ADDR	(STAB0_PAGE << PAGE_SHIFT)
> +#define STAB0_VIRT_ADDR	(PAGE_OFFSET + STAB0_PHYS_ADDR)

Urg.. this one's a bit curly.  PAGE_OFFSET is right here, but moving
the kernel away from there will mean that (. = x) in head.S no longer
means the physical address will be x.  STAB0_PAGE is used in the
iSeries hv data to give the offset within the load area of the stab
page, but the phys addr is used on pSeries in mtasr so it needs to be
a real physical address.  So I think you actually need:
	#define STAB0_PAGE	<some number>
	#define STAB0_PHYS	((STAB0_PAGE<<PAGE_SHIFT) + (KERNELBASE-PAGE_OFFSET))
	#define STAB0_VIRT	(PAGE_OFFSET + STAB0_PHYS)

And in head.S you'll need either
	. = (STAB0_PAGE << PAGE_SHIFT)
or
	. = (STAB0_VIRT - KERNELBASE)
rather than the current
       .  = STAB0_PHYS

> Index: work/include/asm-ppc64/page.h
> ===================================================================
> --- work.orig/include/asm-ppc64/page.h
> +++ work/include/asm-ppc64/page.h
> @@ -204,14 +204,14 @@ extern u64 ppc64_pft_size;		/* Log 2 of 
>  #define VMALLOC_OFFSET		ASM_CONST(0xD000000000000000)
>  
>  #define VMALLOC_REGION_ID	(VMALLOC_OFFSET >> REGION_SHIFT)
> -#define KERNEL_REGION_ID   (KERNELBASE >> REGION_SHIFT)
> +#define KERNEL_REGION_ID	(PAGE_OFFSET >> REGION_SHIFT)

Could rename this LINEAR_REGION_ID or somesuch for clarity

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/people/dgibson

More information about the Linuxppc64-dev mailing list