[PATCH v3] irqchip/riscv-aplic: fix asynchronous probing

[PATCH v3] irqchip/riscv-aplic: fix asynchronous probing

[Vladimir Kondratiev]

boolean flag don't protect from race when probing several instances
asynchronously. On system with multiple APLICs, if probed asynchronously,
warning triggered as below.

[    0.227210] riscv-aplic 67560000.interrupt-controller: 257 interrupts directly connected to 8 CPUs
[    0.227210] riscv-aplic 674e0000.interrupt-controller: 257 interrupts directly connected to 8 CPUs
[    0.227224] ------------[ cut here ]------------
[    0.227227] list_add double add: new=ffffffff8130ced0, prev=ffffffff8130ced0, next=ffffffff81316d40.
[    0.227302] WARNING: lib/list_debug.c:35 at __list_add_valid_or_report+0xe0/0xe6, CPU#4: kworker/u97:0/211
[    0.227322] Modules linked in:
[    0.227333] CPU: 4 UID: 0 PID: 211 Comm: kworker/u97:0 Not tainted 7.3.0-rc6 #1 PREEMPT_RT
[    0.227341] Hardware name: EyeQ7H evaluation board (DT)
[    0.227346] Workqueue: async async_run_entry_fn
[    0.227358] epc : __list_add_valid_or_report+0xe0/0xe6
[    0.227366]  ra : __list_add_valid_or_report+0xe0/0xe6
[    0.227375] epc : ffffffff804a08a0 ra : ffffffff804a08a0 sp : ffffffc600823b90
[    0.227379]  gp : ffffffff81389e70 tp : ffffffd60135ee00 t0 : 0000000000000000
[    0.227383]  t1 : 3fffffffffffffff t2 : 0000000000000000 s0 : ffffffc600823bb0
[    0.227387]  s1 : ffffffff8130ced0 a0 : 0000000000000058 a1 : ffffffd9efc9e5c0
[    0.227391]  a2 : ffffffd9efd5c510 a3 : 0000000000000000 a4 : 0000000000000000
[    0.227395]  a5 : 0000000000000000 a6 : ffffffd9efc6e5c0 a7 : bfffffffffffe000
[    0.227399]  s2 : 0000000000000000 s3 : ffffffff8130ce28 s4 : 0000000000000000
[    0.227402]  s5 : ffffffd6018b65c8 s6 : 0000000000000000 s7 : ffffffd6012fd605
[    0.227406]  s8 : ffffffd6010c3400 s9 : 0000000000000000 s10: 0000000000000000
[    0.227409]  s11: 0000000000000000 t3 : 3fffffffffffe000 t4 : 0000000000000020
[    0.227413]  t5 : 0000000000000000 t6 : ffffffc6008238f8 ssp : 0000000000000000
[    0.227417] status: 0000000200000120 badaddr: ffffffff804a08a0 cause: 0000000000000003
[    0.227422] [<ffffffff804a08a0>] __list_add_valid_or_report+0xe0/0xe6
[    0.227432] [<ffffffff80558226>] register_syscore+0x30/0x60
[    0.227444] [<ffffffff804ca0fe>] aplic_probe+0xd2/0xfa
[    0.227453] [<ffffffff80559b2e>] platform_probe+0x3e/0x7a
[    0.227461] [<ffffffff8055731a>] really_probe+0x88/0x35c
[    0.227469] [<ffffffff805576ae>] __driver_probe_device+0xc0/0x13a
[    0.227478] [<ffffffff805577f4>] driver_probe_device+0x38/0xb2
[    0.227486] [<ffffffff80557ba6>] __driver_attach_async_helper+0x3c/0xc4
[    0.227493] [<ffffffff80054a0e>] async_run_entry_fn+0x28/0x154
[    0.227500] [<ffffffff8004482a>] process_one_work+0x14e/0x3d2
[    0.227509] [<ffffffff800452fe>] worker_thread+0x144/0x286
[    0.227516] [<ffffffff8004ca3e>] kthread+0xee/0x10e
[    0.227524] [<ffffffff8001050a>] ret_from_fork_kernel+0x18/0x14e
[    0.227532] [<ffffffff807bef36>] ret_from_fork_kernel_asm+0x16/0x18
[    0.227542] ---[ end trace 0000000000000000 ]---
[    0.228475] pinctrl-single 48508000.pinctrl: 512 pins, size 2048
[    0.228500] pinctrl-single 53838000.pinctrl: 512 pins, size 2048
[    0.228723] pinctrl-single 67107000.pinctrl: 512 pins, size 2048
[    0.227210] riscv-aplic 674e0000.interrupt-controller: 257 interrupts directly connected to 8 CPUs
[    0.227224] ------------[ cut here ]------------
[    0.227227] list_add double add: new=ffffffff8130ced0, prev=ffffffff8130ced0, next=ffffffff81316d40.
[    0.227302] WARNING: lib/list_debug.c:35 at __list_add_valid_or_report+0xe0/0xe6, CPU#4: kworker/u97:0/211
[    0.227322] Modules linked in:
[    0.227333] CPU: 4 UID: 0 PID: 211 Comm: kworker/u97:0 Not tainted 7.3.0-rc6 #1 PREEMPT_RT
[    0.227341] Hardware name: EyeQ7H evaluation board (DT)
[    0.227346] Workqueue: async async_run_entry_fn
[    0.227358] epc : __list_add_valid_or_report+0xe0/0xe6
[    0.227366]  ra : __list_add_valid_or_report+0xe0/0xe6
[    0.227375] epc : ffffffff804a08a0 ra : ffffffff804a08a0 sp : ffffffc600823b90
[    0.227379]  gp : ffffffff81389e70 tp : ffffffd60135ee00 t0 : 0000000000000000
[    0.227383]  t1 : 3fffffffffffffff t2 : 0000000000000000 s0 : ffffffc600823bb0
[    0.227387]  s1 : ffffffff8130ced0 a0 : 0000000000000058 a1 : ffffffd9efc9e5c0
[    0.227391]  a2 : ffffffd9efd5c510 a3 : 0000000000000000 a4 : 0000000000000000
[    0.227395]  a5 : 0000000000000000 a6 : ffffffd9efc6e5c0 a7 : bfffffffffffe000
[    0.227399]  s2 : 0000000000000000 s3 : ffffffff8130ce28 s4 : 0000000000000000
[    0.227402]  s5 : ffffffd6018b65c8 s6 : 0000000000000000 s7 : ffffffd6012fd605
[    0.227406]  s8 : ffffffd6010c3400 s9 : 0000000000000000 s10: 0000000000000000
[    0.227409]  s11: 0000000000000000 t3 : 3fffffffffffe000 t4 : 0000000000000020
[    0.227413]  t5 : 0000000000000000 t6 : ffffffc6008238f8 ssp : 0000000000000000
[    0.227417] status: 0000000200000120 badaddr: ffffffff804a08a0 cause: 0000000000000003
[    0.227422] [<ffffffff804a08a0>] __list_add_valid_or_report+0xe0/0xe6
[    0.227432] [<ffffffff80558226>] register_syscore+0x30/0x60
[    0.227444] [<ffffffff804ca0fe>] aplic_probe+0xd2/0xfa
[    0.227453] [<ffffffff80559b2e>] platform_probe+0x3e/0x7a
[    0.227461] [<ffffffff8055731a>] really_probe+0x88/0x35c
[    0.227469] [<ffffffff805576ae>] __driver_probe_device+0xc0/0x13a
[    0.227478] [<ffffffff805577f4>] driver_probe_device+0x38/0xb2
[    0.227486] [<ffffffff80557ba6>] __driver_attach_async_helper+0x3c/0xc4
[    0.227493] [<ffffffff80054a0e>] async_run_entry_fn+0x28/0x154
[    0.227500] [<ffffffff8004482a>] process_one_work+0x14e/0x3d2
[    0.227509] [<ffffffff800452fe>] worker_thread+0x144/0x286
[    0.227516] [<ffffffff8004ca3e>] kthread+0xee/0x10e
[    0.227524] [<ffffffff8001050a>] ret_from_fork_kernel+0x18/0x14e
[    0.227532] [<ffffffff807bef36>] ret_from_fork_kernel_asm+0x16/0x18
[    0.227542] ---[ end trace 0000000000000000 ]---

