Re: [PATCH v2] spl: introduce SPL_XIP to config

Hi Nikita,
From: Nikita Shubin nikita.shubin@maquefel.me Sent: Wednesday, August 31, 2022 3:25 PM To: u-boot@lists.denx.de Cc: linux@yadro.com; Nikita Shubin n.shubin@yadro.com; Rick Jian-Zhi Chen(陳建志) rick@andestech.com; Leo Yu-Chi Liang(梁育齊) ycliang@andestech.com; Simon Glass sjg@chromium.org; Bin Meng bmeng.cn@gmail.com; Heinrich Schuchardt xypron.glpk@gmx.de; Ilias Apalodimas ilias.apalodimas@linaro.org; Alexandru Gagniuc mr.nuke.me@gmail.com; Andrew Davis afd@ti.com; Alper Nebi Yasak alpernebiyasak@gmail.com Subject: [PATCH v2] spl: introduce SPL_XIP to config
From: Nikita Shubin n.shubin@yadro.com
U-Boot and SPL don't necessary share the same location, so we might end with U-Boot SPL in read-only memory (XIP) and U-Boot in read-write memory.
In case of non XIP boot mode, we rely on such variables as "hart_lottery" and "available_harts_lock" which we use as atomics.
The problem is that CONFIG_XIP also propagate to main U-Boot, not only SPL, so we need CONFIG_SPL_XIP to distinguish SPL XIP from other XIP modes.
This adds an option special for SPL to behave it in XIP manner and we don't use hart_lottery and available_harts_lock, during start proccess.
Signed-off-by: Nikita Shubin n.shubin@yadro.com
v1->v2: Sean Anderson: - used Kconfig description suggested by Sean - indeed more cleaner and understandable
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 | 7 +++++++ 6 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index 9f5fa0bcb3..5d8163b19f 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 !CONFIG_IS_ENABLED(XIP) u32 hart_lottery __section(".data") = 0;
/* diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index ac81783a90..c3c859e667 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 !CONFIG_IS_ENABLED(XIP) /* * Pick hart to initialize global data and run U-Boot. The other harts * wait for initialization to complete. @@ -150,7 +150,7 @@ call_harts_early_init: /* save the boot hart id to global_data */ SREG tp, GD_BOOT_HART(gp)
-#ifndef CONFIG_XIP +#if !CONFIG_IS_ENABLED(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..a4d3cf430b 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 !CONFIG_IS_ENABLED(XIP) ulong available_harts; #endif }; diff --git a/arch/riscv/lib/asm-offsets.c b/arch/riscv/lib/asm-offsets.c index f1fe089b3d..c4f48c8373 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 !CONFIG_IS_ENABLED(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..f8b756291f 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 !CONFIG_IS_ENABLED(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..777eff5e47 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -27,6 +27,13 @@ config SPL_FRAMEWORK supports MMC, NAND and YMODEM and other methods loading of U-Boot and the Linux Kernel. If unsure, say Y.
Would you please move the below SPL_XIP to arch/riscv/Kconfig and aside the CONFIG_XIP. Since hart_lottery and available_harts_lock only be used by RISC-V currently.
And also please help to change CONFIG_XIP to CONFIG_SPL_XIP in the following defconfig, or the behavior will be unexpected for AE350. ./ae350_rv64_spl_xip_defconfig:CONFIG_XIP=y ./ae350_rv32_spl_xip_defconfig:CONFIG_XIP=y
Other looks great for me.
Thanks, Rick
+config SPL_XIP
bool "Enable XIP mode for SPL"
help
Support booting SPL from read-only memory (such as XIP). Don't rely on
lock variables (for example hart_lottery and available_harts_lock)
since they cannot be modified.
config SPL_FRAMEWORK_BOARD_INIT_F bool "Define a generic function board_init_f" depends on SPL_FRAMEWORK -- 2.35.1

On Fri, 2 Sep 2022 15:25:06 +0800 Rick Chen rickchen36@gmail.com wrote:
Hello Rick!
Hi Nikita,
From: Nikita Shubin nikita.shubin@maquefel.me Sent: Wednesday, August 31, 2022 3:25 PM To: u-boot@lists.denx.de Cc: linux@yadro.com; Nikita Shubin n.shubin@yadro.com; Rick Jian-Zhi Chen(陳建志) rick@andestech.com; Leo Yu-Chi Liang(梁育齊) ycliang@andestech.com; Simon Glass sjg@chromium.org; Bin Meng bmeng.cn@gmail.com; Heinrich Schuchardt xypron.glpk@gmx.de; Ilias Apalodimas ilias.apalodimas@linaro.org; Alexandru Gagniuc mr.nuke.me@gmail.com; Andrew Davis afd@ti.com; Alper Nebi Yasak alpernebiyasak@gmail.com Subject: [PATCH v2] spl: introduce SPL_XIP to config
From: Nikita Shubin n.shubin@yadro.com
U-Boot and SPL don't necessary share the same location, so we might end with U-Boot SPL in read-only memory (XIP) and U-Boot in read-write memory.
In case of non XIP boot mode, we rely on such variables as "hart_lottery" and "available_harts_lock" which we use as atomics.
The problem is that CONFIG_XIP also propagate to main U-Boot, not only SPL, so we need CONFIG_SPL_XIP to distinguish SPL XIP from other XIP modes.
This adds an option special for SPL to behave it in XIP manner and we don't use hart_lottery and available_harts_lock, during start proccess.
Signed-off-by: Nikita Shubin n.shubin@yadro.com
v1->v2: Sean Anderson: - used Kconfig description suggested by Sean - indeed more cleaner and understandable --- 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 | 7 +++++++ 6 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index 9f5fa0bcb3..5d8163b19f 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 !CONFIG_IS_ENABLED(XIP) u32 hart_lottery __section(".data") = 0;
/* diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index ac81783a90..c3c859e667 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 !CONFIG_IS_ENABLED(XIP) /* * Pick hart to initialize global data and run U-Boot. The other harts * wait for initialization to complete. @@ -150,7 +150,7 @@ call_harts_early_init: /* save the boot hart id to global_data */ SREG tp, GD_BOOT_HART(gp)
-#ifndef CONFIG_XIP +#if !CONFIG_IS_ENABLED(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..a4d3cf430b 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 !CONFIG_IS_ENABLED(XIP) ulong available_harts; #endif }; diff --git a/arch/riscv/lib/asm-offsets.c b/arch/riscv/lib/asm-offsets.c index f1fe089b3d..c4f48c8373 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 !CONFIG_IS_ENABLED(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..f8b756291f 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 !CONFIG_IS_ENABLED(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..777eff5e47 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -27,6 +27,13 @@ config SPL_FRAMEWORK supports MMC, NAND and YMODEM and other methods loading of U-Boot and the Linux Kernel. If unsure, say Y.
Would you please move the below SPL_XIP to arch/riscv/Kconfig and aside the CONFIG_XIP. Since hart_lottery and available_harts_lock only be used by RISC-V currently.
And also please help to change CONFIG_XIP to CONFIG_SPL_XIP in the following defconfig, or the behavior will be unexpected for AE350. ./ae350_rv64_spl_xip_defconfig:CONFIG_XIP=y ./ae350_rv32_spl_xip_defconfig:CONFIG_XIP=y
No problem. Thanks for catching ae350.
Other looks great for me.
Thanks, Rick
+config SPL_XIP
bool "Enable XIP mode for SPL"
help
Support booting SPL from read-only memory (such as XIP).
Don't rely on
lock variables (for example hart_lottery and
available_harts_lock)
since they cannot be modified.
config SPL_FRAMEWORK_BOARD_INIT_F bool "Define a generic function board_init_f" depends on SPL_FRAMEWORK -- 2.35.1

From: Nikita Shubin n.shubin@yadro.com
U-Boot and SPL don't necessary share the same location, so we might end with U-Boot SPL in read-only memory (XIP) and U-Boot in read-write memory.
In case of non XIP boot mode, we rely on such variables as "hart_lottery" and "available_harts_lock" which we use as atomics.
The problem is that CONFIG_XIP also propagate to main U-Boot, not only SPL, so we need CONFIG_SPL_XIP to distinguish SPL XIP from other XIP modes.
This adds an option special for SPL to behave it in XIP manner and we don't use hart_lottery and available_harts_lock, during start proccess.
Signed-off-by: Nikita Shubin n.shubin@yadro.com --- v2->v3: Rick Chen: - move SPL_XIP to arch/riscv/Kconfig right after XIP - change ae350_spl defconfig's to use SPL_XIP instead of XIP --- arch/riscv/Kconfig | 7 +++++++ 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 +- configs/ae350_rv32_spl_xip_defconfig | 2 +- configs/ae350_rv64_spl_xip_defconfig | 2 +- 8 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 78e964db12..c042506a64 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -269,6 +269,13 @@ config XIP from a NOR flash memory without copying the code to ram. Say yes here if U-Boot boots from flash directly.
+config SPL_XIP + bool "Enable XIP mode for SPL" + help + If SPL starts in read-only memory (XIP for example) then we shouldn't + rely on lock variables (for example hart_lottery and available_harts_lock), + this affects only SPL, other stages should proceed as non-XIP. + config SHOW_REGS bool "Show registers on unhandled exception"
diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c index 3ffcbbd23f..0f323b26b3 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 !CONFIG_IS_ENABLED(XIP) u32 hart_lottery __section(".data") = 0;
/* diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S index b7f21ab63e..de9d078da1 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 !CONFIG_IS_ENABLED(XIP) /* * Pick hart to initialize global data and run U-Boot. The other harts * wait for initialization to complete. @@ -152,7 +152,7 @@ call_harts_early_init: /* save the boot hart id to global_data */ SREG tp, GD_BOOT_HART(gp)
-#ifndef CONFIG_XIP +#if !CONFIG_IS_ENABLED(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 095484a635..b3c79e1760 100644 --- a/arch/riscv/include/asm/global_data.h +++ b/arch/riscv/include/asm/global_data.h @@ -27,7 +27,7 @@ struct arch_global_data { #if CONFIG_IS_ENABLED(SMP) struct ipi_data ipi[CONFIG_NR_CPUS]; #endif -#ifndef CONFIG_XIP +#if !CONFIG_IS_ENABLED(XIP) ulong available_harts; #endif }; diff --git a/arch/riscv/lib/asm-offsets.c b/arch/riscv/lib/asm-offsets.c index f1fe089b3d..c4f48c8373 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 !CONFIG_IS_ENABLED(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..f8b756291f 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 !CONFIG_IS_ENABLED(XIP) /* skip if hart is not available */ if (!(gd->arch.available_harts & (1 << reg))) continue; diff --git a/configs/ae350_rv32_spl_xip_defconfig b/configs/ae350_rv32_spl_xip_defconfig index c7b6ea4730..67c1e35c55 100644 --- a/configs/ae350_rv32_spl_xip_defconfig +++ b/configs/ae350_rv32_spl_xip_defconfig @@ -11,7 +11,7 @@ CONFIG_SPL=y CONFIG_SYS_LOAD_ADDR=0x100000 CONFIG_TARGET_AX25_AE350=y CONFIG_RISCV_SMODE=y -CONFIG_XIP=y +CONFIG_SPL_XIP=y CONFIG_DISTRO_DEFAULTS=y CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xffff00 diff --git a/configs/ae350_rv64_spl_xip_defconfig b/configs/ae350_rv64_spl_xip_defconfig index a197c97736..baee9bfe4a 100644 --- a/configs/ae350_rv64_spl_xip_defconfig +++ b/configs/ae350_rv64_spl_xip_defconfig @@ -12,7 +12,7 @@ CONFIG_SYS_LOAD_ADDR=0x100000 CONFIG_TARGET_AX25_AE350=y CONFIG_ARCH_RV64I=y CONFIG_RISCV_SMODE=y -CONFIG_XIP=y +CONFIG_SPL_XIP=y CONFIG_DISTRO_DEFAULTS=y CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xfffe70
participants (2)
-
Nikita Shubin
-
Rick Chen