public inbox for linux-parisc@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH AUTOSEL 6.19-5.10] parisc: Prevent interrupts during reboot
       [not found] <20260214005825.3665084-1-sashal@kernel.org>
@ 2026-02-16 10:21 ` Geert Uytterhoeven
  2026-02-16 13:12   ` Sasha Levin
  0 siblings, 1 reply; 4+ messages in thread
From: Geert Uytterhoeven @ 2026-02-16 10:21 UTC (permalink / raw)
  To: Sasha Levin
  Cc: patches, stable, Helge Deller, guoren, neil.armstrong, brauner,
	yelangyan, schuster.simon, linux-csky, Parisc List

Hi Sasha

Cc linux-parisc

How did you (or the LLM?) came up with that CC list?!?

On Sat, 14 Feb 2026 at 01:58, Sasha Levin <sashal@kernel.org> wrote:
>
> From: Helge Deller <deller@gmx.de>
>
> [ Upstream commit 35ac5a728c878594f2ea6c43b57652a16be3c968 ]
>
> Signed-off-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>
> LLM Generated explanations, may be completely bogus:
>
> Now I have all the information I need for a thorough analysis. Here is
> my complete assessment:
>
> ---
>
> ## Detailed Analysis: "parisc: Prevent interrupts during reboot"
>
> ### 1. COMMIT MESSAGE ANALYSIS
>
> The commit message is minimal: "parisc: Prevent interrupts during
> reboot". It clearly states the problem it's fixing -- interrupts are not
> properly disabled during the reboot path on PA-RISC systems. The author
> is **Helge Deller**, the long-time parisc maintainer and the most
> authoritative developer for this architecture. The commit has no
> `Fixes:` tag or `Cc: stable` (as expected for autosel candidates).
>
> ### 2. CODE CHANGE ANALYSIS
>
> The change is **one single effective line of code** plus a comment:
>
> ```c
> /* prevent interrupts during reboot */
> set_eiem(0);
> ```
>
> This is inserted into `machine_restart()` in
> `arch/parisc/kernel/process.c` immediately after
> `pdc_chassis_send_status(PDC_CHASSIS_DIRECT_SHUTDOWN)` and before
> `pdc_do_reset()`.
>
> **What `set_eiem(0)` does:** On PA-RISC, the EIEM (External Interrupt
> Enable Mask, Control Register 15) controls which external interrupts can
> fire. Setting it to 0 **masks all external interrupts at the hardware
> level**, preventing any interrupt from being delivered to the CPU. This
> is defined as:
>
> ```82:82:arch/parisc/include/asm/special_insns.h
> #define set_eiem(val)   mtctl(val, CR_EIEM)
> ```
>
> **The bug:** Without this line, external interrupts remain enabled
> during the entire reboot sequence. This means:
>
> a) **Deadlock risk in `pdc_do_reset()`**: The `pdc_do_reset()` function
> acquires `pdc_lock` via `spin_lock_irqsave()`:
>
> ```1236:1246:arch/parisc/kernel/firmware.c
> int pdc_do_reset(void)
> {
>         int retval;
>         unsigned long flags;
>
>         spin_lock_irqsave(&pdc_lock, flags);
>         retval = mem_pdc_call(PDC_BROADCAST_RESET, PDC_DO_RESET);
>         spin_unlock_irqrestore(&pdc_lock, flags);
>
>         return retval;
> }
> ```
>
> While `spin_lock_irqsave` disables local interrupts, the PA-RISC EIEM
> hardware mask is a separate mechanism. On PA-RISC, the external
> interrupt delivery path goes through the EIEM -- an interrupt fires only
> if the corresponding EIEM bit is set AND the EIRR (External Interrupt
> Request Register) bit is set. If a hardware interrupt fires between
> `pdc_chassis_send_status()` (which also uses `pdc_lock`) and
> `pdc_do_reset()`, or during the firmware calls themselves, it could
> interfere with the reset process.
>
> b) **Interference with firmware reset**: `pdc_do_reset()` calls into PDC
> firmware (`mem_pdc_call(PDC_BROADCAST_RESET, PDC_DO_RESET)`). Firmware
> calls on PA-RISC are sensitive to the processor state. An interrupt
> arriving during or between firmware calls can corrupt the reset
> sequence, potentially causing the machine to **hang instead of
> rebooting**.
>
> c) **The `gsc_writel(CMD_RESET, COMMAND_GLOBAL)` fallback**: If
> `pdc_do_reset()` returns (on machines that don't implement
> `PDC_BROADCAST_RESET`), the code tries a hardware reset via
> `gsc_writel`. Interrupts during this path are equally problematic.
>
> ### 3. ESTABLISHED PATTERN IN PARISC AND OTHER ARCHITECTURES
>
> **PA-RISC internal precedent:**
> - `parisc_terminate()` in `traps.c` uses the exact same pattern:
>   `set_eiem(0)` followed by `local_irq_disable()` before critical
>   shutdown operations (line 428-429)
> - The SMP CPU hotplug code (`smp.c:481`) uses `set_eiem(0)` to disable
>   all external interrupts when taking a CPU offline
>
> **Other architectures ALL disable interrupts before reset:**
> - ARM: `local_irq_disable()` at line 136 of `arch/arm/kernel/reboot.c`
> - ARM64: `local_irq_disable()` at line 141 of
>   `arch/arm64/kernel/process.c`
> - x86: `local_irq_disable()` at line 100 of `arch/x86/kernel/reboot.c`
> - xtensa: `local_irq_disable()` at line 524 of
>   `arch/xtensa/kernel/setup.c`
> - nios2: `local_irq_disable()` at line 49 of
>   `arch/nios2/kernel/process.c`
> - csky: `local_irq_disable()` at line 25 of `arch/csky/kernel/power.c`
> - MIPS falcon: `local_irq_disable()` at line 37 of
>   `arch/mips/lantiq/falcon/reset.c`
>
> PA-RISC was the **outlier** in not disabling interrupts before reboot.
> This commit fixes that deficiency.
>
> ### 4. CLASSIFICATION
>
> This is a **bug fix** -- specifically fixing a potential hang/crash
> during reboot caused by unmasked interrupts during firmware calls. It is
> NOT a new feature, optimization, or cleanup.
>
> ### 5. SCOPE AND RISK ASSESSMENT
>
> - **Lines changed**: 3 (1 blank line, 1 comment, 1 functional line)
> - **Files touched**: 1 (`arch/parisc/kernel/process.c`)
> - **Subsystem**: parisc architecture, reboot path
> - **Risk**: **Extremely low**. Disabling interrupts before a system
>   reset is universally accepted as correct and necessary. The
>   `set_eiem(0)` pattern is already used in the same codebase for similar
>   critical paths.
>
> ### 6. USER IMPACT
>
> - **Who is affected**: All PA-RISC Linux users who reboot their machines
> - **What happens without the fix**: Potential hang during reboot,
>   requiring a hard power cycle. This could be intermittent and timing-
>   dependent, making it hard to reproduce but very real.
> - **Severity if triggered**: System hang requiring manual intervention
>   (power cycle)
>
> ### 7. STABILITY INDICATORS
>
> - Author is the subsystem maintainer (highest trust level)
> - Pattern is well-established across all other architectures
> - The exact same `set_eiem(0)` call is used in the parisc
>   `parisc_terminate()` function already
> - The change is trivially small and obviously correct
>
> ### 8. DEPENDENCY CHECK
>
> - No dependencies on other commits
> - The `set_eiem()` macro has existed since the earliest parisc code
> - The `machine_restart()` function is largely unchanged across all
>   stable trees
> - This will apply cleanly to any kernel version that has parisc support
>
> ### Summary
>
> This is a trivially small, obviously correct fix that prevents
> interrupts during the reboot sequence on PA-RISC systems. Without it, an
> untimely interrupt can interfere with PDC firmware calls during reset,
> potentially causing the machine to hang instead of rebooting. Every
> other Linux architecture disables interrupts before reboot; parisc was
> the outlier. The fix uses the same `set_eiem(0)` pattern already used in
> `parisc_terminate()` and SMP CPU offline code. The risk is essentially
> zero -- there is no conceivable way that disabling interrupts right
> before a hardware reset could cause a regression.
>
> **YES**
>
>  arch/parisc/kernel/process.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
> index e64ab5d2a40d6..703644e5bfc4a 100644
> --- a/arch/parisc/kernel/process.c
> +++ b/arch/parisc/kernel/process.c
> @@ -85,6 +85,9 @@ void machine_restart(char *cmd)
>  #endif
>         /* set up a new led state on systems shipped with a LED State panel */
>         pdc_chassis_send_status(PDC_CHASSIS_DIRECT_SHUTDOWN);
> +
> +       /* prevent interrupts during reboot */
> +       set_eiem(0);
>
>         /* "Normal" system reset */
>         pdc_do_reset();
> --
> 2.51.0
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH AUTOSEL 6.19-5.10] parisc: Prevent interrupts during reboot
  2026-02-16 10:21 ` [PATCH AUTOSEL 6.19-5.10] parisc: Prevent interrupts during reboot Geert Uytterhoeven
@ 2026-02-16 13:12   ` Sasha Levin
  2026-02-16 13:28     ` Geert Uytterhoeven
  0 siblings, 1 reply; 4+ messages in thread
From: Sasha Levin @ 2026-02-16 13:12 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: patches, stable, Helge Deller, guoren, neil.armstrong, brauner,
	yelangyan, schuster.simon, linux-csky, Parisc List

On Mon, Feb 16, 2026 at 11:21:25AM +0100, Geert Uytterhoeven wrote:
>Hi Sasha
>
>Cc linux-parisc
>
>How did you (or the LLM?) came up with that CC list?!?

Interesting...

$ ~/linux/scripts/get_maintainer.pl --pattern-depth=1 --no-rolestats --nor --nos 0001-parisc-Prevent-interrupts-during-reboot.patch 
Neil Armstrong <neil.armstrong@linaro.org>
"Guo Ren (Alibaba Damo Academy)" <guoren@kernel.org>
Christian Brauner <brauner@kernel.org>
Geert Uytterhoeven <geert@linux-m68k.org>
Andreas Larsson <andreas@gaisler.com>
Helge Deller <deller@gmx.de>
Langyan Ye <yelangyan@huaqin.corp-partner.google.com>
Simon Schuster <schuster.simon@siemens-energy.com>

I think that I'll fix it by replacing --pattern-depth with --nogit --nogit-fallback:

$ ~/linux/scripts/get_maintainer.pl --no-git --nogit-fallback --no-rolestats --nor --nos 0001-parisc-Prevent-interrupts-during-reboot.patch 
"James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
Helge Deller <deller@gmx.de>
linux-parisc@vger.kernel.org
linux-kernel@vger.kernel.org


Thanks for reporting this!

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH AUTOSEL 6.19-5.10] parisc: Prevent interrupts during reboot
  2026-02-16 13:12   ` Sasha Levin
