Linux-csky Archive mirror
 help / color / mirror / Atom feed
From: Leonardo Bras <leobras@redhat.com>
To: Guo Ren <guoren@kernel.org>
Cc: paul.walmsley@sifive.com, anup@brainfault.org,
	peterz@infradead.org, mingo@redhat.com, will@kernel.org,
	palmer@rivosinc.com, longman@redhat.com, boqun.feng@gmail.com,
	tglx@linutronix.de, paulmck@kernel.org, rostedt@goodmis.org,
	rdunlap@infradead.org, catalin.marinas@arm.com,
	conor.dooley@microchip.com, xiaoguang.xing@sophgo.com,
	bjorn@rivosinc.com, alexghiti@rivosinc.com,
	keescook@chromium.org, greentime.hu@sifive.com,
	ajones@ventanamicro.com, jszhang@kernel.org, wefu@redhat.com,
	wuwei2016@iscas.ac.cn, linux-arch@vger.kernel.org,
	linux-riscv@lists.infradead.org, linux-doc@vger.kernel.org,
	kvm@vger.kernel.org, virtualization@lists.linux-foundation.org,
	linux-csky@vger.kernel.org, Guo Ren <guoren@linux.alibaba.com>
Subject: Re: [PATCH V11 03/17] riscv: Use Zicbop in arch_xchg when available
Date: Tue, 19 Sep 2023 02:13:39 -0300	[thread overview]
Message-ID: <ZQkuA7WloWIIteVR@redhat.com> (raw)
In-Reply-To: <CAJF2gTQLBNy9uS4AF+UgD+ew3BN1dLs0f0+z0jzpieR75kv_Dw@mail.gmail.com>

On Sun, Sep 17, 2023 at 10:34:36PM +0800, Guo Ren wrote:
> On Sat, Sep 16, 2023 at 9:25 AM Leonardo Bras <leobras@redhat.com> wrote:
> >
> > On Fri, Sep 15, 2023 at 08:36:31PM +0800, Guo Ren wrote:
> > > On Wed, Sep 13, 2023 at 4:50 PM Leonardo Bras <leobras@redhat.com> wrote:
> > > >
> > > > On Sun, Sep 10, 2023 at 04:28:57AM -0400, guoren@kernel.org wrote:
> > > > > From: Guo Ren <guoren@linux.alibaba.com>
> > > > >
> > > > > Cache-block prefetch instructions are HINTs to the hardware to
> > > > > indicate that software intends to perform a particular type of
> > > > > memory access in the near future. Enable ARCH_HAS_PREFETCHW and
> > > > > improve the arch_xchg for qspinlock xchg_tail.
> > > > >
> > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
> > > > > Signed-off-by: Guo Ren <guoren@kernel.org>
> > > > > ---
> > > > >  arch/riscv/Kconfig                 | 15 +++++++++++++++
> > > > >  arch/riscv/include/asm/cmpxchg.h   |  4 +++-
> > > > >  arch/riscv/include/asm/hwcap.h     |  1 +
> > > > >  arch/riscv/include/asm/insn-def.h  |  5 +++++
> > > > >  arch/riscv/include/asm/processor.h | 13 +++++++++++++
> > > > >  arch/riscv/kernel/cpufeature.c     |  1 +
> > > > >  6 files changed, 38 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > > index e9ae6fa232c3..2c346fe169c1 100644
> > > > > --- a/arch/riscv/Kconfig
> > > > > +++ b/arch/riscv/Kconfig
> > > > > @@ -617,6 +617,21 @@ config RISCV_ISA_ZICBOZ
> > > > >
> > > > >          If you don't know what to do here, say Y.
> > > > >
> > > > > +config RISCV_ISA_ZICBOP
> > > > > +     bool "Zicbop extension support for cache block prefetch"
> > > > > +     depends on MMU
> > > > > +     depends on RISCV_ALTERNATIVE
> > > > > +     default y
> > > > > +     help
> > > > > +        Adds support to dynamically detect the presence of the ZICBOP
> > > > > +        extension (Cache Block Prefetch Operations) and enable its
> > > > > +        usage.
> > > > > +
> > > > > +        The Zicbop extension can be used to prefetch cache block for
> > > > > +        read/write/instruction fetch.
> > > > > +
> > > > > +        If you don't know what to do here, say Y.
> > > > > +
> > > > >  config TOOLCHAIN_HAS_ZIHINTPAUSE
> > > > >       bool
> > > > >       default y
> > > > > diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> > > > > index 702725727671..56eff7a9d2d2 100644
> > > > > --- a/arch/riscv/include/asm/cmpxchg.h
> > > > > +++ b/arch/riscv/include/asm/cmpxchg.h
> > > > > @@ -11,6 +11,7 @@
> > > > >
> > > > >  #include <asm/barrier.h>
> > > > >  #include <asm/fence.h>
> > > > > +#include <asm/processor.h>
> > > > >
> > > > >  #define __arch_xchg_masked(prepend, append, r, p, n)                 \
> > > > >  ({                                                                   \
> > > > > @@ -25,6 +26,7 @@
> > > > >                                                                       \
> > > > >       __asm__ __volatile__ (                                          \
> > > > >              prepend                                                  \
> > > > > +            PREFETCHW_ASM(%5)                                        \
> > > > >              "0:      lr.w %0, %2\n"                                  \
> > > > >              "        and  %1, %0, %z4\n"                             \
> > > > >              "        or   %1, %1, %z3\n"                             \
> > > > > @@ -32,7 +34,7 @@
> > > > >              "        bnez %1, 0b\n"                                  \
> > > > >              append                                                   \
> > > > >              : "=&r" (__retx), "=&r" (__rc), "+A" (*(__ptr32b))       \
> > > > > -            : "rJ" (__newx), "rJ" (~__mask)                          \
> > > > > +            : "rJ" (__newx), "rJ" (~__mask), "rJ" (__ptr32b)         \
> > > > >              : "memory");                                             \
> > > > >                                                                       \
> > > > >       r = (__typeof__(*(p)))((__retx & __mask) >> __s);               \
> > > > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > > > > index b7b58258f6c7..78b7b8b53778 100644
> > > > > --- a/arch/riscv/include/asm/hwcap.h
> > > > > +++ b/arch/riscv/include/asm/hwcap.h
> > > > > @@ -58,6 +58,7 @@
> > > > >  #define RISCV_ISA_EXT_ZICSR          40
> > > > >  #define RISCV_ISA_EXT_ZIFENCEI               41
> > > > >  #define RISCV_ISA_EXT_ZIHPM          42
> > > > > +#define RISCV_ISA_EXT_ZICBOP         43
> > > > >
> > > > >  #define RISCV_ISA_EXT_MAX            64
> > > > >
> > > > > diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> > > > > index 6960beb75f32..dc590d331894 100644
> > > > > --- a/arch/riscv/include/asm/insn-def.h
> > > > > +++ b/arch/riscv/include/asm/insn-def.h
> > > > > @@ -134,6 +134,7 @@
> > > > >
> > > > >  #define RV_OPCODE_MISC_MEM   RV_OPCODE(15)
> > > > >  #define RV_OPCODE_SYSTEM     RV_OPCODE(115)
> > > > > +#define RV_OPCODE_PREFETCH   RV_OPCODE(19)
> > > > >
> > > > >  #define HFENCE_VVMA(vaddr, asid)                             \
> > > > >       INSN_R(OPCODE_SYSTEM, FUNC3(0), FUNC7(17),              \
> > > > > @@ -196,4 +197,8 @@
> > > > >       INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0),              \
> > > > >              RS1(base), SIMM12(4))
> > > > >
> > > > > +#define CBO_prefetchw(base)                                  \
> > > > > +     INSN_R(OPCODE_PREFETCH, FUNC3(6), FUNC7(0),             \
> > > > > +            RD(x0), RS1(base), RS2(x0))
> > > > > +
> > > >
> > > > I understand that here you create the instruction via bitfield, following
> > > > the ISA, and this enables using instructions not available on the
> > > > toolchain.
> > > >
> > > > It took me some time to find the document with this instruction, so please
> > > > add this to the commit msg:
> > > >
> > > > https://github.com/riscv/riscv-CMOs/blob/master/specifications/cmobase-v1.0.pdf
> > > > Page 23.
> > > >
> > > > IIUC, the instruction is "prefetch.w".
> > > >
> > > > Maybe I am missing something, but in the document the rs2 field
> > > > (PREFETCH.W) contains a 0x3, while the above looks to have a 0 instead.
> > > >
> > > > rs2 field = 0x0 would be a prefetch.i (instruction prefetch) instead.
> > > >
> > > > Is the above correct, or am I missing something?
> > > Oh, you are right. My fault, thx for pointing out. It should be:
> > > +       INSN_R(OPCODE_PREFETCH, FUNC3(6), FUNC7(0),             \
> > > +              RD(x0), RS1(base), RS2(x3))
> >
> > Now I am curious to check if / how will this impact performance. :)
> > (Please let me know)
> Ref:
> commit 0ea366f5e1b6 ("arm64: atomics: prefetch the destination word
> for write prior to stxr")
> commit 86d231459d6d ("bpf: cpumap memory prefetchw optimizations for
> struct page")

Oh, I understand that prefetch.w is very useful for performance :)

What I meant is that previously this patch was issuing a prefetch.i,
and now it's issuing a prefetch.w (as intended). 

What got me curious is how much would it impact the performance to change 
the prefetch.i to prefetch.w. :)

