[PATCH] powerpc: Better setup of boot page TLB entry

[PATCH] powerpc: Better setup of boot page TLB entry

[Trent Piepho]

The initial TLB mapping for the kernel boot didn't set the memory coherent
attribute, MAS2[M], in SMP mode.

If this code supported booting a secondary processor, which it doesn't yet,
but suppose it did, then when a secondary processor boots, it would have
probably signaled the primary processor by setting a variable called
something like __secondary_hold_acknowledge.  However, due to the lack of
the M bit, the primary processor would not have snooped the transaction
(even if a transaction were broadcast).  If primary CPU's L1 D-cache had a
copy, it would not have been flushed and the CPU would have never seen the
ack.  Which would have resulted in the primary CPU spinning for a long
time, perhaps a full second before it would have given up, while it would
have waited for the ack from the secondary CPU that it wouldn't have been
able to see because of the stale cache.

The value of MAS2 for the boot page TLB1 entry is a compile time constant,
so there is no need to calculate it in powerpc assembly language.

Also, from the MPC8572 manual section 6.12.5.3, "Bits that represent
offsets within a page are ignored and should be cleared." Existing code
didn't clear them, this code does.

The same when the page of KERNELBASE is found; we don't need to use asm to
mask the lower 12 bits off.

In the code that computes the address to rfi from, don't hard code the
offset to 24 bytes, but have the assembler figure that out for us.

Signed-off-by: Trent Piepho <tpiepho@freescale.com>
Acked-by: Kumar Gala <galak@kernel.crashing.org>
---
 arch/powerpc/include/asm/mmu-fsl-booke.h |    2 ++
 arch/powerpc/kernel/head_fsl_booke.S     |   22 +++++++++++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/include/asm/mmu-fsl-booke.h b/arch/powerpc/include/asm/mmu-fsl-booke.h
index 925d93c..5588a41 100644
--- a/arch/powerpc/include/asm/mmu-fsl-booke.h
+++ b/arch/powerpc/include/asm/mmu-fsl-booke.h
@@ -40,6 +40,8 @@
 #define MAS2_M		0x00000004
 #define MAS2_G		0x00000002
 #define MAS2_E		0x00000001
+#define MAS2_EPN_MASK(size)		(~0 << (2*(size) + 10))
+#define MAS2_VAL(addr, size, flags)	((addr) & MAS2_EPN_MASK(size) | (flags))
 
 #define MAS3_RPN	0xFFFFF000
 #define MAS3_U0		0x00000200
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 590304c..e621eac 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -235,36 +235,40 @@ skpinv:	addi	r6,r6,1				/* Increment */
 	tlbivax 0,r9
 	TLBSYNC
 
+/* The mapping only needs to be cache-coherent on SMP */
+#ifdef CONFIG_SMP
+#define	M_IF_SMP	MAS2_M
+#else
+#define M_IF_SMP	0
+#endif
+
 /* 6. Setup KERNELBASE mapping in TLB1[0] */
 	lis	r6,0x1000		/* Set MAS0(TLBSEL) = TLB1(1), ESEL = 0 */
 	mtspr	SPRN_MAS0,r6
 	lis	r6,(MAS1_VALID|MAS1_IPROT)@h
 	ori	r6,r6,(MAS1_TSIZE(BOOKE_PAGESZ_64M))@l
 	mtspr	SPRN_MAS1,r6
-	li	r7,0
-	lis	r6,PAGE_OFFSET@h
-	ori	r6,r6,PAGE_OFFSET@l
-	rlwimi	r6,r7,0,20,31
+	lis	r6,MAS2_VAL(PAGE_OFFSET, BOOKE_PAGESZ_64M, M_IF_SMP)@h
+	ori	r6,r6,MAS2_VAL(PAGE_OFFSET, BOOKE_PAGESZ_64M, M_IF_SMP)@l
 	mtspr	SPRN_MAS2,r6
 	mtspr	SPRN_MAS3,r8
 	tlbwe
 
 /* 7. Jump to KERNELBASE mapping */
-	lis	r6,KERNELBASE@h
-	ori	r6,r6,KERNELBASE@l
-	rlwimi	r6,r7,0,20,31
+	lis	r6,(KERNELBASE & ~0xfff)@h
+	ori	r6,r6,(KERNELBASE & ~0xfff)@l
 	lis	r7,MSR_KERNEL@h
 	ori	r7,r7,MSR_KERNEL@l
 	bl	1f			/* Find our address */
 1:	mflr	r9
 	rlwimi	r6,r9,0,20,31
-	addi	r6,r6,24
+	addi	r6,r6,(2f - 1b)
 	mtspr	SPRN_SRR0,r6
 	mtspr	SPRN_SRR1,r7
 	rfi				/* start execution out of TLB1[0] entry */
 
 /* 8. Clear out the temp mapping */
-	lis	r7,0x1000	/* Set MAS0(TLBSEL) = 1 */
+2:	lis	r7,0x1000	/* Set MAS0(TLBSEL) = 1 */
 	rlwimi	r7,r5,16,4,15	/* Setup MAS0 = TLBSEL | ESEL(r5) */
 	mtspr	SPRN_MAS0,r7
 	tlbre
-- 
1.5.4.1

Re: [PATCH] powerpc: Better setup of boot page TLB entry

[Kumar Gala]

On Nov 19, 2008, at 1:14 PM, Trent Piepho wrote:

> The initial TLB mapping for the kernel boot didn't set the memory  
> coherent
> attribute, MAS2[M], in SMP mode.
>
> If this code supported booting a secondary processor, which it  
> doesn't yet,
> but suppose it did, then when a secondary processor boots, it would  
> have
> probably signaled the primary processor by setting a variable called
> something like __secondary_hold_acknowledge.  However, due to the  
> lack of
> the M bit, the primary processor would not have snooped the  
> transaction
> (even if a transaction were broadcast).  If primary CPU's L1 D-cache  
> had a
> copy, it would not have been flushed and the CPU would have never  
> seen the
> ack.  Which would have resulted in the primary CPU spinning for a long
> time, perhaps a full second before it would have given up, while it  
> would
> have waited for the ack from the secondary CPU that it wouldn't have  
> been
> able to see because of the stale cache.
>
> The value of MAS2 for the boot page TLB1 entry is a compile time  
> constant,
> so there is no need to calculate it in powerpc assembly language.
>
> Also, from the MPC8572 manual section 6.12.5.3, "Bits that represent
> offsets within a page are ignored and should be cleared." Existing  
> code
> didn't clear them, this code does.
>
> The same when the page of KERNELBASE is found; we don't need to use  
> asm to
> mask the lower 12 bits off.
>
> In the code that computes the address to rfi from, don't hard code the
> offset to 24 bytes, but have the assembler figure that out for us.
>
> Signed-off-by: Trent Piepho <tpiepho@freescale.com>
> Acked-by: Kumar Gala <galak@kernel.crashing.org>
> ---
> arch/powerpc/include/asm/mmu-fsl-booke.h |    2 ++
> arch/powerpc/kernel/head_fsl_booke.S     |   22 +++++++++++++---------
> 2 files changed, 15 insertions(+), 9 deletions(-)

applied to next.

- k

Re: [PATCH] powerpc: Better setup of boot page TLB entry

[Milton Miller]

On Thu Nov 20 at 06:14:30 EST in 2008, Trent Piepho wrote:
> The initial TLB mapping for the kernel boot didn't set the memory 
> coherent
> attribute, MAS2[M], in SMP mode.


> Also, from the MPC8572 manual section 6.12.5.3, "Bits that represent
> offsets within a page are ignored and should be cleared." Existing code
> didn't clear them, this code does.
>
> The same when the page of KERNELBASE is found; we don't need to use 
> asm to
> mask the lower 12 bits off.
>
> In the code that computes the address to rfi from, don't hard code the
> offset to 24 bytes, but have the assembler figure that out for us.

The expressions are still overly complex.

...
> -       li      r7,0
> -       lis     r6,PAGE_OFFSET at h
> -       ori     r6,r6,PAGE_OFFSET at l
> -       rlwimi  r6,r7,0,20,31
> +       lis     r6,MAS2_VAL(PAGE_OFFSET, BOOKE_PAGESZ_64M, M_IF_SMP)@h
> +       ori     r6,r6,MAS2_VAL(PAGE_OFFSET, BOOKE_PAGESZ_64M, 
> M_IF_SMP)@l

I'm fine with this part, even if the expression is a bit long.  You 
might consider using LOAD_REG_IMMEDIATE from asm/ppc_asm.h to avoid 
duplicating the expression.

>         mtspr   SPRN_MAS2,r6
>         mtspr   SPRN_MAS3,r8
>         tlbwe
>
>  /* 7. Jump to KERNELBASE mapping */
> -       lis     r6,KERNELBASE at h
> -       ori     r6,r6,KERNELBASE at l
> -       rlwimi  r6,r7,0,20,31
> +       lis     r6,(KERNELBASE & ~0xfff)@h
> +       ori     r6,r6,(KERNELBASE & ~0xfff)@l

Why do you need to mask off the bottom bits of KERNEL_BASE?  Just 
trying to keep the instruction effect identical?

First of all, if its not aligned, then its likely other parts of the 
kernel will break.  We could put a BUILD_BUG_ON somewhere (in c) if 
that check is required.

Second, it makes the expression longer and more complex (requiring 
parenthesis).

>         lis     r7,MSR_KERNEL at h
>         ori     r7,r7,MSR_KERNEL at l
>         bl      1f                      /* Find our address */
>  1:     mflr    r9
>         rlwimi  r6,r9,0,20,31

Third, this just inserted the offset into those bits, overwriting any 
previous value they had.

> -       addi    r6,r6,24
> +       addi    r6,r6,(2f - 1b)

and while doing assembler math is better than the hand computed 24, how 
about doing li r9,2f@l and just inserting that into r6?  Unless you 
expect step 8 to cross a page from the 1b label above.  But if you are 
that close to a page boundary than assuming 1b is in the page at 
KERNEL_BASE would seem to be suspect.

For that matter, just do LOAD_ADDR(2f) or LOAD_REG_IMMEDIATE(2f), which 
would give the same result, as KERNEL_BASE should be reflected the 
linked address of the kernel (I assume that you are not doing dynamic 
runtime relocation like ppc64 on the code up to here, otherwise the 
previous suggestion still works).

>         mtspr   SPRN_SRR0,r6
>         mtspr   SPRN_SRR1,r7
>         rfi                             /* start execution out of 
> TLB1[0] entry */
>
>  /* 8. Clear out the temp mapping */
> -       lis     r7,0x1000       /* Set MAS0(TLBSEL) = 1 */
> +2:     lis     r7,0x1000       /* Set MAS0(TLBSEL) = 1 */
>         rlwimi  r7,r5,16,4,15   /* Setup MAS0 = TLBSEL | ESEL(r5) */
>         mtspr   SPRN_MAS0,r7
>         tlbre

milton

Re: [PATCH] powerpc: Better setup of boot page TLB entry

[Trent Piepho]

On Sat, 22 Nov 2008, Milton Miller wrote:
> On Thu Nov 20 at 06:14:30 EST in 2008, Trent Piepho wrote:
>>  -       li      r7,0
>>  -       lis     r6,PAGE_OFFSET at h
>>  -       ori     r6,r6,PAGE_OFFSET at l
>>  -       rlwimi  r6,r7,0,20,31
>>  +       lis     r6,MAS2_VAL(PAGE_OFFSET, BOOKE_PAGESZ_64M, M_IF_SMP)@h
>>  +       ori     r6,r6,MAS2_VAL(PAGE_OFFSET, BOOKE_PAGESZ_64M, M_IF_SMP)@l
>
> I'm fine with this part, even if the expression is a bit long.  You might 
> consider using LOAD_REG_IMMEDIATE from asm/ppc_asm.h to avoid duplicating the 
> expression.

LOAD_REG_IMMEDIATE isn't used at all in that file, while lis/ori is used
many times.  In fact, there only one call of LOAD_REG_IMMEDIATE in all of
arch/powerpc/kernel/*.S.  So I think lis/ori is more easily recognized. 
And to be honest, I find switching syntax from assembly language
instructions to C style macros that generate instructions to be
aesthetically ugly.

It would be nice if the assembler provided a "liw" macro instruction that
would load an immediate.  When the assembler knows the immediate value, it
could even generate shorter sequences in some cases, like when the upper
16 bits are all zero.

>>  /* 7. Jump to KERNELBASE mapping */
>>  -       lis     r6,KERNELBASE at h
>>  -       ori     r6,r6,KERNELBASE at l
>>  -       rlwimi  r6,r7,0,20,31
>>  +       lis     r6,(KERNELBASE & ~0xfff)@h
>>  +       ori     r6,r6,(KERNELBASE & ~0xfff)@l
>
> Why do you need to mask off the bottom bits of KERNEL_BASE?  Just trying to 
> keep the instruction effect identical?

