[PATCH 1/2] microblaze: start.S: Factor out exception setup code to __setup_exceptions

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] microblaze: start.S: Factor out exception setup code to __setup_exceptions

Ovidiu Panait
Currently, the exceptions setup code is duplicated in pre-relocation and
post-relocation init. Factor out this code to __setup_exceptions asm
routine to get rid of the duplication.

__setup_exceptions is called with a relocation offset parameter (r5)
which is set to zero for pre-reloc init and gd->reloc_off for post-reloc
exception setup.

Signed-off-by: Ovidiu Panait <[hidden email]>
---

 arch/microblaze/cpu/start.S | 183 +++++++++++++++++-------------------
 1 file changed, 88 insertions(+), 95 deletions(-)

diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S
index cbec299b7d..f3be014317 100644
--- a/arch/microblaze/cpu/start.S
+++ b/arch/microblaze/cpu/start.S
@@ -13,13 +13,6 @@
  .text
  .global _start
 _start:
- /*
- * reserve registers:
- * r10: Stores little/big endian offset for vectors
- * r2: Stores imm opcode
- * r3: Stores brai opcode
- */
-
  mts rmsr, r0 /* disable cache */
 
  addi r8, r0, __end
@@ -38,6 +31,84 @@ _start:
  mts rshr, r1
  addi r1, r1, -4 /* Decrement SP to top of memory */
 
+ /* Setup vectors with pre-relocation symbols */
+ or r5, r0, r0
+ bralid r15, __setup_exceptions
+ nop
+#endif /* CONFIG_SPL_BUILD */
+
+ /* Flush cache before enable cache */
+ addik r5, r0, 0
+ addik r6, r0, XILINX_DCACHE_BYTE_SIZE
+ bralid r15, flush_cache
+ nop
+
+ /* enable instruction and data cache */
+ mfs r12, rmsr
+ ori r12, r12, 0x1a0
+ mts rmsr, r12
+
+ /* TODO: Redo this code to call board_init_f_*() */
+clear_bss:
+ /* clear BSS segments */
+ addi r5, r0, __bss_start
+ addi r4, r0, __bss_end
+ cmp r6, r5, r4
+ beqi r6, 3f
+2:
+ swi     r0, r5, 0 /* write zero to loc */
+ addi    r5, r5, 4 /* increment to next loc */
+ cmp     r6, r5, r4 /* check if we have reach the end */
+ bnei    r6, 2b
+3: /* jumping to board_init */
+#ifdef CONFIG_DEBUG_UART
+ bralid r15, debug_uart_init
+ nop
+#endif
+#ifndef CONFIG_SPL_BUILD
+ or r5, r0, r0 /* flags - empty */
+ addi    r31, r0, _gd
+#if CONFIG_VAL(SYS_MALLOC_F_LEN)
+ addi r6, r0, CONFIG_SYS_INIT_SP_OFFSET
+ swi r6, r31, GD_MALLOC_BASE
+#endif
+ brai board_init_f
+#else
+ addi r31, r0, _gd
+#if CONFIG_VAL(SYS_MALLOC_F_LEN)
+ addi r6, r0, CONFIG_SPL_STACK_ADDR
+ swi r6, r31, GD_MALLOC_BASE
+#endif
+ brai board_init_r
+#endif
+1: bri 1b
+
+ .section .bss
+.align 4
+_gd:
+         .space  GENERATED_GBL_DATA_SIZE
+
+#ifndef CONFIG_SPL_BUILD
+ .text
+ .ent __setup_exceptions
+ .align 2
+/*
+ * Set up reset, interrupt, user exception and hardware exception vectors.
+ *
+ * Parameters:
+ * r5 - relocation offset (zero when setting up vectors before
+ *      relocation, and gd->reloc_off when setting up vectors after
+ *      relocation)
+ *    - the relocation offset is added to the _exception_handler,
+ *      _interrupt_handler and _hw_exception_handler symbols to reflect the
+ *      post-relocation memory addresses
+ *
+ * Reserve registers:
+ * r10: Stores little/big endian offset for vectors
+ * r2: Stores imm opcode
+ * r3: Stores brai opcode
+ */
+__setup_exceptions:
  /* Find-out if u-boot is running on BIG/LITTLE endian platform
  * There are some steps which is necessary to keep in mind:
  * 1. Setup offset value to r6
@@ -76,7 +147,7 @@ _start:
  swi r2, r0, 0x8 /* user vector exception - imm opcode */
  swi r3, r0, 0xC /* user vector exception - brai opcode */
 
