[PATCH] board_f: Add support for CONFIG_OF_BOARD_FIXUP for XIP images

When U-Boot is running from flash memory (execute in place) then gd->fdt_blob before relocation points to read-only flash memory.
So U-Boot calls board_fix_fdt() with read-only gd->fdt_blob pointer which cause immediate CPU crash when callback is trying to modify gd->fdt_blob.
Fix this issue by introducing a new config option OF_INITIAL_DTB_READONLY which moves fix_fdt callback after the reloc_fdt callback. This makes CONFIG_OF_BOARD_FIXUP working also if U-Boot before relocation is not running from read/write (S)RAM memory.
This is required for mpc85xx boards when booting from flash NOR.
Signed-off-by: Pali Rohár pali@kernel.org --- common/board_f.c | 8 +++++++- dts/Kconfig | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 18e2246733b0..56b4eea24885 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -911,7 +911,7 @@ static const init_fnc_t init_sequence_f[] = { * - board info struct */ setup_dest_addr, -#ifdef CONFIG_OF_BOARD_FIXUP +#if defined(CONFIG_OF_BOARD_FIXUP) && !defined(CONFIG_OF_INITIAL_DTB_READONLY) fix_fdt, #endif #ifdef CONFIG_PRAM @@ -926,6 +926,10 @@ static const init_fnc_t init_sequence_f[] = { reserve_board, reserve_global_data, reserve_fdt, +#if defined(CONFIG_OF_BOARD_FIXUP) && defined(CONFIG_OF_INITIAL_DTB_READONLY) + reloc_fdt, + fix_fdt, +#endif reserve_bootstage, reserve_bloblist, reserve_arch, @@ -936,7 +940,9 @@ static const init_fnc_t init_sequence_f[] = { setup_bdinfo, display_new_sp, INIT_FUNC_WATCHDOG_RESET +#if !defined(CONFIG_OF_BOARD_FIXUP) || !defined(CONFIG_OF_INITIAL_DTB_READONLY) reloc_fdt, +#endif reloc_bootstage, reloc_bloblist, setup_reloc, diff --git a/dts/Kconfig b/dts/Kconfig index bc5f22029ff9..0bc3d741f8bc 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -105,6 +105,12 @@ config OF_EMBED
endchoice
+config OF_INITIAL_DTB_READONLY + bool "Initial DTB for DT control is read-only" + help + If initial DTB for DT control is read-only (e.g. points to + memory-mapped flash memory), then set this option. + config OF_BOARD bool "Provided by the board (e.g a previous loader) at runtime" default y if SANDBOX || OF_HAS_PRIOR_STAGE

Can you rename the option to OF_FDT_READONLY_BEFORE_RELOC ?
The INITIAL in the name may confuse people.
The functions are called *_fdt (reloc_fdt, fix_fdt), so maybe put FDT instead of DTB in the config option name.
Marek

On Sunday 28 August 2022 05:47:51 Marek Behún wrote:
Can you rename the option to OF_FDT_READONLY_BEFORE_RELOC ?
I do not care what is the name of this option. All other options for this configuration have OF and DTB in its name, not FDT, and that is why I chose the current name.
The INITIAL in the name may confuse people.
The functions are called *_fdt (reloc_fdt, fix_fdt), so maybe put FDT instead of DTB in the config option name.
Marek

From: Pali Rohár pali@kernel.org
When U-Boot is running from flash memory (execute in place) then gd->fdt_blob before relocation points to read-only flash memory.
So U-Boot calls board_fix_fdt() with read-only gd->fdt_blob pointer which cause immediate CPU crash when callback is trying to modify gd->fdt_blob.
Fix this issue by introducing a new config option OF_DTB_READONLY_BEFORE_RELOC which moves fix_fdt callback after the reloc_fdt callback. This makes CONFIG_OF_BOARD_FIXUP working also if U-Boot before relocation is not running from read/write (S)RAM memory.
This is required for mpc85xx boards when booting from flash NOR.
Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún kabel@kernel.org --- Changes since v1: - just changed the new Kconfig option name and associated help string --- common/board_f.c | 8 +++++++- dts/Kconfig | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 18e2246733..35fb4a0753 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -911,7 +911,7 @@ static const init_fnc_t init_sequence_f[] = { * - board info struct */ setup_dest_addr, -#ifdef CONFIG_OF_BOARD_FIXUP +#if defined(CONFIG_OF_BOARD_FIXUP) && !defined(CONFIG_OF_DTB_READONLY_BEFORE_RELOC) fix_fdt, #endif #ifdef CONFIG_PRAM @@ -926,6 +926,10 @@ static const init_fnc_t init_sequence_f[] = { reserve_board, reserve_global_data, reserve_fdt, +#if defined(CONFIG_OF_BOARD_FIXUP) && defined(CONFIG_OF_DTB_READONLY_BEFORE_RELOC) + reloc_fdt, + fix_fdt, +#endif reserve_bootstage, reserve_bloblist, reserve_arch, @@ -936,7 +940,9 @@ static const init_fnc_t init_sequence_f[] = { setup_bdinfo, display_new_sp, INIT_FUNC_WATCHDOG_RESET +#if !defined(CONFIG_OF_BOARD_FIXUP) || !defined(CONFIG_OF_DTB_READONLY_BEFORE_RELOC) reloc_fdt, +#endif reloc_bootstage, reloc_bloblist, setup_reloc, diff --git a/dts/Kconfig b/dts/Kconfig index bc5f22029f..29e5248b15 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -105,6 +105,12 @@ config OF_EMBED
endchoice
+config OF_DTB_READONLY_BEFORE_RELOC + bool "Initial DTB (before relocation) for DT control is read-only" + help + If initial (before relocation) DTB for DT control is read-only, + (e.g. points to memory-mapped flash memory), set this option. + config OF_BOARD bool "Provided by the board (e.g a previous loader) at runtime" default y if SANDBOX || OF_HAS_PRIOR_STAGE

Simon, could you review this patch? If not, who can do it?
On Sunday 28 August 2022 17:19:29 Marek Behún wrote:
From: Pali Rohár pali@kernel.org
When U-Boot is running from flash memory (execute in place) then gd->fdt_blob before relocation points to read-only flash memory.
So U-Boot calls board_fix_fdt() with read-only gd->fdt_blob pointer which cause immediate CPU crash when callback is trying to modify gd->fdt_blob.
Fix this issue by introducing a new config option OF_DTB_READONLY_BEFORE_RELOC which moves fix_fdt callback after the reloc_fdt callback. This makes CONFIG_OF_BOARD_FIXUP working also if U-Boot before relocation is not running from read/write (S)RAM memory.
This is required for mpc85xx boards when booting from flash NOR.
Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún kabel@kernel.org
Changes since v1:
- just changed the new Kconfig option name and associated help string
common/board_f.c | 8 +++++++- dts/Kconfig | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/common/board_f.c b/common/board_f.c index 18e2246733..35fb4a0753 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -911,7 +911,7 @@ static const init_fnc_t init_sequence_f[] = { * - board info struct */ setup_dest_addr, -#ifdef CONFIG_OF_BOARD_FIXUP +#if defined(CONFIG_OF_BOARD_FIXUP) && !defined(CONFIG_OF_DTB_READONLY_BEFORE_RELOC) fix_fdt, #endif #ifdef CONFIG_PRAM @@ -926,6 +926,10 @@ static const init_fnc_t init_sequence_f[] = { reserve_board, reserve_global_data, reserve_fdt, +#if defined(CONFIG_OF_BOARD_FIXUP) && defined(CONFIG_OF_DTB_READONLY_BEFORE_RELOC)
- reloc_fdt,
- fix_fdt,
+#endif reserve_bootstage, reserve_bloblist, reserve_arch, @@ -936,7 +940,9 @@ static const init_fnc_t init_sequence_f[] = { setup_bdinfo, display_new_sp, INIT_FUNC_WATCHDOG_RESET +#if !defined(CONFIG_OF_BOARD_FIXUP) || !defined(CONFIG_OF_DTB_READONLY_BEFORE_RELOC) reloc_fdt, +#endif reloc_bootstage, reloc_bloblist, setup_reloc, diff --git a/dts/Kconfig b/dts/Kconfig index bc5f22029f..29e5248b15 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -105,6 +105,12 @@ config OF_EMBED
endchoice
+config OF_DTB_READONLY_BEFORE_RELOC
- bool "Initial DTB (before relocation) for DT control is read-only"
- help
If initial (before relocation) DTB for DT control is read-only,
(e.g. points to memory-mapped flash memory), set this option.
config OF_BOARD bool "Provided by the board (e.g a previous loader) at runtime" default y if SANDBOX || OF_HAS_PRIOR_STAGE -- 2.35.1

Hi,
On Sun, 28 Aug 2022 at 09:19, Marek Behún kabel@kernel.org wrote:
From: Pali Rohár pali@kernel.org
When U-Boot is running from flash memory (execute in place) then gd->fdt_blob before relocation points to read-only flash memory.
So U-Boot calls board_fix_fdt() with read-only gd->fdt_blob pointer which cause immediate CPU crash when callback is trying to modify gd->fdt_blob.
Fix this issue by introducing a new config option OF_DTB_READONLY_BEFORE_RELOC which moves fix_fdt callback after the reloc_fdt callback. This makes CONFIG_OF_BOARD_FIXUP working also if U-Boot before relocation is not running from read/write (S)RAM memory.
This is required for mpc85xx boards when booting from flash NOR.
Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún kabel@kernel.org
Changes since v1:
- just changed the new Kconfig option name and associated help string
common/board_f.c | 8 +++++++- dts/Kconfig | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-)
Can we just make this the normal behaviour? I think it makes more sense to change the FDT after we have relocated it.
Regards, Simon

On Monday 10 October 2022 17:48:58 Simon Glass wrote:
Hi,
On Sun, 28 Aug 2022 at 09:19, Marek Behún kabel@kernel.org wrote:
From: Pali Rohár pali@kernel.org
When U-Boot is running from flash memory (execute in place) then gd->fdt_blob before relocation points to read-only flash memory.
So U-Boot calls board_fix_fdt() with read-only gd->fdt_blob pointer which cause immediate CPU crash when callback is trying to modify gd->fdt_blob.
Fix this issue by introducing a new config option OF_DTB_READONLY_BEFORE_RELOC which moves fix_fdt callback after the reloc_fdt callback. This makes CONFIG_OF_BOARD_FIXUP working also if U-Boot before relocation is not running from read/write (S)RAM memory.
This is required for mpc85xx boards when booting from flash NOR.
Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún kabel@kernel.org
Changes since v1:
- just changed the new Kconfig option name and associated help string
common/board_f.c | 8 +++++++- dts/Kconfig | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-)
Can we just make this the normal behaviour? I think it makes more sense to change the FDT after we have relocated it.
Regards, Simon
Marek? Any comments?

+ Tom
On Wednesday 02 November 2022 00:23:03 Pali Rohár wrote:
On Monday 10 October 2022 17:48:58 Simon Glass wrote:
Hi,
On Sun, 28 Aug 2022 at 09:19, Marek Behún kabel@kernel.org wrote:
From: Pali Rohár pali@kernel.org
When U-Boot is running from flash memory (execute in place) then gd->fdt_blob before relocation points to read-only flash memory.
So U-Boot calls board_fix_fdt() with read-only gd->fdt_blob pointer which cause immediate CPU crash when callback is trying to modify gd->fdt_blob.
Fix this issue by introducing a new config option OF_DTB_READONLY_BEFORE_RELOC which moves fix_fdt callback after the reloc_fdt callback. This makes CONFIG_OF_BOARD_FIXUP working also if U-Boot before relocation is not running from read/write (S)RAM memory.
This is required for mpc85xx boards when booting from flash NOR.
Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún kabel@kernel.org
Changes since v1:
- just changed the new Kconfig option name and associated help string
common/board_f.c | 8 +++++++- dts/Kconfig | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-)
Can we just make this the normal behaviour? I think it makes more sense to change the FDT after we have relocated it.
Regards, Simon
Marek? Any comments?

On Mon, Nov 21, 2022 at 06:42:01PM +0100, Pali Rohár wrote:
- Tom
On Wednesday 02 November 2022 00:23:03 Pali Rohár wrote:
On Monday 10 October 2022 17:48:58 Simon Glass wrote:
Hi,
On Sun, 28 Aug 2022 at 09:19, Marek Behún kabel@kernel.org wrote:
From: Pali Rohár pali@kernel.org
When U-Boot is running from flash memory (execute in place) then gd->fdt_blob before relocation points to read-only flash memory.
So U-Boot calls board_fix_fdt() with read-only gd->fdt_blob pointer which cause immediate CPU crash when callback is trying to modify gd->fdt_blob.
Fix this issue by introducing a new config option OF_DTB_READONLY_BEFORE_RELOC which moves fix_fdt callback after the reloc_fdt callback. This makes CONFIG_OF_BOARD_FIXUP working also if U-Boot before relocation is not running from read/write (S)RAM memory.
This is required for mpc85xx boards when booting from flash NOR.
Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún kabel@kernel.org
Changes since v1:
- just changed the new Kconfig option name and associated help string
common/board_f.c | 8 +++++++- dts/Kconfig | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-)
Can we just make this the normal behaviour? I think it makes more sense to change the FDT after we have relocated it.
Can we?

On Mon, 21 Nov 2022 12:45:32 -0500 Tom Rini trini@konsulko.com wrote:
On Mon, Nov 21, 2022 at 06:42:01PM +0100, Pali Roh_r wrote:
- Tom
On Wednesday 02 November 2022 00:23:03 Pali Roh_r wrote:
On Monday 10 October 2022 17:48:58 Simon Glass wrote:
Hi,
On Sun, 28 Aug 2022 at 09:19, Marek Beh_n kabel@kernel.org wrote:
From: Pali Roh_r pali@kernel.org
When U-Boot is running from flash memory (execute in place) then gd->fdt_blob before relocation points to read-only flash memory.
So U-Boot calls board_fix_fdt() with read-only gd->fdt_blob pointer which cause immediate CPU crash when callback is trying to modify gd->fdt_blob.
Fix this issue by introducing a new config option OF_DTB_READONLY_BEFORE_RELOC which moves fix_fdt callback after the reloc_fdt callback. This makes CONFIG_OF_BOARD_FIXUP working also if U-Boot before relocation is not running from read/write (S)RAM memory.
This is required for mpc85xx boards when booting from flash NOR.
Signed-off-by: Pali Roh_r pali@kernel.org Signed-off-by: Marek Beh_n kabel@kernel.org
Changes since v1:
- just changed the new Kconfig option name and associated help string
common/board_f.c | 8 +++++++- dts/Kconfig | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-)
Can we just make this the normal behaviour? I think it makes more sense to change the FDT after we have relocated it.
Can we?
Sorry, I didn't notice this thread. Do we know if there aren't boards that change fdt before reloc?
Marek
participants (4)
-
Marek Behún
-
Pali Rohár
-
Simon Glass
-
Tom Rini