Hi Mykola,
Mykola Kvach <xakep.ama...@gmail.com> writes:
> From: Mykola Kvach <mykola_kv...@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_kv...@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