- addik r6, r0, _exception_handler
+ addik r6, r5, _exception_handler
  sw r6, r1, r0
  /*
  * BIG ENDIAN memory map for user exception
@@ -109,7 +180,7 @@ _start:
  swi r2, r0, 0x10 /* interrupt - imm opcode */
  swi r3, r0, 0x14 /* interrupt - brai opcode */
 
- addik r6, r0, _interrupt_handler
+ addik r6, r5, _interrupt_handler
  sw r6, r1, r0
  lhu r7, r1, r10
  rsubi r8, r10, 0x12
@@ -121,67 +192,18 @@ _start:
  swi r2, r0, 0x20 /* hardware exception - imm opcode */
  swi r3, r0, 0x24 /* hardware exception - brai opcode */
 
- addik r6, r0, _hw_exception_handler
+ addik r6, r5, _hw_exception_handler
  sw r6, r1, r0
  lhu r7, r1, r10
  rsubi r8, r10, 0x22
  sh r7, r0, r8
  rsubi r8, r10, 0x26
  sh r6, r0, r8
-#endif /* CONFIG_SPL_BUILD */
-
- /* Flush cache before enable cache */
- addik r5, r0, 0
- addik r6, r0, XILINX_DCACHE_BYTE_SIZE
- bralid r15, flush_cache
- nop
-
- /* enable instruction and data cache */
- mfs r12, rmsr
- ori r12, r12, 0x1a0
- mts rmsr, r12
 
- /* TODO: Redo this code to call board_init_f_*() */
-clear_bss:
- /* clear BSS segments */
- addi r5, r0, __bss_start
- addi r4, r0, __bss_end
- cmp r6, r5, r4
- beqi r6, 3f
-2:
- swi     r0, r5, 0 /* write zero to loc */
- addi    r5, r5, 4 /* increment to next loc */
- cmp     r6, r5, r4 /* check if we have reach the end */
- bnei    r6, 2b
-3: /* jumping to board_init */
-#ifdef CONFIG_DEBUG_UART
- bralid r15, debug_uart_init
- nop
-#endif
-#ifndef CONFIG_SPL_BUILD
- or r5, r0, r0 /* flags - empty */
- addi    r31, r0, _gd
-#if CONFIG_VAL(SYS_MALLOC_F_LEN)
- addi r6, r0, CONFIG_SYS_INIT_SP_OFFSET
- swi r6, r31, GD_MALLOC_BASE
-#endif
- brai board_init_f
-#else
- addi r31, r0, _gd
-#if CONFIG_VAL(SYS_MALLOC_F_LEN)
- addi r6, r0, CONFIG_SPL_STACK_ADDR
- swi r6, r31, GD_MALLOC_BASE
-#endif
- brai board_init_r
-#endif
-1: bri 1b
-
- .section .bss
-.align 4
-_gd:
-         .space  GENERATED_GBL_DATA_SIZE
+ rtsd r15, 8
+ or r0, r0, r0
+ .end __setup_exceptions
 
-#ifndef CONFIG_SPL_BUILD
 /*
  * Read 16bit little endian
  */
@@ -249,39 +271,10 @@ relocate_code:
  addi r24, r0, CONFIG_SYS_TEXT_BASE /* Get reloc offset */
  rsub r23, r24, r23 /* keep - this is already here gd->reloc_off */
 