Yes, so it was clear I wasn't changing what the code did here.  And to
make it clear we only wanted the page number from KERNEL_BASE.  It's an
obvious expression and a compile time constant, merely saving a few
characters in the source doesn't seem worth much.  I realize it's
unnecessary since those bits get masked off in the wlwimi a few
instructions later.

Really all I wanted to fix the was memory coherency on SMP bug.  But the
code for MAS2 was stupid, so I had to change that to fix the bug in a
non-ugly way.  But then r7 didn't need to be zeroed and the (unnecessary)
"rlwimi r6,r7,0,20,31" would no longer be doing what's it's supposed to,
so I fixed that too.

> First of all, if its not aligned, then its likely other parts of the kernel 
> will break.  We could put a BUILD_BUG_ON somewhere (in c) if that check is 
> required.

I seems like "Require KERNEL_BASE to be page aligned and modify code to
depend on said requirement" belongs in another patch.

>>  -       addi    r6,r6,24
>>  +       addi    r6,r6,(2f - 1b)
>
> and while doing assembler math is better than the hand computed 24, how about 
> doing li r9,2f@l and just inserting that into r6?  Unless you expect step 8 
> to cross a page from the 1b label above.  But if you are that close to a page 
> boundary than assuming 1b is in the page at KERNEL_BASE would seem to be 
> suspect.
>
> For that matter, just do LOAD_ADDR(2f) or LOAD_REG_IMMEDIATE(2f), which would 
> give the same result, as KERNEL_BASE should be reflected the linked address 
> of the kernel (I assume that you are not doing dynamic runtime relocation 
> like ppc64 on the code up to here, otherwise the previous suggestion still 
> works).

