Re: [XEN][PATCH] xen/evtchn: enable build optimization for evtchn_move_pirqs()/send_guest_pirq()

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

On 17.07.25 16:10, Jan Beulich wrote:
> On 17.07.2025 15:01, Grygorii Strashko wrote:
> > --- a/xen/common/event_channel.c
> > +++ b/xen/common/event_channel.c
> > @@ -975,6 +975,9 @@ void send_guest_pirq(struct domain *d, const struct pirq 
> > *pirq)
> >       int port;
> >       struct evtchn *chn;
> >   
> > +    if (!IS_ENABLED(CONFIG_HAS_PIRQ))
> > +        return;
> > +
> >       /*
> >        * PV guests: It should not be possible to race with __evtchn_close(). The
> >        *     caller of this function must synchronise with pirq_guest_unbind().
> Isn't this function unreachable on Arm, and hence a Misra rule 2.1 violation,
> requiring #ifdef around the entire function to address?
Yes. It's unused on Arm, only x86 is an user.
I can put it under ifdef.

> > @@ -1710,10 +1713,15 @@ void evtchn_destroy_final(struct domain *d)
> >   void evtchn_move_pirqs(struct vcpu *v)
> >   {
> >       struct domain *d = v->domain;
> > -    const cpumask_t *mask = cpumask_of(v->processor);
> > +    const cpumask_t *mask;
> This change shouldn't be necessary; compilers ought to be able to DCE the
> code.
Unfortunately not, with "-O1" more code is generated as cpumask_of() is 
complicated inside.

> >       unsigned int port;
> >       struct evtchn *chn;
> >   
> > +    if (!IS_ENABLED(CONFIG_HAS_PIRQ))
> Nit (style): Missing blanks (see other nearby if()-s).
>
> I wonder though whether we wouldn't better have x86'es arch_move_irqs()
> invoke this function, and then #ifdef it out here altogether as well.
Do you mean as in the below diff?

diff --git a/xen/arch/x86/include/asm/irq.h b/xen/arch/x86/include/asm/irq.h
index 3c73073b71b3..6d2bdfc9df1a 100644
--- a/xen/arch/x86/include/asm/irq.h
+++ b/xen/arch/x86/include/asm/irq.h
@@ -224,7 +224,7 @@ void cleanup_domain_irq_mapping(struct domain *d);
 
 bool cpu_has_pending_apic_eoi(void);
 
-static inline void arch_move_irqs(struct vcpu *v) { }
+void arch_move_irqs(struct vcpu *v);
 
 struct msi_info;
 int allocate_and_map_gsi_pirq(struct domain *d, int index, int *pirq_p);
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 556134f85aa0..b8d8f202119d 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1851,6 +1851,10 @@ void pirq_guest_unbind(struct domain *d, struct pirq 
*pirq)
         cleanup_domain_irq_pirq(d, irq, pirq);
 }
 
+void arch_move_irqs(struct vcpu *v) {
+    evtchn_move_pirqs(v);
+}
+
 static bool pirq_guest_force_unbind(struct domain *d, struct pirq *pirq)
 {
     struct irq_desc *desc;
diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index 13fdf57e57b9..ad6032fb2865 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -642,7 +642,6 @@ int sched_init_vcpu(struct vcpu *v)
 static void vcpu_move_irqs(struct vcpu *v)
 {
     arch_move_irqs(v);
-    evtchn_move_pirqs(v);
 }


--
Best regards,
-grygorii