On Mon, 17 Dec 2018 14:27:38 +0100, Michal Simek wrote: > On 03. 12. 18 14:42, Alexander Graf wrote: > > On 03.12.18 14:14, Michal Simek wrote: > >> +Alex > >> > >> On 28. 11. 18 11:47, Michael Tretter wrote: > >>> The FSBL copies "Total Partition Word Length" * 4 bytes from the boot.bin, > >>> which implies that the partition size is 4 byte aligned. When writing the > >>> partition, mkimage calculates "Total Partition Word Length" by dividing > >>> the size by 4. This implicitly cuts unaligned bytes at the end of the > >>> added binary. > >>> > >>> Instead of rounding down, the size must be round up to 4 bytes and the > >>> binary padded accordingly. > >>> > >>> Signed-off-by: Michael Tretter <m.tret...@pengutronix.de> > >>> --- > >>> tools/zynqmpbif.c | 18 ++++++++++++++---- > >>> 1 file changed, 14 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/tools/zynqmpbif.c b/tools/zynqmpbif.c > >>> index 6c8f66055d..885a037da6 100644 > >>> --- a/tools/zynqmpbif.c > >>> +++ b/tools/zynqmpbif.c > >>> @@ -316,19 +316,29 @@ static int bif_add_pmufw(struct bif_entry *bf, > >>> const char *data, size_t len) > >>> return 0; > >>> } > >>> > >>> -static int bif_add_part(struct bif_entry *bf, const char *data, size_t > >>> len) > >>> +static int bif_add_part(struct bif_entry *bf, char *data, size_t len) > >>> { > >>> size_t parthdr_offset = 0; > >>> + size_t len_padded = ROUND(len, 4); > >>> + > >>> struct partition_header parthdr = { > >>> - .len_enc = cpu_to_le32(len / 4), > >>> - .len_unenc = cpu_to_le32(len / 4), > >>> - .len = cpu_to_le32(len / 4), > >>> + .len_enc = cpu_to_le32(len_padded / 4), > >>> + .len_unenc = cpu_to_le32(len_padded / 4), > >>> + .len = cpu_to_le32(len_padded / 4), > >>> .entry_point = cpu_to_le64(bf->entry), > >>> .load_address = cpu_to_le64(bf->load), > >>> }; > >>> int r; > >>> uint32_t csum; > >>> > >>> + if (len != len_padded) { > >>> + data = realloc(data, len_padded); > >>> + while (len < len_padded) { > >>> + data[len] = 0; > >>> + len++; > >>> + } > >>> + } > >>> + > >>> if (bf->flags & (1ULL << BIF_FLAG_PMUFW_IMAGE)) > >>> return bif_add_pmufw(bf, data, len); > >>> > >>> > >> > >> I am not using this for creating boot image but I have tested it on > >> regular example and it is still working. > >> > >> I have applied this to my branch. > >> Alex: If you have any comment on this please let us know and I can > >> remove it from my queue. > > > > I don't think I've ever had a not-multiple-of-4 payload, so I never ran > > into this. > > I was playing with U-Boot on Friday and I found one thing related to > this commit. You can get "not-multiple-of-4 payload" quite simply just > but enabling OF_SEPARATE for SPL which is generating (I am not 100% sure > if all case) aligned u-boot-spl-nodtb.bin and then appends cat > <filterred dtb> >> u-boot-spl-nodtb.bin which is ending up in > "not-multiple-of-4 payload" very often. > > I was trying to enable OF_SEPARATE because of > "fdt: Add warning about CONFIG_OF_EMBED" > (sha1: 841d5fbae4e993476fa87d8933db0cd58d3c2d41) > which is working fine for jtag bootmode but not for SD one (I expect > also others will be broken). > > But I found that you are still pure len(non aligned) in > image_size/image_stored_size instead of aligned one. Which caused that > boot.bin with "not-multiple-of-4 payload" is not handled properly by > bootrom. > > After this patch, boot.bin started to work. > > --- a/tools/zynqmpbif.c > +++ b/tools/zynqmpbif.c > @@ -425,8 +425,8 @@ static int bif_add_part(struct bif_entry *bf, const > char *data, size_t len) > if (!bif_output.header->image_offset) > bif_output.header->image_offset = > cpu_to_le32(bf->offset); > - bif_output.header->image_size = cpu_to_le32(len); > - bif_output.header->image_stored_size = cpu_to_le32(len); > + bif_output.header->image_size = cpu_to_le32(len_padded); > + bif_output.header->image_stored_size = > cpu_to_le32(len_padded); > > > Anyway xilinx bootgen has issue with this too and also zynqmpimage.c. > > Can you please retest it again but I am quite sure you need to also > update header with padded len?
The image_size affects only the first stage bootloader. As I use the FSBL for that, I didn't run into this issue. I can reproduce the issue by writing a size that is not multiple of 4 into either image_size or image_stored_size. The FSBL does not boot, if the image_size is larger than the size of the image, but not 4 byte aligned. It does boot, if image_size is larger and 4 byte aligned. I would have expected that the bootrom ignores the bytes between image_size and partition_size and just reads image_size bytes. Therefore, aligning the image_size looks like a proper fix, but the reason for the patch is different. Michael > > Thanks, > Michal > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot