Hi Bin, On 10 August 2015 at 21:05, Bin Meng <bmeng...@gmail.com> wrote: > Hi Simon, > > On Tue, Aug 11, 2015 at 10:51 AM, Simon Glass <s...@chromium.org> wrote: >> Hi Bin, >> >> On 10 August 2015 at 20:45, Bin Meng <bmeng...@gmail.com> wrote: >>> Hi Simon, >>> >>> On Mon, Aug 10, 2015 at 11:48 PM, Simon Glass <s...@chromium.org> wrote: >>>> Hi Bin, >>>> >>>> On 7 August 2015 at 18:47, Bin Meng <bmeng...@gmail.com> wrote: >>>>> Hi Simon, >>>>> >>>>> On Sat, Aug 8, 2015 at 3:26 AM, Simon Glass <s...@chromium.org> wrote: >>>>>> Hi Bin, >>>>>> >>>>>> On 7 August 2015 at 03:28, Bin Meng <bmeng...@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...@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.
Well at least one of those could be found by looking at the length of the 'data' property. But yes I suppose we may as well include everything. > >> 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. Yes and hopefully one day Intel will make microcode update from FSP easier and we can go back to using device tree normally. I agree two copies is a waste but it doesn't seem like a problem for now. I agree that dropping the microcode from the fdt isn't worth it, but if we want to later, then fdtgrep can do it pretty easily. Mostly I just think that the fdt microcode files are much more friendly and readable than the binary dumps. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot