On Wed, 10 Dec 2014 12:40:57 +0100 Laszlo Ersek lersek@redhat.com wrote:
[snip]
This is okay, but please consider adding a *loud* comment here.
The dtb that QEMU exports at the moment is 64KB in size (FDT_MAX_SIZE in "device_tree.c"). See
qemu-system-aarch64 -M virt -machine dumpdtb=virt.dtb
I'm sure you've noticed that; I'll assume that your first new BuildGuidHob() call failed. That's because the BuildGuidHob() function in "MdePkg/Library/PeiHobLib/HobLib.c" limits the size to
(0xFFF8 - sizeof (EFI_HOB_GUID_TYPE)))
Which is why you probably added fdt_pack().
The 64KB DTB that qemu currently exports for the virt machtype carries actually less than 5KB of payload, the rest is padding (for in-place updates I guess), which fdt_pack() trims in-place.
First question: can you guarantee that fdt_pack() trims the DTB in-place? CC'ing David Gibson.
Yes, fdt_pack() will always modify the dtb in place. If there's nothing to pack it will just be a no-op, it will never expand the dtb or move it elsewhere.
This is the initial DTB we're talking about, the one that QEMU places at the base of DRAM in advance (and at reset).
Assuming the above packing (trimming) occurs in-place, we do have a nice margin here (58+ KB) for *normal* FDT nodes (ie. nodes that describe devices), but this change definitely shuts the door on any future ideas to place anything "blob-like" in the DTB. That's okay, and it does match our qemu plans, but you should still explain it in a loud comment at the top. (Also, CC'ing Peter.)
As noted later in the thread it's probably not a good idea to rely on the passed in dtb containing expansion roon - you should use fdt_open_into() to allocate extra space for yourself. You have two options: 1) (easy) Estimate some amount of padding space you think will be sufficient and fdt_open_into before you begin any dtb processing.
2) (harder but more robust with extensive dtb modifications) Write wrappers around the libfdt functions which alter the dtb which will catch -FDT_ERR_NOSPACE errors and re-allocate the dtb at that point.
Note that fdt_open_into() (and fdt_move()) are safe to use with a destination buffer that overlaps the source buffer - and in particular can be used in-place.