On 10/19/20 6:02 PM, Marek Vasut wrote:
On 10/20/20 12:58 AM, Tom Rini wrote:
On Tue, Oct 20, 2020 at 12:54:35AM +0200, Marek Vasut wrote:
On 10/20/20 12:45 AM, Tom Rini wrote:
On Mon, Oct 19, 2020 at 11:59:22PM +0200, Marek Vasut wrote:
On 10/19/20 11:50 PM, Reuben Dowle wrote:
The alignment of 8 bytes would also work if code was expecting 4 byte
alignment. So the explanation you give for reverting this does not make sense
to me.
Well, since U-Boot 2020.10-rc5, any STM32MP1 board does no longer boot
and if I revert this patch, it works again (per git bisect). But this
also applies to any other arm32 boards which load fitImage in SPL, all
of those boards are broken in U-Boot 2020.10.
It seems that the end of the U-Boot image is at 4-byte aligned offset on
arm32, and that is where the DT is also loaded ; but your patch forces
the alignment to 8-bytes, so suddenly the DT location is 4-bytes off.
I think this needs some more investigation to figure out what's going
on and where the underlying bugs are. This section of the code is where
U-Boot is saying it will copy the device tree to. If we're using a
device tree in place that's NOT being copied (and someone else has
ensured 8 byte alignment of) we need to set the address of where the
device tree is, at that time.
The problem is that the previous alignment was 4 byte, now it is 8 byte
and that breaks all the other assumptions. So, this patch should be
reverted to fix the platforms which used to work (or use ALIGN(...4),
which is the same as reverting it really).
And likely the signed image which caused the breakage should be
generated with mkimage -E / -B 8, which would insure the alignment, so
then there is no need to change anything in the code itself.
4 byte alignment is wrong.
Please elaborate why is 4 byte alignment wrong? 8 byte alignment
obviously breaks existing boards which are in-tree and worked for
multiple releases.
The reverted change linked to some kernel documentation that requires
64-bit alignment. I agree with the alignment requirement.
Im my opinion, there are two things that need to be done:
First is to look at an ALIGNED address for the fdt. A summary inspection
of board_fdt_blob_setup() tells us this is done via the "_end" linker
symbol.
Second is to put things in the right place. For FIT, the code, as is, is
correct, but this alignment is not guaranteed for legacy images. I think
somebody mentioned changing the arguments to mkimage to achieve this.
I've tried to fix the first point by aligning the _end symbol (appendix
A). Unfortunately, this is causing other build issues that I don't know
how to deal with.
Alex
APPENDIX A:
-- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -196,7 +196,6 @@ SECTIONS
* for FIT images.
* See common/spl/spl_fit.c: spl_fit_append_fdt
*/
+ . = ALIGN(8);
.end :
{
*(.__end)
