[U-Boot] [PATCH 4/4] armv7: cache: remove flush on un-aligned invalidate

[Anton Staaf]

On Tue, Aug 9, 2011 at 11:48 PM, Aneesh V <aneesh at ti.com> wrote:
> Hi Anton,
>
> On Tuesday 09 August 2011 10:09 PM, Anton Staaf wrote:
>>
>> I'm not sure what the larger context of this change is, but it seems
>> like a bad idea to me.  There are a lot of locations in U-Boot that
>
> Please see this thread for the context.
> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/105113/focus=105135

Ahh, I missed this thread (Its title became stale).  :)  But I
completely agree with the outcome, we need to fix the unaligned buffer
problem.

>
>> will end up causing an unaligned invalidate (ext2 and dos file system
>> code in particular).  And this change will cause those unaligned
>> invalidates to possibly throw away stores to adjacent variables.  If
>
> No. Those partial cache-lines on the boundary are left alone. They are
> not invalidated. So, it still affects only the party calling the
> invalidate.

Ahh, you are correct.  I missed that the change would cause fewer
cache lines to be invalidated.  In this case I am much happier with
this change.  In light of this I still think the warning is a little
mild, since it means that the driver that called the invalidate is
certainly going to get the wrong values.  Perhaps changing it to an
error would be good (I realize that functionally it would be
identical, but it would be more potent psychologically).  I don't
think an assert is warranted in this case since as Albert points out
it would prevent "online" debugging of U-Boot which is a very useful
way of working.

>> you are going to make this change you should at least assert instead
>> of just printing a warning.  And there should be a concerted effort to
>> clean up the buffer management in U-Boot so that invalidates will
>> never be unaligned.  This is also a departure from the cache
>> management implementations in the Linux kernel, not that U-Boot has to
>> do exactly what they do, but I feel they have the correct
>> implementation, from the perspective of ensuring that all stores
>> actually make it to main memory.
>
> Yes, I had implemented it in line with the kernel apporach. However,
> with un-aligned buffers there is no perfect solution anyway. So, I
> don't have a strong opinion on this. Leaving alone the boundary
> cache-lines and printing a big warning seems reasonable enough.

Yup.

> best regards,
> Aneesh
>

I have a number of patches to fix unaligned buffer use in the
filesystem (ext2, dos, mmc and partition) code that I will start
sending upstream today.  Some of them will be more RFCs than patch
sets.  :)

Thanks,
    Anton