Re: [PATCH RFC] riscv: disable local interrupts and stop other CPUs before restart

["Troy Mitchell" <troy.mitchell@linux.dev>]

On Wed Mar 11, 2026 at 5:49 PM CST, Troy Mitchell wrote:
> On Wed Mar 11, 2026 at 2:47 PM CST, Aurelien Jarno wrote:
>> Hi Troy,
>>
>> On 2026-03-11 10:51, Troy Mitchell wrote:
>>> Currently, the RISC-V implementation of machine_restart() directly calls
>>> do_kernel_restart() without disabling local interrupts or stopping other
>>> CPUs. This missing architectural setup causes fatal issues for systems
>>> that rely on external peripherals (e.g., I2C PMICs) to execute the system
>>> restart when CONFIG_PREEMPT_RCU is enabled.
>>> 
>>> When a restart handler relies on the I2C subsystem, the I2C core checks
>>> i2c_in_atomic_xfer_mode() to decide whether to use the sleepable xfer
>>> or the polling atomic_xfer. This check evaluates to true if
>>> (!preemptible() || irqs_disabled()).
>>> 
>>> During do_kernel_restart(), the restart handlers are invoked via
>>> atomic_notifier_call_chain(), which holds an RCU read lock.
>>> The behavior diverges based on the preemption model:
>>> 1. Under CONFIG_PREEMPT_VOLUNTARY or CONFIG_PREEMPT_NONE, rcu_read_lock()
>>>    implicitly disables preemption. preemptible() evaluates to false, and
>>>    the I2C core correctly routes to the atomic, polling transfer path.
>>> 2. Under CONFIG_PREEMPT_RCU, rcu_read_lock() does NOT disable preemption.
>>>    Since machine_restart() left local interrupts enabled, irqs_disabled()
>>>    is false, and preempt_count is 0. Consequently, preemptible() evaluates
>>>    to true.
>>> 
>>> As a result, the I2C core falsely assumes a sleepable context and routes
>>> the transfer to the standard master_xfer path. This inevitably triggers a
>>> schedule() call while holding the RCU read lock, resulting in a fatal splat:
>>> "Voluntary context switch within RCU read-side critical section!" and
>>> a system hang.
>>> 
>>> Align RISC-V with other major architectures (e.g., ARM64) by adding
>>> local_irq_disable() and smp_send_stop() to machine_restart().
>>> - local_irq_disable() guarantees a strict atomic context, forcing sub-
>>>   systems like I2C to always fall back to polling mode.
>>> - smp_send_stop() ensures exclusive hardware access by quiescing other
>>>   CPUs, preventing them from holding bus locks (e.g., I2C spinlocks)
>>>   during the final restart phase.
>>> 
>>> Signed-off-by: Troy Mitchell <troy.mitchell@linux.dev>
>>> ---
>>>  arch/riscv/kernel/reset.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>
>> Thanks. I have been debugging that and it matches my analysis.
>>
>>> diff --git a/arch/riscv/kernel/reset.c b/arch/riscv/kernel/reset.c
>>> index 912288572226..7a5dcfdc3674 100644
>>> --- a/arch/riscv/kernel/reset.c
>>> +++ b/arch/riscv/kernel/reset.c
>>> @@ -5,6 +5,7 @@
>>>  
>>>  #include <linux/reboot.h>
>>>  #include <linux/pm.h>
>>> +#include <linux/smp.h>
>>>  
>>>  static void default_power_off(void)
>>>  {
>>> @@ -17,6 +18,10 @@ EXPORT_SYMBOL(pm_power_off);
>>>  
>>>  void machine_restart(char *cmd)
>>>  {
>>> +	/* Disable interrupts first */
>>> +	local_irq_disable();
>>> +	smp_send_stop();
>>> +
>>>  	do_kernel_restart(cmd);
>>>  	while (1);
>>>  }
>>> 
>>
>> I have started to change the power reset driver to call the I2C code 
>> from a workqueue instead of directly from the notifier call back, but 
>> that's just papering over the issue.
> Since the requirements for i2c_in_atomic() weren't being met, I initially
> considered disabling interrupts before the p1 restart code.
>
> However, I didn't feel that was a generic enough solution, so I looked into
> the architecture-level implementation. That's when I realized how bare-bones
> the current RISC-V machine_restart() actually is..
>
FYI, the discussion with Aurelien started here [1].

Link: https://lore.kernel.org/all/DGZM6WUAVPPS.20Y0NIZYI4572@linux.spacemit.com/ [1]


                          - Troy
>>
>> Your approach is much better and 
>> aligns riscv64 with other architectures, which is important as we might 
>> have shared PMIC drivers.
>>
>> Therefore:
>>
>> Tested-by: Aurelien Jarno <aurelien@aurel32.net>
>> Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
> Thanks. :)
>
>                                     - Troy
>>
>> Regards
>> Aurelien