From: Conor Dooley <conor.dooley@microchip.com>
To: Guo Ren <guoren@kernel.org>
Cc: Conor Dooley <conor@kernel.org>, <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>,
<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>,
<leobras@redhat.com>, <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 00/17] riscv: Add Native/Paravirt qspinlock support
Date: Mon, 11 Sep 2023 13:52:59 +0100 [thread overview]
Message-ID: <20230911-nimbly-outcome-496efae7adc6@wendy> (raw)
In-Reply-To: <CAJF2gTSP1rxVhuwOKyWiE2vFFijJFc2aKRU2=0rTK9nDc8AbsQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5010 bytes --]
On Mon, Sep 11, 2023 at 11:36:27AM +0800, Guo Ren wrote:
> On Mon, Sep 11, 2023 at 3:45 AM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Sun, Sep 10, 2023 at 05:49:13PM +0800, Guo Ren wrote:
> > > On Sun, Sep 10, 2023 at 5:32 PM Conor Dooley <conor@kernel.org> wrote:
> > > >
> > > > On Sun, Sep 10, 2023 at 05:16:46PM +0800, Guo Ren wrote:
> > > > > On Sun, Sep 10, 2023 at 4:58 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > >
> > > > > > On Sun, Sep 10, 2023 at 04:28:54AM -0400, guoren@kernel.org wrote:
> > > > > >
> > > > > > > Changlog:
> > > > > > > V11:
> > > > > > > - Based on Leonardo Bras's cmpxchg_small patches v5.
> > > > > > > - Based on Guo Ren's Optimize arch_spin_value_unlocked patch v3.
> > > > > > > - Remove abusing alternative framework and use jump_label instead.
> > > > > >
> > > > > > btw, I didn't say that using alternatives was the problem, it was
> > > > > > abusing the errata framework to perform feature detection that I had
> > > > > > a problem with. That's not changed in v11.
> > > > > I've removed errata feature detection. The only related patches are:
> > > > > - riscv: qspinlock: errata: Add ERRATA_THEAD_WRITE_ONCE fixup
> > > > > - riscv: qspinlock: errata: Enable qspinlock for T-HEAD processors
> > > > >
> > > > > Which one is your concern? Could you reply on the exact patch thread? Thx.
> > > >
> > > > riscv: qspinlock: errata: Enable qspinlock for T-HEAD processors
> > > >
> > > > Please go back and re-read the comments I left on v11 about using the
> > > > errata code for feature detection.
> > > >
> > > > > > A stronger forward progress guarantee is not an erratum, AFAICT.
> > > >
> > > > > Sorry, there is no erratum of "stronger forward progress guarantee" in the V11.
> > > >
> > > > "riscv: qspinlock: errata: Enable qspinlock for T-HEAD processors" still
> > > > uses the errata framework to detect the presence of the stronger forward
> > > > progress guarantee in v11.
> > > Oh, thx for pointing it out. I could replace it with this:
> > >
> > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > > index 88690751f2ee..4be92766d3e3 100644
> > > --- a/arch/riscv/kernel/setup.c
> > > +++ b/arch/riscv/kernel/setup.c
> > > @@ -310,7 +310,8 @@ static void __init riscv_spinlock_init(void)
> > > {
> > > #ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> > > if (!enable_qspinlock_key &&
> > > - (sbi_get_firmware_id() != SBI_EXT_BASE_IMPL_ID_KVM)) {
> > > + (sbi_get_firmware_id() != SBI_EXT_BASE_IMPL_ID_KVM) &&
> > > + (sbi_get_mvendorid() != THEAD_VENDOR_ID)) {
> > > static_branch_disable(&combo_qspinlock_key);
> > > pr_info("Ticket spinlock: enabled\n");
> > > } else {
> >
> > As I said on v11, I am opposed to feature probing using mvendorid & Co,
> > partially due to the exact sort of check here to see if the kernel is
> > running as a KVM guest. IMO, whether a platform has this stronger
> KVM can't use any fairness lock, so forcing it using a Test-Set lock
> or paravirt qspinlock is the right way. KVM is not a vendor platform.
My point is that KVM should be telling the guest what additional features
it is capable of using, rather than the kernel making some assumptions
based on$vendorid etc that are invalid when the kernel is running as a
KVM guest.
If the mvendorid etc related assumptions were dropped, the kernel would
then default away from your qspinlock & there'd not be a need to
special-case KVM AFAICT.
> > guarantee needs to be communicated by firmware, using ACPI or DT.
> > I made some comments on v11, referring similar discussion about the
> > thead vector stuff. Please go take a look at that.
> I prefer forcing T-HEAD processors using qspinlock, but if all people
> thought it must be in the ACPI or DT, I would compromise. Then, I
> would delete the qspinlock cmdline param patch and move it into DT.
>
> By the way, what's the kind of DT format? How about:
I added the new "riscv,isa-extensions" property in part to make
communicating vendor extensions like this easier. Please try to use
that. "qspinlock" is software configuration though, the vendor extension
should focus on the guarantee of strong forward progress, since that is
the non-standard aspect of your IP.
A commandline property may still be desirable, to control the locking
method used, since the DT should be a description of the hardware, not
for configuring software policy in your operating system.
Thanks,
Conor.
> cpus {
> #address-cells = <1>;
> #size-cells = <0>;
> + qspinlock;
> cpu0: cpu@0 {
> compatible = "sifive,bullet0", "riscv";
> device_type = "cpu";
> i-cache-block-size = <64>;
> i-cache-sets = <128>;
>
> --
> Best Regards
> Guo Ren
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
next prev parent reply other threads:[~2023-09-11 22:25 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
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 [this message]
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=20230911-nimbly-outcome-496efae7adc6@wendy \
--to=conor.dooley@microchip.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@kernel.org \
--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=leobras@redhat.com \
--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).