[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] xen/irq: Delete the pirq_cleanup_check() macro
On 7/30/25 01:31, Andrew Cooper wrote: > MISRA Rule 5.5 objects to a macro aliasing a function, which is what > pirq_cleanup_check() does. The macro is an overly-complicated way of writing: > > if ( !pirq->evtchn ) > pirq_cleanup_check(pirq, d); > > There are only a handful of users, so expand it inline to be plain regular C. > > Doing this shows one path now needing braces, and one path in > evtchn_bind_pirq() where the expanded form simplies back to no delta, as it > follows an unconditional clear of info->evtchn. > > No functional change; The compiled hypervisors are the same. > > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Reviewed-by: Dmytro Prokopchuk1 <dmytro_prokopchuk1@xxxxxxxx> > --- > CC: Anthony PERARD <anthony.perard@xxxxxxxxxx> > CC: Michal Orzel <michal.orzel@xxxxxxx> > CC: Jan Beulich <jbeulich@xxxxxxxx> > CC: Julien Grall <julien@xxxxxxx> > CC: Roger Pau Monné <roger.pau@xxxxxxxxxx> > CC: Stefano Stabellini <sstabellini@xxxxxxxxxx> > CC: Dmytro Prokopchuk1 <dmytro_prokopchuk1@xxxxxxxx> > CC: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx> > > Clearly the compiler had already found the simplification in > evtchn_bind_pirq(). > > Yes, I know I could integrate the !info->evtchn into the outer if() condition, > but that's an even bigger mess. > --- > xen/arch/x86/irq.c | 11 +++++++---- > xen/common/event_channel.c | 5 ++++- > xen/drivers/passthrough/x86/hvm.c | 9 ++++++--- > xen/include/xen/irq.h | 3 --- > 4 files changed, 17 insertions(+), 11 deletions(-) > > diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c > index 556134f85aa0..1ed85c0c114e 100644 > --- a/xen/arch/x86/irq.c > +++ b/xen/arch/x86/irq.c > @@ -1325,7 +1325,8 @@ static void clear_domain_irq_pirq(struct domain *d, int > irq, struct pirq *pirq) > static void cleanup_domain_irq_pirq(struct domain *d, int irq, > struct pirq *pirq) > { > - pirq_cleanup_check(pirq, d); > + if ( !pirq->evtchn ) > + pirq_cleanup_check(pirq, d); > radix_tree_delete(&d->arch.irq_pirq, irq); > } > > @@ -1383,7 +1384,7 @@ struct pirq *alloc_pirq_struct(struct domain *d) > return pirq; > } > > -void (pirq_cleanup_check)(struct pirq *pirq, struct domain *d) > +void pirq_cleanup_check(struct pirq *pirq, struct domain *d) > { > /* > * Check whether all fields have their default values, and delete > @@ -2823,7 +2824,8 @@ int map_domain_emuirq_pirq(struct domain *d, int pirq, > int emuirq) > radix_tree_int_to_ptr(pirq)); > break; > default: > - pirq_cleanup_check(info, d); > + if ( !info->evtchn ) > + pirq_cleanup_check(info, d); > return err; > } > } > @@ -2858,7 +2860,8 @@ int unmap_domain_pirq_emuirq(struct domain *d, int pirq) > if ( info ) > { > info->arch.hvm.emuirq = IRQ_UNBOUND; > - pirq_cleanup_check(info, d); > + if ( !info->evtchn ) > + pirq_cleanup_check(info, d); > } > if ( emuirq != IRQ_PT ) > radix_tree_delete(&d->arch.hvm.emuirq_pirq, emuirq); > diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c > index c8c1bfa615df..ed48fbb55d67 100644 > --- a/xen/common/event_channel.c > +++ b/xen/common/event_channel.c > @@ -741,11 +741,14 @@ int evtchn_close(struct domain *d1, int port1, bool > guest) > if ( !is_hvm_domain(d1) || > domain_pirq_to_irq(d1, pirq->pirq) <= 0 || > unmap_domain_pirq_emuirq(d1, pirq->pirq) < 0 ) > + { > /* > * The successful path of unmap_domain_pirq_emuirq() will > have > * called pirq_cleanup_check() already. > */ > - pirq_cleanup_check(pirq, d1); > + if ( !pirq->evtchn ) > + pirq_cleanup_check(pirq, d1); > + } > } > unlink_pirq_port(chn1, d1->vcpu[chn1->notify_vcpu_id]); > break; > diff --git a/xen/drivers/passthrough/x86/hvm.c > b/xen/drivers/passthrough/x86/hvm.c > index a2ca7e0e570c..b73bb550554d 100644 > --- a/xen/drivers/passthrough/x86/hvm.c > +++ b/xen/drivers/passthrough/x86/hvm.c > @@ -329,7 +329,8 @@ int pt_irq_create_bind( > pirq_dpci->gmsi.gvec = 0; > pirq_dpci->dom = NULL; > pirq_dpci->flags = 0; > - pirq_cleanup_check(info, d); > + if ( !info->evtchn ) > + pirq_cleanup_check(info, d); > write_unlock(&d->event_lock); > return rc; > } > @@ -536,7 +537,8 @@ int pt_irq_create_bind( > hvm_irq_dpci->link_cnt[link]--; > } > pirq_dpci->flags = 0; > - pirq_cleanup_check(info, d); > + if ( !info->evtchn ) > + pirq_cleanup_check(info, d); > write_unlock(&d->event_lock); > xfree(girq); > xfree(digl); > @@ -693,7 +695,8 @@ int pt_irq_destroy_bind( > */ > pt_pirq_softirq_reset(pirq_dpci); > > - pirq_cleanup_check(pirq, d); > + if ( !pirq->evtchn ) > + pirq_cleanup_check(pirq, d); > } > > write_unlock(&d->event_lock); > diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h > index 95034c0d6bb5..6071b00f621e 100644 > --- a/xen/include/xen/irq.h > +++ b/xen/include/xen/irq.h > @@ -185,9 +185,6 @@ extern struct pirq *pirq_get_info(struct domain *d, int > pirq); > > void pirq_cleanup_check(struct pirq *pirq, struct domain *d); > > -#define pirq_cleanup_check(pirq, d) \ > - (!(pirq)->evtchn ? pirq_cleanup_check(pirq, d) : (void)0) > - > extern void pirq_guest_eoi(struct pirq *pirq); > extern void desc_guest_eoi(struct irq_desc *desc, struct pirq *pirq); > extern int pirq_guest_unmask(struct domain *d); > > base-commit: b5070a959667d60e627017d44fc5b5b1c5e98777
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |