[U-Boot] [PATCH v2 15/27] x86: Integrate Tunnel Creek processor microcode

[Simon Glass]

Hi Bin,

On 11 December 2014 at 20:00, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Thu, Dec 11, 2014 at 11:50 PM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Bin,
>>
>> On 11 December 2014 at 08:01, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> Hi Simon,
>>>
>>> On Thu, Dec 11, 2014 at 10:34 PM, Simon Glass <sjg at chromium.org> wrote:
>>>> Hi Bin,
>>>>
>>>> On 9 December 2014 at 23:24, Simon Glass <sjg at chromium.org> wrote:
>>>>> Hi Bin.
>>>>>
>>>>>
>>>>> On 9 December 2014 at 23:17, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>>>> Hi Simon,
>>>>>>
>>>>>> On Wed, Dec 10, 2014 at 2:09 PM, Simon Glass <sjg at chromium.org> wrote:
>>>>>>> Hi Bin,
>>>>>>>
>>>>>>> On 9 December 2014 at 07:50, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>>>>>> Integrate the processor microcode version 1.05 for Tunnel Creek,
>>>>>>>> CPUID device 20661h.
>>>>>>>>
>>>>>>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Changes in v2: None
>>>>>>>
>>>>>>> We are going to end up with microcode both in the device tree and in
>>>>>>> an assembler include file.
>>>>>>>
>>>>>>> This seems unfortunate. I wonder if we can always put it in the device
>>>>>>> tree, but then optionally also compile it into the image using a
>>>>>>> symbol (as we do in dts/Makefile for CONFIG_OF_EMBED). This could be
>>>>>>> done with fdtget to read the binary output of the property perhaps. I
>>>>>>> haven't looked at it in detail though.
>>>>>>>
>>>>>>
>>>>>> This will do for the embedded case, but it won't help for the separate dtb case.
>>>>>>
>>>>>>> Maybe this is too clumsy but it would be good to have it in one
>>>>>>> format. Or maybe we should have a tool which can generate either
>>>>>>> format.
>>>>>>
>>>>>> I don't think we are able to parse device tree in that early phase
>>>>>> (before car_init), to get the microcode content from dtb. I know in
>>>>>> some Intel processors, the microcode update are required before CAR
>>>>>> initialization, as without the microcode update, the CAR
>>>>>> initialization will fail.
>>>>>
>>>>> Right, I meant that we could perhaps do this in the build system -
>>>>> i.e. compile in the microcode after reading it from the device tree.
>>>>>
>>>>> Not thrilled with the idea...
>>>>
>>>> I wonder if we could do this:
>>>>
>>>> 1. Put the microcode in a .dtsi file, perhaps with a tool to do this
>>>> automatically.
>>>> 3. Build the board
>>>> 2. Use something like this to extract the hex data manualy:
>>>
>>> 'manually' means we don't let the build system to do this 'automatically'?
>>
>> For now, since I suspect automatically might be hard for you.
>>
>>>
>>>>
>>>> fdtget -tx /path/to/u-boot.dtb /microcode/update at 0 data | sed 's/ /, 0x/g'
>>>>
>>>> then put this in a file that you compile. That way if we come up with
>>>> a clever solution later we can use it. Unfortunately it means
>>>> submitting the same microcode update in a .c format for now, but have
>>>> a path to fixing it.
>>>>
>>>
>>> Sorry I am confused. In what format should we submit the microcode
>>> data? Is it dtsi or .c? Intel normally releases the microcode update
>>> in .h format.
>>
>> We should have a tool which converts Intel's format into a device tree
>> .dtsi include file. I'd like that .dtsi to be the file we commit to
>> the U-Boot source tree. However since this is not suitable for your
>> platform then we need to figure out an approach to convert it to a
>> form you need. I believe all you really need to know is the address of
>> the microcode update, and I feel there must be a way to make this work
>> automatically in the build system.
>>
>> My suggestion above was just a short-term approach to get you unblocked.
>>
>>>
>>> BTW: I have cleaned up the remaining coding convention issues for ths
>>> FSP codes. Do you want to continue reviewing other patches in this v2,
>>> so that I can fix all issues in v3? Do you think we need address this
>>> microcode update in v3? I think we can leave this issue for this
>>> series, and address this microcode issue in another patch series, as
>>> it may touch the chromebook_link codes.
>>
>> Yes I will get to the others. Overall it all looks fine so don't
>> expect anything major. I'm happy to sort out the microcode in a later
>> series if you prefer, but we do need to fix it.
>
> Yes, we can address the microcode issue in a later series.
>
> For this v2 series, I would assume I can just send v3 with the
> remaining U-Boot coding convention changes and it is ready to apply?
> Let me know if you have different thoughts.

I sent through a few other comments that you saw, but once they are
resolved, yes. I plan to test this and then apply it immediately so
long as we can do this by early next week. Normally we would wait for
the merge window, but as this bare x86 support is entirely new in this
cycle anyway I see no problem with just going ahead, and in fact I
think waiting would be worse since you have improved fixed a few
problems and generalised in some areas.

I'm not entirely sure this will make it to mainline - I will check
with Tom. But he may be OK with it for the same reasons I am.

Regards,
Simon