I'm not sure if this code can be relocated or not.  If it isn't now, using
non-position independent code won't make it any easier to make it
relocatable.  It looks like the "bl 1f ; 1: mflr" sequence is used 13
times in arch/powerpc/kernel/*.S, I wonder if all of them are unnecessary?

Re: [PATCH] powerpc: Better setup of boot page TLB entry

[Kumar Gala]

On Nov 22, 2008, at 10:01 PM, Trent Piepho wrote:

> On Sat, 22 Nov 2008, Milton Miller wrote:
>> On Thu Nov 20 at 06:14:30 EST in 2008, Trent Piepho wrote:
>>> -       li      r7,0
>>> -       lis     r6,PAGE_OFFSET at h
>>> -       ori     r6,r6,PAGE_OFFSET at l
>>> -       rlwimi  r6,r7,0,20,31
>>> +       lis     r6,MAS2_VAL(PAGE_OFFSET, BOOKE_PAGESZ_64M,  
>>> M_IF_SMP)@h
>>> +       ori     r6,r6,MAS2_VAL(PAGE_OFFSET, BOOKE_PAGESZ_64M,  
>>> M_IF_SMP)@l
>>
>> I'm fine with this part, even if the expression is a bit long.  You  
>> might
>> consider using LOAD_REG_IMMEDIATE from asm/ppc_asm.h to avoid  
>> duplicating the
>> expression.
>
> LOAD_REG_IMMEDIATE isn't used at all in that file, while lis/ori is  
> used
> many times.  In fact, there only one call of LOAD_REG_IMMEDIATE in  
> all of
> arch/powerpc/kernel/*.S.  So I think lis/ori is more easily  
> recognized.
> And to be honest, I find switching syntax from assembly language
> instructions to C style macros that generate instructions to be
> aesthetically ugly.
>
> It would be nice if the assembler provided a "liw" macro instruction  
> that
> would load an immediate.  When the assembler knows the immediate  
> value, it
> could even generate shorter sequences in some cases, like when the  
> upper
> 16 bits are all zero.

I agree lis/ori is what should be used in this file and am not  
interested in changing it at this point.

>>> /* 7. Jump to KERNELBASE mapping */
>>> -       lis     r6,KERNELBASE at h
>>> -       ori     r6,r6,KERNELBASE at l
>>> -       rlwimi  r6,r7,0,20,31
>>> +       lis     r6,(KERNELBASE & ~0xfff)@h
>>> +       ori     r6,r6,(KERNELBASE & ~0xfff)@l
>>
>> Why do you need to mask off the bottom bits of KERNEL_BASE?  Just  
>> trying to
>> keep the instruction effect identical?
>
> Yes, so it was clear I wasn't changing what the code did here.  And to
> make it clear we only wanted the page number from KERNEL_BASE.  It's  
> an
> obvious expression and a compile time constant, merely saving a few
> characters in the source doesn't seem worth much.  I realize it's
> unnecessary since those bits get masked off in the wlwimi a few
> instructions later.
>
> Really all I wanted to fix the was memory coherency on SMP bug.  But  
> the
> code for MAS2 was stupid, so I had to change that to fix the bug in a
> non-ugly way.  But then r7 didn't need to be zeroed and the  
> (unnecessary)
> "rlwimi r6,r7,0,20,31" would no longer be doing what's it's supposed  
> to,
> so I fixed that too.
>
>> First of all, if its not aligned, then its likely other parts of  
>> the kernel
>> will break.  We could put a BUILD_BUG_ON somewhere (in c) if that  
>> check is
>> required.
>
> I seems like "Require KERNEL_BASE to be page aligned and modify code  
> to
> depend on said requirement" belongs in another patch.
>
>>> -       addi    r6,r6,24
>>> +       addi    r6,r6,(2f - 1b)
>>
>> and while doing assembler math is better than the hand computed 24,  
>> how about
>> doing li r9,2f@l and just inserting that into r6?  Unless you  
>> expect step 8
>> to cross a page from the 1b label above.  But if you are that close  
>> to a page
>> boundary than assuming 1b is in the page at KERNEL_BASE would seem  
>> to be
>> suspect.
>>
>> For that matter, just do LOAD_ADDR(2f) or LOAD_REG_IMMEDIATE(2f),  
>> which would
>> give the same result, as KERNEL_BASE should be reflected the linked  
>> address
>> of the kernel (I assume that you are not doing dynamic runtime  
>> relocation
>> like ppc64 on the code up to here, otherwise the previous  
>> suggestion still
>> works).
>
> I'm not sure if this code can be relocated or not.  If it isn't now,  
> using
> non-position independent code won't make it any easier to make it
> relocatable.  It looks like the "bl 1f ; 1: mflr" sequence is used 13
> times in arch/powerpc/kernel/*.S, I wonder if all of them are  
> unnecessary?

We support relocation of this kernel so any changes need to be tried  
out w/CONFIG_RELOCATABLE enabled.

I'm fine with the patch as is and any other changes should be follow  
on patches.

- k

Re: [PATCH] powerpc: Better setup of boot page TLB entry

[Milton Miller]

On Nov 23, 2008, at 11:01 PM, Kumar Gala wrote:
> On Nov 22, 2008, at 10:01 PM, Trent Piepho wrote:
>> On Sat, 22 Nov 2008, Milton Miller wrote:
>>> On Thu Nov 20 at 06:14:30 EST in 2008, Trent Piepho wrote:
>>>> -       li      r7,0
>>>> -       lis     r6,PAGE_OFFSET at h
>>>> -       ori     r6,r6,PAGE_OFFSET at l
>>>> -       rlwimi  r6,r7,0,20,31
>>>> +       lis     r6,MAS2_VAL(PAGE_OFFSET, BOOKE_PAGESZ_64M, 
>>>> M_IF_SMP)@h
>>>> +       ori     r6,r6,MAS2_VAL(PAGE_OFFSET, BOOKE_PAGESZ_64M, 
>>>> M_IF_SMP)@l
>>>
>>> I'm fine with this part, even if the expression is a bit long.  You 
>>> might
>>> consider using LOAD_REG_IMMEDIATE from asm/ppc_asm.h to avoid 
>>> duplicating the
>>> expression.
>>
>> LOAD_REG_IMMEDIATE isn't used at all in that file, while lis/ori is 
>> used
>> many times.  In fact, there only one call of LOAD_REG_IMMEDIATE in 
>> all of
>> arch/powerpc/kernel/*.S.  So I think lis/ori is more easily 
>> recognized.
>> And to be honest, I find switching syntax from assembly language
>> instructions to C style macros that generate instructions to be
>> aesthetically ugly.

The macro is more useful for the 64 bit case where it uses its arguemnt 
4 times.  The usage was severely reduced with the 64 bit relocatable 
patch, where *IMMEDIATE was not relocated but LOAD_ADDR was.

>> It would be nice if the assembler provided a "liw" macro instruction 
>> that
>> would load an immediate.  When the assembler knows the immediate 
>> value, it
>> could even generate shorter sequences in some cases, like when the 
>> upper
>> 16 bits are all zero.

One thing that is nice about powerpc assembly is that all instructions 
and macros are fixed length.  That means we don't need to do multiple 
passes to figure out how many bytes we need for that source line.  In 
fact, I think all the standard macros are just alias formatting of one 
instruction.   At a minimum, you would only want the zero detection 
when it was a constant.


> I agree lis/ori is what should be used in this file and am not 
> interested in changing it at this point.

It was only a light consider comment and would not have prompted the 
email.


>
>>>> /* 7. Jump to KERNELBASE mapping */
>>>> -       lis     r6,KERNELBASE at h
>>>> -       ori     r6,r6,KERNELBASE at l
>>>> -       rlwimi  r6,r7,0,20,31
>>>> +       lis     r6,(KERNELBASE & ~0xfff)@h
>>>> +       ori     r6,r6,(KERNELBASE & ~0xfff)@l
>>>
>>> Why do you need to mask off the bottom bits of KERNEL_BASE?  Just 
>>> trying to
>>> keep the instruction effect identical?
>>
>> Yes, so it was clear I wasn't changing what the code did here.  And to
>> make it clear we only wanted the page number from KERNEL_BASE.  It's 
>> an
>> obvious expression and a compile time constant, merely saving a few
>> characters in the source doesn't seem worth much.  I realize it's
>> unnecessary since those bits get masked off in the wlwimi a few
>> instructions later.

