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