- addik r6, r0, 0x2 /* BIG/LITTLE endian offset */
- lwi r7, r0, 0x28
- swi r6, r0, 0x28 /* used first unused MB vector */
- lbui r10, r0, 0x28 /* used first unused MB vector */
- swi r7, r0, 0x28
-
-#ifdef CONFIG_SYS_USR_EXCEP
- addik r6, r0, _exception_handler
- addk r6, r6, r23 /* add offset */
- sw r6, r1, r0
- lhu r7, r1, r10
- rsubi r8, r10, 0xa
- sh r7, r0, r8
- rsubi r8, r10, 0xe
- sh r6, r0, r8
-#endif
- addik r6, r0, _hw_exception_handler
- addk r6, r6, r23 /* add offset */
- sw r6, r1, r0
- lhu r7, r1, r10
- rsubi r8, r10, 0x22
- sh r7, r0, r8
- rsubi r8, r10, 0x26
- sh r6, r0, r8
-
- addik r6, r0, _interrupt_handler
- addk r6, r6, r23 /* add offset */
- sw r6, r1, r0
- lhu r7, r1, r10
- rsubi r8, r10, 0x12
- sh r7, r0, r8
- rsubi r8, r10, 0x16
- sh r6, r0, r8
+ /* Setup vectors with post-relocation symbols */
+ add r5, r0, r23 /* load gd->reloc_off to r5 */
+ bralid r15, __setup_exceptions
+ nop
 
  /* Check if GOT exist */
  addik r21, r23, _got_start
--
2.17.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] microblaze: start.S: Use board_init_f_alloc/init in early init

Ovidiu Panait
Implement early init by calling generic board_init_f_alloc_reserve and
board_init_f_init_reserve functions:
* drop SYS_MALLOC_F_LEN related code, as allocation and gd->malloc_base
  assignment are taken care of by the generic functions
* drop _gd logic

Signed-off-by: Ovidiu Panait <[hidden email]>
---

 arch/microblaze/cpu/start.S | 45 ++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S
index f3be014317..1f580b1112 100644
--- a/arch/microblaze/cpu/start.S
+++ b/arch/microblaze/cpu/start.S
@@ -17,25 +17,40 @@ _start:
 
  addi r8, r0, __end
  mts rslr, r8
- /* TODO: Redo this code to call board_init_f_*() */
+
 #if defined(CONFIG_SPL_BUILD)
  addi r1, r0, CONFIG_SPL_STACK_ADDR
- mts rshr, r1
- addi r1, r1, -4 /* Decrement SP to top of memory */
-#else
-#if CONFIG_VAL(SYS_MALLOC_F_LEN)
- addi r1, r0, CONFIG_SYS_INIT_SP_OFFSET - CONFIG_VAL(SYS_MALLOC_F_LEN)
 #else
  addi r1, r0, CONFIG_SYS_INIT_SP_OFFSET
 #endif
+
  mts rshr, r1
  addi r1, r1, -4 /* Decrement SP to top of memory */
 
+ /* Call board_init_f_alloc_reserve with the current stack pointer as
+ * parameter. */
+ add r5, r0, r1
+ bralid r15, board_init_f_alloc_reserve
+ nop
+
+ /* board_init_f_alloc_reserve returns a pointer to the allocated area
+ * in r3. Set the new stack pointer below this area. */
+ add r1, r0, r3
+ mts rshr, r1
+ addi r1, r1, -4
+
+ /* Call board_init_f_init_reserve with the address returned by
+ * board_init_f_alloc_reserve as parameter. */
+ add r5, r0, r3
+ bralid r15, board_init_f_init_reserve
+ nop
+
+#if !defined(CONFIG_SPL_BUILD)
  /* Setup vectors with pre-relocation symbols */
  or r5, r0, r0
  bralid r15, __setup_exceptions
  nop
-#endif /* CONFIG_SPL_BUILD */
+#endif
 
  /* Flush cache before enable cache */
  addik r5, r0, 0
@@ -48,7 +63,6 @@ _start:
  ori r12, r12, 0x1a0
  mts rmsr, r12
 
- /* TODO: Redo this code to call board_init_f_*() */
 clear_bss:
  /* clear BSS segments */
  addi r5, r0, __bss_start
@@ -67,27 +81,12 @@ clear_bss:
 #endif
 #ifndef CONFIG_SPL_BUILD
  or r5, r0, r0 /* flags - empty */
