[U-Boot] [PATCH v2] ARM926ejs: Add routines to invalidate D-Cache

[Aneesh V]

Hi Albert,

On Friday 05 August 2011 06:47 PM, Albert ARIBAUD wrote:
> (BTW: responders to this thread please stop using my @free.fr address. I
> just noticed the big pile of U-Boot related messages that went to an
> account which I do not use for U-Boot any more)
>
> On 05/08/2011 13:51, Aneesh V wrote:
>> Hi Albert,
>>
>> On Friday 05 August 2011 04:33 PM, Albert ARIBAUD wrote:
>>> Hi Aneesh,
>>>
>>> On 05/08/2011 12:47, Aneesh V wrote:
>>>> Hi Eric,
>>>>
>>>> On Friday 05 August 2011 04:03 PM, Hong Xu wrote:
>>>>> Hi Aneesh,
>>>> [snip ..]
>>>>>>
>>>>>> IMHO, Hong's approach is correct. If the buffer that is
>>>>>> invalidated is
>>>>>> not aligned to cache-line, one cache-line at the respective boundary
>>>>>> may have to be flushed to make sure the invalidation doesn't affect
>>>>>> somebody else's memory.
>>>>>>
>>>>>> The solution is for drivers to ensure that any buffer that needs
>>>>>> to be
>>>>>> invalidated is aligned to cache-line boundary at both ends. The above
>>>>>> approach puts this onus on the driver. I have documented the
>>>>>> alignment
>>>>>> requirement in my recent patch series for fixing arm cache problems.
>>>>>
>>>>> I have not noticed the patch series. ;-)
>>>>> If we put the alignment burden to the driver, I'm afraid many drivers
>>>>> which make use of something like a DMA controller have to modify the
>>>>> code heavily. This sounds not good. :)
>>>>
>>>> We have a fundamental problem when it comes to invalidating an
>>>> un-aligned buffer. Either you flush the boundary lines and corrupt your
>>>> buffer at boundaries OR you invalidate without flushing and corrupt
>>>> memory around your buffer. Both are not good! The only real solution is
>>>> to have aligned buffers, if you want to have D-cache enabled and do DMA
>>>> at the same time.
>>>
>>> Plus, there should not be *heavy* modifications; DMA engines tend to use
>>> essentially two types of memory-resident objects: data buffers and
>>> buffer descriptors. There's only a small handful of places in the driver
>>> code to look at to find where these objects are allocated and how.
>>>
>>> So I stand by my opinion: since the cache invalidation routine should
>>> only be called with cache-aligned objects, there is no requirement to
>>> flush the first (resp. last) cache line in case of unaligned start
>>> (resp.stop), and I don't want cache operations performed when they are
>>> not required.
>>
>> IMHO, flushing is better, because the person who commits the
>> mistake of invalidating the un-aligned buffer is the one who is
>> affected and is likely to fix the issue soon. If we didn't flush, the
>> resulting corruption will cause totally random errors that will be hard
>> to debug. Doing an extra flush operation for a maximum of 2 lines
>> doesn't cost us anything. This is the approach followed by the kernel
>> too.
>
> As pointed out by Reinhard, flushing while invalidating is only almost
> good, and not required at all if alignment requirements are followed.

I don't dispute that having buffers aligned is the ideal scenario. The
question is about error-handling the situation when this requirement is
not met.

>
> Especially, I don't buy the argument that "the person who commits the
> mistake of invalidating the un-aligned buffer is the one who is affected
> and is likely to fix the issue soon". The issue might not appear right
> after the call to flush is added; it might appear quite later, after
> several reorganizations of the ordering of data in RAM, and affect some
> completely unrelated person doing something completely unrelated.

I don't get how a flush can affect an un-related person unless the
surrounding buffer also happens to be a DMA receive buffer. Please note
that flush is something that can happen to any dirty line as part of
the cache line replacement done by hardware when new lines are brought
into the cache, it's not such a dangerous thing for normal memory
locations.

>
> OTOH, aligning buffers on cache boundaries removes the need to flush
> within invalidates, and will ensure no other data is at any risk.

If the buffers are aligned, the flush operation will not get executed.
So, what is the risk?

>
> Between an implementation that "should cause no issue" and an
> implementation that "cannot cause issues", I definitely favor the
> latter: so that's a no on my side to any flushing while invalidating a
> range.

Let's look at the different cases:

Let A be an un-aligned DMA receive buffer. Let B be a buffer lying
next to A belonging to another program.

Case I - A is aligned
1. Invalidate with flush - no issue
2. Invalidate without flush - no issue

Case II - A is un-aligned, B is DMA receive buffer:
1. Invalidate with flush - corrupts B if DMA is on-going.
2. Invalidate without flush - no issue.

Case III - A is un-aligned, B is normal memory.
1. Invalidate with flush - A corrupted, no issue for B.
2. Invalidate without flush - no issue for A, B is corrupted.

Case I doesn't cause any issue either way. Case II would be rather
rare. If it happens, it's more likely that B belongs to the same driver
owning A. Case III is the more common error case and you can see that
Invalidate with Flush is better because it doesn't affect B.

best regards,
Aneesh