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

[Simon Glass]

Hi Andrew,

On 29 June 2015 at 08:36, Andrew Bradford <andrew at bradfordembedded.com> wrote:
> Hi Simon,
>
> On 06/07 18:58, Simon Glass wrote:
>> 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.
>
> Are you saying that then for a board that could have more than one
> microcode used that the device tree for said board would actually list
> two different dtsi microcode files and then have a handler that is able
> to pass both to the FSP?
>
> I don't get the impression that Intel is going to improve the FSP for
> Baytrail at this point.
>
> I'd like to get Bin's microcode patch included into u-boot so it can be
> used with newer stepping E3800 parts, what's the best way I can help?
>

My understanding is that you can pass several microcode blobs to the
FSP and it will load the correct one (as Bin says above). If that is
untrue then this will not work.

My preference is that we modify ifdtool to support collecting the
microcode blobs together and put them into a single place in the ROM.
This could be run from the Makefile as another ifdtool step. It would
find the various microcode nodes, extract each blob of binary data,
pack them together and put them in a specified place in ROM.

Regards,
Simon