Fixes: b330fbfd34d7 ("irqchip/riscv-aplic: Register syscore operations only once")
Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@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
---
 drivers/irqchip/irq-riscv-aplic-main.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
index d9afb6ae98cf56d6fd9f858c31a3d23546c3b450..54e1d5602f15a04127e900c0a4983022d49da553 100644
--- a/drivers/irqchip/irq-riscv-aplic-main.c
+++ b/drivers/irqchip/irq-riscv-aplic-main.c
@@ -116,14 +116,12 @@ static struct syscore aplic_syscore = {
 	.ops = &aplic_syscore_ops,
 };
 
-static bool aplic_syscore_registered __ro_after_init;
+static atomic_t aplic_syscore_registered = ATOMIC_INIT(0);
 
 static void aplic_syscore_init(void)
 {
-	if (!aplic_syscore_registered) {
+	if (!atomic_cmpxchg(&aplic_syscore_registered, 0, 1))
 		register_syscore(&aplic_syscore);
-		aplic_syscore_registered = true;
-	}
 }
 
 static int aplic_pm_notifier(struct notifier_block *nb, unsigned long action, void *data)

---
base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
change-id: 20261012-aplic-async-probe-81624e035607

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

Re: [PATCH v3] irqchip/riscv-aplic: fix asynchronous probing

