[U-Boot] [PATCH] cmd: net: flush cache cacheline aligned

[Stefan Agner <stefan@agner.ch>]

On 2016-08-04 10:46, Tom Rini wrote:
> On Wed, Aug 03, 2016 at 07:43:24PM -0700, Stefan Agner wrote:
>> On 2016-08-03 16:18, Joe Hershberger wrote:
>> > On Tue, Aug 2, 2016 at 2:20 AM, Stefan Agner <stefan@agner.ch> wrote:
>> >> From: Stefan Agner <stefan.agner@toradex.com>
>> >>
>> >> Flush loaded data cacheline aligned. This avoids warnings such as
>> >> CACHE: Misaligned operation at range [81000000, 816d0fa8]
>> >>
>> >> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
>> >> ---
>> >
>> > This was already rejected once.
>> > http://lists.denx.de/pipermail/u-boot/2012-April/121564.html
>>
>> Oh I see, and in the end the message was converted to a debug() call, in
>> essence turning the whole problem back under a stone... :-)
>>
>> FWIW, I largely support Mike Frysinger's position in that discussion,
>> and think it should be fine to flush these extra bytes...
> 
> I re-read most of that thread last-night and yes, it seems like rather
> than solving the problem we just put it into a debug().  But if I
> followed Mike's point right, we shouldn't be having a debug there
> either, the flushes are fine?  And as to Marek's point there about
> driver bugs, it's perhaps only sub-optimal rather than a broken thing?

[added Mike to CC]

It seems that back then another cache driver was affected
(arch/arm/cpu/arm926ejs/cache.c).

I don't agree with Mike: We definitely should warn if a cache flush is
unaligned. Hence I think Siomons change is the right thing to do. Not
sure if all cache driver behave the same, but the ARMv7 cache driver
(arch/arm/cpu/armv7/cache_v7.c) flushes a unaligned line. This might not
be an issue in that particular case in cmd/net.c (since there are likely
no critical data immediately following the loaded binary), but it
definitely can be an issue if we flush something on the heap: Assume we
have an array of 32 byte long DMA descriptors, and the cache line is 64
bytes. If a driver tries to flush one descriptor only,  we would always
silently flush a second DMA descriptor... If that other descriptor is
updated by the DMA controller, we would run into an issue. So if a
driver flushes cache line unaligned, we should warn!

Since the cache driver anyway flushes cache aligned, the only thing this
patch changes is actually to be explicit about that... And compared to
the patch above, this patch uses CONFIG_SYS_CACHELINE_SIZE, which makes
it clear that this is not a DMA alignment issue but a cache line
alignment issue...

So I vote for either explicitly flush cache aligned (what this patch is
doing) or remove that flush entirely...

--
Stefan