[PATCH v6 13/13] riscv: Add qspinlock support

[Guo Ren]

On Fri, Nov 29, 2024 at 12:19 AM Conor Dooley <conor at kernel.org> wrote:
>
> On Thu, Nov 28, 2024 at 03:50:09PM +0100, Alexandre Ghiti wrote:
> > On 28/11/2024 15:14, Conor Dooley wrote:
> > > On Thu, Nov 28, 2024 at 01:41:36PM +0000, Will Deacon wrote:
> > > > On Thu, Nov 28, 2024 at 12:56:55PM +0000, Conor Dooley wrote:
> > > > > On Sun, Nov 03, 2024 at 03:51:53PM +0100, Alexandre Ghiti wrote:
> > > > > > In order to produce a generic kernel, a user can select
> > > > > > CONFIG_COMBO_SPINLOCKS which will fallback at runtime to the ticket
> > > > > > spinlock implementation if Zabha or Ziccrse are not present.
> > > > > >
> > > > > > Note that we can't use alternatives here because the discovery of
> > > > > > extensions is done too late and we need to start with the qspinlock
> > > > > > implementation because the ticket spinlock implementation would pollute
> > > > > > the spinlock value, so let's use static keys.
> > > > > >
> > > > > > This is largely based on Guo's work and Leonardo reviews at [1].
> > > > > >
> > > > > > Link: https://lore.kernel.org/linux-riscv/20231225125847.2778638-1-guoren@kernel.org/ [1]
> > > > > > Signed-off-by: Guo Ren <guoren at kernel.org>
> > > > > > Signed-off-by: Alexandre Ghiti <alexghiti at rivosinc.com>
> > > > > This patch (now commit ab83647fadae2 ("riscv: Add qspinlock support"))
> > > > > breaks boot on polarfire soc. It dies before outputting anything to the
> > > > > console. My .config has:
> > > > >
> > > > > # CONFIG_RISCV_TICKET_SPINLOCKS is not set
> > > > > # CONFIG_RISCV_QUEUED_SPINLOCKS is not set
> > > > > CONFIG_RISCV_COMBO_SPINLOCKS=y
> > > > I pointed out some of the fragility during review:
> > > >
> > > > https://lore.kernel.org/all/20241111164259.GA20042@willie-the-truck/
> > > >
> > > > so I'm kinda surprised it got merged tbh :/
> > > Maybe it could be reverted rather than having a broken boot with the
> > > default settings in -rc1.
> >
> >
> > No need to rush before we know what's happening,I guess you bisected to this
> > commit right?
>
> The symptom is a failure to boot, without any console output, of course
> I bisected it before blaming something specific. But I don't think it is
> "rushing" as having -rc1 broken with an option's default is a massive pain
> in the arse when it comes to testing.
>
> > I don't have this soc, so can you provide $stval/$sepc/$scause, a config, a
> > kernel, anything?
>
> I don't have the former cos it died immediately on boot. config is
> attached. It reproduces in QEMU so you don't need any hardware.
If QEMU could reproduce, could you provide a dmesg by the below method?

Qemu cmd append: -s -S
ref: https://qemu-project.gitlab.io/qemu/system/gdb.html

Connect gdb and in console:
1. file vmlinux
2. source ./Documentation/admin-guide/kdump/gdbmacros.txt
3. dmesg

Then, we could get the kernel's early boot logs from memory.

>
> > Does the polarfire soc provide Ziccrse?
>
> I don't think that is relevant because ziccrse is not listed in the dts,
> so the kernel should not be assuming that LR/SC has a forward progress
> guarantee. It's not even getting as far as riscv_spinlock_init() given
> several things before that should be emitting logs, so it doesn't even
> get to make any decisions about Ziccrse.



-- 
Best Regards
 Guo Ren