diff mbox series

[3/3] board_f.c: Insure 16 alignment of start_addr_sp and reserved memory

Message ID 20200109171123.16348-4-patrick.delaunay@st.com
State Superseded
Headers show
Series | expand

Commit Message

Patrick Delaunay Jan. 9, 2020, 5:11 p.m. UTC
Add a function reserve_sp() 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 at st.com>
---

 common/board_f.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

Comments

Heinrich Schuchardt Jan. 9, 2020, 5:55 p.m. UTC | #1
On 1/9/20 6:11 PM, Patrick Delaunay wrote:
> Add a function reserve_sp() to reserved memory with 16 bits alignment

I guess this is a typo:

%s/bits/bytes/

Best regards

Heinrich

> 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 at st.com>
> ---
>
>   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 e21f533634..0302ee4a6e 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -470,6 +470,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_sp(size_t size)
> +{
> +	return ALIGN_DOWN(gd->start_addr_sp - size, 16);
> +}
> +
>   #ifdef CONFIG_SYS_NONCACHED_MEMORY
>   static int reserve_noncached(void)
>   {
> @@ -495,7 +506,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_sp(TOTAL_MALLOC_LEN);
>   	debug("Reserving %dk for malloc() at: %08lx\n",
>   	      TOTAL_MALLOC_LEN >> 10, gd->start_addr_sp);
>   #ifdef CONFIG_SYS_NONCACHED_MEMORY
> @@ -509,7 +520,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_sp(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",
> @@ -528,7 +539,7 @@ static int setup_machine(void)
>
>   static int reserve_global_data(void)
>   {
> -	gd->start_addr_sp -= sizeof(gd_t);
> +	gd->start_addr_sp = reserve_sp(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);
> @@ -546,7 +557,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_sp(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);
> @@ -561,12 +572,7 @@ static int reserve_bootstage(void)
>   #ifdef CONFIG_BOOTSTAGE
>   	int size = bootstage_get_size();
>
> -	gd->start_addr_sp -= size;
> -	/*
> -	 * Insure 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_sp(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);
> @@ -583,8 +589,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_sp(16);
>
>   	/*
>   	 * let the architecture-specific code tailor gd->start_addr_sp and
> @@ -596,8 +601,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_sp(CONFIG_BLOBLIST_SIZE);
>   	gd->new_bloblist = map_sysmem(gd->start_addr_sp, CONFIG_BLOBLIST_SIZE);
>   #endif
>
>
Patrick Delaunay Jan. 10, 2020, 8:47 a.m. UTC | #2
Hi,

> From: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Sent: jeudi 9 janvier 2020 18:56
> 
> On 1/9/20 6:11 PM, Patrick Delaunay wrote:
> > Add a function reserve_sp() to reserved memory with 16 bits alignment
> 
> I guess this is a typo:
> 
> %s/bits/bytes/

Yes error in commit message "16 bytes alignment"... 
I will correct it in V2, thanks.

Regards

Patrick
 
> Best regards
> 
> Heinrich
> 
> > 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 at st.com>
> > ---
> >
> >   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
> > e21f533634..0302ee4a6e 100644
> > --- a/common/board_f.c
> > +++ b/common/board_f.c
> > @@ -470,6 +470,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_sp(size_t size) {
> > +	return ALIGN_DOWN(gd->start_addr_sp - size, 16); }
> > +
> >   #ifdef CONFIG_SYS_NONCACHED_MEMORY
> >   static int reserve_noncached(void)
> >   {
> > @@ -495,7 +506,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_sp(TOTAL_MALLOC_LEN);
> >   	debug("Reserving %dk for malloc() at: %08lx\n",
> >   	      TOTAL_MALLOC_LEN >> 10, gd->start_addr_sp);
> >   #ifdef CONFIG_SYS_NONCACHED_MEMORY
> > @@ -509,7 +520,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_sp(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", @@ -
> 528,7
> > +539,7 @@ static int setup_machine(void)
> >
> >   static int reserve_global_data(void)
> >   {
> > -	gd->start_addr_sp -= sizeof(gd_t);
> > +	gd->start_addr_sp = reserve_sp(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); @@ -546,7 +557,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_sp(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); @@ -561,12 +572,7 @@
> > static int reserve_bootstage(void)
> >   #ifdef CONFIG_BOOTSTAGE
> >   	int size = bootstage_get_size();
> >
> > -	gd->start_addr_sp -= size;
> > -	/*
> > -	 * Insure 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_sp(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);
> > @@ -583,8 +589,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_sp(16);
> >
> >   	/*
> >   	 * let the architecture-specific code tailor gd->start_addr_sp and
> > @@ -596,8 +601,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_sp(CONFIG_BLOBLIST_SIZE);
> >   	gd->new_bloblist = map_sysmem(gd->start_addr_sp,
> CONFIG_BLOBLIST_SIZE);
> >   #endif
> >
> >
diff mbox series

Patch

diff --git a/common/board_f.c b/common/board_f.c
index e21f533634..0302ee4a6e 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -470,6 +470,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_sp(size_t size)
+{
+	return ALIGN_DOWN(gd->start_addr_sp - size, 16);
+}
+
 #ifdef CONFIG_SYS_NONCACHED_MEMORY
 static int reserve_noncached(void)
 {
@@ -495,7 +506,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_sp(TOTAL_MALLOC_LEN);
 	debug("Reserving %dk for malloc() at: %08lx\n",
 	      TOTAL_MALLOC_LEN >> 10, gd->start_addr_sp);
 #ifdef CONFIG_SYS_NONCACHED_MEMORY
@@ -509,7 +520,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_sp(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",
@@ -528,7 +539,7 @@  static int setup_machine(void)
 
 static int reserve_global_data(void)
 {
-	gd->start_addr_sp -= sizeof(gd_t);
+	gd->start_addr_sp = reserve_sp(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);
@@ -546,7 +557,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_sp(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);
@@ -561,12 +572,7 @@  static int reserve_bootstage(void)
 #ifdef CONFIG_BOOTSTAGE
 	int size = bootstage_get_size();
 
-	gd->start_addr_sp -= size;
-	/*
-	 * Insure 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_sp(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);
@@ -583,8 +589,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_sp(16);
 
 	/*
 	 * let the architecture-specific code tailor gd->start_addr_sp and
@@ -596,8 +601,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_sp(CONFIG_BLOBLIST_SIZE);
 	gd->new_bloblist = map_sysmem(gd->start_addr_sp, CONFIG_BLOBLIST_SIZE);
 #endif