[PATCHv2] gpio: mvebu: keep resume masks within the irqchip cache

[PATCHv2] gpio: mvebu: keep resume masks within the irqchip cache

[Rosen Penev]

mvebu_gpio_resume() writes the edge/level mask registers saved at
suspend time straight back to hardware, bypassing the irqchip's
mask_cache_priv.  genirq skips mask_irq() for a line it already
considers masked, so restoring a bit in hardware that genirq thinks
is still masked leaves that line unmasked behind genirq's back.  An
asserted level line then has nobody to ack it, and the moment
interrupts are re-enabled the chained handler storms, hanging resume.

AND the restored mask values with the matching irqchip mask cache so
only lines genirq currently considers unmasked are unmasked again.
Read the caches under gc->lock to keep them consistent with the
mask/unmask handlers.

Tested on Helios4 (armhf): 5 suspend cycles woken by magic packet,
no hang; mvebu_gpio_resume() returns in 6 usecs.

Assisted-by: LLM
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: Unlock spinlock before regmap_write.
 drivers/gpio/gpio-mvebu.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index c57758019e92..267e6d5bacd0 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -1049,6 +1049,8 @@ static int mvebu_gpio_suspend(struct platform_device *pdev, pm_message_t state)
 static int mvebu_gpio_resume(struct platform_device *pdev)
 {
 	struct mvebu_gpio_chip *mvchip = platform_get_drvdata(pdev);
+	u32 edge_cache = ~0U, level_cache = ~0U;
+	unsigned long flags;
 	int i;
 
 	regmap_write(mvchip->regs, GPIO_OUT_OFF + mvchip->offset,
@@ -1060,32 +1062,51 @@ static int mvebu_gpio_resume(struct platform_device *pdev)
 	regmap_write(mvchip->regs, GPIO_IN_POL_OFF + mvchip->offset,
 		     mvchip->in_pol_reg);
 
+	/*
+	 * genirq skips mask_irq() for a line it already considers masked, so
+	 * unmasking one behind its back leaves an asserted level line that
+	 * nobody masks. Restore only bits the irqchip cache still has set.
+	 *
+	 * Snapshot the caches under the raw spinlock, but release it before
+	 * the regmap writes below: regmap_write() takes a sleepable lock on
+	 * PREEMPT_RT.
+	 */
+	if (mvchip->domain) {
+		struct irq_chip_generic *gc;
+
+		gc = irq_get_domain_generic_chip(mvchip->domain, 0);
+		raw_spin_lock_irqsave(&gc->lock, flags);
+		level_cache = gc->chip_types[0].mask_cache_priv;
+		edge_cache = gc->chip_types[1].mask_cache_priv;
+		raw_spin_unlock_irqrestore(&gc->lock, flags);
+	}
+
 	switch (mvchip->soc_variant) {
 	case MVEBU_GPIO_SOC_VARIANT_ORION:
 	case MVEBU_GPIO_SOC_VARIANT_A8K:
 		regmap_write(mvchip->regs, GPIO_EDGE_MASK_OFF + mvchip->offset,
-			     mvchip->edge_mask_regs[0]);
+			     mvchip->edge_mask_regs[0] & edge_cache);
 		regmap_write(mvchip->regs, GPIO_LEVEL_MASK_OFF + mvchip->offset,
-			     mvchip->level_mask_regs[0]);
+			     mvchip->level_mask_regs[0] & level_cache);
 		break;
 	case MVEBU_GPIO_SOC_VARIANT_MV78200:
 		for (i = 0; i < 2; i++) {
 			regmap_write(mvchip->regs,
 				     GPIO_EDGE_MASK_MV78200_OFF(i),
-				     mvchip->edge_mask_regs[i]);
+				     mvchip->edge_mask_regs[i] & edge_cache);
 			regmap_write(mvchip->regs,
 				     GPIO_LEVEL_MASK_MV78200_OFF(i),
-				     mvchip->level_mask_regs[i]);
+				     mvchip->level_mask_regs[i] & level_cache);
 		}
 		break;
 	case MVEBU_GPIO_SOC_VARIANT_ARMADAXP:
 		for (i = 0; i < 4; i++) {
 			regmap_write(mvchip->regs,
 				     GPIO_EDGE_MASK_ARMADAXP_OFF(i),
-				     mvchip->edge_mask_regs[i]);
+				     mvchip->edge_mask_regs[i] & edge_cache);
 			regmap_write(mvchip->regs,
 				     GPIO_LEVEL_MASK_ARMADAXP_OFF(i),
-				     mvchip->level_mask_regs[i]);
+				     mvchip->level_mask_regs[i] & level_cache);
 		}
 		break;
 	default:
