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

[Bin Meng]

Hi Simon,

On Thu, Aug 6, 2015 at 10:02 PM, Simon Glass <sjg at chromium.org> wrote:
> 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.
>

Actually I was working on a patch series for this but suspended. I
will rebase and send out the series tomorrow.

Regards,
Bin