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 ?

It's just that on 32bit
platforms 4-but-not-8 byte alignment tends to not be fatal but on 64bit
platforms it is.

[...]

Reply via email to