[PATCH v5 06/12] xen/arm: irq: Restore state of local IRQs during system resume

Mykola Kvach posted 12 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v5 06/12] xen/arm: irq: Restore state of local IRQs during system resume
Posted by Mykola Kvach 2 months, 2 weeks ago
From: Mykola Kvach <mykola_kvach@epam.com>

On ARM, the first 32 interrupts (SGIs and PPIs) are banked per-CPU
and not restored by gic_resume (for secondary cpus).

This patch introduces restore_local_irqs_on_resume, a function that
restores the state of local interrupts on the target CPU during
system resume.

It iterates over all local IRQs and re-enables those that were not
disabled, reprogramming their routing and affinity accordingly.

The function is invoked from start_secondary, ensuring that local IRQ
state is restored early during CPU bring-up after suspend.

Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
 xen/arch/arm/irq.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 148f184f8b..b3ff38dc27 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -113,6 +113,47 @@ static int init_local_irq_data(unsigned int cpu)
     return 0;
 }
 
+/*
+ * The first 32 interrupts (PPIs and SGIs) are per-CPU,
+ * so call this function on the target CPU to restore them.
+ *
+ * SPIs are restored via gic_resume.
+ */
+static void restore_local_irqs_on_resume(void)
+{
+    int irq;
+
+    if ( system_state != SYS_STATE_resume )
+        return;
+
+    spin_lock(&local_irqs_type_lock);
+
+    for ( irq = 0; irq < NR_LOCAL_IRQS; irq++ )
+    {
+        struct irq_desc *desc = irq_to_desc(irq);
+
+        spin_lock(&desc->lock);
+
+        if ( test_bit(_IRQ_DISABLED, &desc->status) )
+        {
+            spin_unlock(&desc->lock);
+            continue;
+        }
+
+        /* it is needed to avoid asserts in below calls */
+        set_bit(_IRQ_DISABLED, &desc->status);
+
+        gic_route_irq_to_xen(desc, GIC_PRI_IRQ);
+
+        /* _IRQ_DISABLED is cleared by below call */
+        desc->handler->startup(desc);
+
+        spin_unlock(&desc->lock);
+    }
+
+    spin_unlock(&local_irqs_type_lock);
+}
+
 static int cpu_callback(struct notifier_block *nfb, unsigned long action,
                         void *hcpu)
 {
@@ -131,6 +172,9 @@ static int cpu_callback(struct notifier_block *nfb, unsigned long action,
             printk(XENLOG_ERR "Unable to allocate local IRQ for CPU%u\n",
                    cpu);
         break;
+    case CPU_STARTING:
+        restore_local_irqs_on_resume();
+        break;
     }
 
     return notifier_from_errno(rc);
-- 
2.48.1
Re: [PATCH v5 06/12] xen/arm: irq: Restore state of local IRQs during system resume
Posted by Volodymyr Babchuk 2 months, 1 week ago
Hi Mykola,

Mykola Kvach <xakep.amatop@gmail.com> writes:

> From: Mykola Kvach <mykola_kvach@epam.com>
>
> On ARM, the first 32 interrupts (SGIs and PPIs) are banked per-CPU
> and not restored by gic_resume (for secondary cpus).
>
> This patch introduces restore_local_irqs_on_resume, a function that
> restores the state of local interrupts on the target CPU during
> system resume.
>
> It iterates over all local IRQs and re-enables those that were not
> disabled, reprogramming their routing and affinity accordingly.
>
> The function is invoked from start_secondary, ensuring that local IRQ
> state is restored early during CPU bring-up after suspend.
>
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
>  xen/arch/arm/irq.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 148f184f8b..b3ff38dc27 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -113,6 +113,47 @@ static int init_local_irq_data(unsigned int cpu)
>      return 0;
>  }
>  
> +/*
> + * The first 32 interrupts (PPIs and SGIs) are per-CPU,
> + * so call this function on the target CPU to restore them.
> + *
> + * SPIs are restored via gic_resume.
> + */
> +static void restore_local_irqs_on_resume(void)
> +{
> +    int irq;
> +
> +    if ( system_state != SYS_STATE_resume )
> +        return;

Maybe it is better to move this check to restore_local_irqs_on_resume()
caller? You can put ASSERT(system_state == SYS_STATE_resume) there
instead.

I am saying this because name of restore_local_irqs_on_resume() implies that it
should be called only on resume.

> +
> +    spin_lock(&local_irqs_type_lock);
> +
> +    for ( irq = 0; irq < NR_LOCAL_IRQS; irq++ )
> +    {
> +        struct irq_desc *desc = irq_to_desc(irq);
> +
> +        spin_lock(&desc->lock);
> +
> +        if ( test_bit(_IRQ_DISABLED, &desc->status) )
> +        {
> +            spin_unlock(&desc->lock);
> +            continue;
> +        }
> +
> +        /* it is needed to avoid asserts in below calls */
> +        set_bit(_IRQ_DISABLED, &desc->status);

Assert in the call below isn't just because. You need to either call
desc->handler->disable() to properly disable the IRQ or justify why it
is fine to just set the bit.

> +
> +        gic_route_irq_to_xen(desc, GIC_PRI_IRQ);
> +
> +        /* _IRQ_DISABLED is cleared by below call */
> +        desc->handler->startup(desc);
> +
> +        spin_unlock(&desc->lock);
> +    }
> +
> +    spin_unlock(&local_irqs_type_lock);
> +}
> +
>  static int cpu_callback(struct notifier_block *nfb, unsigned long action,
>                          void *hcpu)
>  {
> @@ -131,6 +172,9 @@ static int cpu_callback(struct notifier_block *nfb, unsigned long action,
>              printk(XENLOG_ERR "Unable to allocate local IRQ for CPU%u\n",
>                     cpu);
>          break;
> +    case CPU_STARTING:
> +        restore_local_irqs_on_resume();
> +        break;
>      }
>  
>      return notifier_from_errno(rc);

-- 
WBR, Volodymyr
Re: [PATCH v5 06/12] xen/arm: irq: Restore state of local IRQs during system resume
Posted by Mykola Kvach 2 months ago
Hi Volodymyr,

On Sat, Aug 23, 2025 at 3:45 AM Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
> Hi Mykola,
>
> Mykola Kvach <xakep.amatop@gmail.com> writes:
>
> > From: Mykola Kvach <mykola_kvach@epam.com>
> >
> > On ARM, the first 32 interrupts (SGIs and PPIs) are banked per-CPU
> > and not restored by gic_resume (for secondary cpus).
> >
> > This patch introduces restore_local_irqs_on_resume, a function that
> > restores the state of local interrupts on the target CPU during
> > system resume.
> >
> > It iterates over all local IRQs and re-enables those that were not
> > disabled, reprogramming their routing and affinity accordingly.
> >
> > The function is invoked from start_secondary, ensuring that local IRQ
> > state is restored early during CPU bring-up after suspend.
> >
> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> > ---
> >  xen/arch/arm/irq.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> >
> > diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> > index 148f184f8b..b3ff38dc27 100644
> > --- a/xen/arch/arm/irq.c
> > +++ b/xen/arch/arm/irq.c
> > @@ -113,6 +113,47 @@ static int init_local_irq_data(unsigned int cpu)
> >      return 0;
> >  }
> >
> > +/*
> > + * The first 32 interrupts (PPIs and SGIs) are per-CPU,
> > + * so call this function on the target CPU to restore them.
> > + *
> > + * SPIs are restored via gic_resume.
> > + */
> > +static void restore_local_irqs_on_resume(void)
> > +{
> > +    int irq;
> > +
> > +    if ( system_state != SYS_STATE_resume )
> > +        return;
>
> Maybe it is better to move this check to restore_local_irqs_on_resume()
> caller? You can put ASSERT(system_state == SYS_STATE_resume) there
> instead.
>
> I am saying this because name of restore_local_irqs_on_resume() implies that it
> should be called only on resume.

Not a problem.
I'll move checking outside the function.

>
> > +
> > +    spin_lock(&local_irqs_type_lock);
> > +
> > +    for ( irq = 0; irq < NR_LOCAL_IRQS; irq++ )
> > +    {
> > +        struct irq_desc *desc = irq_to_desc(irq);
> > +
> > +        spin_lock(&desc->lock);
> > +
> > +        if ( test_bit(_IRQ_DISABLED, &desc->status) )
> > +        {
> > +            spin_unlock(&desc->lock);
> > +            continue;
> > +        }
> > +
> > +        /* it is needed to avoid asserts in below calls */
> > +        set_bit(_IRQ_DISABLED, &desc->status);
>
> Assert in the call below isn't just because. You need to either call
> desc->handler->disable() to properly disable the IRQ or justify why it
> is fine to just set the bit.

Got it. I’ll call the disable handler here instead.

>
> > +
> > +        gic_route_irq_to_xen(desc, GIC_PRI_IRQ);
> > +
> > +        /* _IRQ_DISABLED is cleared by below call */
> > +        desc->handler->startup(desc);
> > +
> > +        spin_unlock(&desc->lock);
> > +    }
> > +
> > +    spin_unlock(&local_irqs_type_lock);
> > +}
> > +
> >  static int cpu_callback(struct notifier_block *nfb, unsigned long action,
> >                          void *hcpu)
> >  {
> > @@ -131,6 +172,9 @@ static int cpu_callback(struct notifier_block *nfb, unsigned long action,
> >              printk(XENLOG_ERR "Unable to allocate local IRQ for CPU%u\n",
> >                     cpu);
> >          break;
> > +    case CPU_STARTING:
> > +        restore_local_irqs_on_resume();
> > +        break;
> >      }
> >
> >      return notifier_from_errno(rc);
>
> --
> WBR, Volodymyr

Best regards,
Mykola