@ 2026-02-16 13:28     ` Geert Uytterhoeven
  2026-02-16 15:48       ` Sasha Levin
  0 siblings, 1 reply; 4+ messages in thread
From: Geert Uytterhoeven @ 2026-02-16 13:28 UTC (permalink / raw)
  To: Sasha Levin
  Cc: patches, stable, Helge Deller, guoren, neil.armstrong, brauner,
	yelangyan, schuster.simon, linux-csky, Parisc List

Hi Sasha,

On Mon, 16 Feb 2026 at 14:12, Sasha Levin <sashal@kernel.org> wrote:
> On Mon, Feb 16, 2026 at 11:21:25AM +0100, Geert Uytterhoeven wrote:
> >Cc linux-parisc
> >
> >How did you (or the LLM?) came up with that CC list?!?
>
> Interesting...
>
> $ ~/linux/scripts/get_maintainer.pl --pattern-depth=1 --no-rolestats --nor --nos 0001-parisc-Prevent-interrupts-during-reboot.patch
> Neil Armstrong <neil.armstrong@linaro.org>
> "Guo Ren (Alibaba Damo Academy)" <guoren@kernel.org>
> Christian Brauner <brauner@kernel.org>
> Geert Uytterhoeven <geert@linux-m68k.org>
> Andreas Larsson <andreas@gaisler.com>
> Helge Deller <deller@gmx.de>
> Langyan Ye <yelangyan@huaqin.corp-partner.google.com>
> Simon Schuster <schuster.simon@siemens-energy.com>

