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

Albert ARIBAUD
2011-08-08T17:56:46Z

Le 08/08/2011 19:34, Marek Vasut a ?crit :

>> Thinking more about the degenerate case -- why not round *up* the start
>> address, and round *down* the stop address, that is, *reduce* the area
>> to the aligned portion rather than *expand* it into the unknown? That
>> would make data in "partially owned" cache lines safe from unwanted
>> invalidation. OTOH, it would not completely invalidate the caller's
>> data, but at least the malfunction would appear in the faulty calling
>> code, not elsewhere.
>
> That'd introduce even stranger behaviour and it'd be even more sickening to
> debug

I tend to agree neither on the "stranger" part, nor on the "sickening".
Can you develop your viewpoint? For your reference, here is mine:

As for "stranger": for me, an unexpected value (either a modification or
an absence thereof) in a variable that no code is using right now, that
is stranger than an unexpected value in a DMA buffer we just happen to
have recently invalidated.

As for "sickening": for me, when a bug occurs, one of the requisites is
to look up the console output, where a message about an unaligned cache
invalidate address will stand out and quite quickly point at the root
cause for that weird DMA buffer content problem -- OTOH, with "expanded"
invalidates, the DMA buffer is OK so one might disregard the warning and
not link it far later with that weird problem where an obscure global
variable couldn't keep its value.

I do understand though that I dismissed flush-in-invalidate with an
argument of "does what it says on the tin", and that one could say my
proposal makes the function do less of what it says on the tin; my point
was about the nature of the action, not its extent.

IOW, the sticker on the tin also says "use only with cacheline-aligned
start and stop", so fair's fair: if you pass unaligned ranges, don't
expect the invalidate to extend beyond the aligned part.

Amicalement,
--
Albert.