Patchwork [U-Boot,03/10] x86: Permit bootstage and timer data to be used prior to relocation

login
register
mail settings
Submitter Simon Glass
Date Dec. 14, 2012, 9:13 p.m.
Message ID <1355519594-5694-3-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/206564/
State Superseded, archived
Delegated to: Simon Glass
Headers show

Comments

Simon Glass - Dec. 14, 2012, 9:13 p.m.
It is useful to be able to access the timer before U-Boot has relocated
so that we can fully support bootstage.

Move the relevant variables to the data region to support this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/x86/cpu/coreboot/coreboot.c |    4 ++--
 arch/x86/cpu/interrupts.c        |    2 +-
 arch/x86/lib/timer.c             |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
Graeme Russ - Dec. 14, 2012, 10:15 p.m.
Hi Simon,

On 15/12/12 08:13, Simon Glass wrote:
> It is useful to be able to access the timer before U-Boot has relocated
> so that we can fully support bootstage.
> 
> Move the relevant variables to the data region to support this.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  arch/x86/cpu/coreboot/coreboot.c |    4 ++--
>  arch/x86/cpu/interrupts.c        |    2 +-
>  arch/x86/lib/timer.c             |    2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c
> index 9c9431e..22474f5 100644
> --- a/arch/x86/cpu/coreboot/coreboot.c
> +++ b/arch/x86/cpu/coreboot/coreboot.c
> @@ -68,8 +68,8 @@ int board_early_init_r(void)
>  void show_boot_progress(int val)
>  {
>  #if MIN_PORT80_KCLOCKS_DELAY
> -	static uint32_t prev_stamp;
> -	static uint32_t base;
> +	static uint32_t prev_stamp __attribute__((section(".data")));
> +	static uint32_t base __attribute__((section(".data")));

NAK

This may work for coreboot where SDRAM is already initialised and you've
loaded U-Boot into RAM, but it will not work when U-Boot is in Flash as all
sections (including .data) are read-only until after relocation.

The stack and Global Data are the only guaranteed read/write locations
prior to relocation

Regards,

Graeme
Simon Glass - Dec. 14, 2012, 10:35 p.m.
Hi Graeme,

On Fri, Dec 14, 2012 at 2:15 PM, Graeme Russ <graeme.russ@gmail.com> wrote:
> Hi Simon,
>
> On 15/12/12 08:13, Simon Glass wrote:
>> It is useful to be able to access the timer before U-Boot has relocated
>> so that we can fully support bootstage.
>>
>> Move the relevant variables to the data region to support this.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>  arch/x86/cpu/coreboot/coreboot.c |    4 ++--
>>  arch/x86/cpu/interrupts.c        |    2 +-
>>  arch/x86/lib/timer.c             |    2 +-
>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c
>> index 9c9431e..22474f5 100644
>> --- a/arch/x86/cpu/coreboot/coreboot.c
>> +++ b/arch/x86/cpu/coreboot/coreboot.c
>> @@ -68,8 +68,8 @@ int board_early_init_r(void)
>>  void show_boot_progress(int val)
>>  {
>>  #if MIN_PORT80_KCLOCKS_DELAY
>> -     static uint32_t prev_stamp;
>> -     static uint32_t base;
>> +     static uint32_t prev_stamp __attribute__((section(".data")));
>> +     static uint32_t base __attribute__((section(".data")));
>
> NAK
>
> This may work for coreboot where SDRAM is already initialised and you've
> loaded U-Boot into RAM, but it will not work when U-Boot is in Flash as all
> sections (including .data) are read-only until after relocation.
>
> The stack and Global Data are the only guaranteed read/write locations
> prior to relocation

Ah yes, I was thinking of your comment that all boards would move to
use coreboot. If that is not the case, then I will need to add
something to global data for the timer. And I don't want to do that
while generic board is in progress since it creates conflicts.

Regards,
Simon

>
> Regards,
>
> Graeme
>
Simon Glass - Dec. 19, 2012, 1:32 a.m.
Hi Graeme,

On Fri, Dec 14, 2012 at 2:35 PM, Simon Glass <sjg@chromium.org> wrote:

> Hi Graeme,
>
> On Fri, Dec 14, 2012 at 2:15 PM, Graeme Russ <graeme.russ@gmail.com>
> wrote:
> > Hi Simon,
> >
> > On 15/12/12 08:13, Simon Glass wrote:
> >> It is useful to be able to access the timer before U-Boot has relocated
> >> so that we can fully support bootstage.
> >>
> >> Move the relevant variables to the data region to support this.
> >>
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >> ---
> >>  arch/x86/cpu/coreboot/coreboot.c |    4 ++--
> >>  arch/x86/cpu/interrupts.c        |    2 +-
> >>  arch/x86/lib/timer.c             |    2 +-
> >>  3 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/x86/cpu/coreboot/coreboot.c
> b/arch/x86/cpu/coreboot/coreboot.c
> >> index 9c9431e..22474f5 100644
> >> --- a/arch/x86/cpu/coreboot/coreboot.c
> >> +++ b/arch/x86/cpu/coreboot/coreboot.c
> >> @@ -68,8 +68,8 @@ int board_early_init_r(void)
> >>  void show_boot_progress(int val)
> >>  {
> >>  #if MIN_PORT80_KCLOCKS_DELAY
> >> -     static uint32_t prev_stamp;
> >> -     static uint32_t base;
> >> +     static uint32_t prev_stamp __attribute__((section(".data")));
> >> +     static uint32_t base __attribute__((section(".data")));
> >
> > NAK
> >
> > This may work for coreboot where SDRAM is already initialised and you've
> > loaded U-Boot into RAM, but it will not work when U-Boot is in Flash as
> all
> > sections (including .data) are read-only until after relocation.
> >
> > The stack and Global Data are the only guaranteed read/write locations
> > prior to relocation
>
> Ah yes, I was thinking of your comment that all boards would move to
> use coreboot. If that is not the case, then I will need to add
> something to global data for the timer. And I don't want to do that
> while generic board is in progress since it creates conflicts.
>

Any thoughts on this point, please? I presume at least I can use RAM once
dram_init() is called. But when running from coreboot the RAM is already
set up...

Regards,
Simon


>
> Regards,
> Simon
>
> >
> > Regards,
> >
> > Graeme
> >
>
Graeme Russ - Dec. 19, 2012, 9:39 p.m.
Hi Simon,

On Wed, Dec 19, 2012 at 12:32 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Graeme,
>
> On Fri, Dec 14, 2012 at 2:35 PM, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Graeme,
>>
>>
>> On Fri, Dec 14, 2012 at 2:15 PM, Graeme Russ <graeme.russ@gmail.com>
>> wrote:
>> > Hi Simon,
>> >
>> > On 15/12/12 08:13, Simon Glass wrote:
>> >> It is useful to be able to access the timer before U-Boot has relocated
>> >> so that we can fully support bootstage.
>> >>
>> >> Move the relevant variables to the data region to support this.
>> >>
>> >> Signed-off-by: Simon Glass <sjg@chromium.org>
>> >> ---
>> >>  arch/x86/cpu/coreboot/coreboot.c |    4 ++--
>> >>  arch/x86/cpu/interrupts.c        |    2 +-
>> >>  arch/x86/lib/timer.c             |    2 +-
>> >>  3 files changed, 4 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/arch/x86/cpu/coreboot/coreboot.c
>> >> b/arch/x86/cpu/coreboot/coreboot.c
>> >> index 9c9431e..22474f5 100644
>> >> --- a/arch/x86/cpu/coreboot/coreboot.c
>> >> +++ b/arch/x86/cpu/coreboot/coreboot.c
>> >> @@ -68,8 +68,8 @@ int board_early_init_r(void)
>> >>  void show_boot_progress(int val)
>> >>  {
>> >>  #if MIN_PORT80_KCLOCKS_DELAY
>> >> -     static uint32_t prev_stamp;
>> >> -     static uint32_t base;
>> >> +     static uint32_t prev_stamp __attribute__((section(".data")));
>> >> +     static uint32_t base __attribute__((section(".data")));
>> >
>> > NAK
>> >
>> > This may work for coreboot where SDRAM is already initialised and you've
>> > loaded U-Boot into RAM, but it will not work when U-Boot is in Flash as
>> > all
>> > sections (including .data) are read-only until after relocation.
>> >
>> > The stack and Global Data are the only guaranteed read/write locations
>> > prior to relocation
>>
>> Ah yes, I was thinking of your comment that all boards would move to
>> use coreboot. If that is not the case, then I will need to add
>> something to global data for the timer. And I don't want to do that
>> while generic board is in progress since it creates conflicts.

I'm trying to remember the context of my comment regarding only having
coreboot boards. But anyway, not having any writeable data prior to
relocation (other than the stack and gd) is a fundamental principle
that we cannot simply avoid (although I must admit I don't know what
the go is with SPL and U-Boot relocation).

> Any thoughts on this point, please? I presume at least I can use RAM once
> dram_init() is called. But when running from coreboot the RAM is already set
> up...

You will need to use global data to store the static variables. That's
what the other arches do

Regards,

Graeme
Simon Glass - Dec. 19, 2012, 10:01 p.m.
Hi Graeme,

On Wed, Dec 19, 2012 at 1:39 PM, Graeme Russ <graeme.russ@gmail.com> wrote:
> Hi Simon,
>
> On Wed, Dec 19, 2012 at 12:32 PM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Graeme,
>>
>> On Fri, Dec 14, 2012 at 2:35 PM, Simon Glass <sjg@chromium.org> wrote:
>>>
>>> Hi Graeme,
>>>
>>>
>>> On Fri, Dec 14, 2012 at 2:15 PM, Graeme Russ <graeme.russ@gmail.com>
>>> wrote:
>>> > Hi Simon,
>>> >
>>> > On 15/12/12 08:13, Simon Glass wrote:
>>> >> It is useful to be able to access the timer before U-Boot has relocated
>>> >> so that we can fully support bootstage.
>>> >>
>>> >> Move the relevant variables to the data region to support this.
>>> >>
>>> >> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> >> ---
>>> >>  arch/x86/cpu/coreboot/coreboot.c |    4 ++--
>>> >>  arch/x86/cpu/interrupts.c        |    2 +-
>>> >>  arch/x86/lib/timer.c             |    2 +-
>>> >>  3 files changed, 4 insertions(+), 4 deletions(-)
>>> >>
>>> >> diff --git a/arch/x86/cpu/coreboot/coreboot.c
>>> >> b/arch/x86/cpu/coreboot/coreboot.c
>>> >> index 9c9431e..22474f5 100644
>>> >> --- a/arch/x86/cpu/coreboot/coreboot.c
>>> >> +++ b/arch/x86/cpu/coreboot/coreboot.c
>>> >> @@ -68,8 +68,8 @@ int board_early_init_r(void)
>>> >>  void show_boot_progress(int val)
>>> >>  {
>>> >>  #if MIN_PORT80_KCLOCKS_DELAY
>>> >> -     static uint32_t prev_stamp;
>>> >> -     static uint32_t base;
>>> >> +     static uint32_t prev_stamp __attribute__((section(".data")));
>>> >> +     static uint32_t base __attribute__((section(".data")));
>>> >
>>> > NAK
>>> >
>>> > This may work for coreboot where SDRAM is already initialised and you've
>>> > loaded U-Boot into RAM, but it will not work when U-Boot is in Flash as
>>> > all
>>> > sections (including .data) are read-only until after relocation.
>>> >
>>> > The stack and Global Data are the only guaranteed read/write locations
>>> > prior to relocation
>>>
>>> Ah yes, I was thinking of your comment that all boards would move to
>>> use coreboot. If that is not the case, then I will need to add
>>> something to global data for the timer. And I don't want to do that
>>> while generic board is in progress since it creates conflicts.
>
> I'm trying to remember the context of my comment regarding only having
> coreboot boards. But anyway, not having any writeable data prior to
> relocation (other than the stack and gd) is a fundamental principle
> that we cannot simply avoid (although I must admit I don't know what
> the go is with SPL and U-Boot relocation).
>
>> Any thoughts on this point, please? I presume at least I can use RAM once
>> dram_init() is called. But when running from coreboot the RAM is already set
>> up...
>
> You will need to use global data to store the static variables. That's
> what the other arches do

That line is blurred a bit now, but OK I will do that. I will put this
series on hold until we see what happens with the arch-specific
global_data series.

Regards,
Simon

>
> Regards,
>
> Graeme

Patch

diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c
index 9c9431e..22474f5 100644
--- a/arch/x86/cpu/coreboot/coreboot.c
+++ b/arch/x86/cpu/coreboot/coreboot.c
@@ -68,8 +68,8 @@  int board_early_init_r(void)
 void show_boot_progress(int val)
 {
 #if MIN_PORT80_KCLOCKS_DELAY
-	static uint32_t prev_stamp;
-	static uint32_t base;
+	static uint32_t prev_stamp __attribute__((section(".data")));
+	static uint32_t base __attribute__((section(".data")));
 
 	/*
 	 * Scale the time counter reading to avoid using 64 bit arithmetics.
diff --git a/arch/x86/cpu/interrupts.c b/arch/x86/cpu/interrupts.c
index dd30a05..60ae950 100644
--- a/arch/x86/cpu/interrupts.c
+++ b/arch/x86/cpu/interrupts.c
@@ -626,7 +626,7 @@  asm(".globl irq_common_entry\n" \
  */
 u64 get_ticks(void)
 {
-	static u64 tick_base;
+	static u64 tick_base __attribute__((section(".data")));
 	u64 now_tick = rdtsc();
 
 	if (!tick_base)
diff --git a/arch/x86/lib/timer.c b/arch/x86/lib/timer.c
index a13424b..6303ca5 100644
--- a/arch/x86/lib/timer.c
+++ b/arch/x86/lib/timer.c
@@ -37,7 +37,7 @@  struct timer_isr_function {
 
 static struct timer_isr_function *first_timer_isr;
 static unsigned long system_ticks;
-static uint64_t base_value;
+static uint64_t base_value __attribute__((section(".data")));
 
 /*
  * register_timer_isr() allows multiple architecture and board specific