You realize it after studying the code, but the next person reading may 
not.   My take is putting this longer expression makes the code harder 
to read and a changelog comment pointing out that the lower bits are 
overwritten would have eliminated the need for this ugly expression.

>> Really all I wanted to fix the was memory coherency on SMP bug.  But 
>> the
>> code for MAS2 was stupid, so I had to change that to fix the bug in a
>> non-ugly way.  But then r7 didn't need to be zeroed and the 
>> (unnecessary)
>> "rlwimi r6,r7,0,20,31" would no longer be doing what's it's supposed 
>> to,
>> so I fixed that too.
>>
>>> First of all, if its not aligned, then its likely other parts of the 
>>> kernel
>>> will break.  We could put a BUILD_BUG_ON somewhere (in c) if that 
>>> check is
>>> required.
>>
>> I seems like "Require KERNEL_BASE to be page aligned and modify code 
>> to
>> depend on said requirement" belongs in another patch.

Well, I could write one, but I don't have any hardware to test.

And the above code depends on the alignement, by the fact that it 
inserts the 4k offset into kernel base.

>>>> -       addi    r6,r6,24
>>>> +       addi    r6,r6,(2f - 1b)
>>>
>>> and while doing assembler math is better than the hand computed 24, 
>>> how about
>>> doing li r9,2f@l and just inserting that into r6?  Unless you expect 
>>> step 8
>>> to cross a page from the 1b label above.  But if you are that close 
>>> to a page
>>> boundary than assuming 1b is in the page at KERNEL_BASE would seem 
>>> to be
>>> suspect.
>>>
>>> For that matter, just do LOAD_ADDR(2f) or LOAD_REG_IMMEDIATE(2f), 
>>> which would
>>> give the same result, as KERNEL_BASE should be reflected the linked 
>>> address
>>> of the kernel (I assume that you are not doing dynamic runtime 
>>> relocation
>>> like ppc64 on the code up to here, otherwise the previous suggestion 
>>> still
>>> works).
>>
>> I'm not sure if this code can be relocated or not.  If it isn't now, 
>> using
>> non-position independent code won't make it any easier to make it
>> relocatable.  It looks like the "bl 1f ; 1: mflr" sequence is used 13
>> times in arch/powerpc/kernel/*.S, I wonder if all of them are 
>> unnecessary?
>
> We support relocation of this kernel so any changes need to be tried 
> out w/CONFIG_RELOCATABLE enabled.

The question was not does the kernel support CONFIG_RELOCATABLE, the 
question was 'will some agent relocate the kernel to be running at its 
loaded location before this code (which appears to be very early) is 
run', or does the RELOCATABLE code for this flavor work by adjusting 
the linear mapping offset (which is what I thought it did).

>
> I'm fine with the patch as is and any other changes should be follow 
> on patches.
>
> - k

I don't have any booke hardware, so I'm less likely to look at this 
code.  Any changes from me would only be compile changes.

milton