On Sun, 06 Aug 2023 22:23:34 PDT (-0700), sorear@xxxxxxxxxxxx wrote:
On Wed, Aug 2, 2023, at 12:46 PM, guoren@xxxxxxxxxx wrote:
From: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
According to qspinlock requirements, RISC-V gives out a weak LR/SC
forward progress guarantee which does not satisfy qspinlock. But
many vendors could produce stronger forward guarantee LR/SC to
ensure the xchg_tail could be finished in time on any kind of
hart. T-HEAD is the vendor which implements strong forward
guarantee LR/SC instruction pairs, so enable qspinlock for T-HEAD
with errata help.
T-HEAD early version of processors has the merge buffer delay
problem, so we need ERRATA_WRITEONCE to support qspinlock.
Signed-off-by: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
Signed-off-by: Guo Ren <guoren@xxxxxxxxxx>
---
arch/riscv/Kconfig.errata | 13 +++++++++++++
arch/riscv/errata/thead/errata.c | 24 ++++++++++++++++++++++++
arch/riscv/include/asm/errata_list.h | 20 ++++++++++++++++++++
arch/riscv/include/asm/vendorid_list.h | 3 ++-
arch/riscv/kernel/cpufeature.c | 3 ++-
5 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
index 4745a5c57e7c..eb43677b13cc 100644
--- a/arch/riscv/Kconfig.errata
+++ b/arch/riscv/Kconfig.errata
@@ -96,4 +96,17 @@ config ERRATA_THEAD_WRITE_ONCE
If you don't know what to do here, say "Y".
+config ERRATA_THEAD_QSPINLOCK
+ bool "Apply T-Head queued spinlock errata"
+ depends on ERRATA_THEAD
+ default y
+ help
+ The T-HEAD C9xx processors implement strong fwd guarantee LR/SC to
+ match the xchg_tail requirement of qspinlock.
+
+ This will apply the QSPINLOCK errata to handle the non-standard
+ behavior via using qspinlock instead of ticket_lock.
+
+ If you don't know what to do here, say "Y".
If this is to be applied, I would like to see a detailed explanation somewhere,
preferably with citations, of:
(a) The memory model requirements for qspinlock
(b) Why, with arguments, RISC-V does not architecturally meet (a)
(c) Why, with arguments, T-HEAD C9xx meets (a)
(d) Why at least one other architecture which defines ARCH_USE_QUEUED_SPINLOCKS
meets (a)
I agree.
Just having a magic fence that makes qspinlocks stop livelocking on some
processors is going to lead to a mess -- I'd argue this means those
processors just don't provide the forward progress guarantee, but we'd
really need something written down about what this new custom
instruction aliasing as a fence does.
As far as I can tell, the RISC-V guarantees concerning constrained LR/SC loops
(livelock freedom but no starvation freedom) are exactly the same as those in
Armv8 (as of 0487F.c) for equivalent loops, and xchg_tail compiles to a
constrained LR/SC loop with guaranteed eventual success (with -O1). Clearly you
disagree; I would like to see your perspective.
It sounds to me like this processor might be quite broken: if it's
permanently holding stores in a buffer we're going to have more issues
than just qspinlock, pretty much anything concurrent is going to have
issues -- and that's not just in the kernel, there's concurrent
userspace code as well.
-s
+
endmenu # "CPU errata selection"
diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
index 881729746d2e..d560dc45c0e7 100644
--- a/arch/riscv/errata/thead/errata.c
+++ b/arch/riscv/errata/thead/errata.c
@@ -86,6 +86,27 @@ static bool errata_probe_write_once(unsigned int stage,
return false;
}
+static bool errata_probe_qspinlock(unsigned int stage,
+ unsigned long arch_id, unsigned long impid)
+{
+ if (!IS_ENABLED(CONFIG_ERRATA_THEAD_QSPINLOCK))
+ return false;
+
+ /*
+ * The queued_spinlock torture would get in livelock without
+ * ERRATA_THEAD_WRITE_ONCE fixup for the early versions of T-HEAD
+ * processors.
+ */
+ if (arch_id == 0 && impid == 0 &&
+ !IS_ENABLED(CONFIG_ERRATA_THEAD_WRITE_ONCE))
+ return false;
+
+ if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
+ return true;
+
+ return false;
+}
+
static u32 thead_errata_probe(unsigned int stage,
unsigned long archid, unsigned long impid)
{
@@ -103,6 +124,9 @@ static u32 thead_errata_probe(unsigned int stage,
if (errata_probe_write_once(stage, archid, impid))
cpu_req_errata |= BIT(ERRATA_THEAD_WRITE_ONCE);
+ if (errata_probe_qspinlock(stage, archid, impid))
+ cpu_req_errata |= BIT(ERRATA_THEAD_QSPINLOCK);
+
return cpu_req_errata;
}
diff --git a/arch/riscv/include/asm/errata_list.h
b/arch/riscv/include/asm/errata_list.h
index fbb2b8d39321..a696d18d1b0d 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -141,6 +141,26 @@ asm volatile(ALTERNATIVE( \
: "=r" (__ovl) : \
: "memory")
+static __always_inline bool
+riscv_has_errata_thead_qspinlock(void)
+{
+ if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE)) {
+ asm_volatile_goto(
+ ALTERNATIVE(
+ "j %l[l_no]", "nop",
+ THEAD_VENDOR_ID,
+ ERRATA_THEAD_QSPINLOCK,
+ CONFIG_ERRATA_THEAD_QSPINLOCK)
+ : : : : l_no);
+ } else {
+ goto l_no;
+ }
+
+ return true;
+l_no:
+ return false;
+}
+
#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/riscv/include/asm/vendorid_list.h
b/arch/riscv/include/asm/vendorid_list.h
index 73078cfe4029..1f1d03877f5f 100644
--- a/arch/riscv/include/asm/vendorid_list.h
+++ b/arch/riscv/include/asm/vendorid_list.h
@@ -19,7 +19,8 @@
#define ERRATA_THEAD_CMO 1
#define ERRATA_THEAD_PMU 2
#define ERRATA_THEAD_WRITE_ONCE 3
-#define ERRATA_THEAD_NUMBER 4
+#define ERRATA_THEAD_QSPINLOCK 4
+#define ERRATA_THEAD_NUMBER 5
#endif
#endif
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index f8dbbe1bbd34..d9694fe40a9a 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -342,7 +342,8 @@ void __init riscv_fill_hwcap(void)
* spinlock value, the only way is to change from queued_spinlock to
* ticket_spinlock, but can not be vice.
*/
- if (!force_qspinlock) {
+ if (!force_qspinlock &&
+ !riscv_has_errata_thead_qspinlock()) {
set_bit(RISCV_ISA_EXT_XTICKETLOCK, isainfo->isa);
}
#endif
--
2.36.1
_______________________________________________
linux-riscv mailing list
linux-riscv@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-riscv
