[U-Boot] [PATCH 1/4] x86: microcode-tool: Write pure data to the dtsi file

[Bin Meng]

Hi Simon,

On Tue, Aug 11, 2015 at 10:51 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 10 August 2015 at 20:45, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Hi Simon,
>>
>> On Mon, Aug 10, 2015 at 11:48 PM, Simon Glass <sjg at chromium.org> wrote:
>>> Hi Bin,
>>>
>>> On 7 August 2015 at 18:47, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>> Hi Simon,
>>>>
>>>> On Sat, Aug 8, 2015 at 3:26 AM, Simon Glass <sjg at chromium.org> wrote:
>>>>> Hi Bin,
>>>>>
>>>>> On 7 August 2015 at 03:28, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>>>> Currently the microcode-tool writes microcode into a data block as
>>>>>> well as the device tree properties which represents the first 48
>>>>>> bytes in the microcode data. Now we change the tool to only write
>>>>>> the microcode without device tree stuff so that multiple microcode
>>>>>> data blocks can be included in a single property.
>>>>>>
>>>>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>>>>> ---
>>>>>>
>>>>>>  tools/microcode-tool.py | 23 ++++++++++++-----------
>>>>>>  1 file changed, 12 insertions(+), 11 deletions(-)
>>>>>
>>>>> I would rather than we use a tool to pack the microcode together (e.g.
>>>>> ifdtool) rather than changing the source data. I realise that the FSP
>>>>> requires this packing, but I quite dislike the approach of making the
>>>>> source files fit the object files.
>>>>>
>>>>
>>>> Could you roughly describe how you want to do using ifdtool? Is the
>>>> microcode source still from dtb?
>>>
>>> Yes I think it can be done by adding an option to generate the
>>> microcode blob (since for non-FSP platforms it is unnecessary). It can
>>> scan the available microcode nodes and pack them into a single block,
>>> then put a pointer to it into the ROM.
>>>
>>
>> This way we have duplicated microcodes in the ROM. One in the device
>> tree and the other one created by ifdtool as a single block. This
>> causes waste of ROM space.
>>
>>> We already add the pointer with this:
>>>
>>> IFDTOOL_FLAGS += -m 0x$(shell $(NM) u-boot |grep _dt_ucode_base_size
>>> |cut -d' ' -f1)
>>>
>>> but now it will need to go in a separate place in the ROM. I suggest
>>> immediately above the device tree.
>>>
>>> See the write_uboot() which does all sorts of wacky stuff already.
>>> Hopefully we can just replace that code with something that creates
>>> the blob.
>>>
>>>>
>>>>> If you like I can take a look at adding this feature to ifdtool.
>>>>>
>>>
>>
>> And I also would like to clean up the microcode dtsi files. The
>> properties below:
>>
>> intel,header-version = <1>;
>> intel,update-revision = <0x105>;
>> intel,date-code = <0x7182011>;
>> intel,processor-signature = <0x20661>;
>> intel,checksum = <0x52558795>;
>> intel,loader-revision = <1>;
>> intel,processor-flags = <0x2>;
>>
>> I think we can remove them. It provides duplicated information as the
>> data property. As data property has to be parsed by us anyway, with
>> these additional properties it adds no value.
>
> Don't you think these are much easier to read than the binary header?
>

Yes, but I still think duplication is not good. And if we want to do
duplication, we should add two more properties
(intel,microcode-data-size and intel,microcode-total-size) to fully
describe the first 48 bytes header. Right now the description is
somewhere in between.

> Also we could avoid repeating the data in the data property if we
> wanted to, since ifdtool can fix this. I only added it because I was
> unable to get microcode setup to work later on in U-Boot.
>

If we use ifdtool, the header data cannot be avoided as FSP needs the
header. It looks like we may add some special handling in ifdtool when
generating FSP based ROM. These microcode blocks can consume lots of
ROM space if we keep two copies. We can have ifdtool to remove the
microcode nodes in the dtb to avoid duplication, but I don't like that
way. I think putting microcode in the device tree is good for future
reference from U-Boot.

Regards,
Bin