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 4C73CC77B75 for ; Fri, 12 May 2023 12:15:27 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 53210862A2; Fri, 12 May 2023 14:15:15 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="oyXRCwtA"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 0CA9086188; Fri, 12 May 2023 07:50:15 +0200 (CEST) Received: from mail-pl1-x62d.google.com (mail-pl1-x62d.google.com [IPv6:2607:f8b0:4864:20::62d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id E75D48619A for ; Fri, 12 May 2023 07:50:10 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ganboing@gmail.com Received: by mail-pl1-x62d.google.com with SMTP id d9443c01a7336-1aaf70676b6so68027495ad.3 for ; Thu, 11 May 2023 22:50:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1683870609; x=1686462609; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject:from:to:cc :subject:date:message-id:reply-to; bh=LUMCL1xQ1RswtsLr1HIW3CH6h0BzbqvPJY7KPGCuKzI=; b=oyXRCwtAqcXu9PwvGhZt2beKZoLa9ftASbHJm0X4sY4UmHuIA0iNH/962uobbeeTnR 6alEpnmlsGaB/w8f9waP15r71ZEI3C61xN/YBAWbMmtoByFUOepuUSdUcW4H2MBQ1JtX ifJ9l/+rTXBLHGUEbH54L9TtKE9VXj0dduOPOQZ1gW5UP0z9R6d7T4YAfKCsNXp8YmhN y939REn2MtOBZ4NcKXHQdqz3I0YhoOzc3yKs8NyYqBxUuxCjqYjc41jUZ+fZiZ4kE55C g9AYmyIFbj6kKIaHxQnLE6Y+bEyX/TzFYIZWq0zMKJqDSYv/DEGxVPwH9jzO5h64yCGR L8Cw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683870609; x=1686462609; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=LUMCL1xQ1RswtsLr1HIW3CH6h0BzbqvPJY7KPGCuKzI=; b=Pbn3uN8Y+1HylC6QcLhhsOKQSrTshXjlj5/ZzI8s291ZtDCsESZ0CTNVHQb0V4omVZ nLfXWWCuKh6LPdX6bS8VFi7eaJqNO6GOJmA7PUPcMv1iHGRzBq92IZDyvbPTWEGeojJr dvs2w+dKGy1aX9c+EXh/DZTDHAQubMtR/gQHBC4QehawSquq6eN6ibPYbFjxLxEWpT1y FD0fiBt0+wtCA/CYCAp/4EHYUfy961Gt2BsYeY1GH4otCIyLtHW3XMLtOXWdt/dTE47C VPUlG3rXMMGJX3FPaMsjws8MKcpYgeeoH8fPEM18wvZq+3KxKHrCjOIWGJNOEU1hGo6e li2g== X-Gm-Message-State: AC+VfDzRy4SqIIe3UHdjRRxFGRiPGr3IVbslZg9rdyjBk7IosjTPSgQj XA414wxmuAGP5BjKQh+qP7Wif9QE4LRLnA== X-Google-Smtp-Source: ACHHUZ4f+usfXbP0V8unfGuuso3XoJHmuM38YYLOSHx0FDFqPWeE7vn8zwCR5Z5S/b8L/iSkEuUuNg== X-Received: by 2002:a17:902:d38c:b0:1ab:675:3e31 with SMTP id e12-20020a170902d38c00b001ab06753e31mr28243806pld.37.1683870609119; Thu, 11 May 2023 22:50:09 -0700 (PDT) Received: from [192.168.0.13] ([172.92.174.136]) by smtp.gmail.com with ESMTPSA id g7-20020a170902868700b001aad4be4503sm6988102plo.2.2023.05.11.22.50.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 11 May 2023 22:50:08 -0700 (PDT) Subject: Re: [PATCH v5 01/17] riscv: cpu: jh7110: Add support for jh7110 SoC To: Yanhong Wang , u-boot@lists.denx.de, 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> From: Bo Gan Message-ID: <9e188d70-78e3-8672-c3ac-125bfb403c03@gmail.com> Date: Thu, 11 May 2023 22:50:07 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20230329034224.26545-2-yanhong.wang@starfivetech.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Mailman-Approved-At: Fri, 12 May 2023 14:15:11 +0200 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 3/28/23 8:42 PM, Yanhong Wang wrote: > +void harts_early_init(void) > +{ > + ulong *ptr; > + u8 *tmp; > + ulong len, remain; > + /* > + * Feature Disable CSR > + * > + * Clear feature disable CSR to '0' to turn on all features for > + * each core. This operation must be in M-mode. > + */ > + 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; > + } > +} Hi Yanhong, I know this is already merged, but it looks wrong to me.`harts_early_init` will be called by all harts in SPL. The per-hart stack sits between __bss_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 stack space, but it's up to the compiler and we can't guarantee that. If it were to save and restore `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. 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 us the benefit of catching the read of uninitialized data. In this approach, the L2_LIM_MEM_END macro is not needed anymore.