Move functions (and their data) to common code, and invoke the functions
on Arm as well. This is in preparation of dropping the register
parameters from handler functions.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
To limit visibility of the per-CPU data item, we may want to consider
making the functions out-of-line ones (in common/irq.c).

--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -221,6 +221,7 @@ void do_IRQ(struct cpu_user_regs *regs,
 {
     struct irq_desc *desc = irq_to_desc(irq);
     struct irqaction *action;
+    struct cpu_user_regs *old_regs = set_irq_regs(regs);
 
     perfc_incr(irqs);
 
@@ -288,6 +289,7 @@ out:
 out_no_end:
     spin_unlock(&desc->lock);
     irq_exit();
+    set_irq_regs(old_regs);
 }
 
 void release_irq(unsigned int irq, const void *dev_id)
--- a/xen/arch/x86/include/asm/irq.h
+++ b/xen/arch/x86/include/asm/irq.h
@@ -70,27 +70,6 @@ extern bool opt_noirqbalance;
 
 extern int opt_irq_vector_map;
 
-/*
- * Per-cpu current frame pointer - the location of the last exception frame on
- * the stack
- */
-DECLARE_PER_CPU(struct cpu_user_regs *, __irq_regs);
-
-static inline struct cpu_user_regs *get_irq_regs(void)
-{
-	return this_cpu(__irq_regs);
-}
-
-static inline struct cpu_user_regs *set_irq_regs(struct cpu_user_regs *new_regs)
-{
-	struct cpu_user_regs *old_regs, **pp_regs = &this_cpu(__irq_regs);
-
-	old_regs = *pp_regs;
-	*pp_regs = new_regs;
-	return old_regs;
-}
-
-
 #define platform_legacy_irq(irq)	((irq) < 16)
 
 void cf_check event_check_interrupt(struct cpu_user_regs *regs);
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -53,8 +53,6 @@ static DEFINE_SPINLOCK(vector_lock);
 
 DEFINE_PER_CPU(vector_irq_t, vector_irq);
 
-DEFINE_PER_CPU(struct cpu_user_regs *, __irq_regs);
-
 static LIST_HEAD(irq_ratelimit_list);
 static DEFINE_SPINLOCK(irq_ratelimit_lock);
 static struct timer irq_ratelimit_timer;
--- a/xen/common/irq.c
+++ b/xen/common/irq.c
@@ -1,6 +1,8 @@
 #include <xen/irq.h>
 #include <xen/errno.h>
 
+DEFINE_PER_CPU(struct cpu_user_regs *, irq_regs);
+
 int init_one_irq_desc(struct irq_desc *desc)
 {
     int err;
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -131,6 +131,26 @@ void cf_check irq_actor_none(struct irq_
 #define irq_disable_none irq_actor_none
 #define irq_enable_none irq_actor_none
 
+/*
+ * Per-cpu interrupted context register state - the top-most interrupt frame
+ * on the stack.
+ */
+DECLARE_PER_CPU(struct cpu_user_regs *, irq_regs);
+
+static inline struct cpu_user_regs *get_irq_regs(void)
+{
+	return this_cpu(irq_regs);
+}
+
+static inline struct cpu_user_regs *set_irq_regs(struct cpu_user_regs *new_regs)
+{
+	struct cpu_user_regs *old_regs, **pp_regs = &this_cpu(irq_regs);
+
+	old_regs = *pp_regs;
+	*pp_regs = new_regs;
+	return old_regs;
+}
+
 struct domain;
 struct vcpu;

Hi Jan,

On 11/01/2024 07:32, Jan Beulich wrote:
> Move functions (and their data) to common code, and invoke the functions
> on Arm as well. This is in preparation of dropping the register
> parameters from handler functions.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

> ---
> To limit visibility of the per-CPU data item, we may want to consider
> making the functions out-of-line ones (in common/irq.c).
> 
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -221,6 +221,7 @@ void do_IRQ(struct cpu_user_regs *regs,
>   {
>       struct irq_desc *desc = irq_to_desc(irq);
>       struct irqaction *action;
> +    struct cpu_user_regs *old_regs = set_irq_regs(regs);
>   
>       perfc_incr(irqs);
>   
> @@ -288,6 +289,7 @@ out:
>   out_no_end:
>       spin_unlock(&desc->lock);
>       irq_exit();
> +    set_irq_regs(old_regs);
>   }

[...]

> +/*
> + * Per-cpu interrupted context register state - the top-most interrupt frame
> + * on the stack.
> + */
> +DECLARE_PER_CPU(struct cpu_user_regs *, irq_regs);
> +
> +static inline struct cpu_user_regs *get_irq_regs(void)
> +{
> +	return this_cpu(irq_regs);
> +}
> +
> +static inline struct cpu_user_regs *set_irq_regs(struct cpu_user_regs *new_regs)
> +{
> +	struct cpu_user_regs *old_regs, **pp_regs = &this_cpu(irq_regs);
> +
> +	old_regs = *pp_regs;
> +	*pp_regs = new_regs;

NIT: As you move the code, can you add a newline before the return?

> +	return old_regs;
> +}
> +
>   struct domain;
>   struct vcpu;

Cheers,

-- 
Julien Grall

On 11/01/2024 7:32 am, Jan Beulich wrote:
> Move functions (and their data) to common code, and invoke the functions
> on Arm as well. This is in preparation of dropping the register
> parameters from handler functions.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> with one wording
suggestion.

> ---
> To limit visibility of the per-CPU data item, we may want to consider
> making the functions out-of-line ones (in common/irq.c).

If we had working LTO, then maybe.  But we don't, and I think we can
reasonably cover (not using) this with normal code review.

> --- a/xen/include/xen/irq.h
> +++ b/xen/include/xen/irq.h
> @@ -131,6 +131,26 @@ void cf_check irq_actor_none(struct irq_
>  #define irq_disable_none irq_actor_none
>  #define irq_enable_none irq_actor_none
>  
> +/*
> + * Per-cpu interrupted context register state - the top-most interrupt frame
> + * on the stack.

The prior wording "last" wasn't great either.

I'd suggest "inner-most".  "top-most" comes with the usual confusions
with stacks being upside down, and inner/outer is more common
terminology with nesting.

~Andrew