diff mbox

[U-Boot,v2,1/1] mpc85xx: Enable pre-relocation malloc for MPC85xx

Message ID 20160404120105.Horde.mwwJa_RF5jlhJUz3K5COSQ9@webmail.df.eu
State Not Applicable
Headers show

Commit Message

Mario Six April 4, 2016, 10:01 a.m. UTC 
Quoting York Sun <york.sun@nxp.com>:

> On 04/01/2016 01:13 AM, Mario Six wrote:
>> To enable DM on MPC85xx, we need pre-relocation malloc, which is
>> implemented in this patch.
>>
>> We also make sure that the IVORs are always 4-aligned on e500 to prevent
>> alignment exceptions caused by code changes in start.S.
>>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>> Cc: York Sun <york.sun@nxp.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> ---
>>
>> v2:
>>  - Correct IVOR alignment and remove nop hack
>>  - Add sanity check for length of malloc area
>>
>>
>>  arch/powerpc/cpu/mpc85xx/cpu_init_early.c |  8 --------
>>  arch/powerpc/cpu/mpc85xx/start.S          | 28 ++++++++++++++++++++++++++++
>>  include/ppc_asm.tmpl                      |  3 +++
>>  3 files changed, 31 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c  
>> b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
>> index 235a635..e6e1688 100644
>> --- a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
>> +++ b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
>> @@ -82,7 +82,6 @@ void setup_ifc(void)
>>  void cpu_init_early_f(void *fdt)
>>  {
>>  	u32 mas0, mas1, mas2, mas3, mas7;
>> -	int i;
>>  #ifdef CONFIG_SYS_FSL_ERRATUM_P1010_A003549
>>  	ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
>>  #endif
>> @@ -95,13 +94,6 @@ void cpu_init_early_f(void *fdt)
>>  	/* Pointer is writable since we allocated a register for it */
>>  	gd = (gd_t *) (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_GBL_DATA_OFFSET);
>>
>> -	/*
>> -	 * Clear initial global data
>> -	 *   we don't use memset so we can share this code with NAND_SPL
>> -	 */
>> -	for (i = 0; i < sizeof(gd_t); i++)
>> -		((char *)gd)[i] = 0;
>> -
>
> I would appreciate a comment here to explain GD had been zeroed by ...
>

Sure, will add in v3.

>>  #ifdef CONFIG_QEMU_E500
>>  	/*
>>  	 * CONFIG_SYS_CCSRBAR_PHYS below may use gd->fdt_blob on ePAPR systems,
>> diff --git a/arch/powerpc/cpu/mpc85xx/start.S  
>> b/arch/powerpc/cpu/mpc85xx/start.S
>> index d867e2a..cc4cf5f 100644
>> --- a/arch/powerpc/cpu/mpc85xx/start.S
>> +++ b/arch/powerpc/cpu/mpc85xx/start.S
>> @@ -1152,6 +1152,34 @@ _start_cont:
>>  	/* Setup the stack in initial RAM,could be L2-as-SRAM or L1 dcache*/
>>  	lis	r3,(CONFIG_SYS_INIT_RAM_ADDR)@h
>>  	ori	r3,r3,((CONFIG_SYS_INIT_SP_OFFSET-16)&~0xf)@l /* Align to 16 */
>> +
>> +#ifdef CONFIG_SYS_MALLOC_F_LEN
>> +
>> +#if CONFIG_SYS_MALLOC_F_LEN + GENERATED_GBL_DATA_SIZE >  
>> CONFIG_SYS_INIT_RAM_SIZE
>> +#error "CONFIG_SYS_MALLOC_F_LEN too large to fit into initial RAM."
>> +#endif
>
> I am not an expert on linking. I wonder if there can be a way to  
> determine the
> max depth of stack. If we can, a check here would be great. If we  
> can't, we have
> to live with it.
>
>> +
>> +	/* Leave 16+ byte for back chain termination and NULL return address */
>> +	subi	r3,r3,((CONFIG_SYS_MALLOC_F_LEN+16+15)&~0xf)
>> +
>> +	/* End of RAM */
>> +	lis	r4,(CONFIG_SYS_INIT_RAM_ADDR)@h
>> +	ori	r4,r4,(CONFIG_SYS_INIT_RAM_SIZE)@l
>> +
>> +	li	r0,0
>> +
>> +1: 	subi 	r4,r4,4
>> +	stw 	r0,0(r4)
>> +	cmplw 	r4,r3
>> +	bne	1b
>> +
>> +	lis	r4,(CONFIG_SYS_INIT_RAM_ADDR)@h
>> +	ori	r4,r4,(CONFIG_SYS_GBL_DATA_OFFSET)@l
>> +
>> +	addi	r3,r3,16	/* Pre-relocation malloc area */
>> +	stw	r3,GD_MALLOC_BASE(r4)
>> +	subi	r3,r3,16
>> +#endif
>>  	li	r0,0
>>  	stw	r0,0(r3)	/* Terminate Back Chain */
>>  	stw	r0,+4(r3)	/* NULL return address. */
>
> I was thinking to add .align 4 before the start of exception vectors.
>

I tried

without the aligns in ppc_asm.tmpl, and that crashes the board, I'm afraid.

>
>
>> diff --git a/include/ppc_asm.tmpl b/include/ppc_asm.tmpl
>> index ba166eb..379c493 100644
>> --- a/include/ppc_asm.tmpl
>> +++ b/include/ppc_asm.tmpl
>> @@ -263,12 +263,14 @@
>>  	b	transfer_to_handler
>>
>>  #define STD_EXCEPTION(n, label, hdlr)				\
>> +.align 4;							\
>>  label:								\
>>  	EXCEPTION_PROLOG(SRR0, SRR1);				\
>>  	addi	r3,r1,STACK_FRAME_OVERHEAD;			\
>>  	EXC_XFER_TEMPLATE(n, label, hdlr, MSR_KERNEL, NOCOPY)	\
>>
>>  #define CRIT_EXCEPTION(n, label, hdlr)				\
>> +.align 4;							\
>>  label:								\
>>  	EXCEPTION_PROLOG(CSRR0, CSRR1);				\
>>  	addi	r3,r1,STACK_FRAME_OVERHEAD;			\
>> @@ -276,6 +278,7 @@ label:								\
>>  	MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE), NOCOPY)		\
>>
>>  #define MCK_EXCEPTION(n, label, hdlr)				\
>> +.align 4;							\
>>  label:								\
>>  	EXCEPTION_PROLOG(MCSRR0, MCSRR1);			\
>>  	addi	r3,r1,STACK_FRAME_OVERHEAD;			\
>> --
>> 2.7.0.GIT
>>
>>

Best regards,

Mario

Comments

York Sun April 4, 2016, 4:04 p.m. UTC | #1 
On 04/04/2016 03:01 AM, Mario Six wrote:
> 
> Quoting York Sun <york.sun@nxp.com>:
> 
>> On 04/01/2016 01:13 AM, Mario Six wrote:
>>> To enable DM on MPC85xx, we need pre-relocation malloc, which is
>>> implemented in this patch.
>>>
>>> We also make sure that the IVORs are always 4-aligned on e500 to prevent
>>> alignment exceptions caused by code changes in start.S.
>>>
>>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>> Cc: York Sun <york.sun@nxp.com>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>> v2:
>>>  - Correct IVOR alignment and remove nop hack
>>>  - Add sanity check for length of malloc area
>>>
>>>
>>>  arch/powerpc/cpu/mpc85xx/cpu_init_early.c |  8 --------
>>>  arch/powerpc/cpu/mpc85xx/start.S          | 28 ++++++++++++++++++++++++++++
>>>  include/ppc_asm.tmpl                      |  3 +++
>>>  3 files changed, 31 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c  
>>> b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
>>> index 235a635..e6e1688 100644
>>> --- a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
>>> +++ b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
>>> @@ -82,7 +82,6 @@ void setup_ifc(void)
>>>  void cpu_init_early_f(void *fdt)
>>>  {
>>>  	u32 mas0, mas1, mas2, mas3, mas7;
>>> -	int i;
>>>  #ifdef CONFIG_SYS_FSL_ERRATUM_P1010_A003549
>>>  	ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
>>>  #endif
>>> @@ -95,13 +94,6 @@ void cpu_init_early_f(void *fdt)
>>>  	/* Pointer is writable since we allocated a register for it */
>>>  	gd = (gd_t *) (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_GBL_DATA_OFFSET);
>>>
>>> -	/*
>>> -	 * Clear initial global data
>>> -	 *   we don't use memset so we can share this code with NAND_SPL
>>> -	 */
>>> -	for (i = 0; i < sizeof(gd_t); i++)
>>> -		((char *)gd)[i] = 0;
>>> -
>>
>> I would appreciate a comment here to explain GD had been zeroed by ...
>>
> 
> Sure, will add in v3.
> 
>>>  #ifdef CONFIG_QEMU_E500
>>>  	/*
>>>  	 * CONFIG_SYS_CCSRBAR_PHYS below may use gd->fdt_blob on ePAPR systems,
>>> diff --git a/arch/powerpc/cpu/mpc85xx/start.S  
>>> b/arch/powerpc/cpu/mpc85xx/start.S
>>> index d867e2a..cc4cf5f 100644
>>> --- a/arch/powerpc/cpu/mpc85xx/start.S
>>> +++ b/arch/powerpc/cpu/mpc85xx/start.S
>>> @@ -1152,6 +1152,34 @@ _start_cont:
>>>  	/* Setup the stack in initial RAM,could be L2-as-SRAM or L1 dcache*/
>>>  	lis	r3,(CONFIG_SYS_INIT_RAM_ADDR)@h
>>>  	ori	r3,r3,((CONFIG_SYS_INIT_SP_OFFSET-16)&~0xf)@l /* Align to 16 */
>>> +
>>> +#ifdef CONFIG_SYS_MALLOC_F_LEN
>>> +
>>> +#if CONFIG_SYS_MALLOC_F_LEN + GENERATED_GBL_DATA_SIZE >  
>>> CONFIG_SYS_INIT_RAM_SIZE
>>> +#error "CONFIG_SYS_MALLOC_F_LEN too large to fit into initial RAM."
>>> +#endif
>>
>> I am not an expert on linking. I wonder if there can be a way to  
>> determine the
>> max depth of stack. If we can, a check here would be great. If we  
>> can't, we have
>> to live with it.
>>
>>> +
>>> +	/* Leave 16+ byte for back chain termination and NULL return address */
>>> +	subi	r3,r3,((CONFIG_SYS_MALLOC_F_LEN+16+15)&~0xf)
>>> +
>>> +	/* End of RAM */
>>> +	lis	r4,(CONFIG_SYS_INIT_RAM_ADDR)@h
>>> +	ori	r4,r4,(CONFIG_SYS_INIT_RAM_SIZE)@l
>>> +
>>> +	li	r0,0
>>> +
>>> +1: 	subi 	r4,r4,4
>>> +	stw 	r0,0(r4)
>>> +	cmplw 	r4,r3
>>> +	bne	1b
>>> +
>>> +	lis	r4,(CONFIG_SYS_INIT_RAM_ADDR)@h
>>> +	ori	r4,r4,(CONFIG_SYS_GBL_DATA_OFFSET)@l
>>> +
>>> +	addi	r3,r3,16	/* Pre-relocation malloc area */
>>> +	stw	r3,GD_MALLOC_BASE(r4)
>>> +	subi	r3,r3,16
>>> +#endif
>>>  	li	r0,0
>>>  	stw	r0,0(r3)	/* Terminate Back Chain */
>>>  	stw	r0,+4(r3)	/* NULL return address. */
>>
>> I was thinking to add .align 4 before the start of exception vectors.
>>
> 
> I tried
> 
> diff --git a/arch/powerpc/cpu/mpc85xx/start.S  
> b/arch/powerpc/cpu/mpc85xx/start.S
> index d867e2a..921c6f5 100644
> --- a/arch/powerpc/cpu/mpc85xx/start.S
> +++ b/arch/powerpc/cpu/mpc85xx/start.S
> @@ -1177,6 +1177,7 @@ _start_cont:
>   	/* NOTREACHED - board_init_f() does not return */
> 
>   #ifndef MINIMAL_SPL
> +	.align 4
>   	.globl	_start_of_vectors
>   _start_of_vectors:
> 
> without the aligns in ppc_asm.tmpl, and that crashes the board, I'm afraid.
> 

Actually it is correct to add the alignment in ppc_asm.tmpl. It wasn't correct
before but we were lucky to run this far.

York
Mario Six April 5, 2016, 12:55 p.m. UTC | #2 
Quoting York Sun <york.sun@nxp.com>:

> On 04/04/2016 03:01 AM, Mario Six wrote:
>>
>> Quoting York Sun <york.sun@nxp.com>:
>>
>>> On 04/01/2016 01:13 AM, Mario Six wrote:
>>>> To enable DM on MPC85xx, we need pre-relocation malloc, which is
>>>> implemented in this patch.
>>>>
>>>> We also make sure that the IVORs are always 4-aligned on e500 to prevent
>>>> alignment exceptions caused by code changes in start.S.
>>>>
>>>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>>> Cc: York Sun <york.sun@nxp.com>
>>>> Cc: Simon Glass <sjg@chromium.org>
>>>> ---
>>>>
>>>> v2:
>>>>  - Correct IVOR alignment and remove nop hack
>>>>  - Add sanity check for length of malloc area
>>>>
>>>>
>>>>  arch/powerpc/cpu/mpc85xx/cpu_init_early.c |  8 --------
>>>>  arch/powerpc/cpu/mpc85xx/start.S          | 28  
>>>> ++++++++++++++++++++++++++++
>>>>  include/ppc_asm.tmpl                      |  3 +++
>>>>  3 files changed, 31 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
>>>> b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
>>>> index 235a635..e6e1688 100644
>>>> --- a/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
>>>> +++ b/arch/powerpc/cpu/mpc85xx/cpu_init_early.c
>>>> @@ -82,7 +82,6 @@ void setup_ifc(void)
>>>>  void cpu_init_early_f(void *fdt)
>>>>  {
>>>>  	u32 mas0, mas1, mas2, mas3, mas7;
>>>> -	int i;
>>>>  #ifdef CONFIG_SYS_FSL_ERRATUM_P1010_A003549
>>>>  	ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
>>>>  #endif
>>>> @@ -95,13 +94,6 @@ void cpu_init_early_f(void *fdt)
>>>>  	/* Pointer is writable since we allocated a register for it */
>>>>  	gd = (gd_t *) (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_GBL_DATA_OFFSET);
>>>>
>>>> -	/*
>>>> -	 * Clear initial global data
>>>> -	 *   we don't use memset so we can share this code with NAND_SPL
>>>> -	 */
>>>> -	for (i = 0; i < sizeof(gd_t); i++)
>>>> -		((char *)gd)[i] = 0;
>>>> -
>>>
>>> I would appreciate a comment here to explain GD had been zeroed by ...
>>>
>>
>> Sure, will add in v3.
>>
>>>>  #ifdef CONFIG_QEMU_E500
>>>>  	/*
>>>>  	 * CONFIG_SYS_CCSRBAR_PHYS below may use gd->fdt_blob on ePAPR systems,
>>>> diff --git a/arch/powerpc/cpu/mpc85xx/start.S
>>>> b/arch/powerpc/cpu/mpc85xx/start.S
>>>> index d867e2a..cc4cf5f 100644
>>>> --- a/arch/powerpc/cpu/mpc85xx/start.S
>>>> +++ b/arch/powerpc/cpu/mpc85xx/start.S
>>>> @@ -1152,6 +1152,34 @@ _start_cont:
>>>>  	/* Setup the stack in initial RAM,could be L2-as-SRAM or L1 dcache*/
>>>>  	lis	r3,(CONFIG_SYS_INIT_RAM_ADDR)@h
>>>>  	ori	r3,r3,((CONFIG_SYS_INIT_SP_OFFSET-16)&~0xf)@l /* Align to 16 */
>>>> +
>>>> +#ifdef CONFIG_SYS_MALLOC_F_LEN
>>>> +
>>>> +#if CONFIG_SYS_MALLOC_F_LEN + GENERATED_GBL_DATA_SIZE >
>>>> CONFIG_SYS_INIT_RAM_SIZE
>>>> +#error "CONFIG_SYS_MALLOC_F_LEN too large to fit into initial RAM."
>>>> +#endif
>>>
>>> I am not an expert on linking. I wonder if there can be a way to
>>> determine the
>>> max depth of stack. If we can, a check here would be great. If we
>>> can't, we have
>>> to live with it.
>>>
>>>> +
>>>> +	/* Leave 16+ byte for back chain termination and NULL return address */
>>>> +	subi	r3,r3,((CONFIG_SYS_MALLOC_F_LEN+16+15)&~0xf)
>>>> +
>>>> +	/* End of RAM */
>>>> +	lis	r4,(CONFIG_SYS_INIT_RAM_ADDR)@h
>>>> +	ori	r4,r4,(CONFIG_SYS_INIT_RAM_SIZE)@l
>>>> +
>>>> +	li	r0,0
>>>> +
>>>> +1: 	subi 	r4,r4,4
>>>> +	stw 	r0,0(r4)
>>>> +	cmplw 	r4,r3
>>>> +	bne	1b
>>>> +
>>>> +	lis	r4,(CONFIG_SYS_INIT_RAM_ADDR)@h
>>>> +	ori	r4,r4,(CONFIG_SYS_GBL_DATA_OFFSET)@l
>>>> +
>>>> +	addi	r3,r3,16	/* Pre-relocation malloc area */
>>>> +	stw	r3,GD_MALLOC_BASE(r4)
>>>> +	subi	r3,r3,16
>>>> +#endif
>>>>  	li	r0,0
>>>>  	stw	r0,0(r3)	/* Terminate Back Chain */
>>>>  	stw	r0,+4(r3)	/* NULL return address. */
>>>
>>> I was thinking to add .align 4 before the start of exception vectors.
>>>
>>
>> I tried
>>
>> diff --git a/arch/powerpc/cpu/mpc85xx/start.S
>> b/arch/powerpc/cpu/mpc85xx/start.S
>> index d867e2a..921c6f5 100644
>> --- a/arch/powerpc/cpu/mpc85xx/start.S
>> +++ b/arch/powerpc/cpu/mpc85xx/start.S
>> @@ -1177,6 +1177,7 @@ _start_cont:
>>   	/* NOTREACHED - board_init_f() does not return */
>>
>>   #ifndef MINIMAL_SPL
>> +	.align 4
>>   	.globl	_start_of_vectors
>>   _start_of_vectors:
>>
>> without the aligns in ppc_asm.tmpl, and that crashes the board, I'm afraid.
>>
>
> Actually it is correct to add the alignment in ppc_asm.tmpl. It  
> wasn't correct
> before but we were lucky to run this far.
>
> York

OK, I'll keep the aligns in ppc_asm.tmpl.

Also, I noticed a mistake on my part: The RAM has to be zeroed in start.S,
regardless of whether CONFIG_SYS_MALLOC_F_LEN is defined or not, since I
removed the zeroing from cpu_init_early.c. I'll fix that in v3, too.

Best regards,

Mario
diff mbox

Patch

diff --git a/arch/powerpc/cpu/mpc85xx/start.S  
b/arch/powerpc/cpu/mpc85xx/start.S
index d867e2a..921c6f5 100644
--- a/arch/powerpc/cpu/mpc85xx/start.S
+++ b/arch/powerpc/cpu/mpc85xx/start.S
@@ -1177,6 +1177,7 @@  _start_cont:
  	/* NOTREACHED - board_init_f() does not return */

  #ifndef MINIMAL_SPL
+	.align 4
  	.globl	_start_of_vectors
  _start_of_vectors: