Hello Sean! On Sat, 13 Aug 2022 00:35:59 -0400 Sean Anderson <sean...@gmail.com> wrote:
> Hi Nikita, > > On 8/11/22 5:37 AM, Nikita Shubin wrote: > > From: Nikita Shubin <n.shu...@yadro.com> > > > > 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 <n.shu...@yadro.com> > > --- > > 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