- addi    r31, r0, _gd
-#if CONFIG_VAL(SYS_MALLOC_F_LEN)
- addi r6, r0, CONFIG_SYS_INIT_SP_OFFSET
- swi r6, r31, GD_MALLOC_BASE
-#endif
  brai board_init_f
 #else
- addi r31, r0, _gd
-#if CONFIG_VAL(SYS_MALLOC_F_LEN)
- addi r6, r0, CONFIG_SPL_STACK_ADDR
- swi r6, r31, GD_MALLOC_BASE
-#endif
  brai board_init_r
 #endif
 1: bri 1b
 
- .section .bss
-.align 4
-_gd:
-         .space  GENERATED_GBL_DATA_SIZE
-
 #ifndef CONFIG_SPL_BUILD
  .text
  .ent __setup_exceptions
--
2.17.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] microblaze: start.S: Use board_init_f_alloc/init in early init

Michal Simek-3


On 20. 09. 20 18:50, Ovidiu Panait wrote:

> Implement early init by calling generic board_init_f_alloc_reserve and
> board_init_f_init_reserve functions:
> * drop SYS_MALLOC_F_LEN related code, as allocation and gd->malloc_base
>   assignment are taken care of by the generic functions
> * drop _gd logic
>
> Signed-off-by: Ovidiu Panait <[hidden email]>
> ---
>
>  arch/microblaze/cpu/start.S | 45 ++++++++++++++++++-------------------
>  1 file changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S
> index f3be014317..1f580b1112 100644
> --- a/arch/microblaze/cpu/start.S
> +++ b/arch/microblaze/cpu/start.S
> @@ -17,25 +17,40 @@ _start:
>  
>   addi r8, r0, __end
>   mts rslr, r8
> - /* TODO: Redo this code to call board_init_f_*() */
> +
>  #if defined(CONFIG_SPL_BUILD)
>   addi r1, r0, CONFIG_SPL_STACK_ADDR
> - mts rshr, r1
> - addi r1, r1, -4 /* Decrement SP to top of memory */
> -#else
> -#if CONFIG_VAL(SYS_MALLOC_F_LEN)
> - addi r1, r0, CONFIG_SYS_INIT_SP_OFFSET - CONFIG_VAL(SYS_MALLOC_F_LEN)
>  #else
>   addi r1, r0, CONFIG_SYS_INIT_SP_OFFSET
>  #endif
> +
>   mts rshr, r1
>   addi r1, r1, -4 /* Decrement SP to top of memory */

You are setting stack high protection in below code that's why no need
to call it twice here but


>  
> + /* Call board_init_f_alloc_reserve with the current stack pointer as
> + * parameter. */
> + add r5, r0, r1
> + bralid r15, board_init_f_alloc_reserve
> + nop

In SPL case this will decrease stack by SYS_MALLOC_F_LEN which is wrong.

This should be fine if you remove CONFIG_SYS_MALLOC_F_LEN from
CONFIG_SPL_STACK_ADDR then behavior will remain the same as was before.
(there is one more rounddown in board_init_f_alloc_reserve() but that
should be fine.

215 # define CONFIG_SPL_STACK_ADDR          (CONFIG_SYS_INIT_RAM_ADDR + \
216                                          CONFIG_SYS_INIT_RAM_SIZE - \
217                                          CONFIG_SYS_MALLOC_F_LEN)


The rest of code looks good to me.

Thanks,
Michal
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] microblaze: start.S: Factor out exception setup code to __setup_exceptions

Michal Simek-4
In reply to this post by Ovidiu Panait


On 20. 09. 20 18:50, Ovidiu Panait wrote:

