[U-Boot] [ PATCH] ARM: cache: runtime flag to suppress the "cache misaligned" message

[Marek Vasut]

On 03/07/2017 03:41 PM, Gary Bisson wrote:
> Hi Marek, All,
> 
> On Tue, Mar 07, 2017 at 03:52:23AM +0100, Marek Vasut wrote:
>> On 03/06/2017 11:21 PM, Steve Rae wrote:
>>> The "chunks" in the "fastboot sparse image" are not aligned,
>>> resulting in many "cached misaligned" messages from
>>> check_cache_range(). Implement a runtime flag to suppress this
>>> message, and use this flag when processing the "fastboot sparse
>>> image".
>>
>> Let me understand this, what we add here is a patch to silence a warning
>> which correctly indicates something totally wrong is happening instead
>> of fixing that problem ?
>>
>> From me, that is a NAK and if my understanding of this is correct, this
>> patch will go in only over my dead body.
>>
>>> Signed-off-by: Steve Rae <steve.rae at raedomain.com> Reported-by: Gary
>>> Bisson <gary.bisson at boundarydevices.com>
>>>
>>> --- A little background: In the code that I am testing, the "sparse"
>>> image is 350,723,024 bytes and is downloaded into an aligned buffer
>>> on the device. This image/buffer contains a header ('a') and 13903
>>> "chunks" (b1/c1 through bx/cx):
>>
>> OT: Can you please fix your mailer to break lines at 80 chars ?
>> I reflowed the mail and the ascii art turned crap.
>>
>>> +---+----+----+----+----+     +----+----+ | a | b1 | c1 | b2 | c2 |
>>> ... | bx | cx | +---+----+----+----+----+     +----+----+ where: a  =
>>> the "sparse image" header (28 bytes) b* = the "chunk" header
>>> (12 bytes) c* = the "chunk" data          (which is always an exact
>>> multiple of CONFIG_SYS_CACHELINE_SIZE) Whenever the "chunk" type is
>>> CHUNK_TYPE_RAW, then the data in the current 'c' (the "chunk" data)
>>> needs to be written into the flash as is, using the pointer to the
>>> first byte of this 'c' as the starting address for the blk_dwrite().
>>> Because of the 28 byte image header, and the 12 byte "chunk"
>>> header(s), it is extremely unlikely that this starting address will
>>> be aligned - resulting in many "CACHE: Misaligned operation at range
>>> [XXXX, YYYY]" messages while these 13093 "chunks" are processed. In
>>> the code that I am testing, this message is being generated by the
>>> call to flush_cache(start_addr, trans_bytes) in the
>>> sdhci_send_command() function.
>>
>> You can set this up such that the [a] part starts at offset +24 bytes, then
>> the [c] will be aligned.
> 
> This will align the first chunk but there's no guarantee the next one
> will be aligned.

OK, so the protocol is basically crappy ?

>>> Copying each "chuck data" to an aligned buffer before calling
>>> blk_dwrite() in order to avoid this message would be highly
>>> inefficient -- the desire is to download the code and flash it as
>>> fast as possible!
>>
>> Fast is great, but not if this makes the transfer unreliable, which will
>> happen if you cannot reliably flush/invalidate cache and you decide to
>> ignore this clear warning you're getting.
> 
> Actually fast isn't the problem, it is more than we can't afford to copy
> each chunk to an aligned buffer because of size constraints. Although we
> could break down "big" chunks to smaller one I guess.

Great

>>> Therefore, I propose this patch which provides a runtime flag to
>>> suppress this warning message, which is used when processing these
>>> fastboot "sparse" images.
>>
>> This is wrong.
> 
> Yes I agree that hiding the warning isn't solving anything. However I
> expected that disabling dcache would remove the warning but it doesn't.

Removing the warning if dcache is OFF is fine.

> Shouldn't the cache maintenance functions like flush_dcache_range()
> check the dcache_status() before doing anything?

They should.

> Regards,
> Gary
> 


-- 
Best regards,
Marek Vasut