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 D7820C00140 for ; Mon, 15 Aug 2022 07:27:20 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 583A184867; Mon, 15 Aug 2022 09:27:18 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=maquefel.me Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=maquefel.me header.i=@maquefel.me header.b="HxHuKMjV"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id EABEE848DB; Mon, 15 Aug 2022 09:27:15 +0200 (CEST) Received: from forward500o.mail.yandex.net (forward500o.mail.yandex.net [IPv6:2a02:6b8:0:1a2d::610]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 704FF84860 for ; Mon, 15 Aug 2022 09:27:12 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=maquefel.me Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=nikita.shubin@maquefel.me Received: from vla3-23c3b031fed5.qloud-c.yandex.net (vla3-23c3b031fed5.qloud-c.yandex.net [IPv6:2a02:6b8:c15:2582:0:640:23c3:b031]) by forward500o.mail.yandex.net (Yandex) with ESMTP id 68C829400C7; Mon, 15 Aug 2022 10:27:11 +0300 (MSK) Received: by vla3-23c3b031fed5.qloud-c.yandex.net (smtp/Yandex) with ESMTPSA id VWhyfH1fGH-R9h0Ur1F; Mon, 15 Aug 2022 10:27:10 +0300 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client certificate not present) X-Yandex-Fwd: 1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=maquefel.me; s=mail; t=1660548430; bh=rS9MZVPoYi7CN9GizYFygWU6wjlWpbwZ8TRzfhJmJzE=; h=Cc:Message-ID:Subject:Date:References:To:From:In-Reply-To; b=HxHuKMjV96gw/kePa0saE/3wHjtGQ81a5zrTUACgNENftxntDZgNh6bvmlSHqOgRm HqsIcFjdqSbasi1WwyrFimeZw/O/eHr5NN2ehRHJybYuEUThtmkpxck6uYHWX9h2hQ +IL3UC+pMm7xUhuVIMGTM7I2JkfzjXg1/4YkUgwg= Authentication-Results: vla3-23c3b031fed5.qloud-c.yandex.net; dkim=pass header.i=@maquefel.me Date: Mon, 15 Aug 2022 10:27:07 +0300 From: Nikita Shubin To: Sean Anderson Cc: linux@yadro.com, Nikita Shubin , Rick Chen , Leo , Simon Glass , Ilias Apalodimas , Heinrich Schuchardt , Sergei Miroshnichenko , Alexandru Gagniuc , Alper Nebi Yasak , Andrew Davis , u-boot@lists.denx.de Subject: Re: [RFC PATCH 1/1] spl: introduce SPL_XIP to config Message-ID: <20220815102707.7215495c@redslave.neermore.group> In-Reply-To: <9afeff6a-07f3-a951-c083-da8ad77a5031@gmail.com> References: <20220811093706.3772-1-nikita.shubin@maquefel.me> <20220811093706.3772-2-nikita.shubin@maquefel.me> <9afeff6a-07f3-a951-c083-da8ad77a5031@gmail.com> X-Mailer: Claws Mail 3.17.7 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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.6 at phobos.denx.de X-Virus-Status: Clean Hello Sean! On Sat, 13 Aug 2022 00:35:59 -0400 Sean Anderson wrote: > Hi Nikita, > > On 8/11/22 5:37 AM, Nikita Shubin wrote: > > From: Nikita Shubin > > > > U-Boot and SPL don't necessary share the same location, so we might > > end with XIP SPL and U-Boot in "normal" memory. > > > > This adds an option special for SPL to behave it in XIP manner and > > don't use hart_lottery and available_harts_lock. > > > > Signed-off-by: Nikita Shubin > > --- > > On a stylistic note, typically cover letters are not used for single > patches (but feel free to use them for larger series). > > > arch/riscv/cpu/cpu.c | 2 +- > > arch/riscv/cpu/start.S | 4 ++-- > > arch/riscv/include/asm/global_data.h | 2 +- > > arch/riscv/lib/asm-offsets.c | 2 +- > > arch/riscv/lib/smp.c | 2 +- > > common/spl/Kconfig | 5 +++++ > > include/configs/scr7_vcu118.h | 2 ++ > > 7 files changed, 13 insertions(+), 6 deletions(-) > > > > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c > > index 9f5fa0bcb3..89528748d7 100644 > > --- a/arch/riscv/cpu/cpu.c > > +++ b/arch/riscv/cpu/cpu.c > > @@ -19,7 +19,7 @@ > > * The variables here must be stored in the data section since > > they are used > > * before the bss section is available. > > */ > > -#ifndef CONFIG_XIP > > +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP) > Shouldn't this be > > #if CONFIG_IS_ENABLED(XIP) > > ? > > ditto for the rest of these I think you are right, i am a bit confused with CONFIG vs CONFIG_IS_ENABLED. > > > u32 hart_lottery __section(".data") = 0; > > > > /* > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > index 26cb877ed1..d824990778 100644 > > --- a/arch/riscv/cpu/start.S > > +++ b/arch/riscv/cpu/start.S > > @@ -122,7 +122,7 @@ call_board_init_f_0: > > call_harts_early_init: > > jal harts_early_init > > > > -#ifndef CONFIG_XIP > > +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP) > > /* > > * Pick hart to initialize global data and run U-Boot. > > The other harts > > * wait for initialization to complete. > > @@ -155,7 +155,7 @@ call_harts_early_init: > > /* save the boot hart id to global_data */ > > SREG tp, GD_BOOT_HART(gp) > > > > -#ifndef CONFIG_XIP > > +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP) > > la t0, available_harts_lock > > amoswap.w.rl zero, zero, 0(t0) > > > > diff --git a/arch/riscv/include/asm/global_data.h > > b/arch/riscv/include/asm/global_data.h index 9a146d1d49..d71e09c5ab > > 100644 --- a/arch/riscv/include/asm/global_data.h > > +++ b/arch/riscv/include/asm/global_data.h > > @@ -30,7 +30,7 @@ int iccm[CONFIG_NR_CPUS]; > > #if CONFIG_IS_ENABLED(SMP) > > struct ipi_data ipi[CONFIG_NR_CPUS]; > > #endif > > -#ifndef CONFIG_XIP > > +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP) > > ulong available_harts; > > #endif > > }; > > diff --git a/arch/riscv/lib/asm-offsets.c > > b/arch/riscv/lib/asm-offsets.c index f1fe089b3d..bcb3c78654 100644 > > --- a/arch/riscv/lib/asm-offsets.c > > +++ b/arch/riscv/lib/asm-offsets.c > > @@ -16,7 +16,7 @@ int main(void) > > { > > DEFINE(GD_BOOT_HART, offsetof(gd_t, arch.boot_hart)); > > DEFINE(GD_FIRMWARE_FDT_ADDR, offsetof(gd_t, > > arch.firmware_fdt_addr)); -#ifndef CONFIG_XIP > > +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP) > > DEFINE(GD_AVAILABLE_HARTS, offsetof(gd_t, > > arch.available_harts)); #endif > > > > diff --git a/arch/riscv/lib/smp.c b/arch/riscv/lib/smp.c > > index ba992100ad..cef324954c 100644 > > --- a/arch/riscv/lib/smp.c > > +++ b/arch/riscv/lib/smp.c > > @@ -45,7 +45,7 @@ static int send_ipi_many(struct ipi_data *ipi, > > int wait) continue; > > } > > > > -#ifndef CONFIG_XIP > > +#if !defined(CONFIG_XIP) && !defined(CONFIG_SPL_XIP) > > /* skip if hart is not available */ > > if (!(gd->arch.available_harts & (1 << reg))) > > continue; > > diff --git a/common/spl/Kconfig b/common/spl/Kconfig > > index 07c03d611d..f24e423fc0 100644 > > --- a/common/spl/Kconfig > > +++ b/common/spl/Kconfig > > @@ -27,6 +27,11 @@ config SPL_FRAMEWORK > > supports MMC, NAND and YMODEM and other methods loading > > of U-Boot and the Linux Kernel. If unsure, say Y. > > > > +config SPL_XIP > > + bool "Support SPL in XIP" > > + help > > + Enable this if SPL is in XIP memory. > > Can you add another sentence or two? What is "XIP memory"? How does > it differ from the standard boot process? In this case i treat "XIP memory" as RO memory, if CONFIG_XIP is enabled (at least for riscv start.S) we don't use such variables as hart_lottery or available_harts_lock, both are locks. The problem is that CONFIG_XIP also propagate to main U-Boot, not only SPL. For example arch/riscv/cpu/start.S: ``` #ifndef CONFIG_XIP la t0, hart_lottery li t1, 1 amoswap.w s2, t1, 0(t0) bnez s2, wait_for_gd_init #else mv gp, s0 bnez tp, secondary_hart_loop #endif ``` where tp contains hartid, so we just skip it and all harts with non-zero hartid's goto secondary_hart_loop. But if we are loading main U-Boot to RAM - this doesn't make sense for us since only SPL is in RO memory. > > Also, it seems like we already have SPL_XIP_SUPPORT. Maybe we can > reuse that? AFAIK this is different, it's just used to indicate that coping U-Boot or kernel is not required if they are located in execute in place capable flash memory. > > > config SPL_FRAMEWORK_BOARD_INIT_F > > bool "Define a generic function board_init_f" > > depends on SPL_FRAMEWORK > > diff --git a/include/configs/scr7_vcu118.h > > b/include/configs/scr7_vcu118.h index 34545255d1..8f3ba36ce1 100644 > > --- a/include/configs/scr7_vcu118.h > > +++ b/include/configs/scr7_vcu118.h > > @@ -29,6 +29,8 @@ > > #define SCR7_OCRAM_REGION_SIZE 0xffff > > > > #define SCR7L2_CACHE 1 > > + > > +#define CONFIG_SYS_UBOOT_BASE CONFIG_SYS_TEXT_BASE > > #endif > > > > #define CONFIG_SYS_FLASH_BASE 0x00000fffff030000 > > > > What is this part doing? It's not mentioned in the commit message. It doesn't have any relation to main part - my bad - a bit relaxed due to this being an RFC, sorry. > > --Sean