Re: [PATCH v2 3/3] MIPS: CI20: Enable SMP with two CPUs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [High] mips: ingenic: invalid lock nesting in xburst_boot_secondary()
- [High] mips: ingenic: boot BUG_ON in mips_smp_ipi_allocate() and
  missing present mask
- [High] mips: ingenic: missing cpu_set_core() skips cache IPIs under
  CONFIG_MIPS_CPS
- [Low] mips: ingenic: 64-bit build failure in xburst_smp_setup() and
  entry asm
--

commit 80faf5187246f2a8b8c7ef1db1522ec3cdf520d4
Author: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>

MIPS: CI20: Enable SMP with two CPUs

This commit enables CONFIG_SMP=y and sets CONFIG_NR_CPUS=2 in ci20_defconfig
to use both cores of the JZ4780 SoC.

Link: https://lists.openwall.net/linux-kernel/2020/05/19/739

> diff --git a/arch/mips/configs/ci20_defconfig b/arch/mips/configs/ci20_defconfig
> index 90536cab417ee..09d65094b1bd2 100644
> --- a/arch/mips/configs/ci20_defconfig
> +++ b/arch/mips/configs/ci20_defconfig
> @@ -22,6 +22,8 @@ CONFIG_EXPERT=y
>  CONFIG_MACH_INGENIC_SOC=y
>  CONFIG_JZ4780_CI20=y
>  CONFIG_HIGHMEM=y
> +CONFIG_SMP=y
> +CONFIG_NR_CPUS=2

[Severity: High]
Does xburst_boot_secondary() acquire non-raw spinlocks while holding the
raw_spinlock_t smp_lock?

arch/mips/ingenic/ingenic-smp.c:xburst_boot_secondary() {
	...
	raw_spin_lock_irqsave(&smp_lock, flags);
	ctrl = read_c0_corectrl() | CORECTRL_SWRST(1) | CORECTRL_RPC(1);
	write_c0_corectrl(ctrl);
	back_to_back_c0_hazard();

	ret = clk_enable(cpu1_clock);
	...
}

When bringing up CPU1 via smp_init() -> cpu_up(1) -> xburst_boot_secondary(),
clk_enable() acquires the non-raw spinlock_t enable_lock in clk_enable_lock()
and cgu->lock in jz4780_core1_enable() while smp_lock is held.  This triggers
an invalid wait context lockdep BUG under CONFIG_PROVE_LOCKING, sleeps in
atomic context on PREEMPT_RT kernels, and holds smp_lock with interrupts
disabled across the 5000 us busy-poll loop in jz4780_core1_enable().

[Severity: High]
Does booting a generic MIPS SMP kernel (CONFIG_MIPS_GENERIC_KERNEL=y,
CONFIG_BOARD_INGENIC=y, CONFIG_SMP=y) on JZ4780 either leave CPU1 offline or
hit BUG_ON(num_present_cpus() > 1) in mips_smp_ipi_allocate()?

xburst_smp_setup() only calls set_cpu_possible(cpu, true), and
smp_prepare_cpus() only runs init_cpu_present(cpu_possible_mask) when
CONFIG_HOTPLUG_CPU is disabled.  When CONFIG_HOTPLUG_CPU=y (such as in
generic_defconfig), CPU1 is never marked present in cpu_present_mask and is
never brought up.

Furthermore, CONFIG_MIPS_GENERIC_KERNEL selects CONFIG_MIPS_GIC, which selects
CONFIG_GENERIC_IRQ_IPI when CONFIG_SMP=y.  During boot, mips_smp_ipi_init()
calls mips_smp_ipi_allocate(cpu_possible_mask), where ipidomain is NULL
because JZ4780 uses its own mailbox IPI handlers instead of registering a
DOMAIN_BUS_IPI irq_domain:

arch/mips/kernel/smp.c:mips_smp_ipi_allocate() {
	...
	if (!ipidomain) {
		BUG_ON(num_present_cpus() > 1);
		return 0;
	}
	...
}

Whenever CPU1 is marked present (for example with CONFIG_HOTPLUG_CPU=n), this
triggers a kernel panic at boot.

[Severity: High]
Does xburst_smp_setup() leave both JZ4780 cores with core ID 0 when
CONFIG_MIPS_CPS=y, causing r4k_on_each_cpu() to skip cross-CPU cache flush
IPIs?

decode_configs() in arch/mips/kernel/cpu-probe.c only calls cpu_set_core()
when CONFIG_MIPS_CPS is disabled, and neither xburst_smp_setup() nor
xburst_init_secondary() calls cpu_set_core().  Under CONFIG_MIPS_CPS=y (such
as in generic_defconfig), both CPU0 and CPU1 retain core ID 0, so
cpus_are_siblings(0, 1) returns true and calculate_cpu_foreign_map() leaves
cpu_foreign_map[0] empty.  r4k_op_needs_ipi() then returns false and skips
all cross-CPU L1 I-cache and D-cache flush IPIs between Core 0 and Core 1,
corrupting memory and instructions across their non-coherent L1 caches.

[Severity: Low]
Does building a 64-bit generic MIPS SMP kernel (CONFIG_MIPS_GENERIC_KERNEL=y,
CONFIG_64BIT=y, CONFIG_BOARD_INGENIC=y, CONFIG_SMP=y) fail in
arch/mips/ingenic/ingenic-smp.c and arch/mips/ingenic/ingenic-smp-entry.S?

CONFIG_BOARD_INGENIC can be enabled on 64-bit generic kernels and selects
CONFIG_MACH_JZ4780 and CONFIG_SYS_SUPPORTS_SMP.  When CONFIG_64BIT=y,
KSEG1ADDR and KSEG0 are not defined in arch/mips/include/asm/addrspace.h,
so xburst_smp_setup() fails to compile at KSEG1ADDR(), and
xburst_secondary_cpu_entry fails to assemble on KSEG0 as well as the 32-bit
la and lw instructions used on 64-bit pointers and unsigned long symbols.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110043131.1985993-1-f.fainelli@gmail.com?part=3