[U-Boot] [PATCH v4] dcache: Dcache line size aligned stack buffer allocation

[Anton Staaf]

On Thu, Sep 1, 2011 at 3:30 AM, Lukasz Majewski <l.majewski at samsung.com> wrote:
>
> ALLOC_CACHE_ALIGN_BUFFER shall be used in functions, which are using
> stack allocated buffers for DMA transfers.
>
> Signed-off-by: Lukasz Majewski <l.majewski at samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> CC: Albert ARIBAUD <albert.u.boot at aribaud.net>
> ---
> Changes for v2:
>        - ./include/cache.h has been removed and replaced with
>        simpler macro added to ./include/common.h
> Changes for v3:
>        - change char * to char
>        - defined table size definition
> Changes for v4:
>        - (type*) added for compiler warning fix
>
> Signed-off-by: Lukasz Majewski <l.majewski at samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> ---
>  include/common.h |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/include/common.h b/include/common.h
> index 12a1074..a74c6e8 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -767,6 +767,11 @@ int cpu_release(int nr, int argc, char * const argv[]);
>  #define ALIGN(x,a)             __ALIGN_MASK((x),(typeof(x))(a)-1)
>  #define __ALIGN_MASK(x,mask)   (((x)+(mask))&~(mask))
>
> +#define ALLOC_CACHE_ALIGN_BUFFER(type, name, size) \
> +       char __##name[size + CONFIG_SYS_CACHELINE_SIZE - 1]; \

It was pointed out to me that we need to make sure that both ends of
the resulting buffer are cache line aligned.  Or put another way, that
the __##name array has enough padding at the beginning and end that an
invalidate will be both aligned to the cache line and not effect
anything defined after the array on the stack.  So the above
definition needs to change to something like:

char __##name[ROUND(size, CONFIG_SYS_CACHELINE_SIZE)  +
CONFIG_SYS_CACHELINE_SIZE - 1];

Another thing that concerns me is that the macro takes a type, but the
size parameter is specified in bytes, not units of the size of the
type.  Would it make sense to specify the size in units of the type?
It would make almost no sense to specify a size that wasn't a multiple
of the size of the type anyway.  If we want to do that the the array
definition becomes:

char __##name[ROUND(size * sizeof(type), CONFIG_SYS_CACHELINE_SIZE)  +
CONFIG_SYS_CACHELINE_SIZE - 1];

And finally, the ROUND macro is written such that it will always
return a value that is larger than it's first parameter.  Thus
ROUND(CONFIG_SYS_CACHELINE_SIZE, CONFIG_SYS_CACHELINE_SIZE) withh not
equal CONFIG_SYS_CACHELINE_SIZE, but actually 2 *
CONFIG_SYS_CACHELINE_SIZE.  I'm not sure if this is intentional.  In
fact, the only use of ROUND that is not to round the value of
CONFIG_SYS_MALLOC_LEN to a multiple of 4096 is in the common/cmd_sf.c
implementation.  And there it looks like the author worked around the
behavior of ROUND by passing "len_arg - 1", instead of len_arg.  So,
it looks like a patch to fix ROUND might be in order as well.  I'll
try and send one today.

-Anton

>
> +       type *name = (type *)  ALIGN(((typeof(CONFIG_SYS_CACHELINE_SIZE))\
> +                                    (__##name)), (CONFIG_SYS_CACHELINE_SIZE));
> +
>  /* Pull in stuff for the build system */
>  #ifdef DO_DEPS_ONLY
>  # include <environment.h>
> --
> 1.7.2.3
>