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