u-boot.lists.denx.de archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/1] spl: introduce SPL_XIP to config
@ 2022-08-11  9:37 Nikita Shubin
  2022-08-11  9:37 ` [RFC PATCH 1/1] " Nikita Shubin
  0 siblings, 1 reply; 12+ messages in thread
From: Nikita Shubin @ 2022-08-11  9:37 UTC (permalink / raw)
  Cc: linux, Nikita Shubin, Rick Chen, Leo, Simon Glass, Bin Meng,
	Ilias Apalodimas, Heinrich Schuchardt, Sergei Miroshnichenko,
	Alexandru Gagniuc, Alper Nebi Yasak, Andrew Davis, u-boot

From: Nikita Shubin <n.shubin@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.

Actually, it seems that we need only hart_lottery and available_harts_lock to 
function like a R/W memory.

May be we should introduce a new section solely for this two locks copy and
initialize it somewhere befor hart lottery or place config variable with 
desired location for locks, but we still end up with a need for a 
"master" hart.

Does anyone have some thoughts about this ?

Nikita Shubin (1):
  spl: introduce SPL_XIP to config

 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(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 12+ messages in thread
* [RFC PATCH 1/1] spl: introduce SPL_XIP to config
  2022-08-11  9:37 [RFC PATCH 0/1] spl: introduce SPL_XIP to config Nikita Shubin
@ 2022-08-11  9:37 ` Nikita Shubin
  2022-08-13  4:35   ` Sean Anderson
  0 siblings, 1 reply; 12+ messages in thread
From: Nikita Shubin @ 2022-08-11  9:37 UTC (permalink / raw)
  Cc: linux, Nikita Shubin, Rick Chen, Leo, Simon Glass, Sean Anderson,
	Ilias Apalodimas, Heinrich Schuchardt, Sergei Miroshnichenko,
	Alexandru Gagniuc, Alper Nebi Yasak, Andrew Davis, u-boot

From: Nikita Shubin <n.shubin@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.shubin@yadro.com>
---
 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)
 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.
+
 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
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/1] spl: introduce SPL_XIP to config
  2022-08-11  9:37 ` [RFC PATCH 1/1] " Nikita Shubin
@ 2022-08-13  4:35   ` Sean Anderson
  2022-08-15  7:27     ` Nikita Shubin
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Anderson @ 2022-08-13  4:35 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: linux, Nikita Shubin, Rick Chen, Leo, Simon Glass,
	Ilias Apalodimas, Heinrich Schuchardt, Sergei Miroshnichenko,
	Alexandru Gagniuc, Alper Nebi Yasak, Andrew Davis, u-boot

Hi Nikita,

On 8/11/22 5:37 AM, Nikita Shubin wrote:
> From: Nikita Shubin <n.shubin@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.shubin@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

>   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?

Also, it seems like we already have SPL_XIP_SUPPORT. Maybe we can reuse that?

>   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.

--Sean

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/1] spl: introduce SPL_XIP to config
  2022-08-13  4:35   ` Sean Anderson
@ 2022-08-15  7:27     ` Nikita Shubin
  2022-08-23  4:25       ` Sean Anderson
  0 siblings, 1 reply; 12+ messages in thread
From: Nikita Shubin @ 2022-08-15  7:27 UTC (permalink / raw)
  To: Sean Anderson
  Cc: linux, Nikita Shubin, Rick Chen, Leo, Simon Glass,
	Ilias Apalodimas, Heinrich Schuchardt, Sergei Miroshnichenko,
	Alexandru Gagniuc, Alper Nebi Yasak, Andrew Davis, u-boot

Hello Sean!

On Sat, 13 Aug 2022 00:35:59 -0400
Sean Anderson <seanga2@gmail.com> wrote:

> Hi Nikita,
> 
> On 8/11/22 5:37 AM, Nikita Shubin wrote:
> > From: Nikita Shubin <n.shubin@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.shubin@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


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [RFC PATCH 1/1] spl: introduce SPL_XIP to config
  2022-08-15  7:27     ` Nikita Shubin
@ 2022-08-23  4:25       ` Sean Anderson
  2022-08-26  8:44         ` [PATCH] " Nikita Shubin
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Anderson @ 2022-08-23  4:25 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: linux, Nikita Shubin, Rick Chen, Leo, Simon Glass,
	Ilias Apalodimas, Heinrich Schuchardt, Sergei Miroshnichenko,
	Alexandru Gagniuc, Alper Nebi Yasak, Andrew Davis, u-boot

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

If you have something like

CONFIG_XIP
CONFIG_SPL_XIP
CONFIG_TPL_XIP

then if(CONFIG_IS_ENABLED(XIP)) works out to roughly

if ((defined(TPL_BUILD) && IS_ENABLED(CONFIG_TPL_XIP)) ||
     (defined(SPL_BUILD) && IS_ENABLED(CONFIG_SPL_XIP)) ||
     (!defined(TPL_BUILD) && !define(SPL_BUILD) && IS_ENABLED(CONFIG_XIP)))

That is, the appropriate config is enabled only when building that stage.
There's some other magic as well to make it work in #if as well, but that's
the gist.

>>
>>>    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.

Great, can you add that to the description?

> 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.

Makes sense

>>
>>>    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
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH] spl: introduce SPL_XIP to config
  2022-08-23  4:25       ` Sean Anderson
@ 2022-08-26  8:44         ` Nikita Shubin
  2022-08-26 14:10           ` Sean Anderson
  0 siblings, 1 reply; 12+ messages in thread
From: Nikita Shubin @ 2022-08-26  8:44 UTC (permalink / raw)
  Cc: Sean Anderson, linux, Nikita Shubin, Rick Chen, Leo, Simon Glass,
	Heinrich Schuchardt, Bin Meng, Ilias Apalodimas,
	Alexandru Gagniuc, Andrew Davis, Alper Nebi Yasak, u-boot

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>
---
rfc->v0:
Sean Anderson:
	- replace defined with CONFIG_IS_ENABLED
	- add proper description to KConfig
---
 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..b3719f9626 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.
 
+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 SPL_FRAMEWORK_BOARD_INIT_F
 	bool "Define a generic function board_init_f"
 	depends on SPL_FRAMEWORK
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH] spl: introduce SPL_XIP to config
  2022-08-26  8:44         ` [PATCH] " Nikita Shubin
@ 2022-08-26 14:10           ` Sean Anderson
  2022-08-31  7:25             ` [PATCH v2] " Nikita Shubin
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Anderson @ 2022-08-26 14:10 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: linux, Nikita Shubin, Rick Chen, Leo, Simon Glass,
	Heinrich Schuchardt, Bin Meng, Ilias Apalodimas,
	Alexandru Gagniuc, Andrew Davis, Alper Nebi Yasak, u-boot

On 8/26/22 4:44 AM, Nikita Shubin wrote:
> 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>
> ---
> rfc->v0:
> Sean Anderson:
> 	- replace defined with CONFIG_IS_ENABLED
> 	- add proper description to KConfig
> ---
>   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..b3719f9626 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.
>   
> +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.
> +

Kconfig descriptions should tell you what should happen when you enable this config.
So the way you have worded this is strange; I would expect something more like

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.

The rest looks good.

--Sean

>   config SPL_FRAMEWORK_BOARD_INIT_F
>   	bool "Define a generic function board_init_f"
>   	depends on SPL_FRAMEWORK
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH v2] spl: introduce SPL_XIP to config
  2022-08-26 14:10           ` Sean Anderson
@ 2022-08-31  7:25             ` Nikita Shubin
       [not found]               ` <HK0PR03MB2994C85219E6217F7F9F4A05C17A9@HK0PR03MB2994.apcprd03.prod.outlook.com>
  0 siblings, 1 reply; 12+ messages in thread
From: Nikita Shubin @ 2022-08-31  7:25 UTC (permalink / raw)
  To: u-boot
  Cc: linux, Nikita Shubin, Rick Chen, Leo, Simon Glass, Bin Meng,
	Heinrich Schuchardt, Ilias Apalodimas, Alexandru Gagniuc,
	Andrew Davis, Alper Nebi Yasak

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.
 
+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


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH v2] spl: introduce SPL_XIP to config
       [not found]               ` <HK0PR03MB2994C85219E6217F7F9F4A05C17A9@HK0PR03MB2994.apcprd03.prod.outlook.com>
@ 2022-09-02  7:25                 ` Rick Chen
  2022-09-02  8:42                   ` Nikita Shubin
  2022-09-02  8:47                   ` [PATCH v3] " Nikita Shubin
  0 siblings, 2 replies; 12+ messages in thread
From: Rick Chen @ 2022-09-02  7:25 UTC (permalink / raw)
  To: nikita.shubin
  Cc: U-Boot Mailing List, linux, Leo Liang, Simon Glass, Bin Meng,
	Heinrich Schuchardt, ilias.apalodimas, mr.nuke.me, afd,
	alpernebiyasak

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

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v2] spl: introduce SPL_XIP to config
  2022-09-02  7:25                 ` Rick Chen
@ 2022-09-02  8:42                   ` Nikita Shubin
  2022-09-02  8:47                   ` [PATCH v3] " Nikita Shubin
  1 sibling, 0 replies; 12+ messages in thread
From: Nikita Shubin @ 2022-09-02  8:42 UTC (permalink / raw)
  To: Rick Chen
  Cc: U-Boot Mailing List, linux, Leo Liang, Simon Glass, Bin Meng,
	Heinrich Schuchardt, ilias.apalodimas, mr.nuke.me, afd,
	alpernebiyasak

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  


^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH v3] spl: introduce SPL_XIP to config
  2022-09-02  7:25                 ` Rick Chen
  2022-09-02  8:42                   ` Nikita Shubin
@ 2022-09-02  8:47                   ` Nikita Shubin
       [not found]                     ` <HK0PR03MB29942A55774ED60C5568E72CC17F9@HK0PR03MB2994.apcprd03.prod.outlook.com>
  1 sibling, 1 reply; 12+ messages in thread
From: Nikita Shubin @ 2022-09-02  8:47 UTC (permalink / raw)
  To: u-boot
  Cc: linux, Sean Anderson, Rick Chen, Nikita Shubin, Rick Chen, Leo,
	Simon Glass, Bin Meng, Ilias Apalodimas, Heinrich Schuchardt

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
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH v3] spl: introduce SPL_XIP to config
       [not found]                     ` <HK0PR03MB29942A55774ED60C5568E72CC17F9@HK0PR03MB2994.apcprd03.prod.outlook.com>
@ 2022-09-08  0:45                       ` Rick Chen
  0 siblings, 0 replies; 12+ messages in thread
From: Rick Chen @ 2022-09-08  0:45 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: U-Boot Mailing List, linux, Sean Anderson, rick, n.shubin,
	Leo Liang, Simon Glass, Bin Meng, ilias.apalodimas,
	Heinrich Schuchardt

> From: Nikita Shubin <nikita.shubin@maquefel.me>
> Sent: Friday, September 02, 2022 4:48 PM
> To: u-boot@lists.denx.de
> Cc: linux@yadro.com; Sean Anderson <seanga2@gmail.com>; Rick Chen <rickchen36@gmail.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>; Ilias Apalodimas <ilias.apalodimas@linaro.org>; Heinrich Schuchardt <xypron.glpk@gmx.de>
> Subject: [PATCH v3] 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>
> ---
> 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(-)

Reviewed-by: Rick Chen <rick@andestech.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-09-08  0:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-11  9:37 [RFC PATCH 0/1] spl: introduce SPL_XIP to config Nikita Shubin
2022-08-11  9:37 ` [RFC PATCH 1/1] " Nikita Shubin
2022-08-13  4:35   ` Sean Anderson
2022-08-15  7:27     ` Nikita Shubin
2022-08-23  4:25       ` Sean Anderson
2022-08-26  8:44         ` [PATCH] " Nikita Shubin
2022-08-26 14:10           ` Sean Anderson
2022-08-31  7:25             ` [PATCH v2] " Nikita Shubin
     [not found]               ` <HK0PR03MB2994C85219E6217F7F9F4A05C17A9@HK0PR03MB2994.apcprd03.prod.outlook.com>
2022-09-02  7:25                 ` Rick Chen
2022-09-02  8:42                   ` Nikita Shubin
2022-09-02  8:47                   ` [PATCH v3] " Nikita Shubin
     [not found]                     ` <HK0PR03MB29942A55774ED60C5568E72CC17F9@HK0PR03MB2994.apcprd03.prod.outlook.com>
2022-09-08  0:45                       ` Rick Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).