Hello Simon,
Am 14.03.2019 um 11:02 schrieb Simon Glass:
Hi Heiko,
On Tue, 12 Mar 2019 at 02:50, Heiko Schocher hs@denx.de wrote:
Hello Simon, Tom,
Am 12.03.2019 um 09:21 schrieb Heiko Schocher:
Hello Simon, Tom,
I am just stumbeld on an am437x basd board over the problem to pass the bootmode from SPL to U-Boot. On am437x the bootmode info get overwritten from SPL stack, and I need this info in U-Boot.
Hack would be to move SPL stack to another address, but we loose than 0xa000 size for stack ... I do not want to go this way..
I thought gd info is passed from SPL to U-Boot, but this is not the case!
Looking into
... 75 bic r0, r0, #7 /* 8-byte alignment for ABI compliance */ 76 mov sp, r0 77 bl board_init_f_alloc_reserve 78 mov sp, r0 79 /* set up gd here, outside any C code */ 80 mov r9, r0 81 bl board_init_f_init_reserve
and common/init/board_init.c:
99 void board_init_f_init_reserve(ulong base) 100 { 101 struct global_data *gd_ptr; 102 103 /* 104 * clear GD entirely and set it up. 105 * Use gd_ptr, as gd may not be properly set yet. 106 */ 107 108 gd_ptr = (struct global_data *)base; 109 /* zero the area */ 110 memset(gd_ptr, '\0', sizeof(*gd)); 111 /* set GD unless architecture did it already */ 112 #if !defined(CONFIG_ARM) 113 arch_setup_gd(gd_ptr); 114 #endif
gd is always initialized with zeros, no chance for passing infos from SPL to U-Boot...
I really thought, that gd_t was intentionally designed for passing data between different U-Boot states, but looking into gd_t definiton in include/asm-generic/global_data.h it is a big ifdef mess and not useable as an "API" between TPL/SPL and U-Boot ...
I thought also, that SPL detects for example ramsize and than passes this info to U-Boot ...
But Ok, I found "common/init/handoff.c" which seems now the way to go, but:
./common/board_f.c
281 static int setup_spl_handoff(void) 282 { 283 #if CONFIG_IS_ENABLED(HANDOFF) 284 gd->spl_handoff = bloblist_find(BLOBLISTT_SPL_HANDOFF, 285 sizeof(struct spl_handoff)); 286 debug("Found SPL hand-off info %p\n", gd->spl_handoff); 287 #endif 288 289 return 0; 290 }
There is gd->spl_handoff used ... how could this work at least on arm, if gd is set to zeros on init ?
Do I miss something obvious?
Sorry for being so stupid, with:
common/board_f.c
853 #ifdef CONFIG_BLOBLIST 854 bloblist_init, 855 #endif 856 setup_spl_handoff,
and common/bloblist.c
216 int bloblist_init(void) 217 { 218 bool expected; 219 int ret = -ENOENT; 220 221 /** 222 * Wed expect to find an existing bloblist in the first phase of U-Boot 223 * that runs 224 */ 225 expected = !u_boot_first_phase(); 226 if (expected) 227 ret = bloblist_check(CONFIG_BLOBLIST_ADDR, 228 CONFIG_BLOBLIST_SIZE);
gd->spl_handoff gets setup through bloblist_find() ...
But beside sandbox there is no current user currently, or?
$ grep -lr BLOBLIST_ADDR . ./test/bloblist.c ./include/bloblist.h ./common/bloblist.c ./common/Kconfig ./board/sandbox/README.sandbox $
Yes that's right, it is not used outside sandbox, although there are patches to use it on x86.
Thanks for clarifying!
I think it is a reasonable idea to allow the gd region to pass from TPL -> SPL -> U-Boot. But we'll need to remove use of CONFIG_IS_ENABLED(), or put shared things at the beginning of the structure.
I thought about putting interchangeable things at the beginning of gd_t.
The big advantage of a bloblist is, that you only need to parse it and you know, what values are really setup from the previous stage.
Passing structs is not ideal, but small ... but may error prone?
Can we always be sure, previous stage sets up all info in gd_t next stage expects?
We need the concept of 'am I the first thing to run'. This is implemented in bloblist as u_boot_first_phase() - see spl.h. If this is true, we must set up the data structure. If false we must find one set up by a previous phase and use it. Bloblist handles this, but perhaps gd could as well?
Yes, this could be used.
Also consider the scenario where there is a read-only TPL programmed in manufacture that never changes, and a read-write SPL + U-Boot that can be upgraded in the field. In this case they may eventually end up being built with different versions of U-Boot. The bloblist structure is intended to handle this by at least checking that the size matches.
Just experimenting with: diff --git a/common/init/board_init.c b/common/init/board_init.c index 526fee35ff..bdb4b6364d 100644 --- a/common/init/board_init.c +++ b/common/init/board_init.c @@ -96,9 +96,10 @@ ulong board_init_f_alloc_reserve(ulong top) * (seemingly useless) incrementation causes no code increase. */
-void board_init_f_init_reserve(ulong base) +void board_init_f_init_reserve(ulong base, ulong oldgd) { struct global_data *gd_ptr; + struct global_data *gd_ptr_old;
/* * clear GD entirely and set it up. @@ -106,8 +107,22 @@ void board_init_f_init_reserve(ulong base) */
gd_ptr = (struct global_data *)base; - /* zero the area */ - memset(gd_ptr, '\0', sizeof(*gd)); + gd_ptr_old = (struct global_data *)oldgd; + if (u_boot_first_phase() || + oldgd == 0 || + !(((gd_ptr->magic == UBOOT_GD_MAGIC) && + (gd_ptr->gd_size == sizeof(struct global_data))))) { + /* setup gd, zero the area ... */ + memset(gd_ptr, '\0', sizeof(*gd)); + /* ... and set magic and size information */ + gd_ptr->magic = UBOOT_GD_MAGIC; + gd_ptr->gd_size = sizeof(struct global_data); + } else { + /* + * in case we get an already initialised gd_pointer + * use the info from already existing oldgd. + */ + } /* set GD unless architecture did it already */ #if !defined(CONFIG_ARM) arch_setup_gd(gd_ptr); diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index 78dcf40bff..3a5063dbe1 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -23,8 +23,11 @@ #include <membuff.h> #include <linux/list.h>
+#define UBOOT_GD_MAGIC 0x19730517 typedef struct global_data { bd_t *bd; + unsigned long magic; + int gd_size; unsigned long flags; unsigned int baudrate; unsigned long cpu_clk; /* CPU clock in Hz! */
(after moving u_boot_first_phase() to include/common.h)
But it is WIP, nothing functional yet...
Related, I feel that we should figure out how to use registers to pass addresses from SPL to U-Boot. On ARM we could use r0 to pass the value of gd, perhaps.
Perhaps, yes.
bye, Heiko