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

[Simon Glass]

Hi,

On 29 June 2015 at 11:57, Simon Glass <sjg at chromium.org> wrote:
> 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.

Any thoughts on this? I don't actually have a newer board to test
with. But I suspect that no one has both an old and a new board.

Regards,
Simon