Re: [Xen-devel] [PATCH v1 1/2] xen/arm: Introduce clean_and_invalidate_xen_dcache() macro

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Thu, 2014-03-06 at 15:50 +0200, Oleksandr Tyshchenko wrote:
> This macro is very similar to clean_xen_dcache(), but it performs
> clean and invalidate dcache.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Please don't add signed-off-by from other people unless they have
explicitly given you one. Adding an S-o-b conveys a specific meaning,
which is to certify the change under the DCO[0] which at least I haven't
done.

If someone has contributed either ideas or code then the appropriate way
to credit them is Suggested-by or similar or a mention in the prose part
("Based on a suggestion by..."). Of course it would also be appropriate
to include a CC to that person and they may then reply with an (Acked|
Reviewed|Signed-off)-by if they think that is appropriate. If someone
contributed code then you could also ask if they are happy/want you to
include their S-o-b.

Sorry to be picky but as I say there is a legal angle to the use of
S-o-b.

(I've not got time to properly review the actual change today, I'll do
that next week, but at first glance it looks good, thanks)

Thanks,
Ian.

[0] http://wiki.xen.org/wiki/Submitting_Xen_Patches#Signing_off_a_patch

> Signed-off-by: Oleksandr Tyshchenko <oleksandr.tyshchenko@xxxxxxxxxxxxxxx>
> ---
>  xen/include/asm-arm/page.h |   26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index e00be9e..34effa1 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -226,7 +226,7 @@ static inline lpae_t mfn_to_xen_entry(unsigned long mfn)
>  /* Actual cacheline size on the boot CPU. */
>  extern size_t cacheline_bytes;
>  
> -/* Function for flushing medium-sized areas.
> +/* Functions for flushing medium-sized areas.
>   * if 'range' is large enough we might want to use model-specific
>   * full-cache flushes. */
>  static inline void clean_xen_dcache_va_range(void *p, unsigned long size)
> @@ -238,7 +238,17 @@ static inline void clean_xen_dcache_va_range(void *p, 
> unsigned long size)
>      dsb();           /* So we know the flushes happen before continuing */
>  }
>  
> -/* Macro for flushing a single small item.  The predicate is always
> +static inline void clean_and_invalidate_xen_dcache_va_range(void *p,
> +             unsigned long size)
> +{
> +    void *end;
> +    dsb();           /* So the CPU issues all writes to the range */
> +    for ( end = p + size; p < end; p += cacheline_bytes )
> +        asm volatile (__clean_and_invalidate_xen_dcache_one(0) : : "r" (p));
> +    dsb();           /* So we know the flushes happen before continuing */
> +}
> +
> +/* Macros for flushing a single small item.  The predicate is always
>   * compile-time constant so this will compile down to 3 instructions in
>   * the common case. */
>  #define clean_xen_dcache(x) do {                                        \
> @@ -253,6 +263,18 @@ static inline void clean_xen_dcache_va_range(void *p, 
> unsigned long size)
>              : : "r" (_p), "m" (*_p));                                   \
>  } while (0)
>  
> +#define clean_and_invalidate_xen_dcache(x) do {                         \
> +    typeof(x) *_p = &(x);                                               \
> +    if ( sizeof(x) > MIN_CACHELINE_BYTES || sizeof(x) > alignof(x) )    \
> +        clean_and_invalidate_xen_dcache_va_range(_p, sizeof(x));        \
> +    else                                                                \
> +        asm volatile (                                                  \
> +            "dsb sy;"   /* Finish all earlier writes */                 \
> +            __clean_and_invalidate_xen_dcache_one(0)                    \
> +            "dsb sy;"   /* Finish flush before continuing */            \
> +            : : "r" (_p), "m" (*_p));                                   \
> +} while (0)
> +
>  /* Flush the dcache for an entire page. */
>  void flush_page_to_ram(unsigned long mfn);
>  



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel