[PATCH v1 1/4] arm/time: Use static irqaction

Mykyta Poturai posted 4 patches 2 weeks, 2 days ago
There is a newer version of this series
[PATCH v1 1/4] arm/time: Use static irqaction
Posted by Mykyta Poturai 2 weeks, 2 days ago
When stopping a core deinit_timer_interrupt is called in non-alloc
context, which causes xfree in release_irq to fail an assert.

To fix this, switch to a statically allocated irqaction that does not
need to be freed in release_irq.

Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
---
 xen/arch/arm/time.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index e74d30d258..6f215de210 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -303,6 +303,20 @@ static void check_timer_irq_cfg(unsigned int irq, const char *which)
            "WARNING: %s-timer IRQ%u is not level triggered.\n", which, irq);
 }
 
+static struct irqaction __read_mostly irq_hyp = {
+    .name = "hyptimer",
+    .handler = htimer_interrupt,
+    .dev_id = NULL,
+    .free_on_release = 0,
+};
+
+static struct irqaction __read_mostly irq_virt = {
+    .name = "virtimer",
+    .handler = vtimer_interrupt,
+    .dev_id = NULL,
+    .free_on_release = 0,
+};
+
 /* Set up the timer interrupt on this CPU */
 void init_timer_interrupt(void)
 {
@@ -314,10 +328,8 @@ void init_timer_interrupt(void)
     WRITE_SYSREG(0, CNTHP_CTL_EL2);   /* Hypervisor's timer disabled */
     isb();
 
-    request_irq(timer_irq[TIMER_HYP_PPI], 0, htimer_interrupt,
-                "hyptimer", NULL);
-    request_irq(timer_irq[TIMER_VIRT_PPI], 0, vtimer_interrupt,
-                   "virtimer", NULL);
+    setup_irq(timer_irq[TIMER_HYP_PPI], 0, &irq_hyp);
+    setup_irq(timer_irq[TIMER_VIRT_PPI], 0, &irq_virt);
 
     check_timer_irq_cfg(timer_irq[TIMER_HYP_PPI], "hypervisor");
     check_timer_irq_cfg(timer_irq[TIMER_VIRT_PPI], "virtual");
-- 
2.34.1
Re: [PATCH v1 1/4] arm/time: Use static irqaction
Posted by Julien Grall 2 weeks, 2 days ago
Hi Mykyta,

On 18/09/2025 13:16, Mykyta Poturai wrote:
> When stopping a core deinit_timer_interrupt is called in non-alloc
> context, which causes xfree in release_irq to fail an assert.
> 
> To fix this, switch to a statically allocated irqaction that does not
> need to be freed in release_irq.
 > > Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
> ---
>   xen/arch/arm/time.c | 20 ++++++++++++++++----
>   1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
> index e74d30d258..6f215de210 100644
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -303,6 +303,20 @@ static void check_timer_irq_cfg(unsigned int irq, const char *which)
>              "WARNING: %s-timer IRQ%u is not level triggered.\n", which, irq);
>   }
>   
> +static struct irqaction __read_mostly irq_hyp = {
> +    .name = "hyptimer",
> +    .handler = htimer_interrupt,
> +    .dev_id = NULL,
> +    .free_on_release = 0,
> +};
> +
> +static struct irqaction __read_mostly irq_virt = {
> +    .name = "virtimer",
> +    .handler = vtimer_interrupt,
> +    .dev_id = NULL,
> +    .free_on_release = 0,
> +};

setup_irq() will update the field "next" in irqaction. So we need one 
instance per call. Effectively, this means one per CPU. Therefore, we 
want to use DEFINE_PER_CPU. This applies to the rest of the series.

Cheers,

-- 
Julien Grall