Still doesn't explain linux-csky?

> I think that I'll fix it by replacing --pattern-depth with --nogit --nogit-fallback:
>
> $ ~/linux/scripts/get_maintainer.pl --no-git --nogit-fallback --no-rolestats --nor --nos 0001-parisc-Prevent-interrupts-during-reboot.patch
> "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>
> Helge Deller <deller@gmx.de>
> linux-parisc@vger.kernel.org
> linux-kernel@vger.kernel.org

Much better!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH AUTOSEL 6.19-5.10] parisc: Prevent interrupts during reboot
  2026-02-16 13:28     ` Geert Uytterhoeven
@ 2026-02-16 15:48       ` Sasha Levin
  0 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-02-16 15:48 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: patches, stable, Helge Deller, guoren, neil.armstrong, brauner,
	yelangyan, schuster.simon, linux-csky, Parisc List

On Mon, Feb 16, 2026 at 02:28:14PM +0100, Geert Uytterhoeven wrote:
>Hi Sasha,
>
>On Mon, 16 Feb 2026 at 14:12, Sasha Levin <sashal@kernel.org> wrote:
>> On Mon, Feb 16, 2026 at 11:21:25AM +0100, Geert Uytterhoeven wrote:
>> >Cc linux-parisc
>> >
>> >How did you (or the LLM?) came up with that CC list?!?
>>
>> Interesting...
>>
>> $ ~/linux/scripts/get_maintainer.pl --pattern-depth=1 --no-rolestats --nor --nos 0001-parisc-Prevent-interrupts-during-reboot.patch
>> Neil Armstrong <neil.armstrong@linaro.org>
>> "Guo Ren (Alibaba Damo Academy)" <guoren@kernel.org>
>> Christian Brauner <brauner@kernel.org>
>> Geert Uytterhoeven <geert@linux-m68k.org>
>> Andreas Larsson <andreas@gaisler.com>
>> Helge Deller <deller@gmx.de>
>> Langyan Ye <yelangyan@huaqin.corp-partner.google.com>
>> Simon Schuster <schuster.simon@siemens-energy.com>
>
>Still doesn't explain linux-csky?

Oh, that was there because the LLM mentioned csky in it's comparison with other
archs and get_maintainer.pl used keyword matching.

If you grab the actual patch that was sent in the AUTOSEL mail:

$ ~/linux/scripts/get_maintainer.pl --pattern-depth=1 --no-rolestats --nor --nos raw
Guo Ren <guoren@kernel.org>
linux-csky@vger.kernel.org

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-02-16 15:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260214005825.3665084-1-sashal@kernel.org>
2026-02-16 10:21 ` [PATCH AUTOSEL 6.19-5.10] parisc: Prevent interrupts during reboot Geert Uytterhoeven
2026-02-16 13:12   ` Sasha Levin
2026-02-16 13:28     ` Geert Uytterhoeven
2026-02-16 15:48       ` Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox