[U-Boot] [PATCH] x86: WIP: Enable D0 stepping microcode for MinnowMax

[Bin Meng]

Hi Simon,

On Sat, Jun 6, 2015 at 12:17 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 4 June 2015 at 20:03, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Hi Simon,
>>
>> On Fri, Jun 5, 2015 at 2:31 AM, Simon Glass <sjg at chromium.org> wrote:
>>> Hi,
>>>
>>> On 4 June 2015 at 10:27, Andrew Bradford <andrew at bradfordembedded.com> wrote:
>>>>
>>>> Hi Bin,
>>>>
>>>> On 06/04 22:21, Bin Meng wrote:
>>>> > Hi Simon,
>>>> >
>>>> > On Thu, Jun 4, 2015 at 8:12 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>> > > This is a temparory hacking for testing U-Boot on a newer version
>>>> > > MinnowMax board.
>>>> > >
>>>> > > Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>>> > > ---
>>>> > >
>>>> > >  arch/x86/dts/minnowmax.dts | 2 +-
>>>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>>>> > >
>>>> > > diff --git a/arch/x86/dts/minnowmax.dts b/arch/x86/dts/minnowmax.dts
>>>> > > index 7103bc5..9e1fcfc 100644
>>>> > > --- a/arch/x86/dts/minnowmax.dts
>>>> > > +++ b/arch/x86/dts/minnowmax.dts
>>>> > > @@ -101,7 +101,7 @@
>>>> > >
>>>> > >         microcode {
>>>> > >                 update at 0 {
>>>> > > -#include "microcode/m0130673322.dtsi"
>>>> > > +#include "microcode/m0130679901.dtsi"
>>>> > >                 };
>>>> > >         };
>>>> > >
>>>> > > --
>>>> >
>>>> > Saket confirmed these two patches resolved the boot problem he saw. So
>>>> > we will need think about how to better support such scenario that
>>>> > different revision board with different stepping CPUs. Could be
>>>> > multiple microcodes in one U-Boot image, or still one microcode with
>>>> > some #ifdef #endif. Note FSP has the capability to accept multiple
>>>> > microcodes as parameters in the FspTempRamInit(), but right now U-Boot
>>>> > only specifies one. How do you think?
>>>>
>>>> Why not just have a minnowmax common dtsi and then top level dts files
>>>> for each revision of the board containing the ways they are different
>>>> (such as microcode)?
>>>
>>> My preference would be to include all the microcode needed by the
>>> board and then pass the correct one to the FSP. Now that we can access
>>> the device tree that should be possible. There is logic in
>>> ./arch/x86/cpu/ivybridge/microcode_intel.c to do this but it may need
>>> a bit of refactorng.
>>>
>>
>> The device tree still cannot be accessed in that early phase before
>> CAR is initialized. The issue is that FSP is designed to have
>> FspTempRamInit() to do both microcode loading and CAR initialization.
>>
>
> Ah yes of course. Most unfortunate.
>
> Maybe we could have ifdtool or the Makefile put all the microcode in a
> single big blob somewhere else in u-boot.rom so that the whole thing
> can be passed to the FSP. In that case I think the FSP will select the
> correct one

Maybe an easy way to handle this is to create a special microcode
section in u-boot.lds and just include all these header files in a
single array in the section. This does not need ifdtool or device tree
support.

> I also wonder if it is possible to load the microcode in our own code
> and pass nothing to the FSP? I think I tried that though and it hung.

Yes, according to FSP external architecture spec, the microcode is a
must. The reason is that on some processors without microcode update
the CAR initialization will fail.

> The FSP design leaves much to b desired.
>
>>> The existing microcode approach (with ifdtool adding a pointer to the
>>> microcode) is a work-around for the FSP problem. Now that Bin has
>>> solved this I'd be keen to remove it an just use device tree.
>>>

Regards,
Bin