[U-Boot] [PATCH 0/5] x86: Move x86 to use board_init_f_mem() instead of assembler

[U-Boot] [PATCH 0/5] x86: Move x86 to use board_init_f_mem() instead of assembler

[Simon Glass]

Much of the early-init assembler on x86 can be removed if we use the new
board_init_f_mem() function. At present only PowerPC uses it, but we should
try to move more archs over.

This has been contemplated for a while but it is time to take the plunge.


Simon Glass (5):
  x86: Remove init_gd() function
  Align global_data to a 16-byte boundary
  Allow arch-specific setting of global_data in board_init_f_mem()
  x86: Move the GDT into global_data
  x86: Switch to using generic global_data setup

 arch/x86/cpu/cpu.c                 | 11 +++--
 arch/x86/cpu/start.S               | 86 +++++---------------------------------
 arch/x86/include/asm/global_data.h |  4 +-
 arch/x86/include/asm/u-boot-x86.h  |  1 -
 common/board_f.c                   | 19 ++++++---
 include/asm-generic/global_data.h  |  2 +-
 include/common.h                   | 40 ++++++++++++++++++
 7 files changed, 75 insertions(+), 88 deletions(-)

-- 
2.5.0.rc2.392.g76e840b

[U-Boot] [PATCH 1/5] x86: Remove init_gd() function

[Simon Glass]

This is declared but no-longer exists. Drop it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/x86/include/asm/u-boot-x86.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h
index 4dae365..1c459d5 100644
--- a/arch/x86/include/asm/u-boot-x86.h
+++ b/arch/x86/include/asm/u-boot-x86.h
@@ -14,7 +14,6 @@ extern char gdt_rom[];
 int arch_cpu_init(void);
 int x86_cpu_init_f(void);
 int cpu_init_f(void);
-void init_gd(gd_t *id, u64 *gdt_addr);
 void setup_gdt(gd_t *id, u64 *gdt_addr);
 /*
  * Setup FSP execution environment GDT to use the one we used in
-- 
2.5.0.rc2.392.g76e840b

[U-Boot] [PATCH 1/5] x86: Remove init_gd() function

[Bin Meng]

On Mon, Aug 3, 2015 at 8:10 AM, Simon Glass <sjg@chromium.org> wrote:
> This is declared but no-longer exists. Drop it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  arch/x86/include/asm/u-boot-x86.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h
> index 4dae365..1c459d5 100644
> --- a/arch/x86/include/asm/u-boot-x86.h
> +++ b/arch/x86/include/asm/u-boot-x86.h
> @@ -14,7 +14,6 @@ extern char gdt_rom[];
>  int arch_cpu_init(void);
>  int x86_cpu_init_f(void);
>  int cpu_init_f(void);
> -void init_gd(gd_t *id, u64 *gdt_addr);
>  void setup_gdt(gd_t *id, u64 *gdt_addr);
>  /*
>   * Setup FSP execution environment GDT to use the one we used in
> --

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH 2/5] Align global_data to a 16-byte boundary

[Simon Glass]

Some archs like to have larger alignment for their global data. Use 16 bytes
which suits all current archs.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/board_f.c                  | 2 +-
 include/asm-generic/global_data.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/board_f.c b/common/board_f.c
index c596083..a947770 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -497,7 +497,7 @@ static int setup_machine(void)
 
 static int reserve_global_data(void)
 {
-	gd->start_addr_sp -= sizeof(gd_t);
+	gd->start_addr_sp = ALIGN(gd->start_addr_sp - sizeof(gd_t), 16);
 	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);
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 2155265..147f646 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -99,7 +99,7 @@ typedef struct global_data {
 	int pcidelay_done;
 #endif
 	struct udevice *cur_serial_dev;	/* current serial device */
-	struct arch_global_data arch;	/* architecture-specific data */
+	struct arch_global_data arch __aligned(16);	/* arch-specific data */
 } gd_t;
 #endif
 
-- 
2.5.0.rc2.392.g76e840b

[U-Boot] [PATCH 2/5] Align global_data to a 16-byte boundary

[Bin Meng]

Hi Simon,

On Mon, Aug 3, 2015 at 8:10 AM, Simon Glass <sjg@chromium.org> wrote:
> Some archs like to have larger alignment for their global data. Use 16 bytes
> which suits all current archs.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  common/board_f.c                  | 2 +-
>  include/asm-generic/global_data.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/common/board_f.c b/common/board_f.c
> index c596083..a947770 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -497,7 +497,7 @@ static int setup_machine(void)
>
>  static int reserve_global_data(void)
>  {
> -       gd->start_addr_sp -= sizeof(gd_t);
> +       gd->start_addr_sp = ALIGN(gd->start_addr_sp - sizeof(gd_t), 16);

This logic is wrong. This alignment adjustment makes there is no
enough room for global data.

>         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);
> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> index 2155265..147f646 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -99,7 +99,7 @@ typedef struct global_data {
>         int pcidelay_done;
>  #endif
>         struct udevice *cur_serial_dev; /* current serial device */
> -       struct arch_global_data arch;   /* architecture-specific data */
> +       struct arch_global_data arch __aligned(16);     /* arch-specific data */
>  } gd_t;
>  #endif
>
> --

Regards,
Bin

[U-Boot] [PATCH 3/5] Allow arch-specific setting of global_data in board_init_f_mem()

[Simon Glass]

At present we have a simple assignment to gd. With some archs this is
implemented as a register or through some other means; a simple assignment
does not suit in all cases.

Change this to a function and add documentation to describe how this all
works.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/board_f.c | 14 +++++++++++---
 include/common.h | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/common/board_f.c b/common/board_f.c
index a947770..3d1f305 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -1024,15 +1024,23 @@ void board_init_f_r(void)
 #endif /* CONFIG_X86 */
 
 #ifndef CONFIG_X86
-ulong board_init_f_mem(ulong top)
+__weak void arch_setup_gdt(struct global_data *gd_ptr)
 {
+	gd = gd_ptr;
+}
+
+ulong board_init_f_mem(ulong top, ulong reserve_size)
+{
+	struct global_data *gd_ptr;
+
 	/* Leave space for the stack we are running with now */
 	top -= 0x40;
 
 	top -= sizeof(struct global_data);
 	top = ALIGN(top, 16);
-	gd = (struct global_data *)top;
-	memset((void *)gd, '\0', sizeof(*gd));
+	gd_ptr = (struct global_data *)top;
+	memset(gd_ptr, '\0', sizeof(*gd));
+	arch_setup_gdt(gd_ptr);
 
 #ifdef CONFIG_SYS_MALLOC_F_LEN
 	top -= CONFIG_SYS_MALLOC_F_LEN;
diff --git a/include/common.h b/include/common.h
index fcc9ae7..029eb1f 100644
--- a/include/common.h
+++ b/include/common.h
@@ -217,6 +217,46 @@ extern char console_buffer[];
 /* arch/$(ARCH)/lib/board.c */
 void board_init_f(ulong);
 void board_init_r(gd_t *, ulong) __attribute__ ((noreturn));
+
+/**
+ * board_init_f_mem() - Allocate global data and set stack position
+ *
+ * This function is called by each architecture very early in the start-up
+ * code to set up the environment for board_init_f(). It allocates space for
+ * global_data (see include/asm-generic/global_data.h) and places the stack
+ * below this.
+ *
+ * This function requires a stack[1] Normally this is at @top. The function
+ * starts allocating space from 64 bytes below @top. First it creates space
+ * for global_data. then it calls arch_setup_gd() which sets gd to point to
+ * the global_data space and can reserve additional bytes of space if
+ * required). Finally it allocates early malloc() memory
+ * (CONFIG_SYS_MALLOC_F_LEN). The new top of the stack is just below this,
+ * and it returned by this function.
+ *
+ * [1] Strictly speaking it would be possible to implement this function
+ * in C on many archs such that it does not require a stack. However this
+ * does not seem hugely important as only 64 byte are wasted.
+ *
+ * @top:	Top of available memory, also normally the top of the stack
+ * @return:	New stack location
+ */
+ulong board_init_f_mem(ulong top, ulong reserve_size);
+
+/**
+ * arch_setup_gdt() - Set up the global_data pointer
+ *
+ * This pointer is special in some architectures and cannot easily be assigned
+ * to. For example on x86 it is implemented by adding a specific record to its
+ * Global Descriptor Table! So we we provide a function to carry out this task.
+ * For most architectures this can simply be:
+ *
+ *    gd = gd_ptr;
+ *
+ * @gd_ptr:	Pointer to global data
+ */
+void arch_setup_gdt(gd_t *gd_ptr);
+
 int checkboard(void);
 int show_board_info(void);
 int checkflash(void);
-- 
2.5.0.rc2.392.g76e840b

[U-Boot] [PATCH 3/5] Allow arch-specific setting of global_data in board_init_f_mem()

[York Sun]

On 08/02/2015 05:10 PM, Simon Glass wrote:
> At present we have a simple assignment to gd. With some archs this is
> implemented as a register or through some other means; a simple assignment
> does not suit in all cases.
> 
> Change this to a function and add documentation to describe how this all
> works.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/board_f.c | 14 +++++++++++---
>  include/common.h | 40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+), 3 deletions(-)
> 
> diff --git a/common/board_f.c b/common/board_f.c
> index a947770..3d1f305 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -1024,15 +1024,23 @@ void board_init_f_r(void)
>  #endif /* CONFIG_X86 */
>  
>  #ifndef CONFIG_X86
> -ulong board_init_f_mem(ulong top)
> +__weak void arch_setup_gdt(struct global_data *gd_ptr)
>  {
> +	gd = gd_ptr;
> +}
> +
> +ulong board_init_f_mem(ulong top, ulong reserve_size)
> +{
> +	struct global_data *gd_ptr;
> +
>  	/* Leave space for the stack we are running with now */
>  	top -= 0x40;
>  
>  	top -= sizeof(struct global_data);
>  	top = ALIGN(top, 16);
> -	gd = (struct global_data *)top;
> -	memset((void *)gd, '\0', sizeof(*gd));
> +	gd_ptr = (struct global_data *)top;
> +	memset(gd_ptr, '\0', sizeof(*gd));
> +	arch_setup_gdt(gd_ptr);
>  
>  #ifdef CONFIG_SYS_MALLOC_F_LEN
>  	top -= CONFIG_SYS_MALLOC_F_LEN;
> diff --git a/include/common.h b/include/common.h
> index fcc9ae7..029eb1f 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -217,6 +217,46 @@ extern char console_buffer[];
>  /* arch/$(ARCH)/lib/board.c */
>  void board_init_f(ulong);
>  void board_init_r(gd_t *, ulong) __attribute__ ((noreturn));
> +
> +/**
> + * board_init_f_mem() - Allocate global data and set stack position
> + *
> + * This function is called by each architecture very early in the start-up
> + * code to set up the environment for board_init_f(). It allocates space for
> + * global_data (see include/asm-generic/global_data.h) and places the stack
> + * below this.
> + *
> + * This function requires a stack[1] Normally this is at @top. The function
> + * starts allocating space from 64 bytes below @top. First it creates space
> + * for global_data. then it calls arch_setup_gd() which sets gd to point to
> + * the global_data space and can reserve additional bytes of space if
> + * required). Finally it allocates early malloc() memory
> + * (CONFIG_SYS_MALLOC_F_LEN). The new top of the stack is just below this,
> + * and it returned by this function.
> + *
> + * [1] Strictly speaking it would be possible to implement this function
> + * in C on many archs such that it does not require a stack. However this
> + * does not seem hugely important as only 64 byte are wasted.
> + *
> + * @top:	Top of available memory, also normally the top of the stack
> + * @return:	New stack location
> + */
> +ulong board_init_f_mem(ulong top, ulong reserve_size);
> +
> +/**
> + * arch_setup_gdt() - Set up the global_data pointer
> + *
> + * This pointer is special in some architectures and cannot easily be assigned
> + * to. For example on x86 it is implemented by adding a specific record to its
> + * Global Descriptor Table! So we we provide a function to carry out this task.
> + * For most architectures this can simply be:
> + *
> + *    gd = gd_ptr;
> + *
> + * @gd_ptr:	Pointer to global data
> + */
> +void arch_setup_gdt(gd_t *gd_ptr);
> +
>  int checkboard(void);
>  int show_board_info(void);
>  int checkflash(void);
> 

This looks good to me.
Reviewed-by: York Sun <yorksun@freescale.com>

York

[U-Boot] [PATCH 3/5] Allow arch-specific setting of global_data in board_init_f_mem()

[Bin Meng]

Hi Simon,

On Mon, Aug 3, 2015 at 8:10 AM, Simon Glass <sjg@chromium.org> wrote:
> At present we have a simple assignment to gd. With some archs this is
> implemented as a register or through some other means; a simple assignment
> does not suit in all cases.
>
> Change this to a function and add documentation to describe how this all
> works.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  common/board_f.c | 14 +++++++++++---
>  include/common.h | 40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/common/board_f.c b/common/board_f.c
> index a947770..3d1f305 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -1024,15 +1024,23 @@ void board_init_f_r(void)
>  #endif /* CONFIG_X86 */
>
>  #ifndef CONFIG_X86
> -ulong board_init_f_mem(ulong top)
> +__weak void arch_setup_gdt(struct global_data *gd_ptr)
>  {
> +       gd = gd_ptr;
> +}
> +
> +ulong board_init_f_mem(ulong top, ulong reserve_size)

What is reserve_size? I don't see it is used by this function. If we
change the signature, we need also update arc and ppc4xx start.S
otherwise they will break.

> +{
> +       struct global_data *gd_ptr;
> +
>         /* Leave space for the stack we are running with now */
>         top -= 0x40;
>
>         top -= sizeof(struct global_data);
>         top = ALIGN(top, 16);
> -       gd = (struct global_data *)top;
> -       memset((void *)gd, '\0', sizeof(*gd));
> +       gd_ptr = (struct global_data *)top;
> +       memset(gd_ptr, '\0', sizeof(*gd));
> +       arch_setup_gdt(gd_ptr);

gdt? Should be just arch_setup_gd()?

>
>  #ifdef CONFIG_SYS_MALLOC_F_LEN
>         top -= CONFIG_SYS_MALLOC_F_LEN;
> diff --git a/include/common.h b/include/common.h
> index fcc9ae7..029eb1f 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -217,6 +217,46 @@ extern char console_buffer[];
>  /* arch/$(ARCH)/lib/board.c */
>  void board_init_f(ulong);
>  void board_init_r(gd_t *, ulong) __attribute__ ((noreturn));
> +
> +/**
> + * board_init_f_mem() - Allocate global data and set stack position
> + *
> + * This function is called by each architecture very early in the start-up
> + * code to set up the environment for board_init_f(). It allocates space for
> + * global_data (see include/asm-generic/global_data.h) and places the stack
> + * below this.
> + *
> + * This function requires a stack[1] Normally this is at @top. The function
> + * starts allocating space from 64 bytes below @top. First it creates space
> + * for global_data. then it calls arch_setup_gd() which sets gd to point to

Nits: . -> ,

> + * the global_data space and can reserve additional bytes of space if
> + * required). Finally it allocates early malloc() memory
> + * (CONFIG_SYS_MALLOC_F_LEN). The new top of the stack is just below this,
> + * and it returned by this function.
> + *
> + * [1] Strictly speaking it would be possible to implement this function
> + * in C on many archs such that it does not require a stack. However this
> + * does not seem hugely important as only 64 byte are wasted.

Where is the 64 bytes it uses? I only see a variable gd_ptr on the stack.

> + *
> + * @top:       Top of available memory, also normally the top of the stack
> + * @return:    New stack location
> + */
> +ulong board_init_f_mem(ulong top, ulong reserve_size);

I don't like the name board_init_f_mem(). It actually does two things:
creates global data pointer and reserve the early malloc() memory
size. Also it looks like it's more like an arch thing instead of a
board's. How about setup_gd_f_mem()?

> +
> +/**
> + * arch_setup_gdt() - Set up the global_data pointer
> + *
> + * This pointer is special in some architectures and cannot easily be assigned
> + * to. For example on x86 it is implemented by adding a specific record to its
> + * Global Descriptor Table! So we we provide a function to carry out this task.
> + * For most architectures this can simply be:
> + *
> + *    gd = gd_ptr;
> + *
> + * @gd_ptr:    Pointer to global data
> + */
> +void arch_setup_gdt(gd_t *gd_ptr);
> +
>  int checkboard(void);
>  int show_board_info(void);
>  int checkflash(void);
> --

Regards,
Bin

[U-Boot] [PATCH 3/5] Allow arch-specific setting of global_data in board_init_f_mem()

[Simon Glass]

Hi Bin,

On 6 August 2015 at 01:15, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Mon, Aug 3, 2015 at 8:10 AM, Simon Glass <sjg@chromium.org> wrote:
>> At present we have a simple assignment to gd. With some archs this is
>> implemented as a register or through some other means; a simple assignment
>> does not suit in all cases.
>>
>> Change this to a function and add documentation to describe how this all
>> works.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  common/board_f.c | 14 +++++++++++---
>>  include/common.h | 40 ++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 51 insertions(+), 3 deletions(-)
>>
>> diff --git a/common/board_f.c b/common/board_f.c
>> index a947770..3d1f305 100644
>> --- a/common/board_f.c
>> +++ b/common/board_f.c
>> @@ -1024,15 +1024,23 @@ void board_init_f_r(void)
>>  #endif /* CONFIG_X86 */
>>
>>  #ifndef CONFIG_X86
>> -ulong board_init_f_mem(ulong top)
>> +__weak void arch_setup_gdt(struct global_data *gd_ptr)
>>  {
>> +       gd = gd_ptr;
>> +}
>> +
>> +ulong board_init_f_mem(ulong top, ulong reserve_size)
>
> What is reserve_size? I don't see it is used by this function. If we
> change the signature, we need also update arc and ppc4xx start.S
> otherwise they will break.

Actually that should have been reverted. I decided to introduce it
later if needed, but found a way around it for x86.

>
>> +{
>> +       struct global_data *gd_ptr;
>> +
>>         /* Leave space for the stack we are running with now */
>>         top -= 0x40;
>>
>>         top -= sizeof(struct global_data);
>>         top = ALIGN(top, 16);
>> -       gd = (struct global_data *)top;
>> -       memset((void *)gd, '\0', sizeof(*gd));
>> +       gd_ptr = (struct global_data *)top;
>> +       memset(gd_ptr, '\0', sizeof(*gd));
>> +       arch_setup_gdt(gd_ptr);
>
> gdt? Should be just arch_setup_gd()?

Yes

>
>>
>>  #ifdef CONFIG_SYS_MALLOC_F_LEN
>>         top -= CONFIG_SYS_MALLOC_F_LEN;
>> diff --git a/include/common.h b/include/common.h
>> index fcc9ae7..029eb1f 100644
>> --- a/include/common.h
>> +++ b/include/common.h
>> @@ -217,6 +217,46 @@ extern char console_buffer[];
>>  /* arch/$(ARCH)/lib/board.c */
>>  void board_init_f(ulong);
>>  void board_init_r(gd_t *, ulong) __attribute__ ((noreturn));
>> +
>> +/**
>> + * board_init_f_mem() - Allocate global data and set stack position
>> + *
>> + * This function is called by each architecture very early in the start-up
>> + * code to set up the environment for board_init_f(). It allocates space for
>> + * global_data (see include/asm-generic/global_data.h) and places the stack
>> + * below this.
>> + *
>> + * This function requires a stack[1] Normally this is at @top. The function
>> + * starts allocating space from 64 bytes below @top. First it creates space
>> + * for global_data. then it calls arch_setup_gd() which sets gd to point to
>
> Nits: . -> ,
>
>> + * the global_data space and can reserve additional bytes of space if
>> + * required). Finally it allocates early malloc() memory
>> + * (CONFIG_SYS_MALLOC_F_LEN). The new top of the stack is just below this,
>> + * and it returned by this function.
>> + *
>> + * [1] Strictly speaking it would be possible to implement this function
>> + * in C on many archs such that it does not require a stack. However this
>> + * does not seem hugely important as only 64 byte are wasted.
>
> Where is the 64 bytes it uses? I only see a variable gd_ptr on the stack.

It's the subtraction at the top of board_init_f_mem().

>
>> + *
>> + * @top:       Top of available memory, also normally the top of the stack
>> + * @return:    New stack location
>> + */
>> +ulong board_init_f_mem(ulong top, ulong reserve_size);
>
> I don't like the name board_init_f_mem(). It actually does two things:
> creates global data pointer and reserve the early malloc() memory
> size. Also it looks like it's more like an arch thing instead of a
> board's. How about setup_gd_f_mem()?

The name is the same as before. It's intended to imply that it gets
the memory ready for board_init_f(). It may expand in future to do
additional setup (e.g. to allocate some other type of space). If we
change the name we would perhaps have to change it again later. At
least this name describes its purpose.

>
>> +
>> +/**
>> + * arch_setup_gdt() - Set up the global_data pointer
>> + *
>> + * This pointer is special in some architectures and cannot easily be assigned
>> + * to. For example on x86 it is implemented by adding a specific record to its
>> + * Global Descriptor Table! So we we provide a function to carry out this task.
>> + * For most architectures this can simply be:
>> + *
>> + *    gd = gd_ptr;
>> + *
>> + * @gd_ptr:    Pointer to global data
>> + */
>> +void arch_setup_gdt(gd_t *gd_ptr);
>> +
>>  int checkboard(void);
>>  int show_board_info(void);
>>  int checkflash(void);
>> --
>
> Regards,
> Bin

Regards,
Simon

[U-Boot] [PATCH 4/5] x86: Move the GDT into global_data

[Simon Glass]

Rather than keeping track of the Global Descriptor Table in its own memory
we may as well put it in global_data with everything else. As a first step,
stop using the separately allocated GDT.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/x86/cpu/cpu.c                 | 9 +++++----
 arch/x86/include/asm/global_data.h | 4 +++-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index 129777c..4f57145 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -136,9 +136,10 @@ static void load_gdt(const u64 *boot_gdt, u16 num_entries)
 	asm volatile("lgdtl %0\n" : : "m" (gdt));
 }
 
-void setup_gdt(gd_t *id, u64 *gdt_addr)
+void setup_gdt(gd_t *new_gd, u64 *gdt_addr)
 {
-	id->arch.gdt = gdt_addr;
+	gdt_addr = new_gd->arch.gdt;
+
 	/* CS: code, read/execute, 4 GB, base 0 */
 	gdt_addr[X86_GDT_ENTRY_32BIT_CS] = GDT_ENTRY(0xc09b, 0, 0xfffff);
 
@@ -146,9 +147,9 @@ void setup_gdt(gd_t *id, u64 *gdt_addr)
 	gdt_addr[X86_GDT_ENTRY_32BIT_DS] = GDT_ENTRY(0xc093, 0, 0xfffff);
 
 	/* FS: data, read/write, 4 GB, base (Global Data Pointer) */
-	id->arch.gd_addr = id;
+	new_gd->arch.gd_addr = new_gd;
 	gdt_addr[X86_GDT_ENTRY_32BIT_FS] = GDT_ENTRY(0xc093,
-		     (ulong)&id->arch.gd_addr, 0xfffff);
+		     (ulong)&new_gd->arch.gd_addr, 0xfffff);
 
 	/* 16-bit CS: code, read/execute, 64 kB, base 0 */
 	gdt_addr[X86_GDT_ENTRY_16BIT_CS] = GDT_ENTRY(0x009b, 0, 0x0ffff);
diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h
index f7e3889..35148ab 100644
--- a/arch/x86/include/asm/global_data.h
+++ b/arch/x86/include/asm/global_data.h
@@ -10,6 +10,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include <asm/processor.h>
+
 enum pei_boot_mode_t {
 	PEI_BOOT_NONE = 0,
 	PEI_BOOT_SOFT_RESET,
@@ -44,6 +46,7 @@ struct mtrr_request {
 
 /* Architecture-specific global data */
 struct arch_global_data {
+	u64 gdt[X86_GDT_NUM_ENTRIES] __aligned(16);
 	struct global_data *gd_addr;	/* Location of Global Data */
 	uint8_t x86;			/* CPU family */
 	uint8_t x86_vendor;		/* CPU vendor */
@@ -68,7 +71,6 @@ struct arch_global_data {
 	/* MRC training data to save for the next boot */
 	char *mrc_output;
 	unsigned int mrc_output_len;
-	void *gdt;			/* Global descriptor table */
 	ulong table;			/* Table pointer from previous loader */
 };
 
-- 
2.5.0.rc2.392.g76e840b

[U-Boot] [PATCH 4/5] x86: Move the GDT into global_data

[Bin Meng]

On Mon, Aug 3, 2015 at 8:10 AM, Simon Glass <sjg@chromium.org> wrote:
> Rather than keeping track of the Global Descriptor Table in its own memory
> we may as well put it in global_data with everything else. As a first step,
> stop using the separately allocated GDT.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  arch/x86/cpu/cpu.c                 | 9 +++++----
>  arch/x86/include/asm/global_data.h | 4 +++-
>  2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
> index 129777c..4f57145 100644
> --- a/arch/x86/cpu/cpu.c
> +++ b/arch/x86/cpu/cpu.c
> @@ -136,9 +136,10 @@ static void load_gdt(const u64 *boot_gdt, u16 num_entries)
>         asm volatile("lgdtl %0\n" : : "m" (gdt));
>  }
>
> -void setup_gdt(gd_t *id, u64 *gdt_addr)
> +void setup_gdt(gd_t *new_gd, u64 *gdt_addr)
>  {
> -       id->arch.gdt = gdt_addr;
> +       gdt_addr = new_gd->arch.gdt;
> +
>         /* CS: code, read/execute, 4 GB, base 0 */
>         gdt_addr[X86_GDT_ENTRY_32BIT_CS] = GDT_ENTRY(0xc09b, 0, 0xfffff);
>
> @@ -146,9 +147,9 @@ void setup_gdt(gd_t *id, u64 *gdt_addr)
>         gdt_addr[X86_GDT_ENTRY_32BIT_DS] = GDT_ENTRY(0xc093, 0, 0xfffff);
>
>         /* FS: data, read/write, 4 GB, base (Global Data Pointer) */
> -       id->arch.gd_addr = id;
> +       new_gd->arch.gd_addr = new_gd;
>         gdt_addr[X86_GDT_ENTRY_32BIT_FS] = GDT_ENTRY(0xc093,
> -                    (ulong)&id->arch.gd_addr, 0xfffff);
> +                    (ulong)&new_gd->arch.gd_addr, 0xfffff);
>
>         /* 16-bit CS: code, read/execute, 64 kB, base 0 */
>         gdt_addr[X86_GDT_ENTRY_16BIT_CS] = GDT_ENTRY(0x009b, 0, 0x0ffff);
> diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h
> index f7e3889..35148ab 100644
> --- a/arch/x86/include/asm/global_data.h
> +++ b/arch/x86/include/asm/global_data.h
> @@ -10,6 +10,8 @@
>
>  #ifndef __ASSEMBLY__
>
> +#include <asm/processor.h>
> +
>  enum pei_boot_mode_t {
>         PEI_BOOT_NONE = 0,
>         PEI_BOOT_SOFT_RESET,
> @@ -44,6 +46,7 @@ struct mtrr_request {
>
>  /* Architecture-specific global data */
>  struct arch_global_data {
> +       u64 gdt[X86_GDT_NUM_ENTRIES] __aligned(16);
>         struct global_data *gd_addr;    /* Location of Global Data */
>         uint8_t x86;                    /* CPU family */
>         uint8_t x86_vendor;             /* CPU vendor */
> @@ -68,7 +71,6 @@ struct arch_global_data {
>         /* MRC training data to save for the next boot */
>         char *mrc_output;
>         unsigned int mrc_output_len;
> -       void *gdt;                      /* Global descriptor table */
>         ulong table;                    /* Table pointer from previous loader */
>  };
>
> --

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH 5/5] x86: Switch to using generic global_data setup

[Simon Glass]

There is quite a bit of assembler code that can be removed if we use the
generic global_data setup. Less arch-specific code makes it easier to add
new features and maintain the start-up code.

Drop the unneeded code and adjust the hooks in board_f.c to cope.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/x86/cpu/cpu.c   |  4 ++-
 arch/x86/cpu/start.S | 86 ++++++----------------------------------------------
 common/board_f.c     |  3 +-
 3 files changed, 15 insertions(+), 78 deletions(-)

diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index 4f57145..b2e0323 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -136,8 +136,10 @@ static void load_gdt(const u64 *boot_gdt, u16 num_entries)
 	asm volatile("lgdtl %0\n" : : "m" (gdt));
 }
 
-void setup_gdt(gd_t *new_gd, u64 *gdt_addr)
+void arch_setup_gdt(gd_t *new_gd)
 {
+	u64 *gdt_addr;
+
 	gdt_addr = new_gd->arch.gdt;
 
 	/* CS: code, read/execute, 4 GB, base 0 */
diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S
index ac711ed..d93bdfc 100644
--- a/arch/x86/cpu/start.S
+++ b/arch/x86/cpu/start.S
@@ -104,8 +104,7 @@ car_init_ret:
 	 *
 	 * top-> CONFIG_SYS_CAR_ADDR + CONFIG_SYS_CAR_SIZE
 	 *	MRC area
-	 *	global_data
-	 *	x86 global descriptor table
+	 *	global_data with x86 global descriptor table
 	 *	early malloc area
 	 *	stack
 	 * bottom-> CONFIG_SYS_CAR_ADDR
@@ -120,13 +119,10 @@ car_init_ret:
 	 * and esi holds the HOB list address returned by the FSP.
 	 */
 #endif
-
-	/* Reserve space on stack for global data */
-	subl	$GENERATED_GBL_DATA_SIZE, %esp
-
-	/* Align global data to 16-byte boundary */
-	andl	$0xfffffff0, %esp
-	post_code(POST_START_STACK)
+	/* Set up global data */
+	mov	%esp, %eax
+	call	board_init_f_mem
+	mov	%eax, %esp
 
 	/*
 	 * Debug UART is available here although it may not be plumbed out
@@ -137,19 +133,12 @@ car_init_ret:
 	 * call  printch
 	 */
 
-	/* Zero the global data since it won't happen later */
-	xorl	%eax, %eax
-	movl	$GENERATED_GBL_DATA_SIZE, %ecx
-	movl	%esp, %edi
-	rep	stosb
-
 #ifdef CONFIG_HAVE_FSP
+	/* Store the HOB list if we have one */
 	test	%esi, %esi
 	jz	skip_hob
 
-	/* Store HOB list */
-	movl	%esp, %edx
-	addl	$GD_HOB_LIST, %edx
+fs	addl	$GD_HOB_LIST, %edx
 	movl	%esi, (%edx)
 
 skip_hob:
@@ -159,35 +148,10 @@ skip_hob:
 	addl	$GD_TABLE, %edx
 	movl	%esi, (%edx)
 #endif
-
-	/* Setup first parameter to setup_gdt, pointer to global_data */
-	movl	%esp, %eax
-
-	/* Reserve space for global descriptor table */
-	subl	$X86_GDT_SIZE, %esp
-
-	/* Align temporary global descriptor table to 16-byte boundary */
-	andl	$0xfffffff0, %esp
-	movl	%esp, %ecx
-
-#if defined(CONFIG_SYS_MALLOC_F_LEN)
-	/* Set up the pre-relocation malloc pool */
-	subl	$CONFIG_SYS_MALLOC_F_LEN, %esp
-	movl	%eax, %edx
-	addl	$GD_MALLOC_BASE, %edx
-	movl	%esp, (%edx)
-#endif
-	/* Store BIST into global_data */
-	movl	%eax, %edx
-	addl	$GD_BIST, %edx
+	/* Store BIST */
+fs	addl	$GD_BIST, %edx
 	movl	%ebp, (%edx)
 
-	/* Set second parameter to setup_gdt() */
-	movl	%ecx, %edx
-
-	/* Setup global descriptor table so gd->xyz works */
-	call	setup_gdt
-
 	/* Set parameter to board_init_f() to boot flags */
 	post_code(POST_START_DONE)
 	xorl	%eax, %eax
@@ -213,37 +177,7 @@ board_init_f_r_trampoline:
 	/* Stack grows down from top of SDRAM */
 	movl	%eax, %esp
 
-	/* Reserve space on stack for global data */
-	subl	$GENERATED_GBL_DATA_SIZE, %esp
-
-	/* Align global data to 16-byte boundary */
-	andl	$0xfffffff0, %esp
-
-	/* Setup first parameter to memcpy() and setup_gdt() */
-	movl	%esp, %eax
-
-	/* Setup second parameter to memcpy() */
-	fs movl 0, %edx
-
-	/* Set third parameter to memcpy() */
-	movl	$GENERATED_GBL_DATA_SIZE, %ecx
-
-	/* Copy global data from CAR to SDRAM stack */
-	call	memcpy
-
-	/* Reserve space for global descriptor table */
-	subl	$X86_GDT_SIZE, %esp
-
-	/* Align global descriptor table to 16-byte boundary */
-	andl	$0xfffffff0, %esp
-
-	/* Set second parameter to setup_gdt() */
-	movl	%esp, %edx
-
-	/* Setup global descriptor table so gd->xyz works */
-	call	setup_gdt
-
-	/* Set if we need to disable CAR */
+	/* See if we need to disable CAR */
 .weak	car_uninit
 	movl	$car_uninit, %eax
 	cmpl	$0, %eax
diff --git a/common/board_f.c b/common/board_f.c
index 3d1f305..ec9d2a8 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -711,6 +711,7 @@ static int jump_to_copy(void)
 	 * with the stack in SDRAM and Global Data in temporary memory
 	 * (CPU cache)
 	 */
+	arch_setup_gdt(gd->new_gd);
 	board_init_f_r_trampoline(gd->start_addr_sp);
 #else
 	relocate_code(gd->start_addr_sp, gd->new_gd, gd->relocaddr);
@@ -1028,6 +1029,7 @@ __weak void arch_setup_gdt(struct global_data *gd_ptr)
 {
 	gd = gd_ptr;
 }
+#endif /* !CONFIG_X86 */
 
 ulong board_init_f_mem(ulong top, ulong reserve_size)
 {
@@ -1049,4 +1051,3 @@ ulong board_init_f_mem(ulong top, ulong reserve_size)
 
 	return top;
 }
-#endif /* !CONFIG_X86 */
-- 
2.5.0.rc2.392.g76e840b

[U-Boot] [PATCH 5/5] x86: Switch to using generic global_data setup

[Bin Meng]

Hi Simon,

On Mon, Aug 3, 2015 at 8:10 AM, Simon Glass <sjg@chromium.org> wrote:
> There is quite a bit of assembler code that can be removed if we use the
> generic global_data setup. Less arch-specific code makes it easier to add
> new features and maintain the start-up code.
>
> Drop the unneeded code and adjust the hooks in board_f.c to cope.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  arch/x86/cpu/cpu.c   |  4 ++-
>  arch/x86/cpu/start.S | 86 ++++++----------------------------------------------
>  common/board_f.c     |  3 +-
>  3 files changed, 15 insertions(+), 78 deletions(-)
>
> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
> index 4f57145..b2e0323 100644
> --- a/arch/x86/cpu/cpu.c
> +++ b/arch/x86/cpu/cpu.c
> @@ -136,8 +136,10 @@ static void load_gdt(const u64 *boot_gdt, u16 num_entries)
>         asm volatile("lgdtl %0\n" : : "m" (gdt));
>  }
>
> -void setup_gdt(gd_t *new_gd, u64 *gdt_addr)
> +void arch_setup_gdt(gd_t *new_gd)
>  {
> +       u64 *gdt_addr;
> +
>         gdt_addr = new_gd->arch.gdt;
>
>         /* CS: code, read/execute, 4 GB, base 0 */
> diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S
> index ac711ed..d93bdfc 100644
> --- a/arch/x86/cpu/start.S
> +++ b/arch/x86/cpu/start.S
> @@ -104,8 +104,7 @@ car_init_ret:
>          *
>          * top-> CONFIG_SYS_CAR_ADDR + CONFIG_SYS_CAR_SIZE
>          *      MRC area
> -        *      global_data
> -        *      x86 global descriptor table
> +        *      global_data with x86 global descriptor table
>          *      early malloc area
>          *      stack
>          * bottom-> CONFIG_SYS_CAR_ADDR
> @@ -120,13 +119,10 @@ car_init_ret:
>          * and esi holds the HOB list address returned by the FSP.
>          */
>  #endif
> -
> -       /* Reserve space on stack for global data */
> -       subl    $GENERATED_GBL_DATA_SIZE, %esp
> -
> -       /* Align global data to 16-byte boundary */
> -       andl    $0xfffffff0, %esp
> -       post_code(POST_START_STACK)
> +       /* Set up global data */
> +       mov     %esp, %eax
> +       call    board_init_f_mem
> +       mov     %eax, %esp
>
>         /*
>          * Debug UART is available here although it may not be plumbed out
> @@ -137,19 +133,12 @@ car_init_ret:
>          * call  printch
>          */
>
> -       /* Zero the global data since it won't happen later */
> -       xorl    %eax, %eax
> -       movl    $GENERATED_GBL_DATA_SIZE, %ecx
> -       movl    %esp, %edi
> -       rep     stosb
> -
>  #ifdef CONFIG_HAVE_FSP
> +       /* Store the HOB list if we have one */
>         test    %esi, %esi
>         jz      skip_hob
>
> -       /* Store HOB list */
> -       movl    %esp, %edx
> -       addl    $GD_HOB_LIST, %edx
> +fs     addl    $GD_HOB_LIST, %edx

I don't see %edx is initialized anywhere?

>         movl    %esi, (%edx)
>
>  skip_hob:
> @@ -159,35 +148,10 @@ skip_hob:
>         addl    $GD_TABLE, %edx
>         movl    %esi, (%edx)
>  #endif
> -
> -       /* Setup first parameter to setup_gdt, pointer to global_data */
> -       movl    %esp, %eax
> -
> -       /* Reserve space for global descriptor table */
> -       subl    $X86_GDT_SIZE, %esp
> -
> -       /* Align temporary global descriptor table to 16-byte boundary */
> -       andl    $0xfffffff0, %esp
> -       movl    %esp, %ecx
> -
> -#if defined(CONFIG_SYS_MALLOC_F_LEN)
> -       /* Set up the pre-relocation malloc pool */
> -       subl    $CONFIG_SYS_MALLOC_F_LEN, %esp
> -       movl    %eax, %edx
> -       addl    $GD_MALLOC_BASE, %edx
> -       movl    %esp, (%edx)
> -#endif
> -       /* Store BIST into global_data */
> -       movl    %eax, %edx
> -       addl    $GD_BIST, %edx
> +       /* Store BIST */
> +fs     addl    $GD_BIST, %edx

Ditto.

>         movl    %ebp, (%edx)
>
> -       /* Set second parameter to setup_gdt() */
> -       movl    %ecx, %edx
> -
> -       /* Setup global descriptor table so gd->xyz works */
> -       call    setup_gdt
> -
>         /* Set parameter to board_init_f() to boot flags */
>         post_code(POST_START_DONE)
>         xorl    %eax, %eax
> @@ -213,37 +177,7 @@ board_init_f_r_trampoline:
>         /* Stack grows down from top of SDRAM */
>         movl    %eax, %esp
>
> -       /* Reserve space on stack for global data */
> -       subl    $GENERATED_GBL_DATA_SIZE, %esp
> -
> -       /* Align global data to 16-byte boundary */
> -       andl    $0xfffffff0, %esp
> -
> -       /* Setup first parameter to memcpy() and setup_gdt() */
> -       movl    %esp, %eax
> -
> -       /* Setup second parameter to memcpy() */
> -       fs movl 0, %edx
> -
> -       /* Set third parameter to memcpy() */
> -       movl    $GENERATED_GBL_DATA_SIZE, %ecx
> -
> -       /* Copy global data from CAR to SDRAM stack */
> -       call    memcpy
> -
> -       /* Reserve space for global descriptor table */
> -       subl    $X86_GDT_SIZE, %esp
> -
> -       /* Align global descriptor table to 16-byte boundary */
> -       andl    $0xfffffff0, %esp
> -
> -       /* Set second parameter to setup_gdt() */
> -       movl    %esp, %edx
> -
> -       /* Setup global descriptor table so gd->xyz works */
> -       call    setup_gdt
> -
> -       /* Set if we need to disable CAR */
> +       /* See if we need to disable CAR */
>  .weak  car_uninit
>         movl    $car_uninit, %eax
>         cmpl    $0, %eax
> diff --git a/common/board_f.c b/common/board_f.c
> index 3d1f305..ec9d2a8 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -711,6 +711,7 @@ static int jump_to_copy(void)
>          * with the stack in SDRAM and Global Data in temporary memory
>          * (CPU cache)
>          */
> +       arch_setup_gdt(gd->new_gd);
>         board_init_f_r_trampoline(gd->start_addr_sp);
>  #else
>         relocate_code(gd->start_addr_sp, gd->new_gd, gd->relocaddr);
> @@ -1028,6 +1029,7 @@ __weak void arch_setup_gdt(struct global_data *gd_ptr)
>  {
>         gd = gd_ptr;
>  }
> +#endif /* !CONFIG_X86 */
>
>  ulong board_init_f_mem(ulong top, ulong reserve_size)
>  {
> @@ -1049,4 +1051,3 @@ ulong board_init_f_mem(ulong top, ulong reserve_size)
>
>         return top;
>  }
> -#endif /* !CONFIG_X86 */

Can we remove the CONFIG_X86 around arch_set_gdt() given it is a
__weak and any arch can implement their own?

> --

Regards,
Bin

[U-Boot] [PATCH 5/5] x86: Switch to using generic global_data setup

[Simon Glass]

Hi Bin,

On 6 August 2015 at 01:16, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Mon, Aug 3, 2015 at 8:10 AM, Simon Glass <sjg@chromium.org> wrote:
>> There is quite a bit of assembler code that can be removed if we use the
>> generic global_data setup. Less arch-specific code makes it easier to add
>> new features and maintain the start-up code.
>>
>> Drop the unneeded code and adjust the hooks in board_f.c to cope.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  arch/x86/cpu/cpu.c   |  4 ++-
>>  arch/x86/cpu/start.S | 86 ++++++----------------------------------------------
>>  common/board_f.c     |  3 +-
>>  3 files changed, 15 insertions(+), 78 deletions(-)
>>
>> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
>> index 4f57145..b2e0323 100644
>> --- a/arch/x86/cpu/cpu.c
>> +++ b/arch/x86/cpu/cpu.c
>> @@ -136,8 +136,10 @@ static void load_gdt(const u64 *boot_gdt, u16 num_entries)
>>         asm volatile("lgdtl %0\n" : : "m" (gdt));
>>  }
>>
>> -void setup_gdt(gd_t *new_gd, u64 *gdt_addr)
>> +void arch_setup_gdt(gd_t *new_gd)
>>  {
>> +       u64 *gdt_addr;
>> +
>>         gdt_addr = new_gd->arch.gdt;
>>
>>         /* CS: code, read/execute, 4 GB, base 0 */
>> diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S
>> index ac711ed..d93bdfc 100644
>> --- a/arch/x86/cpu/start.S
>> +++ b/arch/x86/cpu/start.S
>> @@ -104,8 +104,7 @@ car_init_ret:
>>          *
>>          * top-> CONFIG_SYS_CAR_ADDR + CONFIG_SYS_CAR_SIZE
>>          *      MRC area
>> -        *      global_data
>> -        *      x86 global descriptor table
>> +        *      global_data with x86 global descriptor table
>>          *      early malloc area
>>          *      stack
>>          * bottom-> CONFIG_SYS_CAR_ADDR
>> @@ -120,13 +119,10 @@ car_init_ret:
>>          * and esi holds the HOB list address returned by the FSP.
>>          */
>>  #endif
>> -
>> -       /* Reserve space on stack for global data */
>> -       subl    $GENERATED_GBL_DATA_SIZE, %esp
>> -
>> -       /* Align global data to 16-byte boundary */
>> -       andl    $0xfffffff0, %esp
>> -       post_code(POST_START_STACK)
>> +       /* Set up global data */
>> +       mov     %esp, %eax
>> +       call    board_init_f_mem
>> +       mov     %eax, %esp
>>
>>         /*
>>          * Debug UART is available here although it may not be plumbed out
>> @@ -137,19 +133,12 @@ car_init_ret:
>>          * call  printch
>>          */
>>
>> -       /* Zero the global data since it won't happen later */
>> -       xorl    %eax, %eax
>> -       movl    $GENERATED_GBL_DATA_SIZE, %ecx
>> -       movl    %esp, %edi
>> -       rep     stosb
>> -
>>  #ifdef CONFIG_HAVE_FSP
>> +       /* Store the HOB list if we have one */
>>         test    %esi, %esi
>>         jz      skip_hob
>>
>> -       /* Store HOB list */
>> -       movl    %esp, %edx
>> -       addl    $GD_HOB_LIST, %edx
>> +fs     addl    $GD_HOB_LIST, %edx
>
> I don't see %edx is initialized anywhere?

What I really want to do is:

movl fs:GD_HOB_LIST, %edx

That doesn't seem to do the right thing, although I'm still testing
fiddling with it.

>
>>         movl    %esi, (%edx)
>>
>>  skip_hob:
>> @@ -159,35 +148,10 @@ skip_hob:
>>         addl    $GD_TABLE, %edx
>>         movl    %esi, (%edx)
>>  #endif
>> -
>> -       /* Setup first parameter to setup_gdt, pointer to global_data */
>> -       movl    %esp, %eax
>> -
>> -       /* Reserve space for global descriptor table */
>> -       subl    $X86_GDT_SIZE, %esp
>> -
>> -       /* Align temporary global descriptor table to 16-byte boundary */
>> -       andl    $0xfffffff0, %esp
>> -       movl    %esp, %ecx
>> -
>> -#if defined(CONFIG_SYS_MALLOC_F_LEN)
>> -       /* Set up the pre-relocation malloc pool */
>> -       subl    $CONFIG_SYS_MALLOC_F_LEN, %esp
>> -       movl    %eax, %edx
>> -       addl    $GD_MALLOC_BASE, %edx
>> -       movl    %esp, (%edx)
>> -#endif
>> -       /* Store BIST into global_data */
>> -       movl    %eax, %edx
>> -       addl    $GD_BIST, %edx
>> +       /* Store BIST */
>> +fs     addl    $GD_BIST, %edx
>
> Ditto.
>
>>         movl    %ebp, (%edx)
>>
>> -       /* Set second parameter to setup_gdt() */
>> -       movl    %ecx, %edx
>> -
>> -       /* Setup global descriptor table so gd->xyz works */
>> -       call    setup_gdt
>> -
>>         /* Set parameter to board_init_f() to boot flags */
>>         post_code(POST_START_DONE)
>>         xorl    %eax, %eax
>> @@ -213,37 +177,7 @@ board_init_f_r_trampoline:
>>         /* Stack grows down from top of SDRAM */
>>         movl    %eax, %esp
>>
>> -       /* Reserve space on stack for global data */
>> -       subl    $GENERATED_GBL_DATA_SIZE, %esp
>> -
>> -       /* Align global data to 16-byte boundary */
>> -       andl    $0xfffffff0, %esp
>> -
>> -       /* Setup first parameter to memcpy() and setup_gdt() */
>> -       movl    %esp, %eax
>> -
>> -       /* Setup second parameter to memcpy() */
>> -       fs movl 0, %edx
>> -
>> -       /* Set third parameter to memcpy() */
>> -       movl    $GENERATED_GBL_DATA_SIZE, %ecx
>> -
>> -       /* Copy global data from CAR to SDRAM stack */
>> -       call    memcpy
>> -
>> -       /* Reserve space for global descriptor table */
>> -       subl    $X86_GDT_SIZE, %esp
>> -
>> -       /* Align global descriptor table to 16-byte boundary */
>> -       andl    $0xfffffff0, %esp
>> -
>> -       /* Set second parameter to setup_gdt() */
>> -       movl    %esp, %edx
>> -
>> -       /* Setup global descriptor table so gd->xyz works */
>> -       call    setup_gdt
>> -
>> -       /* Set if we need to disable CAR */
>> +       /* See if we need to disable CAR */
>>  .weak  car_uninit
>>         movl    $car_uninit, %eax
>>         cmpl    $0, %eax
>> diff --git a/common/board_f.c b/common/board_f.c
>> index 3d1f305..ec9d2a8 100644
>> --- a/common/board_f.c
>> +++ b/common/board_f.c
>> @@ -711,6 +711,7 @@ static int jump_to_copy(void)
>>          * with the stack in SDRAM and Global Data in temporary memory
>>          * (CPU cache)
>>          */
>> +       arch_setup_gdt(gd->new_gd);
>>         board_init_f_r_trampoline(gd->start_addr_sp);
>>  #else
>>         relocate_code(gd->start_addr_sp, gd->new_gd, gd->relocaddr);
>> @@ -1028,6 +1029,7 @@ __weak void arch_setup_gdt(struct global_data *gd_ptr)
>>  {
>>         gd = gd_ptr;
>>  }
>> +#endif /* !CONFIG_X86 */
>>
>>  ulong board_init_f_mem(ulong top, ulong reserve_size)
>>  {
>> @@ -1049,4 +1051,3 @@ ulong board_init_f_mem(ulong top, ulong reserve_size)
>>
>>         return top;
>>  }
>> -#endif /* !CONFIG_X86 */
>
> Can we remove the CONFIG_X86 around arch_set_gdt() given it is a
> __weak and any arch can implement their own?

Unfortunately I get a compile error when assigning to gd - it is not a
register like on ARM, etc.

Regards,
Simon