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.coop...@citrix.com>
Reviewed-by: Dmytro Prokopchuk1 <dmytro_prokopch...@epam.com>
> ---
> CC: Anthony PERARD <anthony.per...@vates.tech>
> CC: Michal Orzel <michal.or...@amd.com>
> CC: Jan Beulich <jbeul...@suse.com>
> CC: Julien Grall <jul...@xen.org>
> CC: Roger Pau Monné <roger....@citrix.com>
> CC: Stefano Stabellini <sstabell...@kernel.org>
> CC: Dmytro Prokopchuk1 <dmytro_prokopch...@epam.com>
> CC: Nicola Vetrini <nicola.vetr...@bugseng.com>
>
> 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
