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

[Simon Glass]

Hi Bin,

On 5 June 2015 at 19:30, Bin Meng <bmeng.cn at gmail.com> wrote:
> 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'd prefer to keep it in the device tree if we can. It is easier to
read. For non-FSP targets this works fine and I live in hope that
Intel might improve the FSP interface such that the single microcode
blob is not necessary.

>
>> 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.

That's really terrible - we can barely even start the CPU without a
microcode update.

>
>> 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,
Simon