From mboxrd@z Thu Jan  1 00:00:00 1970
From: Graeme Russ <graeme.russ@gmail.com>
Date: Sun, 18 Nov 2012 12:07:44 +1100
Subject: [U-Boot] [PATCH 13/57] x86: Move new_gd_addr to arch_global_data
In-Reply-To: <1353100842-20126-14-git-send-email-sjg@chromium.org>
References: <1353100842-20126-1-git-send-email-sjg@chromium.org>
	<1353100842-20126-14-git-send-email-sjg@chromium.org>
Message-ID: <50A834E0.8000506@gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Simon,

On 11/17/2012 08:19 AM, Simon Glass wrote:
> Move this field into arch_global_data and tidy up.
> 
> This will certainly break x86, so will need Graeme's help to sort out

Yes, it most certainly will break x86 :)

> the problem. I would prefer not to put the architecture-specific stuff
> at the top of global_data since we relying on that seems even more ugly.

The fix is not that hard though...

The whole point of putting gdt_addr at the top of the global data structure
is to guarantee that is is the very fist void * in gd. The trick is how we
use the 'F' segment. By loading the fs register with the physical address
of gd, virtual address 0 of fs contains the address of gd.

But really, we can put the address of gd anywhere, as long as we set fs to
be that address

> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  arch/x86/cpu/cpu.c                 |    2 +-
>  arch/x86/include/asm/global_data.h |   12 +++++++++---
>  arch/x86/lib/init_helpers.c        |    2 +-
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
> index e9bb0d7..c276aa6 100644
> --- a/arch/x86/cpu/cpu.c
> +++ b/arch/x86/cpu/cpu.c
> @@ -92,7 +92,7 @@ static void load_gdt(const u64 *boot_gdt, u16 num_entries)
>  
>  void init_gd(gd_t *id, u64 *gdt_addr)
>  {
> -	id->gd_addr = (ulong)id;
> +	id->arch.gd_addr = (ulong)id;
>  	setup_gdt(id, gdt_addr);

If the original code had been:

setup_gdt(&(id->gd_addr), gdt_addr);

There would have been no reliance on gd_addr being the first member of gd.
So change this to:

setup_gdt(&(id->arch.gd_addr), gdt_addr);

And you should be pretty much set - and gd_addr can be anywhere in the arch gd.

>  }
>  
> diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h
> index 3df83bb..d2eb00a 100644
> --- a/arch/x86/include/asm/global_data.h
> +++ b/arch/x86/include/asm/global_data.h
> @@ -28,8 +28,16 @@
>  
>  /* Architecture-specific global data */
>  struct arch_global_data {
> -	unsigned long gdt_addr;		/* Location of GDT */
> +	/*
> +	 * NOTE: gd_addr MUST be first member of struct global_data!
> +	 *
> +	 * But it now isn't, so this is sure to break x86. Can we change
> +	 * x86 to not require this? I don't think we should put the
> +	 * arch data first in global_data...
> +	 */

Yes we can - see above

>  	unsigned long new_gd_addr;	/* New location of Global Data */
> +	unsigned long gd_addr;		/* Location of Global Data */
> +	unsigned long gdt_addr;		/* Location of GDT */
>  };
>  
>  /*
> @@ -41,8 +49,6 @@ struct arch_global_data {
>   */
>  
>  typedef	struct global_data {
> -	/* NOTE: gd_addr MUST be first member of struct global_data! */
> -	unsigned long	gd_addr;	/* Location of Global Data */
>  	bd_t		*bd;
>  	unsigned long	flags;
>  	unsigned int	baudrate;
> diff --git a/arch/x86/lib/init_helpers.c b/arch/x86/lib/init_helpers.c
> index 05cadcd..ac789c2 100644
> --- a/arch/x86/lib/init_helpers.c
> +++ b/arch/x86/lib/init_helpers.c
> @@ -126,7 +126,7 @@ int copy_gd_to_ram_f_r(void)
>  	 * in-RAM copy of Global Data (calculate_relocation_address()
>  	 * has already calculated the in-RAM location of the GDT)
>  	 */
> -	ram_gd->gd_addr = (ulong)ram_gd;
> +	ram_gd->arch.gd_addr = (ulong)ram_gd;
>  	init_gd(ram_gd, (u64 *)gd->arch.gdt_addr);
>  
>  	return 0;
> 

Regards,

Graeme