Hi Alper,
On Thu, 3 Mar 2022 at 14:17, Alper Nebi Yasak alpernebiyasak@gmail.com wrote:
On 24/02/2022 02:00, Simon Glass wrote:
The current implementation sets up the FIT entries but then deletes the 'generator' ones so they don't appear in the final image.
They still show up in the fdtmap if I add one to rockchip-u-boot.dtsi:
$ binman ls -i u-boot-rockchip.bin Name Image-pos Size Entry-type Offset
main-section 0 103520 section 0 mkimage 0 1a000 mkimage 0 fit 1a000 e8d74 fit 1a000 u-boot 1a644 b1898 section 644 u-boot-nodtb 1a644 b1898 u-boot-nodtb 0 @atf-SEQ 0 0 section 0 atf-bl31 0 0 atf-bl31 0 @tee-SEQ 0 0 section 0 tee-os 0 0 tee-os 0 @fdt-SEQ 0 0 section 0 fdtmap 102d74 79c fdtmap 102d74
But not simple-bin.map:
ImagePos Offset Size Name 00000000 00000000 00103520 main-section 00000000 00000000 0001a000 mkimage 0001a000 0001a000 000e8d74 fit 0001a644 00000644 000b1898 u-boot 0001a644 00000000 000b1898 u-boot-nodtb 00102d74 00102d74 0000079c fdtmap
This happens in v1 as well, but I hadn't checked it then.
(The "fit" size and "fdtmap" offset are off by 0x10 too, but I'm not sure why yet.)
Well let's fix this after the series. We need tests for the map and so on.
This is a bit clumsy. We cannot build the image more than once, since the generator entries are lost during the first build. Binman requires that calling BuildSectionData() multiple times returns a valid result each time.
I think the generator entries should be turned into concrete entries and be removed in _gen_entries(), so BuildSectionData() should work on the entries that were generated. This way the individual entries would show up in fdtmap and could be binman-extracted/replaced as well.
That makes sense, but I'm not sure how to implement it. The split-elf thing needs the contents of the entries which is not available at the start. It is likely available before BuildSectionData() is called, but not necessarily. So the template needs to hang around at least as long as that.
I think there is something we could do here, but it isn't quite clear to me. Perhaps we need a expand-nodes-based-on-contents phase in control.py ? Eek...
For FDTs we can generate blob entries for each file, for ELFs maybe we can split them into files like the script used to do (bl31_0x<addr>.bin) and do the same?
I'm not a big fan of adding files. Binman should be able to hold everything in memory. It does generate files for later inspection though, so yes we could add this feature.
Maybe some new entry types, "data" for arbitrary data unrelated to a file whose contents we can set, or "elf-segment" that can extract a segment from an ELF file (but I don't like the information loss there).
Yes I quite like the idea of new entry types. In fact I was hoping to turn split-elf into an entry type (one that generated multiple entries). But I decided to stop before doing that since we really need to gets the code in there, fix the bugs /move forward with some of the ideas you and others have.
Keep a separate, private list which includes the generator nodes and use that where needed, to correct this problem. Ensure that the missing list includes removed generator entries too.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
I'm more and more convinced that generator nodes needs a thorough redesign, but the patch is consistent with status quo, so:
Reviewed-by: Alper Nebi Yasak alpernebiyasak@gmail.com
tools/binman/etype/fit.py | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-)
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index e1b056f56e..30b20a07a2 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -163,10 +163,15 @@ class Entry_fit(Entry_section): key: relative path to entry Node (from the base of the FIT) value: Entry_section object comprising the contents of this node
_priv_entries: Internal copy of _entries which includes 'generator'entries which are used to create the FIT, but should not beprocessed as real entries. This is set up once we have theentriesMaybe this could be "_templates" or "_entry_generators" and only keep track of the generator entries.
Again I think we can revisit that. I feel it is a bit messy right now, too.
""" super().__init__(section, etype, node) self._fit = None self._fit_props = {}
self._priv_entries = {} for pname, prop in self._node.props.items(): if pname.startswith('fit,'):[...]
Regards, Simon