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

[Bin Meng]

Hi Simon,

On Fri, Dec 12, 2014 at 11:57 AM, Simon Glass <sjg at chromium.org> wrote:
> 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.

Thanks for reviewing other patches. It would be good if this series
can catch up with the last train of v2015.01 :) Now it is time for me
to gear up for the v3.

Regards,
Bin