Message ID | TY3P286MB261182CE7C47A535BB3AC113980DA@TY3P286MB2611.JPNP286.PROD.OUTLOOK.COM |
---|---|
State | Superseded |
Delegated to: | Andes |
Headers | show |
Series | arch: riscv: jh7110: Correctly zero L2 LIM | expand |
Hi Shengyu, On Tue, Aug 08, 2023 at 08:39:56PM +0800, Shengyu Qu wrote: > Add the actual support code for SPL_ZERO_MEM_BEFORE_USE and remove > existing Starfive JH7110's L2 LIM clean code, since existing code has > following issues: > 1. Each hart (in the middle of a function call) overwriting its own > stack and other harts' stacks. > (data-race and data-corruption) > 2. Lottery winner hart can be doing "board_init_f_init_reserve", > while other harts are in the middle of zeroing L2 LIM. > (data-race) > > Signed-off-by: Bo Gan <ganboing@gmail.com> > Signed-off-by: Shengyu Qu <wiagn233@outlook.com> > --- > Changes since v2: > - Fix typo (ZERO_MEM_BEFORE_USE to SPL_ZERO_MEM_BEFORE_USE) > --- > arch/riscv/cpu/jh7110/spl.c | 25 ------------------------- > arch/riscv/cpu/start.S | 12 ++++++++++++ > common/init/board_init.c | 3 +++ > 3 files changed, 15 insertions(+), 25 deletions(-) > > diff --git a/arch/riscv/cpu/jh7110/spl.c b/arch/riscv/cpu/jh7110/spl.c > index 72adcefa0e..4047b10efe 100644 > --- a/arch/riscv/cpu/jh7110/spl.c > +++ b/arch/riscv/cpu/jh7110/spl.c > @@ -13,7 +13,6 @@ > #include <init.h> > > #define CSR_U74_FEATURE_DISABLE 0x7c1 > -#define L2_LIM_MEM_END 0x81FFFFFUL > > DECLARE_GLOBAL_DATA_PTR; > > @@ -59,9 +58,6 @@ int spl_soc_init(void) > > void harts_early_init(void) > { > - ulong *ptr; > - u8 *tmp; > - ulong len, remain; > /* > * Feature Disable CSR > * > @@ -70,25 +66,4 @@ void harts_early_init(void) > */ > if (CONFIG_IS_ENABLED(RISCV_MMODE)) > csr_write(CSR_U74_FEATURE_DISABLE, 0); > - > - /* clear L2 LIM memory > - * set __bss_end to 0x81FFFFF region to zero > - * The L2 Cache Controller supports ECC. ECC is applied to SRAM. > - * If it is not cleared, the ECC part is invalid, and an ECC error > - * will be reported when reading data. > - */ > - ptr = (ulong *)&__bss_end; > - len = L2_LIM_MEM_END - (ulong)&__bss_end; > - remain = len % sizeof(ulong); > - len /= sizeof(ulong); > - > - while (len--) > - *ptr++ = 0; > - > - /* clear the remain bytes */ > - if (remain) { > - tmp = (u8 *)ptr; > - while (remain--) > - *tmp++ = 0; > - } > } > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > index 59d58a5a57..930309d8d2 100644 > --- a/arch/riscv/cpu/start.S > +++ b/arch/riscv/cpu/start.S > @@ -111,6 +111,18 @@ call_board_init_f: > * It's essential before any function call, otherwise, we get data-race. > */ > > +/* clear stack if necessary */ > +#if CONFIG_IS_ENABLED(SPL_ZERO_MEM_BEFORE_USE) I think what you have in v2 patch was the correct one. Could you refer to include/linux/kconfig.h to check if this usage fits your expectation ? > +clear_stack: > + li t1, 1 > + slli t1, t1, CONFIG_STACK_SIZE_SHIFT > + sub t1, sp, t1 > +clear_stack_loop: > + SREG zero, 0(t1) /* t1 is always 16 byte aligned */ > + addi t1, t1, REGBYTES > + blt t1, sp, clear_stack_loop > +#endif > + > call_board_init_f_0: > /* find top of reserve space */ > #if CONFIG_IS_ENABLED(SMP) > diff --git a/common/init/board_init.c b/common/init/board_init.c > index 96ffb79a98..51d9ec9a13 100644 > --- a/common/init/board_init.c > +++ b/common/init/board_init.c > @@ -162,6 +162,9 @@ void board_init_f_init_reserve(ulong base) > #if CONFIG_VAL(SYS_MALLOC_F_LEN) > /* go down one 'early malloc arena' */ > gd->malloc_base = base; > +#if CONFIG_IS_ENABLED(SPL_ZERO_MEM_BEFORE_USE) Ditto. Best regards, Leo > + memset((void *)base, '\0', CONFIG_VAL(SYS_MALLOC_F_LEN)); > +#endif > #endif > > if (CONFIG_IS_ENABLED(SYS_REPORT_STACK_F_USAGE)) > -- > 2.41.0 >
Hi Leo, Seems you are right. I'll send v4 to fix this. Thank you. Best regards, Shengyu > Hi Shengyu, > > On Tue, Aug 08, 2023 at 08:39:56PM +0800, Shengyu Qu wrote: >> Add the actual support code for SPL_ZERO_MEM_BEFORE_USE and remove >> existing Starfive JH7110's L2 LIM clean code, since existing code has >> following issues: >> 1. Each hart (in the middle of a function call) overwriting its own >> stack and other harts' stacks. >> (data-race and data-corruption) >> 2. Lottery winner hart can be doing "board_init_f_init_reserve", >> while other harts are in the middle of zeroing L2 LIM. >> (data-race) >> >> Signed-off-by: Bo Gan <ganboing@gmail.com> >> Signed-off-by: Shengyu Qu <wiagn233@outlook.com> >> --- >> Changes since v2: >> - Fix typo (ZERO_MEM_BEFORE_USE to SPL_ZERO_MEM_BEFORE_USE) >> --- >> arch/riscv/cpu/jh7110/spl.c | 25 ------------------------- >> arch/riscv/cpu/start.S | 12 ++++++++++++ >> common/init/board_init.c | 3 +++ >> 3 files changed, 15 insertions(+), 25 deletions(-) >> >> diff --git a/arch/riscv/cpu/jh7110/spl.c b/arch/riscv/cpu/jh7110/spl.c >> index 72adcefa0e..4047b10efe 100644 >> --- a/arch/riscv/cpu/jh7110/spl.c >> +++ b/arch/riscv/cpu/jh7110/spl.c >> @@ -13,7 +13,6 @@ >> #include <init.h> >> >> #define CSR_U74_FEATURE_DISABLE 0x7c1 >> -#define L2_LIM_MEM_END 0x81FFFFFUL >> >> DECLARE_GLOBAL_DATA_PTR; >> >> @@ -59,9 +58,6 @@ int spl_soc_init(void) >> >> void harts_early_init(void) >> { >> - ulong *ptr; >> - u8 *tmp; >> - ulong len, remain; >> /* >> * Feature Disable CSR >> * >> @@ -70,25 +66,4 @@ void harts_early_init(void) >> */ >> if (CONFIG_IS_ENABLED(RISCV_MMODE)) >> csr_write(CSR_U74_FEATURE_DISABLE, 0); >> - >> - /* clear L2 LIM memory >> - * set __bss_end to 0x81FFFFF region to zero >> - * The L2 Cache Controller supports ECC. ECC is applied to SRAM. >> - * If it is not cleared, the ECC part is invalid, and an ECC error >> - * will be reported when reading data. >> - */ >> - ptr = (ulong *)&__bss_end; >> - len = L2_LIM_MEM_END - (ulong)&__bss_end; >> - remain = len % sizeof(ulong); >> - len /= sizeof(ulong); >> - >> - while (len--) >> - *ptr++ = 0; >> - >> - /* clear the remain bytes */ >> - if (remain) { >> - tmp = (u8 *)ptr; >> - while (remain--) >> - *tmp++ = 0; >> - } >> } >> diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S >> index 59d58a5a57..930309d8d2 100644 >> --- a/arch/riscv/cpu/start.S >> +++ b/arch/riscv/cpu/start.S >> @@ -111,6 +111,18 @@ call_board_init_f: >> * It's essential before any function call, otherwise, we get data-race. >> */ >> >> +/* clear stack if necessary */ >> +#if CONFIG_IS_ENABLED(SPL_ZERO_MEM_BEFORE_USE) > I think what you have in v2 patch was the correct one. > > Could you refer to include/linux/kconfig.h to check if > this usage fits your expectation ? > >> +clear_stack: >> + li t1, 1 >> + slli t1, t1, CONFIG_STACK_SIZE_SHIFT >> + sub t1, sp, t1 >> +clear_stack_loop: >> + SREG zero, 0(t1) /* t1 is always 16 byte aligned */ >> + addi t1, t1, REGBYTES >> + blt t1, sp, clear_stack_loop >> +#endif >> + >> call_board_init_f_0: >> /* find top of reserve space */ >> #if CONFIG_IS_ENABLED(SMP) >> diff --git a/common/init/board_init.c b/common/init/board_init.c >> index 96ffb79a98..51d9ec9a13 100644 >> --- a/common/init/board_init.c >> +++ b/common/init/board_init.c >> @@ -162,6 +162,9 @@ void board_init_f_init_reserve(ulong base) >> #if CONFIG_VAL(SYS_MALLOC_F_LEN) >> /* go down one 'early malloc arena' */ >> gd->malloc_base = base; >> +#if CONFIG_IS_ENABLED(SPL_ZERO_MEM_BEFORE_USE) > Ditto. > > Best regards, > Leo >> + memset((void *)base, '\0', CONFIG_VAL(SYS_MALLOC_F_LEN)); >> +#endif >> #endif >> >> if (CONFIG_IS_ENABLED(SYS_REPORT_STACK_F_USAGE)) >> -- >> 2.41.0 >>
diff --git a/arch/riscv/cpu/jh7110/spl.c b/arch/riscv/cpu/jh7110/spl.c index 72adcefa0e..4047b10efe 100644 --- a/arch/riscv/cpu/jh7110/spl.c +++ b/arch/riscv/cpu/jh7110/spl.c @@ -13,7 +13,6 @@ #include <init.h> #define CSR_U74_FEATURE_DISABLE 0x7c1 -#define L2_LIM_MEM_END 0x81FFFFFUL DECLARE_GLOBAL_DATA_PTR; @@ -59,9 +58,6 @@ int spl_soc_init(void) void harts_early_init(void) { - ulong *ptr; - u8 *tmp; - ulong len, remain; /* * Feature Disable CSR * @@ -70,25 +66,4 @@ void harts_early_init(void) */ if (CONFIG_IS_ENABLED(RISCV_MMODE)) csr_write(CSR_U74_FEATURE_DISABLE, 0); - - /* clear L2 LIM memory - * set __bss_end to 0x81FFFFF region to zero - * The L2 Cache Controller supports ECC. ECC is applied to SRAM. - * If it is not cleared, the ECC part is invalid, and an ECC error - * will be reported when reading data. - */ - ptr = (ulong *)&__bss_end; - len = L2_LIM_MEM_END - (ulong)&__bss_end; - remain = len % sizeof(ulong); - len /= sizeof(ulong); - - while (len--) - *ptr++ = 0; - - /* clear the remain bytes */ - if (remain) { - tmp = (u8 *)ptr; - while (remain--) - *tmp++ = 0; - } } diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index 59d58a5a57..930309d8d2 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -111,6 +111,18 @@ call_board_init_f: * It's essential before any function call, otherwise, we get data-race. */ +/* clear stack if necessary */ +#if CONFIG_IS_ENABLED(SPL_ZERO_MEM_BEFORE_USE) +clear_stack: + li t1, 1 + slli t1, t1, CONFIG_STACK_SIZE_SHIFT + sub t1, sp, t1 +clear_stack_loop: + SREG zero, 0(t1) /* t1 is always 16 byte aligned */ + addi t1, t1, REGBYTES + blt t1, sp, clear_stack_loop +#endif + call_board_init_f_0: /* find top of reserve space */ #if CONFIG_IS_ENABLED(SMP) diff --git a/common/init/board_init.c b/common/init/board_init.c index 96ffb79a98..51d9ec9a13 100644 --- a/common/init/board_init.c +++ b/common/init/board_init.c @@ -162,6 +162,9 @@ void board_init_f_init_reserve(ulong base) #if CONFIG_VAL(SYS_MALLOC_F_LEN) /* go down one 'early malloc arena' */ gd->malloc_base = base; +#if CONFIG_IS_ENABLED(SPL_ZERO_MEM_BEFORE_USE) + memset((void *)base, '\0', CONFIG_VAL(SYS_MALLOC_F_LEN)); +#endif #endif if (CONFIG_IS_ENABLED(SYS_REPORT_STACK_F_USAGE))