Patchwork [v3] powerpc/4xx: work around CHIP11 errata in a more PAGE_SIZE-friendly way

login
register
mail settings
Submitter Hollis Blanchard
Date Nov. 24, 2008, 8:07 p.m.
Message ID <1227557235.17746.25.camel@localhost.localdomain>
Download mbox | patch
Permalink /patch/10475/
State Superseded, archived
Headers show

Comments

Hollis Blanchard - Nov. 24, 2008, 8:07 p.m.
On Fri, 2008-11-14 at 16:09 -0600, Hollis Blanchard wrote:
> 
> If this is all too much, then I'm close to giving up and burning a
> 64KB page, which requires only ALIGN_DOWN() in the kernel.

ppc: force memory size to be a multiple of PAGE_SIZE

Ensure that total memory size is page-aligned, because otherwise
bootmem.c gets upset.

This error case was triggered by using 64 KiB pages in the kernel while
arch/powerpc/boot/4xx.c arbitrarily reduced the amount of memory by 4096 (to
work around the "CHIP11" errata which affects the last 256 bytes of physical memory).

Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
---
This is on a common code path, and lmb_enforce_memory_limit() will now
always take action, so wider testing would be good.

This patch supercedes http://patchwork.ozlabs.org/patch/8211/ .
Michael Ellerman - Nov. 25, 2008, 12:10 a.m.
On Mon, 2008-11-24 at 14:07 -0600, Hollis Blanchard wrote:
> On Fri, 2008-11-14 at 16:09 -0600, Hollis Blanchard wrote:
> > 
> > If this is all too much, then I'm close to giving up and burning a
> > 64KB page, which requires only ALIGN_DOWN() in the kernel.
> 
> ppc: force memory size to be a multiple of PAGE_SIZE
> 
> Ensure that total memory size is page-aligned, because otherwise
> bootmem.c gets upset.
> 
> This error case was triggered by using 64 KiB pages in the kernel while
> arch/powerpc/boot/4xx.c arbitrarily reduced the amount of memory by 4096 (to
> work around the "CHIP11" errata which affects the last 256 bytes of physical memory).
> 
> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
> ---
> This is on a common code path, and lmb_enforce_memory_limit() will now
> always take action, so wider testing would be good.
> 
> This patch supercedes http://patchwork.ozlabs.org/patch/8211/ .
> 
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -1200,6 +1200,11 @@ void __init early_init_devtree(void *par
>  	early_reserve_mem();
>  	phyp_dump_reserve_mem();
>  
> +	/* Ensure that total memory size is page-aligned, because otherwise
> +	 * bootmem.c gets upset. */
> +	lmb_analyze();
> +	memory_limit = lmb_phys_mem_size() & PAGE_MASK;

All of the current code using memory_limit looks like it'll be safe with
this change, although there are several cases of this we could remove:

if (memory_limit && <some other condition>)

Because memory_limit will now always be true.

Still, I think it would be better to only set memory_limit when the mem
size is not a multiple of the PAGE_SIZE - so that memory_limit retains
it's function as both the value of the limit and a boolean.

cheers
Milton Miller - Nov. 25, 2008, 5:10 p.m.
On Nov 24, 2008, at 6:10 PM, Michael Ellerman wrote:
> On Mon, 2008-11-24 at 14:07 -0600, Hollis Blanchard wrote:
>> On Fri, 2008-11-14 at 16:09 -0600, Hollis Blanchard wrote:
>>>
>>> If this is all too much, then I'm close to giving up and burning a
>>> 64KB page, which requires only ALIGN_DOWN() in the kernel.
>>
>> ppc: force memory size to be a multiple of PAGE_SIZE
>>
>> Ensure that total memory size is page-aligned, because otherwise
>> bootmem.c gets upset.
>>
>> This error case was triggered by using 64 KiB pages in the kernel 
>> while
>> arch/powerpc/boot/4xx.c arbitrarily reduced the amount of memory by 
>> 4096 (to
>> work around the "CHIP11" errata which affects the last 256 bytes of 
>> physical memory).
>>
>> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>> ---
>> This is on a common code path, and lmb_enforce_memory_limit() will now
>> always take action, so wider testing would be good.
>>
>> This patch supercedes http://patchwork.ozlabs.org/patch/8211/ .
>>
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -1200,6 +1200,11 @@ void __init early_init_devtree(void *par
>>  	early_reserve_mem();
>>  	phyp_dump_reserve_mem();
>>
>> +	/* Ensure that total memory size is page-aligned, because otherwise
>> +	 * bootmem.c gets upset. */
>> +	lmb_analyze();
>> +	memory_limit = lmb_phys_mem_size() & PAGE_MASK;
>
> All of the current code using memory_limit looks like it'll be safe 
> with
> this change, although there are several cases of this we could remove:
>
> if (memory_limit && <some other condition>)
>
> Because memory_limit will now always be true.

memory_limit was the result of parsing mem= from the command line.   
Does this break that?

>
> Still, I think it would be better to only set memory_limit when the mem
> size is not a multiple of the PAGE_SIZE - so that memory_limit retains
> it's function as both the value of the limit and a boolean.

I would have expected this trimming to occur where we actually transfer 
the memory from lmb to bootmem, since it is bootmem that has the 
aligned size requirement.

milton
Hollis Blanchard - Nov. 25, 2008, 9:17 p.m.
On Tue, 2008-11-25 at 11:10 -0600, Milton Miller wrote:
> On Nov 24, 2008, at 6:10 PM, Michael Ellerman wrote:
> > On Mon, 2008-11-24 at 14:07 -0600, Hollis Blanchard wrote:
> >> On Fri, 2008-11-14 at 16:09 -0600, Hollis Blanchard wrote:
> >>>
> >>> If this is all too much, then I'm close to giving up and burning a
> >>> 64KB page, which requires only ALIGN_DOWN() in the kernel.
> >>
> >> ppc: force memory size to be a multiple of PAGE_SIZE
> >>
> >> Ensure that total memory size is page-aligned, because otherwise
> >> bootmem.c gets upset.
> >>
> >> This error case was triggered by using 64 KiB pages in the kernel 
> >> while
> >> arch/powerpc/boot/4xx.c arbitrarily reduced the amount of memory by 
> >> 4096 (to
> >> work around the "CHIP11" errata which affects the last 256 bytes of 
> >> physical memory).
> >>
> >> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
> >> ---
> >> This is on a common code path, and lmb_enforce_memory_limit() will now
> >> always take action, so wider testing would be good.
> >>
> >> This patch supercedes http://patchwork.ozlabs.org/patch/8211/ .
> >>
> >> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> >> --- a/arch/powerpc/kernel/prom.c
> >> +++ b/arch/powerpc/kernel/prom.c
> >> @@ -1200,6 +1200,11 @@ void __init early_init_devtree(void *par
> >>  	early_reserve_mem();
> >>  	phyp_dump_reserve_mem();
> >>
> >> +	/* Ensure that total memory size is page-aligned, because otherwise
> >> +	 * bootmem.c gets upset. */
> >> +	lmb_analyze();
> >> +	memory_limit = lmb_phys_mem_size() & PAGE_MASK;
> >
> > All of the current code using memory_limit looks like it'll be safe 
> > with
> > this change, although there are several cases of this we could remove:
> >
> > if (memory_limit && <some other condition>)
> >
> > Because memory_limit will now always be true.
> 
> memory_limit was the result of parsing mem= from the command line.   
> Does this break that?

If you're asking me, no, the patch doesn't break that. early_parse_mem()
already aligns memory_limit, so the additional mask does nothing.

> > Still, I think it would be better to only set memory_limit when the mem
> > size is not a multiple of the PAGE_SIZE - so that memory_limit retains
> > it's function as both the value of the limit and a boolean.
> 
> I would have expected this trimming to occur where we actually transfer 
> the memory from lmb to bootmem, since it is bootmem that has the 
> aligned size requirement.

No good; since LMB knows the memory exists, it allocates space for the
unflattened device tree in the last page, and then uses
reserve_bootmem() for that space and again, bootmem BUG()s.

Anyways, the LMB interface is wide, with lots of users directly
accessing LMB data structures. There are enough permutations of highmem,
crashdump kernels, et al that I don't feel comfortable trying to audit
and test all the LMB users.

Patch

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -1200,6 +1200,11 @@  void __init early_init_devtree(void *par
 	early_reserve_mem();
 	phyp_dump_reserve_mem();
 
+	/* Ensure that total memory size is page-aligned, because otherwise
+	 * bootmem.c gets upset. */
+	lmb_analyze();
+	memory_limit = lmb_phys_mem_size() & PAGE_MASK;
+
 	lmb_enforce_memory_limit(memory_limit);
 	lmb_analyze();