[PATCH v5 0/2] irqchip/riscv-aplic: fix probing of multiple APLIC instances

[PATCH v5 0/2] irqchip/riscv-aplic: fix probing of multiple APLIC instances

[Vladimir Kondratiev]

The APLIC driver does not set probe_type, so it follows
PROBE_DEFAULT_STRATEGY and gets probed asynchronously when the command
line asks for it (driver_async_probe=). The probe path of an APLIC
instance modifies state shared by all instances without any locking,
for example the registration of the syscore operations. On a system with
several APLIC instances this triggers a "list_add double add" report from
CHECK_DATA_CORRUPTION, because two instances both call register_syscore()
for the same node.

Looking at the problem showed a second, unrelated issue: the flag guarding
the syscore registration is marked __ro_after_init, but with MSI the probe
can be deferred until the IMSIC is available, and then writes the flag
after init memory was made read-only.

Patch 1 fixes the latter and is independent of asynchronous probing.

Patch 2 marks the driver PROBE_FORCE_SYNCHRONOUS. The shared state also
includes the global aplics list, aplic_direct_parent_irq (together with
the CPU hotplug state registered for it) and the per-CPU IDC structures,
so making probes mutually exclusive is simpler and less error-prone than
adding a lock for each of them. An interrupt controller is needed early
and gains nothing from probing in parallel.

Reproduced by booting with "driver_async_probe=riscv-aplic" on the
not yet upstreamed "Mobileye eyeq7h" platform with 3 APLIC instances.

Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
---
Changes in v5:
- split the __ro_after_init fix out into its own patch
- drop atomic_t/atomic_cmpxchg() for the registration flag
- mark the driver PROBE_FORCE_SYNCHRONOUS instead of protecting each piece
  of shared state separately
- Link to v4: https://patch.msgid.link/20261020-aplic-async-probe-v4-1-68bc35e0ece1@mobileye.com

Changes in v4:
- rework commit message
- rebase onto v7.3
- Link to v3: https://lore.kernel.org/r/20261012-aplic-async-probe-v3-1-5cf52dc28690@mobileye.com

Changes in v3:
- drop __ro_after_init attribute due to write to read-only page concern
- Link to v2: https://lore.kernel.org/r/20261012-aplic-async-probe-v2-1-868d7684c9d6@mobileye.com

Changes in v2:
- fixed potential access to read-only page
- Link to v1: https://lore.kernel.org/r/20261012-aplic-async-probe-v1-1-4c2c4f5b0916@mobileye.com

To: Anup Patel <anup@brainfault.org>
To: Thomas Gleixner <tglx@kernel.org>
To: Radu Rendec <radu@rendec.net>
To: Paul Walmsley <pjw@kernel.org>
To: Palmer Dabbelt <palmer@dabbelt.com>
To: Albert Ou <aou@eecs.berkeley.edu>
To: Alexandre Ghiti <alex@ghiti.fr>
To: Jessica Liu <liu.xuemei1@zte.com.cn>
Cc: linux-riscv@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

---
Vladimir Kondratiev (2):
      irqchip/riscv-aplic: do not mark the syscore registered flag ro_after_init
      irqchip/riscv-aplic: force synchronous probing

 drivers/irqchip/irq-riscv-aplic-main.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
---
base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
change-id: 20261012-aplic-async-probe-81624e035607

Best regards,
--  
Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>

[PATCH v5 1/2] irqchip/riscv-aplic: do not mark the syscore registered flag ro_after_init

[Vladimir Kondratiev]

aplic_syscore_registered is marked __ro_after_init, but it is written
from aplic_probe(), which can run after init. With MSI, aplic_msi_setup()
returns -EPROBE_DEFER while the MSI parent (IMSIC) is not ready, so the
later successful probe writes the flag after it has been made read-only.
That write faults.

Drop the __ro_after_init annotation so the flag stays writable.

Fixes: b330fbfd34d7 ("irqchip/riscv-aplic: Register syscore operations only once")
Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
---
 drivers/irqchip/irq-riscv-aplic-main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
index d9afb6ae98cf..5369078f3d00 100644
--- a/drivers/irqchip/irq-riscv-aplic-main.c
+++ b/drivers/irqchip/irq-riscv-aplic-main.c
@@ -116,7 +116,11 @@ static struct syscore aplic_syscore = {
 	.ops = &aplic_syscore_ops,
 };
 
-static bool aplic_syscore_registered __ro_after_init;
+/*
+ * Not __ro_after_init: probe can be deferred (e.g. MSI mode waiting for the
+ * IMSIC) and complete after the init memory is made read-only.
+ */
+static bool aplic_syscore_registered;
 
 static void aplic_syscore_init(void)
 {

-- 
2.43.0

[PATCH v5 2/2] irqchip/riscv-aplic: force synchronous probing

[Vladimir Kondratiev]

The APLIC driver leaves probe_type unset, so it follows
PROBE_DEFAULT_STRATEGY and is probed asynchronously when the command line
requests it (driver_async_probe=). The probe of an APLIC instance updates
state shared by all instances without any locking:

- the check-then-set of aplic_syscore_registered; two instances can both
  observe it as false and both call register_syscore(), inserting the same
  list node twice and tripping CHECK_DATA_CORRUPTION ("list_add double
  add") in __list_add_valid_or_report();
- the global aplics list, modified by aplic_pm_add()/aplic_pm_remove();
- aplic_direct_parent_irq, where more than one instance can pass the check
  and register its own CPU hotplug state;

Interrupt controllers are required early and have no benefit from
parallel probing, so rather than adding locks around each of them mark the
driver PROBE_FORCE_SYNCHRONOUS. Probes then run one after another from
driver registration, and deferred probe retries are serialized by the
single deferred probe work item, so the shared state is never accessed
concurrently. Probing is also blocked across suspend, so it cannot overlap
with the syscore callbacks either.

Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
---
 drivers/irqchip/irq-riscv-aplic-main.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
index 5369078f3d00..3c10dbf3b492 100644
--- a/drivers/irqchip/irq-riscv-aplic-main.c
+++ b/drivers/irqchip/irq-riscv-aplic-main.c
@@ -118,7 +118,8 @@ static struct syscore aplic_syscore = {
 
 /*
  * Not __ro_after_init: probe can be deferred (e.g. MSI mode waiting for the
- * IMSIC) and complete after the init memory is made read-only.
+ * IMSIC) and complete after the init memory is made read-only. Probing is
+ * synchronous (see aplic_driver), so no further serialization is needed.
  */
 static bool aplic_syscore_registered;
 
@@ -413,6 +414,12 @@ static struct platform_driver aplic_driver = {
 		.name		= "riscv-aplic",
 		.of_match_table	= aplic_match,
 		.acpi_match_table = ACPI_PTR(aplic_acpi_match),
+		/*
+		 * Probes of multiple APLIC instances share global state (the
+		 * syscore registration and the parent interrupt of the
+		 * direct mode) and must not run concurrently.
+		 */
+		.probe_type	= PROBE_FORCE_SYNCHRONOUS,
 	},
 	.probe = aplic_probe,
 };

-- 
2.43.0