From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michal Simek <michal.simek@xilinx.com>
Date: Mon, 3 Dec 2018 15:33:41 +0100
Subject: [U-Boot] [PATCH] tools: zynqmpimage: round up partition size
In-Reply-To: <b513f35e-9330-f898-a712-67220a2bf5a8@suse.de>
References: <20181128104730.26780-1-m.tretter@pengutronix.de>
 <aaa9039c-123a-6208-83ff-b8af03a7ba97@xilinx.com>
 <b513f35e-9330-f898-a712-67220a2bf5a8@suse.de>
Message-ID: <bd1df4e1-6004-4627-400a-7b282a42fb73@xilinx.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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

ok. Let me just wait a little bit to finish this discussion.

Thanks,
Michal