[U-Boot] x86 FSP - delayed SDRAM init?

[Simon Glass]

Hi Bin,

On 21 January 2015 at 21:45, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Thu, Jan 22, 2015 at 11:42 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Bin,
>>
>> In the Baytrail FSP docs I see a note about the HOB passing back the
>> 'Boot Loader Temporary Memory Data HOB'. This seems to be a copy of
>> the entire temporary memory space. I wonder if we could recover struct
>> global_data from this?
>>
>
> Yes, I think so. And I have verified this temporary memory space
> indeed contains stack contents before fsp_init() from U-Boot shell on
> Crown Bay. But the overall process might be complicated. See below.
>
>> If so, then we could move the fsp_init stuff to dram_init(), perhaps?
>>
>> But perhaps this is a feature of only this FSP?
>>
>
> I believe this is a feature defined by the FSP architecture spec, so
> every FSP should support that.
>
> Technically it should be no problem to call fsp_init() from
> arch_cpu_init() or even later dram_init(). However, I would say doing
> so brings us more harm than good. The following points are what I
> thought about before:
>
> 1). fsp_init() takes one parameter 'stack_top' to setup another stack
> after DRAM is initialized. This means everything on the previous CAR
> stack will need to migrate to the new stack below 'stack_top'. This
> includes global data, early malloc pointers, arch_cpu_init() stack
> variables and its return address.
> 2). Copy previous global_data to the new places under stack_top, and
> fix up gd->arch.gd_addr.
> 3). The initf_malloc() is called before arch_cpu_init() so we need fix
> up the early malloc pointers manually (gd->malloc_base and
> gd->malloc_limit)
> 4). Fix up the stack variables and return address of arch_cpu_init()
> on the new stack.
> 5). On Tunnel Creek, if we call setup_gdt() in start.S, later
> fsp_init() in arch_cpu_init() will fail to bring up the thread 1
> (Tunnel Creek supports SMT), the reason of which is unknown to me yet
> (FSP is a black box). It looks to me that FSP is assuming GDT only
> contains two entries (32-bit 4G flat address) before calling
> fsp_init().
>
> I have not looked into this any further, so the above points might not
> be 100% right. I would say with these modifications, the codes are
> more difficult to understand.

I don't disagree with any of this, but I've had a bit more of a look.

How about something awful like this:

- start.S
- do the initram thing
- call board_init_f()
- there is no hob pointer, so CPU init does very little
- it runs only to dram_init()
- dram_init() calls the fsp_init() and ends up back at another function
- that function sets up global_data again, calls board_init_f() again
   - passes a boot flag to suppress the banner the second time
- this time there is a hob pointer, so CPU init can complete
- things boot normally

It is hideous. The question is which way is worse.

What we really need is to split fsp_init() into two parts:

1. Set up DRAM
2. Turn off CAR (called when WE decide we no-longer need it)

Then we could make this work as with the non-FSP mode.

By joining the two, they have made it even harder than it should be.
Of course, no one wins with binary blobs, but the 'continuation
function' should have been a red flag when this design was done at
Intel.

BTW for me on minnowmax fsp_init() hangs. Since it is a binary blob I
am not sure how to debug it. There is no error printed / returned. I'm
not even sure how to make it output debug info. No post codes are
available. Sigh.

Regards,
Simon