> Currently, the exceptions setup code is duplicated in pre-relocation and
> post-relocation init. Factor out this code to __setup_exceptions asm
> routine to get rid of the duplication.
>
> __setup_exceptions is called with a relocation offset parameter (r5)
> which is set to zero for pre-reloc init and gd->reloc_off for post-reloc
> exception setup.
>
> Signed-off-by: Ovidiu Panait <[hidden email]>
> ---
>
>  arch/microblaze/cpu/start.S | 183 +++++++++++++++++-------------------
>  1 file changed, 88 insertions(+), 95 deletions(-)
>
> diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S
> index cbec299b7d..f3be014317 100644
> --- a/arch/microblaze/cpu/start.S
> +++ b/arch/microblaze/cpu/start.S
> @@ -13,13 +13,6 @@
>   .text
>   .global _start
>  _start:
> - /*
> - * reserve registers:
> - * r10: Stores little/big endian offset for vectors
> - * r2: Stores imm opcode
> - * r3: Stores brai opcode
> - */
> -
>   mts rmsr, r0 /* disable cache */
>  
>   addi r8, r0, __end
> @@ -38,6 +31,84 @@ _start:
>   mts rshr, r1
>   addi r1, r1, -4 /* Decrement SP to top of memory */
>  
> + /* Setup vectors with pre-relocation symbols */
> + or r5, r0, r0
> + bralid r15, __setup_exceptions
> + nop

Please align code with the rest which is tab between asm instruction and
arguments.


> +#endif /* CONFIG_SPL_BUILD */
> +
> + /* Flush cache before enable cache */
> + addik r5, r0, 0
> + addik r6, r0, XILINX_DCACHE_BYTE_SIZE
> + bralid r15, flush_cache
> + nop
> +
> + /* enable instruction and data cache */
> + mfs r12, rmsr
> + ori r12, r12, 0x1a0
> + mts rmsr, r12
> +
> + /* TODO: Redo this code to call board_init_f_*() */
> +clear_bss:
> + /* clear BSS segments */
> + addi r5, r0, __bss_start
> + addi r4, r0, __bss_end
> + cmp r6, r5, r4
> + beqi r6, 3f
> +2:
> + swi     r0, r5, 0 /* write zero to loc */
> + addi    r5, r5, 4 /* increment to next loc */
> + cmp     r6, r5, r4 /* check if we have reach the end */
> + bnei    r6, 2b
> +3: /* jumping to board_init */
> +#ifdef CONFIG_DEBUG_UART
> + bralid r15, debug_uart_init
> + nop
> +#endif
> +#ifndef CONFIG_SPL_BUILD
> + or r5, r0, r0 /* flags - empty */
> + addi    r31, r0, _gd
> +#if CONFIG_VAL(SYS_MALLOC_F_LEN)
> + addi r6, r0, CONFIG_SYS_INIT_SP_OFFSET
> + swi r6, r31, GD_MALLOC_BASE
> +#endif
> + brai board_init_f
> +#else
> + addi r31, r0, _gd
> +#if CONFIG_VAL(SYS_MALLOC_F_LEN)
> + addi r6, r0, CONFIG_SPL_STACK_ADDR
> + swi r6, r31, GD_MALLOC_BASE
> +#endif
> + brai board_init_r
> +#endif
> +1: bri 1b
> +
> + .section .bss
> +.align 4
> +_gd:
> +         .space  GENERATED_GBL_DATA_SIZE
> +
> +#ifndef CONFIG_SPL_BUILD
> + .text
> + .ent __setup_exceptions
> + .align 2
> +/*
> + * Set up reset, interrupt, user exception and hardware exception vectors.
> + *
> + * Parameters:
> + * r5 - relocation offset (zero when setting up vectors before
> + *      relocation, and gd->reloc_off when setting up vectors after
> + *      relocation)
> + *    - the relocation offset is added to the _exception_handler,
> + *      _interrupt_handler and _hw_exception_handler symbols to reflect the
> + *      post-relocation memory addresses
> + *
> + * Reserve registers:
> + * r10: Stores little/big endian offset for vectors
> + * r2: Stores imm opcode
> + * r3: Stores brai opcode

This is the biggest problem I have with this series. This function is
called from early code which is just the code below with return here
where certain registers are used and I expect this is working well.
But when this function is called from relocate code function these
registers can be used already and this function is not saving them.

I would rather see that regs saved to stack just for sure.

Thanks,
Michal

--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs