[U-Boot] ARM - cache and alignment

[Marek Vasut]

On 01/16/2017 08:16 PM, Jean-Jacques Hiblot wrote:
> 
> 
> On 16/01/2017 17:00, Marek Vasut wrote:
>> On 01/16/2017 02:29 PM, Jean-Jacques Hiblot wrote:
>>> Tom, Marek
>> Hi,
>>
>>> At the moment, whenever an unaligned address is used in cache operations
>>> (invalidate_dcache_range, or flush_dcache_range), the whole request is
>>> discarded  for am926ejs. for armV7 or armV8 only the aligned part is
>>> maintained. This is probably what is causing the bug addressed in
>>> 8133f43d1cd. There are a lot of unaligned buffers used in DMA operations
>>> and for all of them, we're possibly handling the cached partially or not
>>> at all. I've seen this when using the environment from a file stored in
>>> a FAT partition. commit 8133f43d1cd addresses this by using a bounce
>>> buffer at the FAT level but it's only one of many cases.
>>>
>>> I think we can do better with unaligned cache operations:
>>>
>>> * flush (writeback + invalidate): Suppose we use address p which is
>>> unaligned, flush_dcache_range() can do the writeback+invalidate on the
>>> whole range [p & ~(line_sz - 1); p + length | (line_sz - 1)]. There
>>> should no problem with that since writeback can happen at any point in
>>> time.
>>>
>>> * invalidation
>>>
>>> It is a bit trickier. here is a pseudo-code:
>>> invalidate_dcache_range(p,length)
>>> {
>>>           write_back_invalidate(first line)
>>>           write_back_invalidate(last line)
>>>           invalidate(all other lines)
>>> }
>>>
>>> Here again this should work fine IF invalidate_dcache_range() is called
>>> BEFORE the DMA operation (again the writeback can happen at time so it's
>>> valid do it here). Calling it only AFTER the operation, may corrupt the
>>> data written by the DMA with old data from CPU. This how I used to
>>> handle unaligned buffers in some other projects.
>>>
>>>
>>> There is however one loophole: a data sitting in the first or the last
>>> line is accessed before the memory is updated by the DMA, then the
>>> first/line will be corrupted. But it's not highly probable as this data
>>> would have to be used in parallel of the DMA (interrupt handling, SMP?,
>>> dma mgt related variable). So it's not perfect but it would still be
>>> better than we have today.
>> Or just fix all the code which complains about unaligned buffers, done.
>> That's the way to go without all the complications above.
> It's not that complex, but it's not perfect. We would need to keep the
> same warning as we have now, but it would make it work in more cases.

The warning is there for that exact reason -- to inform you something's
wrong.

> Tracking every possible unaligned buffer that gets invalidated is not a
> trivial job. Most of the time the buffer is allocated in a upper layer
> and passed down to a driver via layers like network stack, block layer
> etc.And in many cases, the warning will come and go depending on how the
> variable aligned on the stack or the heap.

I didn't observe this much in fact. I usually see the buffers coming it
aligned or being allocated in drivers. Also, I think that's why the RC
cycle is there, so we can test the next release and fix these issues.


-- 
Best regards,
Marek Vasut