On Mon, 03 Dec 2018 14:42:59 +0100, 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.
>
> The one thing that I'm not fully sure of is the realloc(). I don't think
> that data is necessarily always a pointer that way allocated using malloc().
>
> If you look at bif_add_bit() for example the data pointer is an offset
> into a pointer from read_full_file(). So realloc() won't work on it.
>
> I think it's safer to just allocate a completely new buffer here and
> copy the payload if unaligned.
Thanks. I thought about aligning data in read_full_file(), but the size
read by read_full_file() is not used in bif_add_elf(). I further
assumed that the bitstream is always correctly aligned (because I never
saw anything else) and will never hit the realloc() path, but I'm
not sure if this is a safe assumption.
As realloc() does not work in bif_add_part(), I can only malloc() a
new aligned buffer and cannot free() the old one. I guess that is fine.
I will send a v2.
Michael
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
