[U-Boot] [PATCH] tools: zynqmpimage: round up partition size

[Michael Tretter]

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.tretter at 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