* [PATCH v4 0/4] Ensure 16 alignment of reserved memory in board_f.c @ 2020-03-10 9:15 Patrick Delaunay 2020-03-10 9:15 ` [PATCH v4 1/4] board_f.c: Ensure gd->new_bootstage alignment Patrick Delaunay ` (3 more replies) 0 siblings, 4 replies; 9+ messages in thread From: Patrick Delaunay @ 2020-03-10 9:15 UTC (permalink / raw) To: u-boot Hi, It is a V4 with only cosmetics udpate (s/insure/ensure) for the V3 serie: http://patchwork.ozlabs.org/project/uboot/list/?series=162918&state=* V2 was: http://patchwork.ozlabs.org/project/uboot/list/?series=154685&state=* After the first correction, only for bootstage alignment, I remove the stm32mp1 workaround in the 2nd patch. The 4th patch is a complete solution to alignment (proposed in comment 5 of http://patchwork.ozlabs.org/patch/1201452/#2327366) I always align the reserved memory to 16 bytes with a new function reserve_stack_aligned(). But this patch causes an issue on ARM 32 bits, as the relocated gd pointer is not initialized with gd->new_gd as expected now in reserve_global_data() but is hard-coded with: relocated gd = gd->bd - GD_SIZE {with GD_SIZE = sizeof(struct global_data)} After the 4rd patch, this assumption in not more true as sizeof(struct global_data) is not always 16 bytes aligned. This issue is solved with the 3rd patch of the serie arm: set the relocated gd with gd->new_gd Only tested on STM32MP157C-EV1 board (ARM32 architecture). Changes in v4: - replace insure by ensure in comment - replace insure by ensure Changes in v3: - rename reserve_sp to reserve_stack_aligned Changes in v2: - import: [U-Boot,v3] board_f.c: Insure gd->new_bootstage alignment - fix commit message s/bits/bytes/ Patrice Chotard (1): board_f.c: Ensure gd->new_bootstage alignment Patrick Delaunay (3): Revert "stm32mp1: remove the imply BOOTSTAGE" arm: set the relocated gd with gd->new_gd board_f.c: Ensure 16 alignment of start_addr_sp and reserved memory arch/arm/lib/crt0.S | 3 +-- arch/arm/mach-stm32mp/Kconfig | 2 ++ common/board_f.c | 27 ++++++++++++++++++--------- 3 files changed, 21 insertions(+), 11 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 1/4] board_f.c: Ensure gd->new_bootstage alignment 2020-03-10 9:15 [PATCH v4 0/4] Ensure 16 alignment of reserved memory in board_f.c Patrick Delaunay @ 2020-03-10 9:15 ` Patrick Delaunay 2020-04-17 21:08 ` Tom Rini 2020-03-10 9:15 ` [PATCH v4 2/4] Revert "stm32mp1: remove the imply BOOTSTAGE" Patrick Delaunay ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Patrick Delaunay @ 2020-03-10 9:15 UTC (permalink / raw) To: u-boot From: Patrice Chotard <patrice.chotard@st.com> In reserve_bootstage(), in case size is odd, gd->new_bootstage is not aligned. In bootstage_relocate(), the platform hangs when getting access to data->record[i].name. To avoid this issue, make gd->new_bootstage 16 byte aligned. To ensure that new_bootstage is 16 byte aligned (at least needed for x86_64 and ARMv8) and new_bootstage starts down to get enough space, ALIGN_DOWN macro is used. Fixes: ac9cd4805c8b ("bootstage: Correct relocation algorithm") Signed-off-by: Patrice Chotard <patrice.chotard@st.com> Reviewed-by: Vikas MANOCHA <vikas.manocha@st.com> Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com> Tested-by: Patrick Delaunay <patrick.delaunay@st.com> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> --- Changes in v4: - replace insure by ensure in comment Changes in v3: None Changes in v2: - import: [U-Boot,v3] board_f.c: Insure gd->new_bootstage alignment common/board_f.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/common/board_f.c b/common/board_f.c index 82a164752a..0427b7b096 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -564,6 +564,11 @@ static int reserve_bootstage(void) int size = bootstage_get_size(); gd->start_addr_sp -= size; + /* + * Ensure that start_addr_sp is aligned down to reserve enough + * space for new_bootstage + */ + gd->start_addr_sp = ALIGN_DOWN(gd->start_addr_sp, 16); gd->new_bootstage = map_sysmem(gd->start_addr_sp, size); debug("Reserving %#x Bytes for bootstage at: %08lx\n", size, gd->start_addr_sp); -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 1/4] board_f.c: Ensure gd->new_bootstage alignment 2020-03-10 9:15 ` [PATCH v4 1/4] board_f.c: Ensure gd->new_bootstage alignment Patrick Delaunay @ 2020-04-17 21:08 ` Tom Rini 0 siblings, 0 replies; 9+ messages in thread From: Tom Rini @ 2020-04-17 21:08 UTC (permalink / raw) To: u-boot On Tue, Mar 10, 2020 at 10:15:02AM +0100, Patrick Delaunay wrote: > From: Patrice Chotard <patrice.chotard@st.com> > > In reserve_bootstage(), in case size is odd, gd->new_bootstage > is not aligned. In bootstage_relocate(), the platform hangs when > getting access to data->record[i].name. > To avoid this issue, make gd->new_bootstage 16 byte aligned. > > To ensure that new_bootstage is 16 byte aligned (at least needed for > x86_64 and ARMv8) and new_bootstage starts down to get enough space, > ALIGN_DOWN macro is used. > > Fixes: ac9cd4805c8b ("bootstage: Correct relocation algorithm") > > Signed-off-by: Patrice Chotard <patrice.chotard@st.com> > Reviewed-by: Vikas MANOCHA <vikas.manocha@st.com> > Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com> > Tested-by: Patrick Delaunay <patrick.delaunay@st.com> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> Applied to u-boot/master, thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 659 bytes Desc: not available URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200417/6bf3b720/attachment.sig> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 2/4] Revert "stm32mp1: remove the imply BOOTSTAGE" 2020-03-10 9:15 [PATCH v4 0/4] Ensure 16 alignment of reserved memory in board_f.c Patrick Delaunay 2020-03-10 9:15 ` [PATCH v4 1/4] board_f.c: Ensure gd->new_bootstage alignment Patrick Delaunay @ 2020-03-10 9:15 ` Patrick Delaunay 2020-04-17 21:08 ` Tom Rini 2020-03-10 9:15 ` [PATCH v4 3/4] arm: set the relocated gd with gd->new_gd Patrick Delaunay 2020-03-10 9:15 ` [PATCH v4 4/4] board_f.c: Ensure 16 alignment of start_addr_sp and reserved memory Patrick Delaunay 3 siblings, 1 reply; 9+ messages in thread From: Patrick Delaunay @ 2020-03-10 9:15 UTC (permalink / raw) To: u-boot This reverts the workaround introduced by the commit 16fec9b0bc1a ("stm32mp1: remove the imply BOOTSTAGE") As the bootstage alignment issue is now solved. Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> --- Changes in v4: None Changes in v3: None Changes in v2: None arch/arm/mach-stm32mp/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/mach-stm32mp/Kconfig b/arch/arm/mach-stm32mp/Kconfig index e4d621dee8..1489cbd5f3 100644 --- a/arch/arm/mach-stm32mp/Kconfig +++ b/arch/arm/mach-stm32mp/Kconfig @@ -62,7 +62,9 @@ config TARGET_ST_STM32MP15x bool "STMicroelectronics STM32MP15x boards" select STM32MP15x imply BOOTCOUNT_LIMIT + imply BOOTSTAGE imply CMD_BOOTCOUNT + imply CMD_BOOTSTAGE imply CMD_CLS if CMD_BMP imply DISABLE_CONSOLE imply PRE_CONSOLE_BUFFER -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 2/4] Revert "stm32mp1: remove the imply BOOTSTAGE" 2020-03-10 9:15 ` [PATCH v4 2/4] Revert "stm32mp1: remove the imply BOOTSTAGE" Patrick Delaunay @ 2020-04-17 21:08 ` Tom Rini 0 siblings, 0 replies; 9+ messages in thread From: Tom Rini @ 2020-04-17 21:08 UTC (permalink / raw) To: u-boot On Tue, Mar 10, 2020 at 10:15:03AM +0100, Patrick Delaunay wrote: > This reverts the workaround introduced by the > commit 16fec9b0bc1a ("stm32mp1: remove the imply BOOTSTAGE") > As the bootstage alignment issue is now solved. > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> Applied to u-boot/master, thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 659 bytes Desc: not available URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200417/06d07aaa/attachment.sig> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 3/4] arm: set the relocated gd with gd->new_gd 2020-03-10 9:15 [PATCH v4 0/4] Ensure 16 alignment of reserved memory in board_f.c Patrick Delaunay 2020-03-10 9:15 ` [PATCH v4 1/4] board_f.c: Ensure gd->new_bootstage alignment Patrick Delaunay 2020-03-10 9:15 ` [PATCH v4 2/4] Revert "stm32mp1: remove the imply BOOTSTAGE" Patrick Delaunay @ 2020-03-10 9:15 ` Patrick Delaunay 2020-04-17 21:08 ` Tom Rini 2020-03-10 9:15 ` [PATCH v4 4/4] board_f.c: Ensure 16 alignment of start_addr_sp and reserved memory Patrick Delaunay 3 siblings, 1 reply; 9+ messages in thread From: Patrick Delaunay @ 2020-03-10 9:15 UTC (permalink / raw) To: u-boot Simplify the arm relocation behavior and get gd directly form new_gd, as it is already done in crt0_64.S: ldr x18, [x18, #GD_NEW_GD] /* x18 <- gd->new_gd */ This patch avoid assumption on new GD location (new GD is below bd - with #GD_SIZE offset). Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> --- Changes in v4: None Changes in v3: None Changes in v2: None arch/arm/lib/crt0.S | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index fb6c37cf51..df9dd83e40 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -127,8 +127,7 @@ ENTRY(_main) ldr r0, [r9, #GD_START_ADDR_SP] /* sp = gd->start_addr_sp */ bic r0, r0, #7 /* 8-byte alignment for ABI compliance */ mov sp, r0 - ldr r9, [r9, #GD_BD] /* r9 = gd->bd */ - sub r9, r9, #GD_SIZE /* new GD is below bd */ + ldr r9, [r9, #GD_NEW_GD] /* r9 <- gd->new_gd */ adr lr, here ldr r0, [r9, #GD_RELOC_OFF] /* r0 = gd->reloc_off */ -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 3/4] arm: set the relocated gd with gd->new_gd 2020-03-10 9:15 ` [PATCH v4 3/4] arm: set the relocated gd with gd->new_gd Patrick Delaunay @ 2020-04-17 21:08 ` Tom Rini 0 siblings, 0 replies; 9+ messages in thread From: Tom Rini @ 2020-04-17 21:08 UTC (permalink / raw) To: u-boot On Tue, Mar 10, 2020 at 10:15:04AM +0100, Patrick Delaunay wrote: > Simplify the arm relocation behavior and get gd directly form new_gd, > as it is already done in crt0_64.S: > > ldr x18, [x18, #GD_NEW_GD] /* x18 <- gd->new_gd */ > > This patch avoid assumption on new GD location (new GD is below bd - > with #GD_SIZE offset). > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> Applied to u-boot/master, thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 659 bytes Desc: not available URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200417/f5709d45/attachment.sig> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 4/4] board_f.c: Ensure 16 alignment of start_addr_sp and reserved memory 2020-03-10 9:15 [PATCH v4 0/4] Ensure 16 alignment of reserved memory in board_f.c Patrick Delaunay ` (2 preceding siblings ...) 2020-03-10 9:15 ` [PATCH v4 3/4] arm: set the relocated gd with gd->new_gd Patrick Delaunay @ 2020-03-10 9:15 ` Patrick Delaunay 2020-04-17 21:08 ` Tom Rini 3 siblings, 1 reply; 9+ messages in thread From: Patrick Delaunay @ 2020-03-10 9:15 UTC (permalink / raw) To: u-boot Add a function reserve_stack_aligned() to reserved memory with 16 bits alignment after the stack pointer (gd->start_addr_sp) and use this new function in board_f.c to reserve all the memory area (malloc, board, gd, fdt, bootstage, stacks). This 16 byte alignment is needed for cast on struct pointer for the reserved memory, for example: + x86_64 ABI: https://reviews.llvm.org/D30049: 16 bytes + ARMv8 Instruction Set Overview: quad word, 16 bytes An other alignment value could be needed for other architecture. Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> Acked-by: Stephen Warren <swarren@nvidia.com> --- Changes in v4: - replace insure by ensure Changes in v3: - rename reserve_sp to reserve_stack_aligned Changes in v2: - fix commit message s/bits/bytes/ common/board_f.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/common/board_f.c b/common/board_f.c index 0427b7b096..2ec5dbaa68 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -472,6 +472,17 @@ static int reserve_uboot(void) return 0; } +/* + * reserve after start_addr_sp the requested size and make the stack pointer + * 16-byte aligned, this alignment is needed for cast on the reserved memory + * ref = x86_64 ABI: https://reviews.llvm.org/D30049: 16 bytes + * = ARMv8 Instruction Set Overview: quad word, 16 bytes + */ +static unsigned long reserve_stack_aligned(size_t size) +{ + return ALIGN_DOWN(gd->start_addr_sp - size, 16); +} + #ifdef CONFIG_SYS_NONCACHED_MEMORY static int reserve_noncached(void) { @@ -497,7 +508,7 @@ static int reserve_noncached(void) /* reserve memory for malloc() area */ static int reserve_malloc(void) { - gd->start_addr_sp = gd->start_addr_sp - TOTAL_MALLOC_LEN; + gd->start_addr_sp = reserve_stack_aligned(TOTAL_MALLOC_LEN); debug("Reserving %dk for malloc() at: %08lx\n", TOTAL_MALLOC_LEN >> 10, gd->start_addr_sp); #ifdef CONFIG_SYS_NONCACHED_MEMORY @@ -511,7 +522,7 @@ static int reserve_malloc(void) static int reserve_board(void) { if (!gd->bd) { - gd->start_addr_sp -= sizeof(bd_t); + gd->start_addr_sp = reserve_stack_aligned(sizeof(bd_t)); gd->bd = (bd_t *)map_sysmem(gd->start_addr_sp, sizeof(bd_t)); memset(gd->bd, '\0', sizeof(bd_t)); debug("Reserving %zu Bytes for Board Info at: %08lx\n", @@ -530,7 +541,7 @@ static int setup_machine(void) static int reserve_global_data(void) { - gd->start_addr_sp -= sizeof(gd_t); + gd->start_addr_sp = reserve_stack_aligned(sizeof(gd_t)); gd->new_gd = (gd_t *)map_sysmem(gd->start_addr_sp, sizeof(gd_t)); debug("Reserving %zu Bytes for Global Data at: %08lx\n", sizeof(gd_t), gd->start_addr_sp); @@ -548,7 +559,7 @@ static int reserve_fdt(void) if (gd->fdt_blob) { gd->fdt_size = ALIGN(fdt_totalsize(gd->fdt_blob) + 0x1000, 32); - gd->start_addr_sp -= gd->fdt_size; + gd->start_addr_sp = reserve_stack_aligned(gd->fdt_size); gd->new_fdt = map_sysmem(gd->start_addr_sp, gd->fdt_size); debug("Reserving %lu Bytes for FDT at: %08lx\n", gd->fdt_size, gd->start_addr_sp); @@ -563,12 +574,7 @@ static int reserve_bootstage(void) #ifdef CONFIG_BOOTSTAGE int size = bootstage_get_size(); - gd->start_addr_sp -= size; - /* - * Ensure that start_addr_sp is aligned down to reserve enough - * space for new_bootstage - */ - gd->start_addr_sp = ALIGN_DOWN(gd->start_addr_sp, 16); + gd->start_addr_sp = reserve_stack_aligned(size); gd->new_bootstage = map_sysmem(gd->start_addr_sp, size); debug("Reserving %#x Bytes for bootstage at: %08lx\n", size, gd->start_addr_sp); @@ -585,8 +591,7 @@ __weak int arch_reserve_stacks(void) static int reserve_stacks(void) { /* make stack pointer 16-byte aligned */ - gd->start_addr_sp -= 16; - gd->start_addr_sp &= ~0xf; + gd->start_addr_sp = reserve_stack_aligned(16); /* * let the architecture-specific code tailor gd->start_addr_sp and @@ -598,8 +603,7 @@ static int reserve_stacks(void) static int reserve_bloblist(void) { #ifdef CONFIG_BLOBLIST - gd->start_addr_sp &= ~0xf; - gd->start_addr_sp -= CONFIG_BLOBLIST_SIZE; + gd->start_addr_sp = reserve_stack_aligned(CONFIG_BLOBLIST_SIZE); gd->new_bloblist = map_sysmem(gd->start_addr_sp, CONFIG_BLOBLIST_SIZE); #endif -- 2.17.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 4/4] board_f.c: Ensure 16 alignment of start_addr_sp and reserved memory 2020-03-10 9:15 ` [PATCH v4 4/4] board_f.c: Ensure 16 alignment of start_addr_sp and reserved memory Patrick Delaunay @ 2020-04-17 21:08 ` Tom Rini 0 siblings, 0 replies; 9+ messages in thread From: Tom Rini @ 2020-04-17 21:08 UTC (permalink / raw) To: u-boot On Tue, Mar 10, 2020 at 10:15:05AM +0100, Patrick Delaunay wrote: > Add a function reserve_stack_aligned() to reserved memory with 16 bits > alignment after the stack pointer (gd->start_addr_sp) and use this new > function in board_f.c to reserve all the memory area (malloc, board, gd, > fdt, bootstage, stacks). > > This 16 byte alignment is needed for cast on struct pointer > for the reserved memory, for example: > + x86_64 ABI: https://reviews.llvm.org/D30049: 16 bytes > + ARMv8 Instruction Set Overview: quad word, 16 bytes > > An other alignment value could be needed for other architecture. > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> > Acked-by: Stephen Warren <swarren@nvidia.com> Applied to u-boot/master, thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 659 bytes Desc: not available URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200417/1394297e/attachment.sig> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-04-17 21:08 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-10 9:15 [PATCH v4 0/4] Ensure 16 alignment of reserved memory in board_f.c Patrick Delaunay 2020-03-10 9:15 ` [PATCH v4 1/4] board_f.c: Ensure gd->new_bootstage alignment Patrick Delaunay 2020-04-17 21:08 ` Tom Rini 2020-03-10 9:15 ` [PATCH v4 2/4] Revert "stm32mp1: remove the imply BOOTSTAGE" Patrick Delaunay 2020-04-17 21:08 ` Tom Rini 2020-03-10 9:15 ` [PATCH v4 3/4] arm: set the relocated gd with gd->new_gd Patrick Delaunay 2020-04-17 21:08 ` Tom Rini 2020-03-10 9:15 ` [PATCH v4 4/4] board_f.c: Ensure 16 alignment of start_addr_sp and reserved memory Patrick Delaunay 2020-04-17 21:08 ` Tom Rini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.