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

[Anton Staaf]

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
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
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.

Thanks,
    Anton

On Tue, Aug 9, 2011 at 4:10 AM, Aneesh V <aneesh at ti.com> wrote:
> Remove the flush of boundary cache-lines done as part
> of invalidate on a non cache-line boundary aligned
> buffer
>
> Also, print a warning when this situation is recognized.
>
> Signed-off-by: Aneesh V <aneesh at ti.com>
> ---
> V2:
>  New in V2
> ---
>  arch/arm/cpu/armv7/cache_v7.c |   14 ++++++++------
>  arch/arm/lib/cache-pl310.c    |   15 +++++++++------
>  2 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c
> index 665f025..3dda56b 100644
> --- a/arch/arm/cpu/armv7/cache_v7.c
> +++ b/arch/arm/cpu/armv7/cache_v7.c
> @@ -181,21 +181,23 @@ static void v7_dcache_inval_range(u32 start, u32 stop, u32 line_len)
>        u32 mva;
>
>        /*
> -        * If start address is not aligned to cache-line flush the first
> -        * line to prevent affecting somebody else's buffer
> +        * If start address is not aligned to cache-line do not
> +        * invalidate the first cache-line
>         */
>        if (start & (line_len - 1)) {
> -               v7_dcache_clean_inval_range(start, start + 1, line_len);
> +               printf("WARNING: %s - start address is not aligned - 0x%08x\n",
> +                       __func__, start);
>                /* move to next cache line */
>                start = (start + line_len - 1) & ~(line_len - 1);
>        }
>
>        /*
> -        * If stop address is not aligned to cache-line flush the last
> -        * line to prevent affecting somebody else's buffer
> +        * If stop address is not aligned to cache-line do not
> +        * invalidate the last cache-line
>         */
>        if (stop & (line_len - 1)) {
> -               v7_dcache_clean_inval_range(stop, stop + 1, line_len);
> +               printf("WARNING: %s - stop address is not aligned - 0x%08x\n",
> +                       __func__, stop);
>                /* align to the beginning of this cache line */
>                stop &= ~(line_len - 1);
>        }
> diff --git a/arch/arm/lib/cache-pl310.c b/arch/arm/lib/cache-pl310.c
> index 36c629c..15fe304 100644
> --- a/arch/arm/lib/cache-pl310.c
> +++ b/arch/arm/lib/cache-pl310.c
> @@ -26,6 +26,7 @@
>  #include <asm/armv7.h>
>  #include <asm/pl310.h>
>  #include <config.h>
> +#include <common.h>
>
>  struct pl310_regs *const pl310 = (struct pl310_regs *)CONFIG_SYS_PL310_BASE;
>
> @@ -89,21 +90,23 @@ void v7_outer_cache_inval_range(u32 start, u32 stop)
>        u32 pa, line_size = 32;
>
>        /*
> -        * If start address is not aligned to cache-line flush the first
> -        * line to prevent affecting somebody else's buffer
> +        * If start address is not aligned to cache-line do not
> +        * invalidate the first cache-line
>         */
>        if (start & (line_size - 1)) {
> -               v7_outer_cache_flush_range(start, start + 1);
> +               printf("WARNING: %s - start address is not aligned - 0x%08x\n",
> +                       __func__, start);
>                /* move to next cache line */
>                start = (start + line_size - 1) & ~(line_size - 1);
>        }
>
>        /*
> -        * If stop address is not aligned to cache-line flush the last
> -        * line to prevent affecting somebody else's buffer
> +        * If stop address is not aligned to cache-line do not
> +        * invalidate the last cache-line
>         */
>        if (stop & (line_size - 1)) {
> -               v7_outer_cache_flush_range(stop, stop + 1);
> +               printf("WARNING: %s - stop address is not aligned - 0x%08x\n",
> +                       __func__, stop);
>                /* align to the beginning of this cache line */
>                stop &= ~(line_size - 1);
>        }
> --
> 1.7.0.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>