| Message ID | 4c6d11b0-32dd-4ce3-a157-a848b6fc3154@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | limit passing around of cpu_user_regs | expand |
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
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,
--- 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;
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).