[MinnowBoard] Intel FSP Feedback

[Simon Glass]

Hi,

Here are a few thoughts from using the FSP to get U-Boot running on
the Minnowmax. The thoughts are in order of what I think is greatest
to least importance. I hope this is useful. It is not intended as
criticism of something which clearly works. But since the FSP claims
to be 'easily incorporated into many existing boot loaders' I think it
could do better.

1. FspInitEntry should do less. The current way it works makes it
impossible for a boot loader to control execution, meaning that the
FSP cannot be nicely integrated. For example, in U-Boot we turn on the
CAR, start up the serial console and print some things before we start
up DRAM. We then do a few more things before relocating U-Boot and
jumping to it. Having the DRAM init force you into a new stack is a
major pain and breaks the U-Boot init sequence. We either need to
start it again, or do the DRAM init really early when serial is not
available. This makes debugging hard.

Suggestion:
FspInitCpu() - sets up the CPU the way you want it (should not require
DRAM to be running)
FspInitRam() - sets up DRAM and returns information about it
FspInitChip() - sets up the SoC peripherals, maybe this requires DRAM?
FspDisableCar() - turn off CAR

Then I can do things 'properly':

- enable CAR
- init the CPU early but perhaps delay things like MMC, etc, until I
have had a chance to decide what should be inited
- display serial output, including DRAM message
- init DRAM and show size
- continue init (on same stack in CAR)
- chosen where to put my stack in DRAM, based on how much there is
- relocate U-Boot to DRAM and jump to it
- continue post-relocation init, etc.

At present I have to use one of these non-ideal sequences:

- enable CAR
- select a random place for the stack in DRAM
- enable DRAM
- start the pre-relocation init sequence (i.e. much later than I
should, pre-serial)
- (stack ends up being in three different places)

or (and I can't make this one work):

- enable CAR
- start the init sequence, display serial output
- init DRAM
- restart the init sequence on the new stack, somehow suppressing the
serial output up until the 'DRAM' message
- run with the stack in DRAM the second time, when really U-Boot
expects it to still be in CAR

I doubt that the approach of merging CPU init, DRAM init and CAR
disable suits any boot loader. It doesn't look like it suits Coreboot
either. But the FSP should act like a library of simple C functions,
and avoid messing with the execution environment to the extent
possible.

2. The FSP should not require any particular GDT, and should allow the
GDT to be in CAR or ROM. This would work as a side-effect of (1) I
think.

3. The CAR init function (TempRamInitEntry) reqiures a stack in ROM,
making it impossible to select the return address at run-time. I think
it would be better to pass the four parameters and the return address
in registers. That way I can go off and search for the microcode,
rather than needing a separate build-time tool which finds the
microcode and builds the table that TempRamInitEntry needs.

4. The things in UPD_DATA_REGION should be fully documented in the FSP guide.

5. The FSP should return an error if something goes wrong, not crash.

6. Allow microcode updates to be applied one at a time, so we don't
have to stuff them all into one big binary blob. This could be another
API call.

7. Even if there is no space for full serial debug, some sort of
mechanism should be provided (e.g. limited serial output, or even some
hex output). At present it is a total black box.

8. Simplify things. Multiple firmware 'files', multiple binaries
blobs, multiple tools to configure and relocate the binary blobs
(including a few Windows program with an incomplete command-line
implementation and a GUI that just crashes on my Ubuntu box)., 64-bit
GUIDs to identify an image.. it's hard to deal with.

Having said all that, I did get it working, including the graphics
console and most peripherals which is nice. I wonder if there is a
'native' way of doing all this init that doesn't require the FSPs?

Regards,
Simon