[Sebastian Andrzej Siewior]

On 2026-10-12 12:12:59 [+0300], Vladimir Kondratiev wrote:
> boolean flag don't protect from race when probing several instances
> asynchronously. On system with multiple APLICs, if probed asynchronously,
> warning triggered as below.

Why is this async? Doesn't __platform_driver_probe() set it to sync?

> [    0.227210] riscv-aplic 67560000.interrupt-controller: 257 interrupts directly connected to 8 CPUs
> [    0.227210] riscv-aplic 674e0000.interrupt-controller: 257 interrupts directly connected to 8 CPUs
> [    0.227224] ------------[ cut here ]------------
> [    0.227227] list_add double add: new=ffffffff8130ced0, prev=ffffffff8130ced0, next=ffffffff81316d40.
> [    0.227302] WARNING: lib/list_debug.c:35 at __list_add_valid_or_report+0xe0/0xe6, CPU#4: kworker/u97:0/211
> [    0.227322] Modules linked in:
> [    0.227333] CPU: 4 UID: 0 PID: 211 Comm: kworker/u97:0 Not tainted 7.3.0-rc6 #1 PREEMPT_RT
> [    0.227341] Hardware name: EyeQ7H evaluation board (DT)
> [    0.227346] Workqueue: async async_run_entry_fn
> [    0.227358] epc : __list_add_valid_or_report+0xe0/0xe6
> [    0.227366]  ra : __list_add_valid_or_report+0xe0/0xe6
> [    0.227375] epc : ffffffff804a08a0 ra : ffffffff804a08a0 sp : ffffffc600823b90
> [    0.227379]  gp : ffffffff81389e70 tp : ffffffd60135ee00 t0 : 0000000000000000
> [    0.227383]  t1 : 3fffffffffffffff t2 : 0000000000000000 s0 : ffffffc600823bb0
> [    0.227387]  s1 : ffffffff8130ced0 a0 : 0000000000000058 a1 : ffffffd9efc9e5c0
> [    0.227391]  a2 : ffffffd9efd5c510 a3 : 0000000000000000 a4 : 0000000000000000
> [    0.227395]  a5 : 0000000000000000 a6 : ffffffd9efc6e5c0 a7 : bfffffffffffe000
> [    0.227399]  s2 : 0000000000000000 s3 : ffffffff8130ce28 s4 : 0000000000000000
> [    0.227402]  s5 : ffffffd6018b65c8 s6 : 0000000000000000 s7 : ffffffd6012fd605
> [    0.227406]  s8 : ffffffd6010c3400 s9 : 0000000000000000 s10: 0000000000000000
> [    0.227409]  s11: 0000000000000000 t3 : 3fffffffffffe000 t4 : 0000000000000020
> [    0.227413]  t5 : 0000000000000000 t6 : ffffffc6008238f8 ssp : 0000000000000000
> [    0.227417] status: 0000000200000120 badaddr: ffffffff804a08a0 cause: 0000000000000003
> [    0.227422] [<ffffffff804a08a0>] __list_add_valid_or_report+0xe0/0xe6
> [    0.227432] [<ffffffff80558226>] register_syscore+0x30/0x60
> [    0.227444] [<ffffffff804ca0fe>] aplic_probe+0xd2/0xfa
> [    0.227453] [<ffffffff80559b2e>] platform_probe+0x3e/0x7a
> [    0.227461] [<ffffffff8055731a>] really_probe+0x88/0x35c
> [    0.227469] [<ffffffff805576ae>] __driver_probe_device+0xc0/0x13a
> [    0.227478] [<ffffffff805577f4>] driver_probe_device+0x38/0xb2
> [    0.227486] [<ffffffff80557ba6>] __driver_attach_async_helper+0x3c/0xc4
> [    0.227493] [<ffffffff80054a0e>] async_run_entry_fn+0x28/0x154
> [    0.227500] [<ffffffff8004482a>] process_one_work+0x14e/0x3d2
> [    0.227509] [<ffffffff800452fe>] worker_thread+0x144/0x286
> [    0.227516] [<ffffffff8004ca3e>] kthread+0xee/0x10e
> [    0.227524] [<ffffffff8001050a>] ret_from_fork_kernel+0x18/0x14e
> [    0.227532] [<ffffffff807bef36>] ret_from_fork_kernel_asm+0x16/0x18
> [    0.227542] ---[ end trace 0000000000000000 ]---
> [    0.228475] pinctrl-single 48508000.pinctrl: 512 pins, size 2048
> [    0.228500] pinctrl-single 53838000.pinctrl: 512 pins, size 2048
> [    0.228723] pinctrl-single 67107000.pinctrl: 512 pins, size 2048
> [    0.227210] riscv-aplic 674e0000.interrupt-controller: 257 interrupts directly connected to 8 CPUs
> [    0.227224] ------------[ cut here ]------------
> [    0.227227] list_add double add: new=ffffffff8130ced0, prev=ffffffff8130ced0, next=ffffffff81316d40.
> [    0.227302] WARNING: lib/list_debug.c:35 at __list_add_valid_or_report+0xe0/0xe6, CPU#4: kworker/u97:0/211
> [    0.227322] Modules linked in:
> [    0.227333] CPU: 4 UID: 0 PID: 211 Comm: kworker/u97:0 Not tainted 7.3.0-rc6 #1 PREEMPT_RT
> [    0.227341] Hardware name: EyeQ7H evaluation board (DT)
> [    0.227346] Workqueue: async async_run_entry_fn
> [    0.227358] epc : __list_add_valid_or_report+0xe0/0xe6
> [    0.227366]  ra : __list_add_valid_or_report+0xe0/0xe6
> [    0.227375] epc : ffffffff804a08a0 ra : ffffffff804a08a0 sp : ffffffc600823b90
> [    0.227379]  gp : ffffffff81389e70 tp : ffffffd60135ee00 t0 : 0000000000000000
> [    0.227383]  t1 : 3fffffffffffffff t2 : 0000000000000000 s0 : ffffffc600823bb0
> [    0.227387]  s1 : ffffffff8130ced0 a0 : 0000000000000058 a1 : ffffffd9efc9e5c0
> [    0.227391]  a2 : ffffffd9efd5c510 a3 : 0000000000000000 a4 : 0000000000000000
> [    0.227395]  a5 : 0000000000000000 a6 : ffffffd9efc6e5c0 a7 : bfffffffffffe000
> [    0.227399]  s2 : 0000000000000000 s3 : ffffffff8130ce28 s4 : 0000000000000000
> [    0.227402]  s5 : ffffffd6018b65c8 s6 : 0000000000000000 s7 : ffffffd6012fd605
> [    0.227406]  s8 : ffffffd6010c3400 s9 : 0000000000000000 s10: 0000000000000000
> [    0.227409]  s11: 0000000000000000 t3 : 3fffffffffffe000 t4 : 0000000000000020
> [    0.227413]  t5 : 0000000000000000 t6 : ffffffc6008238f8 ssp : 0000000000000000
> [    0.227417] status: 0000000200000120 badaddr: ffffffff804a08a0 cause: 0000000000000003
> [    0.227422] [<ffffffff804a08a0>] __list_add_valid_or_report+0xe0/0xe6
> [    0.227432] [<ffffffff80558226>] register_syscore+0x30/0x60
> [    0.227444] [<ffffffff804ca0fe>] aplic_probe+0xd2/0xfa
> [    0.227453] [<ffffffff80559b2e>] platform_probe+0x3e/0x7a
> [    0.227461] [<ffffffff8055731a>] really_probe+0x88/0x35c
> [    0.227469] [<ffffffff805576ae>] __driver_probe_device+0xc0/0x13a
> [    0.227478] [<ffffffff805577f4>] driver_probe_device+0x38/0xb2
> [    0.227486] [<ffffffff80557ba6>] __driver_attach_async_helper+0x3c/0xc4
> [    0.227493] [<ffffffff80054a0e>] async_run_entry_fn+0x28/0x154
> [    0.227500] [<ffffffff8004482a>] process_one_work+0x14e/0x3d2
> [    0.227509] [<ffffffff800452fe>] worker_thread+0x144/0x286
> [    0.227516] [<ffffffff8004ca3e>] kthread+0xee/0x10e
> [    0.227524] [<ffffffff8001050a>] ret_from_fork_kernel+0x18/0x14e
> [    0.227532] [<ffffffff807bef36>] ret_from_fork_kernel_asm+0x16/0x18
> [    0.227542] ---[ end trace 0000000000000000 ]---

Would you mind providing a coherent description of the problem and
implemented solution instead of slapping console output?

> Fixes: b330fbfd34d7 ("irqchip/riscv-aplic: Register syscore operations only once")
> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
> ---
> Changes in v3:
> - drop __ro_after_init attribute due to write to read-only page concern

What concern? That __ro_after_init can stay. It is built-in only.

> diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
> index d9afb6ae98cf56d6fd9f858c31a3d23546c3b450..54e1d5602f15a04127e900c0a4983022d49da553 100644
> --- a/drivers/irqchip/irq-riscv-aplic-main.c
> +++ b/drivers/irqchip/irq-riscv-aplic-main.c
> @@ -116,14 +116,12 @@ static struct syscore aplic_syscore = {
>  	.ops = &aplic_syscore_ops,
>  };
>  
> -static bool aplic_syscore_registered __ro_after_init;
> +static atomic_t aplic_syscore_registered = ATOMIC_INIT(0);
>  
>  static void aplic_syscore_init(void)
>  {
> -	if (!aplic_syscore_registered) {
> +	if (!atomic_cmpxchg(&aplic_syscore_registered, 0, 1))
>  		register_syscore(&aplic_syscore);
> -		aplic_syscore_registered = true;
> -	}
>  }
>  
>  static int aplic_pm_notifier(struct notifier_block *nb, unsigned long action, void *data)
> 

Sebastian

Re: [PATCH v3] irqchip/riscv-aplic: fix asynchronous probing

[Vladimir Kondratiev]

On 10/13/26 18:27, Sebastian Andrzej Siewior wrote:
> EXTERNAL EMAIL: Do not click any links or open any attachments unless you trust the sender and know the content is safe.
> 
> On 2026-10-12 12:12:59 [+0300], Vladimir Kondratiev wrote:
>> boolean flag don't protect from race when probing several instances
>> asynchronously. On system with multiple APLICs, if probed asynchronously,
>> warning triggered as below.
> 
> Why is this async? Doesn't __platform_driver_probe() set it to sync?

Yes it does but aplic driver uses

builtin_platform_driver(aplic_driver);

that takes different route. Easy way to directly proof it: I boot with

driver_async_probe=* dyndbg=\"func __driver_attach +p\"

to request asynchronous probe and debug prints in the __driver_attach
shows:
[    0.236483] platform 48760000.interrupt-controller: probing driver riscv-aplic asynchronously
[    0.236513] platform 674e0000.interrupt-controller: probing driver riscv-aplic asynchronously
[    0.236539] platform 67560000.interrupt-controller: probing driver riscv-aplic asynchronously
 
> Would you mind providing a coherent description of the problem and
> implemented solution instead of slapping console output?

Sorry if it was not clear; shall I explain in details why pattern

if (!done) {
	do_something();
	done = true;
}

is not SMP safe? Add detailed explanation to commit message and shorten console output?

Reason for asynchronous probing is simple - to reduce boot time, it is critical for embedded
systems. Think of system managing car rear camera and proximity sensors, it should be ready when you
start driving, this lefts very short time for the kernel to boot.

>> ---
>> Changes in v3:
>> - drop __ro_after_init attribute due to write to read-only page concern
> 
> What concern? That __ro_after_init can stay. It is built-in only.
See comment to v2 by sachiko-bot$kernel.org here
https://lore.kernel.org/all/sashiko-outbox-167856@kernel.org/
quote:
---cut---
[Severity: High]

Will this trigger a fatal page fault if the probe is deferred?



The aplic_syscore_registered variable retains the __ro_after_init attribute,

which places it in a memory section that becomes strictly read-only after

the kernel initialization phase completes.



Platform driver probes can be deferred (for instance, waiting for an MSI

domain) and run in a workqueue after the init phase is done.



If a late probe calls aplic_syscore_init(), atomic_cmpxchg() will attempt

to write 1 to this read-only memory page, which causes a store fault (or

AMO fault on RISC-V).



Does the __ro_after_init attribute need to be removed here to safely

support late and deferred probing?

---cut---

and indeed, aplic_msi_setup() can return -EPROBE_DEFER so concern is valid and
existed in original code, it is not related to asynchronous probing

> 
>> diff --git a/drivers/irqchip/irq-riscv-aplic-main.c b/drivers/irqchip/irq-riscv-aplic-main.c
>> index d9afb6ae98cf56d6fd9f858c31a3d23546c3b450..54e1d5602f15a04127e900c0a4983022d49da553 100644
>> --- a/drivers/irqchip/irq-riscv-aplic-main.c
>> +++ b/drivers/irqchip/irq-riscv-aplic-main.c
>> @@ -116,14 +116,12 @@ static struct syscore aplic_syscore = {
>>  	.ops = &aplic_syscore_ops,
>>  };
>>  
>> -static bool aplic_syscore_registered __ro_after_init;
>> +static atomic_t aplic_syscore_registered = ATOMIC_INIT(0);
>>  
>>  static void aplic_syscore_init(void)
>>  {
>> -	if (!aplic_syscore_registered) {
>> +	if (!atomic_cmpxchg(&aplic_syscore_registered, 0, 1))
>>  		register_syscore(&aplic_syscore);
>> -		aplic_syscore_registered = true;
>> -	}
>>  }
>>  
>>  static int aplic_pm_notifier(struct notifier_block *nb, unsigned long action, void *data)
>>Thanks, Vladimir

Re: [PATCH v3] irqchip/riscv-aplic: fix asynchronous probing

[Sebastian Andrzej Siewior]

On 2026-10-14 11:15:05 [+0300], Vladimir Kondratiev wrote:
> On 10/13/26 18:27, Sebastian Andrzej Siewior wrote:
> > EXTERNAL EMAIL: Do not click any links or open any attachments unless you trust the sender and know the content is safe.
> > 
> > On 2026-10-12 12:12:59 [+0300], Vladimir Kondratiev wrote:
> >> boolean flag don't protect from race when probing several instances
> >> asynchronously. On system with multiple APLICs, if probed asynchronously,
> >> warning triggered as below.
> > 
> > Why is this async? Doesn't __platform_driver_probe() set it to sync?
> 
> Yes it does but aplic driver uses
> 
> builtin_platform_driver(aplic_driver);
> 
> that takes different route. Easy way to directly proof it: I boot with
> 
> driver_async_probe=* dyndbg=\"func __driver_attach +p\"
> 
> to request asynchronous probe and debug prints in the __driver_attach
> shows:
> [    0.236483] platform 48760000.interrupt-controller: probing driver riscv-aplic asynchronously
> [    0.236513] platform 674e0000.interrupt-controller: probing driver riscv-aplic asynchronously
> [    0.236539] platform 67560000.interrupt-controller: probing driver riscv-aplic asynchronously

Does "driver_async_probe=*" mark _all_ drivers as safe and you run into
a problem where a driver isn't safe? Does the "dyndbg=" argument show
the driver is probed asynchronously if you omit the "driver_async_probe"
argument?
So the question is do we want to let it probe asynchronously or should
we force it to sync.

> > Would you mind providing a coherent description of the problem and
> > implemented solution instead of slapping console output?
> 
> Sorry if it was not clear; shall I explain in details why pattern
> 
> if (!done) {
> 	do_something();
> 	done = true;
> }
> 
> is not SMP safe? Add detailed explanation to commit message and shorten console output?

No console output at all. You need to coherently explain the problem
without adding console output. This is a simple problem, there is no
need to for a huge commit message here. Also there is no need for SMP
here as we do have kernel preemption.

> Reason for asynchronous probing is simple - to reduce boot time, it is critical for embedded
> systems. Think of system managing car rear camera and proximity sensors, it should be ready when you
> start driving, this lefts very short time for the kernel to boot.

So the driver does not support async probing and you add it and call it
a fix? I don't mind async probing but it should be declared what the
actual intention is. And maybe the driver should be marked async in the
end.

> >> ---
> >> Changes in v3:
> >> - drop __ro_after_init attribute due to write to read-only page concern
> > 
> > What concern? That __ro_after_init can stay. It is built-in only.
> See comment to v2 by sachiko-bot$kernel.org here
> https://lore.kernel.org/all/sashiko-outbox-167856@kernel.org/
> quote:
> ---cut---
> [Severity: High]
> 
> Will this trigger a fatal page fault if the probe is deferred?
> 
…
> Platform driver probes can be deferred (for instance, waiting for an MSI
> 
> domain) and run in a workqueue after the init phase is done.
…
> and indeed, aplic_msi_setup() can return -EPROBE_DEFER so concern is valid and
> existed in original code, it is not related to asynchronous probing

I would not expect the MSI domain to be probed after kernel booting via
a module. This is the scenario where this would explode.
And if you consider this a problem you should address it and denote it
in the commit description as such and not silently changing it as part
of another change without any explanation.

Sebastian

Re: [PATCH v3] irqchip/riscv-aplic: fix asynchronous probing

[Vladimir Kondratiev]

On 10/14/26 13:53, Sebastian Andrzej Siewior wrote:
> EXTERNAL EMAIL: Do not click any links or open any attachments unless you trust the sender and know the content is safe.
> 
> On 2026-10-14 11:15:05 [+0300], Vladimir Kondratiev wrote:
>> On 10/13/26 18:27, Sebastian Andrzej Siewior wrote:
>>> EXTERNAL EMAIL: Do not click any links or open any attachments unless you trust the sender and know the content is safe.
>>>
>>> On 2026-10-12 12:12:59 [+0300], Vladimir Kondratiev wrote:
>>>> boolean flag don't protect from race when probing several instances
>>>> asynchronously. On system with multiple APLICs, if probed asynchronously,
>>>> warning triggered as below.
>>>
>>> Why is this async? Doesn't __platform_driver_probe() set it to sync?
>>
>> Yes it does but aplic driver uses
>>
>> builtin_platform_driver(aplic_driver);
>>
>> that takes different route. Easy way to directly proof it: I boot with
>>
>> driver_async_probe=* dyndbg=\"func __driver_attach +p\"
>>
>> to request asynchronous probe and debug prints in the __driver_attach
>> shows:
>> [    0.236483] platform 48760000.interrupt-controller: probing driver riscv-aplic asynchronously
>> [    0.236513] platform 674e0000.interrupt-controller: probing driver riscv-aplic asynchronously
>> [    0.236539] platform 67560000.interrupt-controller: probing driver riscv-aplic asynchronously
> 
> Does "driver_async_probe=*" mark _all_ drivers as safe and you run into
> a problem where a driver isn't safe? Does the "dyndbg=" argument show
> the driver is probed asynchronously if you omit the "driver_async_probe"
> argument?

Not exactly, "driver_async_probe=*" in bootargs request only drivers that has
PROBE_DEFAULT_STRATEGY to be probed asynchronously.

aplic_driver don't set probe_type thus it defaults to PROBE_DEFAULT_STRATEGY,
quote exact definition from kdoc:
 * @PROBE_DEFAULT_STRATEGY: Used by drivers that work equally well
 *	whether probed synchronously or asynchronously.

If driver is no capable of async probing it should use PROBE_FORCE_SYNCHRONOUS.
3-rd option for probe_type is PROBE_PREFER_ASYNCHRONOUS

Direct test shows exactly this: that "dyndbg=" argument prints
"probing driver riscv-aplic asynchronously" if I set "driver_async_probe=*"
and prints nothing, meaning driver probed synchronously,
without "driver_async_probe=*"

> So the question is do we want to let it probe asynchronously or should
> we force it to sync.

I see no reason for PROBE_FORCE_SYNCHRONOUS to prohibit asynchronous probing.
code itself is capable of asynchronous probing. Preference for particular
platform can be specified in the bootargs.
> 
>>> Would you mind providing a coherent description of the problem and
>>> implemented solution instead of slapping console output?
>>
>> Sorry if it was not clear; shall I explain in details why pattern
>>
>> if (!done) {
>> 	do_something();
>> 	done = true;
>> }
>>
>> is not SMP safe? Add detailed explanation to commit message and shorten console output?
> 
> No console output at all. You need to coherently explain the problem
> without adding console output. This is a simple problem, there is no
> need to for a huge commit message here. Also there is no need for SMP
> here as we do have kernel preemption.

Got it. Will do. 

> 
>> Reason for asynchronous probing is simple - to reduce boot time, it is critical for embedded
>> systems. Think of system managing car rear camera and proximity sensors, it should be ready when you
>> start driving, this lefts very short time for the kernel to boot.
> 
> So the driver does not support async probing and you add it and call it
> a fix? I don't mind async probing but it should be declared what the
> actual intention is. And maybe the driver should be marked async in the
> end.

As above, drive declares it is agnostic to probe type, supporting both. So
I do think it is a fix.

>>>> ---
>>>> Changes in v3:
>>>> - drop __ro_after_init attribute due to write to read-only page concern
>>>
>>> What concern? That __ro_after_init can stay. It is built-in only.
>> See comment to v2 by sachiko-bot$kernel.org here
>> https://lore.kernel.org/all/sashiko-outbox-167856@kernel.org/
>> quote:
>> ---cut---
>> [Severity: High]
>>
>> Will this trigger a fatal page fault if the probe is deferred?
>>
> …
>> Platform driver probes can be deferred (for instance, waiting for an MSI
>>
>> domain) and run in a workqueue after the init phase is done.
> …
>> and indeed, aplic_msi_setup() can return -EPROBE_DEFER so concern is valid and
>> existed in original code, it is not related to asynchronous probing
> 
> I would not expect the MSI domain to be probed after kernel booting via
> a module. This is the scenario where this would explode.
> And if you consider this a problem you should address it and denote it
> in the commit description as such and not silently changing it as part
> of another change without any explanation.

OK, will add explanation for this.
This driver can't be a module, it is "builtin_platform_driver(aplic_driver);"
but still probe may be deferred if of node has "msi-parent", see
aplic_msi_setup() - it can return -EPROBE_DEFER;

Thanks, Vladimir

Re: [PATCH v3] irqchip/riscv-aplic: fix asynchronous probing

[Sebastian Andrzej Siewior]

On 2026-10-15 09:19:26 [+0300], Vladimir Kondratiev wrote:
> Not exactly, "driver_async_probe=*" in bootargs request only drivers that has
> PROBE_DEFAULT_STRATEGY to be probed asynchronously.

okay.

> > I would not expect the MSI domain to be probed after kernel booting via
> > a module. This is the scenario where this would explode.
> > And if you consider this a problem you should address it and denote it
> > in the commit description as such and not silently changing it as part
> > of another change without any explanation.
> 
> OK, will add explanation for this.
> This driver can't be a module, it is "builtin_platform_driver(aplic_driver);"
> but still probe may be deferred if of node has "msi-parent", see
> aplic_msi_setup() - it can return -EPROBE_DEFER;

The driver providing MSI capabilities (the MSI domain for this device)
must a module. This is the requirement so that _this_ driver returns
EPROBE_DEFER and *then* once the MSI domain driver is loaded as a module
(after boot) this driver is probed again and succeeds.

> Thanks, Vladimir

Sebastian

Re: [PATCH v3] irqchip/riscv-aplic: fix asynchronous probing

[Vladimir Kondratiev]

On 10/15/26 09:32, Sebastian Andrzej Siewior wrote:
> The driver providing MSI capabilities (the MSI domain for this device)
> must a module. This is the requirement so that _this_ driver returns
> EPROBE_DEFER and *then* once the MSI domain driver is loaded as a module
> (after boot) this driver is probed again and succeeds.
Why module? for "riscv-aplic", "msi-parent" shall be "riscv-imsic" and
it is "builtin_platform_driver", see
drivers/irqchip/irq-riscv-imsic-platform.c

Thanks, Vladimir

Re: [PATCH v3] irqchip/riscv-aplic: fix asynchronous probing

[Sebastian Andrzej Siewior]

On 2026-10-15 11:11:24 [+0300], Vladimir Kondratiev wrote:
> On 10/15/26 09:32, Sebastian Andrzej Siewior wrote:
> > The driver providing MSI capabilities (the MSI domain for this device)
> > must a module. This is the requirement so that _this_ driver returns
> > EPROBE_DEFER and *then* once the MSI domain driver is loaded as a module
> > (after boot) this driver is probed again and succeeds.
> Why module? for "riscv-aplic", "msi-parent" shall be "riscv-imsic" and
> it is "builtin_platform_driver", see
> drivers/irqchip/irq-riscv-imsic-platform.c

But _how_ do you want to make deferred probing happen that will change
aplic_syscore_registered after init without using modules?

> Thanks, Vladimir

Sebastian