From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 75C15C7EE24 for ; Tue, 16 May 2023 06:49:15 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 634F686086; Tue, 16 May 2023 08:49:13 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=starfivetech.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 74C0E857D5; Tue, 16 May 2023 08:49:12 +0200 (CEST) Received: from ex01.ufhost.com (ex01.ufhost.com [61.152.239.75]) by phobos.denx.de (Postfix) with ESMTP id 7C32786086 for ; Tue, 16 May 2023 08:49:07 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=starfivetech.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=yanhong.wang@starfivetech.com Received: from EXMBX165.cuchost.com (unknown [175.102.18.54]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "EXMBX165", Issuer "EXMBX165" (not verified)) by ex01.ufhost.com (Postfix) with ESMTP id 0540824E309; Tue, 16 May 2023 14:49:05 +0800 (CST) Received: from EXMBX073.cuchost.com (172.16.6.83) by EXMBX165.cuchost.com (172.16.6.75) with Microsoft SMTP Server (TLS) id 15.0.1497.42; Tue, 16 May 2023 14:49:04 +0800 Received: from [192.168.120.42] (171.223.208.138) by EXMBX073.cuchost.com (172.16.6.83) with Microsoft SMTP Server (TLS) id 15.0.1497.42; Tue, 16 May 2023 14:49:03 +0800 Message-ID: Date: Tue, 16 May 2023 14:49:02 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.10.1 Subject: Re: [PATCH v5 01/17] riscv: cpu: jh7110: Add support for jh7110 SoC To: Bo Gan , , Rick Chen , Leo , Lukasz Majewski , Sean Anderson CC: Lee Kuan Lim , Jianlong Huang , Emil Renner Berthing References: <20230329034224.26545-1-yanhong.wang@starfivetech.com> <20230329034224.26545-2-yanhong.wang@starfivetech.com> <9e188d70-78e3-8672-c3ac-125bfb403c03@gmail.com> Content-Language: en-US From: yanhong wang In-Reply-To: <9e188d70-78e3-8672-c3ac-125bfb403c03@gmail.com> Content-Type: text/plain; charset="UTF-8" X-Originating-IP: [171.223.208.138] X-ClientProxiedBy: EXCAS066.cuchost.com (172.16.6.26) To EXMBX073.cuchost.com (172.16.6.83) X-YovoleRuleAgent: yovoleflag Content-Transfer-Encoding: quoted-printable X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean On 2023/5/12 13:50, Bo Gan wrote: > On 3/28/23 8:42 PM, Yanhong Wang wrote: >> +void harts_early_init(void) >> +{ >> +=C2=A0=C2=A0=C2=A0 ulong *ptr; >> +=C2=A0=C2=A0=C2=A0 u8 *tmp; >> +=C2=A0=C2=A0=C2=A0 ulong len, remain; >> +=C2=A0=C2=A0=C2=A0 /* >> +=C2=A0=C2=A0=C2=A0=C2=A0 * Feature Disable CSR >> +=C2=A0=C2=A0=C2=A0=C2=A0 * >> +=C2=A0=C2=A0=C2=A0=C2=A0 * Clear feature disable CSR to '0' to turn o= n all features for >> +=C2=A0=C2=A0=C2=A0=C2=A0 * each core. This operation must be in M-mod= e. >> +=C2=A0=C2=A0=C2=A0=C2=A0 */ >> +=C2=A0=C2=A0=C2=A0 if (CONFIG_IS_ENABLED(RISCV_MMODE)) >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 csr_write(CSR_U74_FEATURE_= DISABLE, 0); >> + >> +=C2=A0=C2=A0=C2=A0 /* clear L2 LIM=C2=A0 memory >> +=C2=A0=C2=A0=C2=A0=C2=A0 * set __bss_end to 0x81FFFFF region to zero >> +=C2=A0=C2=A0=C2=A0=C2=A0 * The L2 Cache Controller supports ECC. ECC = is applied to SRAM. >> +=C2=A0=C2=A0=C2=A0=C2=A0 * If it is not cleared, the ECC part is inva= lid, and an ECC error >> +=C2=A0=C2=A0=C2=A0=C2=A0 * will be reported when reading data. >> +=C2=A0=C2=A0=C2=A0=C2=A0 */ >> +=C2=A0=C2=A0=C2=A0 ptr =3D (ulong *)&__bss_end; >> +=C2=A0=C2=A0=C2=A0 len =3D L2_LIM_MEM_END - (ulong)&__bss_end; >> +=C2=A0=C2=A0=C2=A0 remain =3D len % sizeof(ulong); >> +=C2=A0=C2=A0=C2=A0 len /=3D sizeof(ulong); >> + >> +=C2=A0=C2=A0=C2=A0 while (len--) >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *ptr++ =3D 0; >> + >> +=C2=A0=C2=A0=C2=A0 /* clear the remain bytes */ >> +=C2=A0=C2=A0=C2=A0 if (remain) { >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 tmp =3D (u8 *)ptr; >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 while (remain--) >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *t= mp++ =3D 0; >> +=C2=A0=C2=A0=C2=A0 } >> +} > Hi Yanhong, I know this is already merged, but it looks wrong to me.`ha= rts_early_init` > will be called by all harts in SPL. The per-hart stack sits between __b= ss_end and L2_LIM_MEM_END. > Zeroing this region could overwrite the hart's stack, and other harts' = stacks. The current > implementation works likely because harts_early_init doesn't use any st= ack space, but it's up to > the compiler and we can't guarantee that. If it were to save and restor= e `ra` register, then we > would crash in function epilogue. Also, we are having data-races here, = because harts are writing > over each other's stack. >=20 > My advice is that we should split the zeroing of L2 LIM into different = places just before the > region is to be used. For stacks, we can let each hart clearing its own= stack, and for the malloc > space, we can do so during malloc initialization. Doing so also gives u= s the benefit of catching > the read of uninitialized data. In this approach, the L2_LIM_MEM_END ma= cro is not needed anymore. Hi=EF=BC=8CBo Gan=EF=BC=8C I agree with you, there is some problem with t= he initialization of the L2 LIM,=20 so, as you suggested, we should split the zeroing of L2 LIM into differen= t places. Something like=EF=BC=9A diff --git a/arch/riscv/cpu/jh7110/spl.c b/arch/riscv/cpu/jh7110/spl.c index 72adcefa0e..574ffc3d33 100644 --- a/arch/riscv/cpu/jh7110/spl.c +++ b/arch/riscv/cpu/jh7110/spl.c @@ -13,7 +13,6 @@ #include =20 #define CSR_U74_FEATURE_DISABLE 0x7c1 -#define L2_LIM_MEM_END 0x81FFFFFUL =20 DECLARE_GLOBAL_DATA_PTR; =20 @@ -61,7 +60,7 @@ void harts_early_init(void) { ulong *ptr; u8 *tmp; - ulong len, remain; + ulong len, remain, init_end; /* * Feature Disable CSR * @@ -77,8 +76,10 @@ void harts_early_init(void) * If it is not cleared, the ECC part is invalid, and an ECC error * will be reported when reading data. */ + init_end =3D CONFIG_SPL_STACK -CONFIG_VAL(SYS_MALLOC_F_LEN) - sizeof(*g= d) + - CONFIG_NR_CPUS * BIT(CONFIG_STACK_SIZE_SHIFT); ptr =3D (ulong *)&__bss_end; - len =3D L2_LIM_MEM_END - (ulong)&__bss_end; + len =3D init_end - (ulong)&__bss_end; remain =3D len % sizeof(ulong); len /=3D sizeof(ulong); =20 diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index dad22bfea8..46da9ec503 100644 --- a/arch/riscv/cpu/start.S +++ b/arch/riscv/cpu/start.S @@ -118,6 +118,20 @@ call_board_init_f_0: mv sp, a0 #endif =20 +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK) && \ + defined(CONFIG_STARFIVE_JH7110) + + /* Set the stack region to zero */ + li t0, 1 + slli t1, t0, CONFIG_STACK_SIZE_SHIFT + mv t0, sp + sub t1, t0, t1 +clear_stack: + SREG zero, 0(t1) + addi t1, t1, REGBYTES + blt t1, t0, clear_stack +#endif + /* Configure proprietary settings and customized CSRs of harts */ call_harts_early_init: jal harts_early_init diff --git a/common/init/board_init.c b/common/init/board_init.c index 96ffb79a98..46e4e4abc7 100644 --- a/common/init/board_init.c +++ b/common/init/board_init.c @@ -162,6 +162,7 @@ void board_init_f_init_reserve(ulong base) #if CONFIG_VAL(SYS_MALLOC_F_LEN) /* go down one 'early malloc arena' */ gd->malloc_base =3D base; + memset((void *)base, 0, CONFIG_VAL(SYS_MALLOC_F_LEN)); #endif =20 if (CONFIG_IS_ENABLED(SYS_REPORT_STACK_F_USAGE))