Re: [PATCH V10 07/19] riscv: qspinlock: errata: Introduce ERRATA_THEAD_QSPINLOCK

[Waiman Long <longman@redhat.com>]

On 9/13/23 14:54, Palmer Dabbelt wrote:
> On Sun, 06 Aug 2023 22:23:34 PDT (-0700), sorear@fastmail.com wrote:
>> On Wed, Aug 2, 2023, at 12:46 PM, guoren@kernel.org wrote:
>>> From: Guo Ren <guoren@linux.alibaba.com>
>>>
>>> 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@linux.alibaba.com>
>>> Signed-off-by: Guo Ren <guoren@kernel.org>
>>> ---
>>>  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

The part of qspinlock that causes problem with many RISC architectures 
is its use of a 16-bit xchg() function call which many RISC 
architectures cannot do it natively and have to be emulated with 
hopefully some forward progress guarantee. Except that one call, the 
other atomic operations are all 32 bit in size.

Cheers,
Longman