Thanks!
Leo


> 
> >
> >
> > >
> > > >
> > > >
> > > > Thanks!
> > > > Leo
> > > >
> > > > >  #endif /* __ASM_INSN_DEF_H */
> > > > > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> > > > > index de9da852f78d..7ad3a24212e8 100644
> > > > > --- a/arch/riscv/include/asm/processor.h
> > > > > +++ b/arch/riscv/include/asm/processor.h
> > > > > @@ -12,6 +12,8 @@
> > > > >  #include <vdso/processor.h>
> > > > >
> > > > >  #include <asm/ptrace.h>
> > > > > +#include <asm/insn-def.h>
> > > > > +#include <asm/hwcap.h>
> > > > >
> > > > >  #ifdef CONFIG_64BIT
> > > > >  #define DEFAULT_MAP_WINDOW   (UL(1) << (MMAP_VA_BITS - 1))
> > > > > @@ -103,6 +105,17 @@ static inline void arch_thread_struct_whitelist(unsigned long *offset,
> > > > >  #define KSTK_EIP(tsk)                (ulong)(task_pt_regs(tsk)->epc)
> > > > >  #define KSTK_ESP(tsk)                (ulong)(task_pt_regs(tsk)->sp)
> > > > >
> > > > > +#define ARCH_HAS_PREFETCHW
> > > > > +#define PREFETCHW_ASM(base)  ALTERNATIVE(__nops(1), \
> > > > > +                                         CBO_prefetchw(base), \
> > > > > +                                         0, \
> > > > > +                                         RISCV_ISA_EXT_ZICBOP, \
> > > > > +                                         CONFIG_RISCV_ISA_ZICBOP)
> > > > > +static inline void prefetchw(const void *ptr)
> > > > > +{
> > > > > +     asm volatile(PREFETCHW_ASM(%0)
> > > > > +             : : "r" (ptr) : "memory");
> > > > > +}
> > > > >
> > > > >  /* Do necessary setup to start up a newly executed thread. */
> > > > >  extern void start_thread(struct pt_regs *regs,
> > > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > > > index ef7b4fd9e876..e0b897db0b97 100644
> > > > > --- a/arch/riscv/kernel/cpufeature.c
> > > > > +++ b/arch/riscv/kernel/cpufeature.c
> > > > > @@ -159,6 +159,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> > > > >       __RISCV_ISA_EXT_DATA(h, RISCV_ISA_EXT_h),
> > > > >       __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
> > > > >       __RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
> > > > > +     __RISCV_ISA_EXT_DATA(zicbop, RISCV_ISA_EXT_ZICBOP),
> > > > >       __RISCV_ISA_EXT_DATA(zicntr, RISCV_ISA_EXT_ZICNTR),
> > > > >       __RISCV_ISA_EXT_DATA(zicsr, RISCV_ISA_EXT_ZICSR),
> > > > >       __RISCV_ISA_EXT_DATA(zifencei, RISCV_ISA_EXT_ZIFENCEI),
> > > > > --
> > > > > 2.36.1
> > > > >
> > > >
> > >
> > >
> > > --
> > > Best Regards
> > >  Guo Ren
> > >
> >
> 
> 
> -- 
> Best Regards
>  Guo Ren
> 


  reply	other threads:[~2023-09-19  5:14 UTC|newest]

Thread overview: 105+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-10  8:28 [PATCH V11 00/17] riscv: Add Native/Paravirt qspinlock support guoren
2023-09-10  8:28 ` [PATCH V11 01/17] asm-generic: ticket-lock: Reuse arch_spinlock_t of qspinlock guoren
2023-09-11 19:05   ` Leonardo Brás
2023-09-13  1:55     ` Guo Ren
2023-09-13  7:59       ` Leonardo Bras
2023-09-10  8:28 ` [PATCH V11 02/17] asm-generic: ticket-lock: Move into ticket_spinlock.h guoren
2023-09-13  8:15   ` Leonardo Bras
2023-09-10  8:28 ` [PATCH V11 03/17] riscv: Use Zicbop in arch_xchg when available guoren
2023-09-13  8:49   ` Leonardo Bras
2023-09-15 12:36     ` Guo Ren
2023-09-16  1:25       ` Leonardo Bras
2023-09-17 14:34         ` Guo Ren
2023-09-19  5:13           ` Leonardo Bras [this message]
2023-09-19  7:53             ` Guo Ren
2023-09-19 14:38               ` Leonardo Bras
2023-09-14 13:47   ` Andrew Jones
2023-09-15  8:22     ` Leonardo Bras
2023-09-15 11:07       ` Andrew Jones
2023-09-15 11:26         ` Conor Dooley
2023-09-15 12:22           ` Andrew Jones
2023-09-15 12:42             ` Conor Dooley
2023-09-16  0:05               ` Conor Dooley
2023-09-15 20:32         ` Leonardo Bras
2023-09-14 14:25   ` Andrew Jones
2023-09-14 14:47     ` Andrew Jones
2023-09-15 11:37       ` Conor Dooley
2023-09-15 12:14         ` Andrew Jones
2023-09-15 12:53           ` Conor Dooley
2023-12-31  8:29   ` guoren
2023-09-10  8:28 ` [PATCH V11 04/17] locking/qspinlock: Improve xchg_tail for number of cpus >= 16k guoren
2023-09-11  2:35   ` Waiman Long
2023-09-11  3:09     ` Guo Ren
2023-09-11 13:03       ` Waiman Long
2023-09-12  1:10         ` Guo Ren
2023-09-13  8:55           ` Leonardo Bras
2023-09-13 12:52             ` Guo Ren
2023-09-13 13:06               ` Waiman Long
2023-09-14  3:45                 ` Guo Ren
2023-09-10  8:28 ` [PATCH V11 05/17] riscv: qspinlock: Add basic queued_spinlock support guoren
2023-09-13 20:28   ` Leonardo Bras
2023-09-14  4:46     ` Guo Ren
2023-09-14  9:43       ` Leonardo Bras
2023-09-15  2:10         ` Guo Ren
2023-09-15  9:08           ` Leonardo Bras
2023-09-17 15:02             ` Guo Ren
2023-09-19  5:20               ` Leonardo Bras
2023-09-10  8:29 ` [PATCH V11 06/17] riscv: qspinlock: Introduce combo spinlock guoren
2023-09-10 11:06   ` Guo Ren
2023-09-13 20:37     ` Leonardo Bras
2023-09-13 20:49       ` Leonardo Bras
2023-09-14  4:49         ` Guo Ren
2023-09-14  7:17           ` Leonardo Bras
2023-09-10  8:29 ` [PATCH V11 07/17] riscv: qspinlock: Introduce qspinlock param for command line guoren
2023-09-11 15:22   ` Waiman Long
2023-09-12  1:06     ` Guo Ren
2023-09-11 15:34   ` Waiman Long
2023-09-12  1:08     ` Guo Ren
2023-09-14  7:32       ` Leonardo Bras
2023-09-14 17:23         ` Waiman Long
2023-09-10  8:29 ` [PATCH V11 08/17] riscv: qspinlock: Add virt_spin_lock() support for KVM guest guoren
2023-09-14  8:02   ` Leonardo Bras
2023-09-17 15:12     ` Guo Ren
2023-09-19  5:30       ` Leonardo Bras
2023-09-19  8:04         ` Guo Ren
2023-09-19 14:40           ` Leonardo Bras
2023-09-10  8:29 ` [PATCH V11 09/17] riscv: qspinlock: errata: Add ERRATA_THEAD_WRITE_ONCE fixup guoren
2023-09-14  8:32   ` Leonardo Bras
2023-09-17 15:15     ` Guo Ren
2023-09-19  5:34       ` Leonardo Bras
2023-09-10  8:29 ` [PATCH V11 10/17] riscv: qspinlock: errata: Enable qspinlock for T-HEAD processors guoren
2023-09-14  9:36   ` Leonardo Bras
2023-09-10  8:29 ` [PATCH V11 11/17] RISC-V: paravirt: pvqspinlock: Add paravirt qspinlock skeleton guoren
2023-09-15  5:42   ` Leonardo Bras
2023-09-17 14:58     ` Guo Ren
2023-09-19  5:43       ` Leonardo Bras
2023-09-10  8:29 ` [PATCH V11 12/17] RISC-V: paravirt: pvqspinlock: Add nopvspin kernel parameter guoren
2023-09-15  6:05   ` Leonardo Bras
2023-09-17 15:03     ` Guo Ren
2023-09-19  5:44       ` Leonardo Bras
2023-09-10  8:29 ` [PATCH V11 13/17] RISC-V: paravirt: pvqspinlock: Add SBI implementation guoren
2023-09-15  6:23   ` Leonardo Bras
2023-09-17 15:06     ` Guo Ren
2023-09-19  5:45       ` Leonardo Bras
2023-09-10  8:29 ` [PATCH V11 14/17] RISC-V: paravirt: pvqspinlock: Add kconfig entry guoren
2023-09-15  6:25   ` Leonardo Bras
2023-09-10  8:29 ` [PATCH V11 15/17] RISC-V: paravirt: pvqspinlock: Add trace point for pv_kick/wait guoren
2023-09-15  6:33   ` Leonardo Bras
2023-09-10  8:29 ` [PATCH V11 16/17] RISC-V: paravirt: pvqspinlock: KVM: Add paravirt qspinlock skeleton guoren
2023-09-15  6:46   ` Leonardo Bras
2023-09-10  8:29 ` [PATCH V11 17/17] RISC-V: paravirt: pvqspinlock: KVM: Implement kvm_sbi_ext_pvlock_kick_cpu() guoren
2023-09-15  6:52   ` Leonardo Bras
2023-09-10  8:58 ` [PATCH V11 00/17] riscv: Add Native/Paravirt qspinlock support Conor Dooley
2023-09-10  9:16   ` Guo Ren
2023-09-10  9:20     ` Guo Ren
2023-09-10  9:31     ` Conor Dooley
2023-09-10  9:49       ` Guo Ren
2023-09-10 19:45         ` Conor Dooley
2023-09-11  3:36           ` Guo Ren
2023-09-11 12:52             ` Conor Dooley
2023-09-12  1:33               ` Guo Ren
2023-09-12  8:07                 ` Conor Dooley
2023-09-12 10:58                   ` Guo Ren
2023-11-06 20:42 ` Leonardo Bras
2023-11-12  4:23   ` Guo Ren
2023-11-13 10:19     ` Leonardo Bras Soares Passos

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZQkuA7WloWIIteVR@redhat.com \
    --to=leobras@redhat.com \
    --cc=ajones@ventanamicro.com \
    --cc=alexghiti@rivosinc.com \
    --cc=anup@brainfault.org \
    --cc=bjorn@rivosinc.com \
    --cc=boqun.feng@gmail.com \
    --cc=catalin.marinas@arm.com \
    --cc=conor.dooley@microchip.com \
    --cc=greentime.hu@sifive.com \
    --cc=guoren@kernel.org \
    --cc=guoren@linux.alibaba.com \
    --cc=jszhang@kernel.org \
    --cc=keescook@chromium.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-csky@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=palmer@rivosinc.com \
    --cc=paul.walmsley@sifive.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rdunlap@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=wefu@redhat.com \
    --cc=will@kernel.org \
    --cc=wuwei2016@iscas.ac.cn \
    --cc=xiaoguang.xing@sophgo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).