-- 
2.55.0

Re: [PATCHv2] gpio: mvebu: keep resume masks within the irqchip cache

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] gpio: mvebu: TOCTOU race in resume causes interrupt storm
--

commit 6b10f9bb173190e25a679e67f7992002ca12fe2f
Author: Rosen Penev <rosenp@gmail.com>

gpio: mvebu: keep resume masks within the irqchip cache

This commit fixes a resume hang in mvebu_gpio_resume by masking restored
values with the matching irqchip mask cache. It reads the caches under
gc->lock to keep them consistent with mask/unmask handlers.

> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index 93b8a08b04b9f..a2796240fe14b 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c

[ ... ]

> @@ -1045,32 +1047,51 @@ static int mvebu_gpio_resume(struct platform_device *pdev)
>  	regmap_write(mvchip->regs, GPIO_IN_POL_OFF + mvchip->offset,
>  		     mvchip->in_pol_reg);
>  
> +	/*
> +	 * genirq skips mask_irq() for a line it already considers masked, so
> +	 * unmasking one behind its back leaves an asserted level line that
> +	 * nobody masks. Restore only bits the irqchip cache still has set.
> +	 *
> +	 * Snapshot the caches under the raw spinlock, but release it before
> +	 * the regmap writes below: regmap_write() takes a sleepable lock on
> +	 * PREEMPT_RT.
> +	 */
> +	if (mvchip->domain) {
> +		struct irq_chip_generic *gc;
> +
> +		gc = irq_get_domain_generic_chip(mvchip->domain, 0);
> +		raw_spin_lock_irqsave(&gc->lock, flags);
> +		level_cache = gc->chip_types[0].mask_cache_priv;
> +		edge_cache = gc->chip_types[1].mask_cache_priv;
> +		raw_spin_unlock_irqrestore(&gc->lock, flags);

[Severity: High]
Does releasing the lock here create a race window?

If a level-triggered interrupt fires immediately after the lock is released,
the IRQ handler runs. Genirq would acquire the lock, mask the hardware, and
clear the bit in mask_cache_priv.

> +	}
> +
>  	switch (mvchip->soc_variant) {
>  	case MVEBU_GPIO_SOC_VARIANT_ORION:
>  	case MVEBU_GPIO_SOC_VARIANT_A8K:
>  		regmap_write(mvchip->regs, GPIO_EDGE_MASK_OFF + mvchip->offset,
> -			     mvchip->edge_mask_regs[0]);
> +			     mvchip->edge_mask_regs[0] & edge_cache);

[Severity: High]
Could writing the stale snapshot here unmask an IRQ that genirq just masked?

If the race described above occurs, mvebu_gpio_resume() continues and writes
its stale snapshot (with the bit set) to the hardware mask register. This
would unmask the line in hardware while genirq's irq_data maintains the
IRQD_IRQ_MASKED flag.

When the interrupt inevitably fires again, could genirq skip masking it
because it is already marked masked, leading to an unhandled infinite
interrupt storm and kernel lockup?


[Severity: High]
This is a pre-existing issue, but does the bitwise AND permanently drop IRQs
that were unmasked during early resume?

If a driver disables an IRQ before suspend, the snapshot bit in
edge_mask_regs is 0. If the driver then calls enable_irq() during an early
resume phase (like .resume_noirq), genirq correctly unmasks the hardware and
sets the cache bit to 1.

When mvebu_gpio_resume() later executes in the .resume phase, it calculates
0 (snapshot) & 1 (cache) = 0 and writes 0 to the hardware. Does this leave
the hardware permanently masked while genirq considers it unmasked, silently
breaking the interrupt?

>  		regmap_write(mvchip->regs, GPIO_LEVEL_MASK_OFF + mvchip->offset,
> -			     mvchip->level_mask_regs[0]);
> +			     mvchip->level_mask_regs[0] & level_cache);
>  		break;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915004406.115230-1-rosenp@gmail.com?part=1

Re: [PATCHv2] gpio: mvebu: keep resume masks within the irqchip cache

[Sebastian Andrzej Siewior]

On 2026-09-14 17:44:06 [-0700], Rosen Penev wrote:
> mvebu_gpio_resume() writes the edge/level mask registers saved at
> suspend time straight back to hardware, bypassing the irqchip's
> mask_cache_priv.  genirq skips mask_irq() for a line it already
> considers masked, so restoring a bit in hardware that genirq thinks
> is still masked leaves that line unmasked behind genirq's back.  An
> asserted level line then has nobody to ack it, and the moment
> interrupts are re-enabled the chained handler storms, hanging resume.

Why is the mask value changed after suspend?

Sebastian