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

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2807 bytes --]

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@kernel.org>
> > > > > Signed-off-by: Alexandre Ghiti <alexghiti@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.

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

[-- Attachment #1.2: qspinlocks_config --]
[-- Type: text/plain, Size: 4678 bytes --]

CONFIG_SYSVIPC=y
CONFIG_POSIX_MQUEUE=y
CONFIG_GENERIC_IRQ_DEBUGFS=y
CONFIG_NO_HZ_IDLE=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_BPF_SYSCALL=y
CONFIG_PREEMPT_RT=y
CONFIG_IKCONFIG=y
CONFIG_IKCONFIG_PROC=y
CONFIG_CGROUPS=y
CONFIG_CGROUP_SCHED=y
CONFIG_CFS_BANDWIDTH=y
CONFIG_CGROUP_BPF=y
CONFIG_NAMESPACES=y
CONFIG_USER_NS=y
CONFIG_CHECKPOINT_RESTORE=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_EXPERT=y
# CONFIG_SYSFS_SYSCALL is not set
CONFIG_PROFILING=y
CONFIG_KEXEC=y
CONFIG_KEXEC_FILE=y
CONFIG_ARCH_MICROCHIP=y
CONFIG_ARCH_RENESAS=y
CONFIG_ARCH_SOPHGO=y
CONFIG_SOC_STARFIVE=y
CONFIG_ARCH_THEAD=y
CONFIG_ARCH_VIRT=y
CONFIG_NONPORTABLE=y
CONFIG_SMP=y
CONFIG_RANDOMIZE_BASE=y
CONFIG_CMDLINE="earlycon keep_bootcon reboot=cold"
CONFIG_HIBERNATION=y
CONFIG_CPU_FREQ=y
CONFIG_VIRTUALIZATION=y
CONFIG_KVM=m
CONFIG_ACPI=y
CONFIG_JUMP_LABEL=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
CONFIG_SPARSEMEM_MANUAL=y
CONFIG_NET=y
CONFIG_PACKET=y
CONFIG_IP_MULTICAST=y
CONFIG_IP_ADVANCED_ROUTER=y
CONFIG_IP_PNP=y
CONFIG_IP_PNP_DHCP=y
CONFIG_IP_PNP_BOOTP=y
CONFIG_IP_PNP_RARP=y
CONFIG_NETLINK_DIAG=y
CONFIG_NET_9P=y
CONFIG_NET_9P_VIRTIO=y
CONFIG_PCI=y
CONFIG_PCIEPORTBUS=y
CONFIG_PCI_HOST_GENERIC=y
CONFIG_PCIE_XILINX=y
CONFIG_PCIE_MICROCHIP_HOST=y
CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y
CONFIG_FW_LOADER_USER_HELPER=y
CONFIG_DEBUG_DRIVER=y
CONFIG_AX45MP_L2_CACHE=y
CONFIG_SIFIVE_CCACHE=y
CONFIG_EFI_ZBOOT=y
CONFIG_MTD=y
CONFIG_MTD_CMDLINE_PARTS=y
CONFIG_MTD_CFI=y
CONFIG_MTD_JEDECPROBE=y
CONFIG_MTD_SPI_NAND=y
CONFIG_MTD_SPI_NOR=y
# CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is not set
CONFIG_OF_OVERLAY=y
CONFIG_ZRAM=y
CONFIG_ZRAM_MEMORY_TRACKING=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_VIRTIO_BLK=y
CONFIG_BLK_DEV_NVME=y
CONFIG_BLK_DEV_SD=y
CONFIG_BLK_DEV_SR=y
CONFIG_SCSI_VIRTIO=y
CONFIG_ATA=y
CONFIG_SATA_AHCI=y
CONFIG_SATA_AHCI_PLATFORM=y
CONFIG_NETDEVICES=y
CONFIG_VIRTIO_NET=y
CONFIG_MACB=y
CONFIG_E1000E=y
CONFIG_R8169=y
CONFIG_MICROSEMI_PHY=y
CONFIG_VITESSE_PHY=y
CONFIG_INPUT_MOUSEDEV=y
CONFIG_INPUT_JOYDEV=m
CONFIG_INPUT_JOYSTICK=y
CONFIG_JOYSTICK_SENSEHAT=m
CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_SERIAL_OF_PLATFORM=y
CONFIG_VIRTIO_CONSOLE=y
CONFIG_HW_RANDOM=y
CONFIG_HW_RANDOM_VIRTIO=y
CONFIG_HW_RANDOM_POLARFIRE_SOC=y
CONFIG_I2C=y
CONFIG_I2C_CHARDEV=y
CONFIG_I2C_MICROCHIP_CORE=y
CONFIG_SPI=y
CONFIG_SPI_MICROCHIP_CORE=y
CONFIG_SPI_MICROCHIP_CORE_QSPI=y
CONFIG_SPI_SIFIVE=y
# CONFIG_PTP_1588_CLOCK is not set
CONFIG_GPIO_SYSFS=y
CONFIG_GPIO_POLARFIRE_SOC=y
CONFIG_GPIO_SIFIVE=y
CONFIG_AUXDISPLAY=y
CONFIG_FB=y
CONFIG_FRAMEBUFFER_CONSOLE=y
CONFIG_SOUND=y
CONFIG_SND=y
CONFIG_SND_SOC=y
CONFIG_SND_SOC_MAX9867=y
CONFIG_USB=y
CONFIG_USB_XHCI_HCD=y
CONFIG_USB_XHCI_PLATFORM=y
CONFIG_USB_EHCI_HCD=y
CONFIG_USB_EHCI_HCD_PLATFORM=y
CONFIG_USB_OHCI_HCD=y
CONFIG_USB_OHCI_HCD_PLATFORM=y
CONFIG_USB_STORAGE=y
CONFIG_USB_UAS=y
CONFIG_USB_MUSB_HDRC=y
CONFIG_USB_MUSB_POLARFIRE_SOC=y
CONFIG_USB_INVENTRA_DMA=y
CONFIG_NOP_USB_XCEIV=y
CONFIG_MMC=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_PLTFM=y
CONFIG_MMC_SDHCI_CADENCE=y
CONFIG_MMC_SPI=y
CONFIG_NEW_LEDS=y
CONFIG_LEDS_CLASS=y
CONFIG_LEDS_GPIO=y
CONFIG_LEDS_PWM=y
CONFIG_RTC_CLASS=y
CONFIG_RTC_DRV_PCF2123=y
CONFIG_DMADEVICES=y
CONFIG_SF_PDMA=y
CONFIG_VIRTIO_PCI=y
CONFIG_VIRTIO_BALLOON=y
CONFIG_VIRTIO_INPUT=y
CONFIG_VIRTIO_MMIO=y
CONFIG_MAILBOX=y
CONFIG_POLARFIRE_SOC_MAILBOX=y
CONFIG_RPMSG_CHAR=y
CONFIG_RPMSG_CTRL=y
CONFIG_RPMSG_VIRTIO=y
CONFIG_POLARFIRE_SOC_SYS_CTRL=y
CONFIG_IIO=y
CONFIG_ADXL345_SPI=y
CONFIG_MCP320X=y
CONFIG_MCP3564=y
CONFIG_PAC1934=y
CONFIG_SD_ADC_MODULATOR=y
CONFIG_HTS221=m
CONFIG_IIO_ST_LSM6DSX=m
CONFIG_IIO_ST_MAGN_3AXIS=m
CONFIG_IIO_ST_PRESS=m
CONFIG_PWM=y
CONFIG_PWM_DEBUG=y
CONFIG_PWM_MICROCHIP_CORE=y
CONFIG_LIBNVDIMM=y
CONFIG_FPGA=y
CONFIG_FPGA_BRIDGE=y
CONFIG_FPGA_REGION=y
CONFIG_FPGA_MGR_MICROCHIP_SPI=y
CONFIG_EXT4_FS=y
CONFIG_EXT4_FS_POSIX_ACL=y
CONFIG_MSDOS_FS=y
CONFIG_VFAT_FS=y
CONFIG_TMPFS=y
CONFIG_TMPFS_POSIX_ACL=y
CONFIG_HUGETLBFS=y
CONFIG_NFS_FS=y
CONFIG_NFS_V3_ACL=y
CONFIG_NFS_V4=y
CONFIG_NFS_V4_1=y
CONFIG_NFS_V4_2=y
CONFIG_NFS_V4_1_MIGRATION=y
CONFIG_ROOT_NFS=y
CONFIG_9P_FS=y
CONFIG_NLS_CODEPAGE_437=y
CONFIG_NLS_ISO8859_1=m
CONFIG_CRYPTO_DEFLATE=y
CONFIG_CRYPTO_ZSTD=y
CONFIG_CRYPTO_USER_API_HASH=y
CONFIG_CRYPTO_DEV_VIRTIO=y
CONFIG_PRINTK_TIME=y
CONFIG_DYNAMIC_DEBUG=y
CONFIG_MAGIC_SYSRQ=y
CONFIG_DEBUG_FS=y
CONFIG_DEBUG_PAGEALLOC=y
CONFIG_SCHED_STACK_END_CHECK=y
CONFIG_DEBUG_VM=y
CONFIG_DEBUG_VM_PGFLAGS=y
CONFIG_DEBUG_MEMORY_INIT=y
CONFIG_DEBUG_PER_CPU_MAPS=y
CONFIG_SOFTLOCKUP_DETECTOR=y
CONFIG_WQ_WATCHDOG=y
CONFIG_PROVE_LOCKING=y
CONFIG_DEBUG_ATOMIC_SLEEP=y
CONFIG_DEBUG_LIST=y
CONFIG_DEBUG_PLIST=y
CONFIG_DEBUG_SG=y
# CONFIG_RCU_TRACE is not set
CONFIG_RCU_EQS_DEBUG=y
CONFIG_SAMPLES=y
CONFIG_MEMTEST=y

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]