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@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