From: Andrew Jones <ajones@ventanamicro.com>
To: kvm-riscv@lists.infradead.org
Subject: [PATCH v2 3/6] riscv: Add Zawrs support for spinlocks
Date: Mon, 22 Apr 2024 10:36:02 +0200 [thread overview]
Message-ID: <20240422-97341bd5e6f69d54eeaba632@orel> (raw)
In-Reply-To: <ZiWCP6f6zZ3dKXfN@andrea>
On Sun, Apr 21, 2024 at 11:16:47PM +0200, Andrea Parri wrote:
> On Fri, Apr 19, 2024 at 03:53:25PM +0200, Andrew Jones wrote:
> > From: Christoph M??llner <christoph.muellner@vrull.eu>
> >
> > RISC-V code uses the generic ticket lock implementation, which calls
> > the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> > Introduce a RISC-V specific implementation of smp_cond_load_relaxed()
> > which applies WRS.NTO of the Zawrs extension in order to reduce power
> > consumption while waiting and allows hypervisors to enable guests to
> > trap while waiting. smp_cond_load_acquire() doesn't need a RISC-V
> > specific implementation as the generic implementation is based on
> > smp_cond_load_relaxed() and smp_acquire__after_ctrl_dep() sufficiently
> > provides the acquire semantics.
> >
> > This implementation is heavily based on Arm's approach which is the
> > approach Andrea Parri also suggested.
> >
> > The Zawrs specification can be found here:
> > https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> >
> > Signed-off-by: Christoph M??llner <christoph.muellner@vrull.eu>
> > Co-developed-by: Andrew Jones <ajones@ventanamicro.com>
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> > arch/riscv/Kconfig | 13 ++++++++
> > arch/riscv/include/asm/barrier.h | 45 ++++++++++++++++++---------
> > arch/riscv/include/asm/cmpxchg.h | 51 +++++++++++++++++++++++++++++++
> > arch/riscv/include/asm/hwcap.h | 1 +
> > arch/riscv/include/asm/insn-def.h | 2 ++
> > arch/riscv/kernel/cpufeature.c | 1 +
> > 6 files changed, 98 insertions(+), 15 deletions(-)
>
> Doesn't apply to riscv/for-next (due to, AFAIU,
>
> https://lore.kernel.org/all/171275883330.18495.10110341843571163280.git-patchwork-notify at kernel.org/ ).
I based it on -rc1. We recently discussed what we should base on, but
I couldn't recall the final decision, so I fell back to the old approach.
I can rebase on for-next or the latest rc if that's the new, improved
approach.
>
> But other than that, this LGTM. One nit below.
>
>
> > -#define __smp_store_release(p, v) \
> > -do { \
> > - compiletime_assert_atomic_type(*p); \
> > - RISCV_FENCE(rw, w); \
> > - WRITE_ONCE(*p, v); \
> > -} while (0)
> > -
> > -#define __smp_load_acquire(p) \
> > -({ \
> > - typeof(*p) ___p1 = READ_ONCE(*p); \
> > - compiletime_assert_atomic_type(*p); \
> > - RISCV_FENCE(r, rw); \
> > - ___p1; \
> > -})
> > -
> > /*
> > * This is a very specific barrier: it's currently only used in two places in
> > * the kernel, both in the scheduler. See include/linux/spinlock.h for the two
> > @@ -70,6 +56,35 @@ do { \
> > */
> > #define smp_mb__after_spinlock() RISCV_FENCE(iorw, iorw)
> >
> > +#define __smp_store_release(p, v) \
> > +do { \
> > + compiletime_assert_atomic_type(*p); \
> > + RISCV_FENCE(rw, w); \
> > + WRITE_ONCE(*p, v); \
> > +} while (0)
> > +
> > +#define __smp_load_acquire(p) \
> > +({ \
> > + typeof(*p) ___p1 = READ_ONCE(*p); \
> > + compiletime_assert_atomic_type(*p); \
> > + RISCV_FENCE(r, rw); \
> > + ___p1; \
> > +})
>
> Unrelated/unmotivated changes.
The relation/motivation was to get the load/store macros in one part of
the file with the barrier macros in another. With this change we have
__mb
__rmb
__wmb
__smp_mb
__smp_rmb
__smp_wmb
smp_mb__after_spinlock
__smp_store_release
__smp_load_acquire
smp_cond_load_relaxed
Without the change, smp_mb__after_spinlock is either after all the
load/stores or in between them.
I didn't think the reorganization was worth its own patch, but I could
split it out (or just drop it).
Thanks,
drew
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Jones <ajones@ventanamicro.com>
To: Andrea Parri <parri.andrea@gmail.com>
Cc: linux-riscv@lists.infradead.org, kvm-riscv@lists.infradead.org,
devicetree@vger.kernel.org, paul.walmsley@sifive.com,
palmer@dabbelt.com, aou@eecs.berkeley.edu,
conor.dooley@microchip.com, anup@brainfault.org,
atishp@atishpatra.org, robh@kernel.org,
krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
christoph.muellner@vrull.eu, heiko@sntech.de,
charlie@rivosinc.com, David.Laight@aculab.com,
luxu.kernel@bytedance.com
Subject: Re: [PATCH v2 3/6] riscv: Add Zawrs support for spinlocks
Date: Mon, 22 Apr 2024 10:36:02 +0200 [thread overview]
Message-ID: <20240422-97341bd5e6f69d54eeaba632@orel> (raw)
In-Reply-To: <ZiWCP6f6zZ3dKXfN@andrea>
On Sun, Apr 21, 2024 at 11:16:47PM +0200, Andrea Parri wrote:
> On Fri, Apr 19, 2024 at 03:53:25PM +0200, Andrew Jones wrote:
> > From: Christoph M??llner <christoph.muellner@vrull.eu>
> >
> > RISC-V code uses the generic ticket lock implementation, which calls
> > the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> > Introduce a RISC-V specific implementation of smp_cond_load_relaxed()
> > which applies WRS.NTO of the Zawrs extension in order to reduce power
> > consumption while waiting and allows hypervisors to enable guests to
> > trap while waiting. smp_cond_load_acquire() doesn't need a RISC-V
> > specific implementation as the generic implementation is based on
> > smp_cond_load_relaxed() and smp_acquire__after_ctrl_dep() sufficiently
> > provides the acquire semantics.
> >
> > This implementation is heavily based on Arm's approach which is the
> > approach Andrea Parri also suggested.
> >
> > The Zawrs specification can be found here:
> > https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> >
> > Signed-off-by: Christoph M??llner <christoph.muellner@vrull.eu>
> > Co-developed-by: Andrew Jones <ajones@ventanamicro.com>
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> > arch/riscv/Kconfig | 13 ++++++++
> > arch/riscv/include/asm/barrier.h | 45 ++++++++++++++++++---------
> > arch/riscv/include/asm/cmpxchg.h | 51 +++++++++++++++++++++++++++++++
> > arch/riscv/include/asm/hwcap.h | 1 +
> > arch/riscv/include/asm/insn-def.h | 2 ++
> > arch/riscv/kernel/cpufeature.c | 1 +
> > 6 files changed, 98 insertions(+), 15 deletions(-)
>
> Doesn't apply to riscv/for-next (due to, AFAIU,
>
> https://lore.kernel.org/all/171275883330.18495.10110341843571163280.git-patchwork-notify@kernel.org/ ).
I based it on -rc1. We recently discussed what we should base on, but
I couldn't recall the final decision, so I fell back to the old approach.
I can rebase on for-next or the latest rc if that's the new, improved
approach.
>
> But other than that, this LGTM. One nit below.
>
>
> > -#define __smp_store_release(p, v) \
> > -do { \
> > - compiletime_assert_atomic_type(*p); \
> > - RISCV_FENCE(rw, w); \
> > - WRITE_ONCE(*p, v); \
> > -} while (0)
> > -
> > -#define __smp_load_acquire(p) \
> > -({ \
> > - typeof(*p) ___p1 = READ_ONCE(*p); \
> > - compiletime_assert_atomic_type(*p); \
> > - RISCV_FENCE(r, rw); \
> > - ___p1; \
> > -})
> > -
> > /*
> > * This is a very specific barrier: it's currently only used in two places in
> > * the kernel, both in the scheduler. See include/linux/spinlock.h for the two
> > @@ -70,6 +56,35 @@ do { \
> > */
> > #define smp_mb__after_spinlock() RISCV_FENCE(iorw, iorw)
> >
> > +#define __smp_store_release(p, v) \
> > +do { \
> > + compiletime_assert_atomic_type(*p); \
> > + RISCV_FENCE(rw, w); \
> > + WRITE_ONCE(*p, v); \
> > +} while (0)
> > +
> > +#define __smp_load_acquire(p) \
> > +({ \
> > + typeof(*p) ___p1 = READ_ONCE(*p); \
> > + compiletime_assert_atomic_type(*p); \
> > + RISCV_FENCE(r, rw); \
> > + ___p1; \
> > +})
>
> Unrelated/unmotivated changes.
The relation/motivation was to get the load/store macros in one part of
the file with the barrier macros in another. With this change we have
__mb
__rmb
__wmb
__smp_mb
__smp_rmb
__smp_wmb
smp_mb__after_spinlock
__smp_store_release
__smp_load_acquire
smp_cond_load_relaxed
Without the change, smp_mb__after_spinlock is either after all the
load/stores or in between them.
I didn't think the reorganization was worth its own patch, but I could
split it out (or just drop it).
Thanks,
drew
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Jones <ajones@ventanamicro.com>
To: Andrea Parri <parri.andrea@gmail.com>
Cc: linux-riscv@lists.infradead.org, kvm-riscv@lists.infradead.org,
devicetree@vger.kernel.org, paul.walmsley@sifive.com,
palmer@dabbelt.com, aou@eecs.berkeley.edu,
conor.dooley@microchip.com, anup@brainfault.org,
atishp@atishpatra.org, robh@kernel.org,
krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
christoph.muellner@vrull.eu, heiko@sntech.de,
charlie@rivosinc.com, David.Laight@aculab.com,
luxu.kernel@bytedance.com
Subject: Re: [PATCH v2 3/6] riscv: Add Zawrs support for spinlocks
Date: Mon, 22 Apr 2024 10:36:02 +0200 [thread overview]
Message-ID: <20240422-97341bd5e6f69d54eeaba632@orel> (raw)
In-Reply-To: <ZiWCP6f6zZ3dKXfN@andrea>
On Sun, Apr 21, 2024 at 11:16:47PM +0200, Andrea Parri wrote:
> On Fri, Apr 19, 2024 at 03:53:25PM +0200, Andrew Jones wrote:
> > From: Christoph M??llner <christoph.muellner@vrull.eu>
> >
> > RISC-V code uses the generic ticket lock implementation, which calls
> > the macros smp_cond_load_relaxed() and smp_cond_load_acquire().
> > Introduce a RISC-V specific implementation of smp_cond_load_relaxed()
> > which applies WRS.NTO of the Zawrs extension in order to reduce power
> > consumption while waiting and allows hypervisors to enable guests to
> > trap while waiting. smp_cond_load_acquire() doesn't need a RISC-V
> > specific implementation as the generic implementation is based on
> > smp_cond_load_relaxed() and smp_acquire__after_ctrl_dep() sufficiently
> > provides the acquire semantics.
> >
> > This implementation is heavily based on Arm's approach which is the
> > approach Andrea Parri also suggested.
> >
> > The Zawrs specification can be found here:
> > https://github.com/riscv/riscv-zawrs/blob/main/zawrs.adoc
> >
> > Signed-off-by: Christoph M??llner <christoph.muellner@vrull.eu>
> > Co-developed-by: Andrew Jones <ajones@ventanamicro.com>
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> > arch/riscv/Kconfig | 13 ++++++++
> > arch/riscv/include/asm/barrier.h | 45 ++++++++++++++++++---------
> > arch/riscv/include/asm/cmpxchg.h | 51 +++++++++++++++++++++++++++++++
> > arch/riscv/include/asm/hwcap.h | 1 +
> > arch/riscv/include/asm/insn-def.h | 2 ++
> > arch/riscv/kernel/cpufeature.c | 1 +
> > 6 files changed, 98 insertions(+), 15 deletions(-)
>
> Doesn't apply to riscv/for-next (due to, AFAIU,
>
> https://lore.kernel.org/all/171275883330.18495.10110341843571163280.git-patchwork-notify@kernel.org/ ).
I based it on -rc1. We recently discussed what we should base on, but
I couldn't recall the final decision, so I fell back to the old approach.
I can rebase on for-next or the latest rc if that's the new, improved
approach.
>
> But other than that, this LGTM. One nit below.
>
>
> > -#define __smp_store_release(p, v) \
> > -do { \
> > - compiletime_assert_atomic_type(*p); \
> > - RISCV_FENCE(rw, w); \
> > - WRITE_ONCE(*p, v); \
> > -} while (0)
> > -
> > -#define __smp_load_acquire(p) \
> > -({ \
> > - typeof(*p) ___p1 = READ_ONCE(*p); \
> > - compiletime_assert_atomic_type(*p); \
> > - RISCV_FENCE(r, rw); \
> > - ___p1; \
> > -})
> > -
> > /*
> > * This is a very specific barrier: it's currently only used in two places in
> > * the kernel, both in the scheduler. See include/linux/spinlock.h for the two
> > @@ -70,6 +56,35 @@ do { \
> > */
> > #define smp_mb__after_spinlock() RISCV_FENCE(iorw, iorw)
> >
> > +#define __smp_store_release(p, v) \
> > +do { \
> > + compiletime_assert_atomic_type(*p); \
> > + RISCV_FENCE(rw, w); \
> > + WRITE_ONCE(*p, v); \
> > +} while (0)
> > +
> > +#define __smp_load_acquire(p) \
> > +({ \
> > + typeof(*p) ___p1 = READ_ONCE(*p); \
> > + compiletime_assert_atomic_type(*p); \
> > + RISCV_FENCE(r, rw); \
> > + ___p1; \
> > +})
>
> Unrelated/unmotivated changes.
The relation/motivation was to get the load/store macros in one part of
the file with the barrier macros in another. With this change we have
__mb
__rmb
__wmb
__smp_mb
__smp_rmb
__smp_wmb
smp_mb__after_spinlock
__smp_store_release
__smp_load_acquire
smp_cond_load_relaxed
Without the change, smp_mb__after_spinlock is either after all the
load/stores or in between them.
I didn't think the reorganization was worth its own patch, but I could
split it out (or just drop it).
Thanks,
drew
next prev parent reply other threads:[~2024-04-22 8:36 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-19 13:53 [PATCH v2 0/6] riscv: Apply Zawrs when available Andrew Jones
2024-04-19 13:53 ` Andrew Jones
2024-04-19 13:53 ` Andrew Jones
2024-04-19 13:53 ` [PATCH v2 1/6] riscv: Provide a definition for 'pause' Andrew Jones
2024-04-19 13:53 ` Andrew Jones
2024-04-19 13:53 ` Andrew Jones
2024-04-19 13:53 ` [PATCH v2 2/6] dt-bindings: riscv: Add Zawrs ISA extension description Andrew Jones
2024-04-19 13:53 ` Andrew Jones
2024-04-19 13:53 ` Andrew Jones
2024-04-19 14:45 ` Conor Dooley
2024-04-19 14:45 ` Conor Dooley
2024-04-19 14:45 ` Conor Dooley
2024-04-19 15:16 ` Andrew Jones
2024-04-19 15:16 ` Andrew Jones
2024-04-19 15:16 ` Andrew Jones
2024-04-19 15:19 ` Conor Dooley
2024-04-19 15:19 ` Conor Dooley
2024-04-19 15:19 ` Conor Dooley
2024-04-19 16:40 ` Charlie Jenkins
2024-04-19 16:40 ` Charlie Jenkins
2024-04-19 16:40 ` Charlie Jenkins
2024-04-21 10:20 ` Andrew Jones
2024-04-21 10:20 ` Andrew Jones
2024-04-21 10:20 ` Andrew Jones
2024-04-22 22:36 ` Charlie Jenkins
2024-04-22 22:36 ` Charlie Jenkins
2024-04-22 22:36 ` Charlie Jenkins
2024-04-23 8:46 ` Andrew Jones
2024-04-23 8:46 ` Andrew Jones
2024-04-23 8:46 ` Andrew Jones
2024-04-23 9:05 ` Conor Dooley
2024-04-23 9:05 ` Conor Dooley
2024-04-23 9:05 ` Conor Dooley
2024-04-23 18:00 ` Charlie Jenkins
2024-04-23 18:00 ` Charlie Jenkins
2024-04-23 18:00 ` Charlie Jenkins
2024-04-23 19:42 ` Charlie Jenkins
2024-04-23 19:42 ` Charlie Jenkins
2024-04-23 19:42 ` Charlie Jenkins
2024-04-24 7:34 ` Andrew Jones
2024-04-24 7:34 ` Andrew Jones
2024-04-24 7:34 ` Andrew Jones
2024-04-24 9:23 ` Christoph Müllner
2024-04-24 9:23 ` Christoph Müllner
2024-04-24 9:23 ` Christoph Müllner
2024-04-24 10:32 ` Andrew Jones
2024-04-24 10:32 ` Andrew Jones
2024-04-24 10:32 ` Andrew Jones
2024-04-19 13:53 ` [PATCH v2 3/6] riscv: Add Zawrs support for spinlocks Andrew Jones
2024-04-19 13:53 ` Andrew Jones
2024-04-19 13:53 ` Andrew Jones
2024-04-19 15:22 ` Conor Dooley
2024-04-19 15:22 ` Conor Dooley
2024-04-19 15:22 ` Conor Dooley
2024-04-21 21:16 ` Andrea Parri
2024-04-21 21:16 ` Andrea Parri
2024-04-21 21:16 ` Andrea Parri
2024-04-22 8:36 ` Andrew Jones [this message]
2024-04-22 8:36 ` Andrew Jones
2024-04-22 8:36 ` Andrew Jones
2024-04-19 13:53 ` [PATCH v2 4/6] riscv: hwprobe: export Zawrs ISA extension Andrew Jones
2024-04-19 13:53 ` Andrew Jones
2024-04-19 13:53 ` Andrew Jones
2024-04-19 13:53 ` [PATCH v2 5/6] KVM: riscv: Support guest wrs.nto Andrew Jones
2024-04-19 13:53 ` Andrew Jones
2024-04-19 13:53 ` Andrew Jones
2024-04-19 13:53 ` [PATCH v2 6/6] KVM: riscv: selftests: Add Zawrs extension to get-reg-list test Andrew Jones
2024-04-19 13:53 ` Andrew Jones
2024-04-19 13:53 ` Andrew Jones
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=20240422-97341bd5e6f69d54eeaba632@orel \
--to=ajones@ventanamicro.com \
--cc=kvm-riscv@lists.infradead.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.