Hi Graeme,
On Sat, Nov 17, 2012 at 5:07 PM, Graeme Russ <graeme.r...@gmail.com> wrote:
> 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 <s...@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.
Thanks for that. I actually understand it now, for better or worse. It
seems to work fine. Actually your clean-up of the global_data really
has paid dividends as things are so much nicer now.
I'm going to resend the whole series rebased to master.
Regards,
Simon
>
>> }
>>
>> 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
