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

[Charlie Jenkins <charlie@rivosinc.com>]

On Tue, Jul 30, 2024 at 10:59:52AM +0200, Clément Léger wrote:
> 
> 
> On 29/07/2024 22:50, Charlie Jenkins wrote:
> > Many of the other architectures use their custom barrier implmentations.
> 
> Hi Charlie,
> 
> Typo: implmentations -> implementations

Thank you! I will fix that.

- Charlie

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