On 7/12/21 5:43 PM, Tom Rini wrote:
On Mon, Jul 12, 2021 at 05:38:33PM +0200, Marek Vasut wrote:
On 7/12/21 5:15 PM, Tom Rini wrote:
On Mon, Jul 12, 2021 at 01:36:14PM +0800, Bin Meng wrote:
On Mon, Jul 12, 2021 at 1:21 PM Reuben Dowle <reuben.do...@4rf.com> wrote:
I submitted an almost identical patch. See
https://github.com/u-boot/u-boot/commit/eb39d8ba5f0d1468b01b89a2a464d18612d3ea76
This patch eventually had to be reverted
(https://github.com/u-boot/u-boot/commit/5675ed7cb645f5ec13958726992daeeed16fd114),
because it was causing issues on some platforms that had FIT on 32 bit
boundary. However I continue to use it in production code, as without it the
boot on my platform aborts.
I don't have time to investigate why this was happening, but you need to check
this code won't just cause exactly the same faults.
Thanks for your information.
+Marek who did the revert
The revert commit message says:
"The commit breaks booting of fitImage by SPL, the system simply
hangs. This is because on arm32, the fitImage and all of its content
can be aligned to 4 bytes and U-Boot expects just that."
I don't understand this. If an address is aligned to 8, it is already
aligned to 4, so how did this commit make the system hang on arm32?
I think this had something to do with embedding contents somewhere in
the image? There is a thread on the ML from then but I don't know how
informative it will end up being.
If I recall this correctly, DT node alignment is 4 byte and that is what DTC
emits. If you have fitImage with embedded data, you basically end up with
/ {
prop1 = "string1";
prop2 = "string2";
};
where the "string2" is aligned to 4 bytes. And that is what U-Boot expects
when it tries to access those data in-place in SPL.
The problem with the reverted patch was that it made U-Boot assume the
alignment is 8 bytes, and that actually works only if you use fitImage with
external data (mkimage -E), but with embedded data (mkimage default) not so
much. That caused off-by-4 error in some cases and that made the SPL hang.
Note, as I indicated in this patch, now with libfdt 1.6.1, the
alignment to 8 byte is a must-have. So we have to do such alignment
anyway.
@Tom may fill in why libfdt commit commit 5e735860c478 ("libfdt: Check
for 8-byte address alignment in fdt_ro_probe_()") was made to have the
8-byte alignment requirement.
Note that it's not so much since libfdt 1.6.1 but that since always the
device tree has required 8 byte alignment.
DT alignment was always 4 byte , no ?
I'm pretty sure, no, 8 byte base alignment is a pretty much always
thing. I don't have a reference handy but I also know I couldn't have
convinced dgibson to add the check otherwise.
DTSpec rev 0.3 says the following and I think you got confused by
section 5.6 which talks about alignment of the entire tree, not its nodes:
5.4 Structure Block
"
Each token in the structure block, and thus the structure block itself,
shall be located at a 4-byte aligned offset from the
beginning of the devicetree blob (see 5.6).
"
5.4.2 Tree structure
"
For each property of the node:
...
– FDT_PROP token
...
* [zeroed padding bytes to align to a 4-byte boundary]
"
5.5 Strings Block
"
The strings block contains strings representing all the property names
used in the tree. These null terminated strings are
simply concatenated together in this section, and referred to from the
structure block by an offset into the strings block.
The strings block has no alignment constraints and may appear at any
offset from the beginning of the devicetree blob.
"
5.6 Alignment
"
As described in the previous sections, the structure and strings blocks
shall have aligned offsets from the beginning of
the devicetree blob. To ensure the in-memory alignment of the blocks, it
is sufficient to ensure that the devicetree as a
whole is loaded at an address aligned to the largest alignment of any of
the subblocks, that is, to an 8-byte boundary.
"
