[U-Boot] [PATCH v2] bootm: Align cache flush begin address

[Bryan O'Donoghue]

On 17/04/18 23:21, Simon Glass wrote:
> Hi Bryan,
> 
> On 17 April 2018 at 03:27, Bryan O'Donoghue <bryan.odonoghue at linaro.org> wrote:
>>
>>
>>
>> On 16/04/18 17:49, Simon Glass wrote:
>>>
>>> On 15 April 2018 at 04:48, Bryan O'Donoghue <bryan.odonoghue at linaro.org> wrote:
>>>>
>>>> commit b4d956f6bc0f ("bootm: Align cache flush end address correctly")
>>>> aligns the end address of the cache flush operation to a cache-line size to
>>>> ensure lower-layers in the code accept the range provided and flush.
>>>>
>>>> A similar action should be taken for the begin address of a cache flush
>>>> operation. The load address may not be aligned to a cache-line boundary, so
>>>> ensure the passed address is aligned.
>>>>
>>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue at linaro.org>
>>>> Reported-by: Breno Matheus Lima <brenomatheus at gmail.com>
>>>> Suggested-by: Tom Rini <trini at konsulko.com>
>>>> Cc: Simon Glass <sjg at chromium.org>
>>>> ---
>>>>    common/bootm.c | 8 +++++++-
>>>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>>
>>> I feel that using an unaligned start address should probably be an
>>> error. Why would that be useful?
>>>
>>> Apart from that:
>>>
>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>
>>> Regards,
>>> Simon
>>>
>>
>> We are booting a kernel that has an entry point aligned a cacheline boundary with a header prefix/load-address that is a negative offset from that.
>>
>> We could go about trying to move the load/ep address of that kernel but, my feeling is that's probably the wrong thing to do, we can just as easily align-down and add to the flush length.
> 
> What header is this? Perhaps it should be updated to be a cache-line
> multiple in size?

I don't think so it's a TEE header, that's pretty fixed.

> I suspect the impact of this patch is minimal, since people hopefully
> don't put data just before the image is loaded. But if they did, and
> the image is loaded using DMA behind the cache, we might have tricky
> bugs. That's why in general I'm not keen on silently messing with the
> cache outside the expected range.

Yes, I agree with both points.

How printing a warning ?

if (flush_start < load)
     flush_len += load - flush_start;

if (flush_start < load) {
     printf("WARNING: unaligned load address 0x%08lx flushing 0x%08lx\n",
            load, flush_start);
     flush_len += load - flush_start;
}