Re: [PATCH 1/2] tools: Add riscv barrier implementation

["Clément Léger" <cleger@rivosinc.com>]

On 29/07/2024 22:50, Charlie Jenkins wrote:
> Many of the other architectures use their custom barrier implmentations.

Hi Charlie,

Typo: implmentations -> implementations

> Use the barrier code from the kernel sources to optimize barriers in
> tools.
> 
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> ---
>  tools/arch/riscv/include/asm/barrier.h | 39 ++++++++++++++++++++++++++++++++++
>  tools/arch/riscv/include/asm/fence.h   | 13 ++++++++++++
>  tools/include/asm/barrier.h            |  2 ++
>  3 files changed, 54 insertions(+)
> 
> diff --git a/tools/arch/riscv/include/asm/barrier.h b/tools/arch/riscv/include/asm/barrier.h
> new file mode 100644
> index 000000000000..6997f197086d
> --- /dev/null
> +++ b/tools/arch/riscv/include/asm/barrier.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copied from the kernel sources to tools/arch/riscv:
> + *
> + * Copyright (C) 2012 ARM Ltd.
> + * Copyright (C) 2013 Regents of the University of California
> + * Copyright (C) 2017 SiFive
> + */
> +
> +#ifndef _TOOLS_LINUX_ASM_RISCV_BARRIER_H
> +#define _TOOLS_LINUX_ASM_RISCV_BARRIER_H
> +
> +#include <asm/fence.h>
> +#include <linux/compiler.h>
> +
> +/* These barriers need to enforce ordering on both devices and memory. */
> +#define mb()		RISCV_FENCE(iorw, iorw)
> +#define rmb()		RISCV_FENCE(ir, ir)
> +#define wmb()		RISCV_FENCE(ow, ow)
> +
> +/* These barriers do not need to enforce ordering on devices, just memory. */
> +#define smp_mb()	RISCV_FENCE(rw, rw)
> +#define smp_rmb()	RISCV_FENCE(r, r)
> +#define smp_wmb()	RISCV_FENCE(w, w)
> +
> +#define smp_store_release(p, v)						\
> +do {									\
> +	RISCV_FENCE(rw, w);						\
> +	WRITE_ONCE(*p, v);						\
> +} while (0)
> +
> +#define smp_load_acquire(p)						\
> +({									\
> +	typeof(*p) ___p1 = READ_ONCE(*p);				\
> +	RISCV_FENCE(r, rw);						\
> +	___p1;								\
> +})
> +
> +#endif /* _TOOLS_LINUX_ASM_RISCV_BARRIER_H */
> diff --git a/tools/arch/riscv/include/asm/fence.h b/tools/arch/riscv/include/asm/fence.h
> new file mode 100644
> index 000000000000..37860e86771d
> --- /dev/null
> +++ b/tools/arch/riscv/include/asm/fence.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copied from the kernel sources to tools/arch/riscv:
> + */
> +
> +#ifndef _ASM_RISCV_FENCE_H
> +#define _ASM_RISCV_FENCE_H
> +
> +#define RISCV_FENCE_ASM(p, s)		"\tfence " #p "," #s "\n"
> +#define RISCV_FENCE(p, s) \
> +	({ __asm__ __volatile__ (RISCV_FENCE_ASM(p, s) : : : "memory"); })
> +
> +#endif	/* _ASM_RISCV_FENCE_H */
> diff --git a/tools/include/asm/barrier.h b/tools/include/asm/barrier.h
> index 8d378c57cb01..0c21678ac5e6 100644
> --- a/tools/include/asm/barrier.h
> +++ b/tools/include/asm/barrier.h
> @@ -8,6 +8,8 @@
>  #include "../../arch/arm64/include/asm/barrier.h"
>  #elif defined(__powerpc__)
>  #include "../../arch/powerpc/include/asm/barrier.h"
> +#elif defined(__riscv)
> +#include "../../arch/riscv/include/asm/barrier.h"
>  #elif defined(__s390__)
>  #include "../../arch/s390/include/asm/barrier.h"
>  #elif defined(__sh__)
> 

Can not really tell for that part except it seems ok to me as well.
Andrea might be a better candidate to add its Rb.

Thanks,

Clément