From: Andrew Cooper <andrew.cooper3@citrix.com>

MISRA Rule 5.5 objects to a macro aliasing a function, which is what
pirq_cleanup_check() does. The macro was originally intended to ensure
the condition 'if (!pirq->evtchn)' is always checked before invoking
the function, avoiding errors across call sites.

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'.

While this complies with MISRA, it shifts the responsibility to
developers to check 'if (!pirq->evtchn)' at call sites.

No functional changes.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
---
Changes in v3:
- added back wording from v1, originally written by Andrew.

Link to v2:
https://patchew.org/Xen/ce37bdf7b5189d314c0f41628dbfb3281358bcf4.1755361782.git.dmytro._5Fprokopchuk1@epam.com/

Link to v1:
https://patchew.org/Xen/20250729223110.3404441-1-andrew.cooper3@citrix.com/
---
 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 556134f85a..1ed85c0c11 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 67700b050a..a3d18bc464 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 a2ca7e0e57..b73bb55055 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 95034c0d6b..6071b00f62 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);
-- 
2.43.0

On 25.08.2025 21:22, Dmytro Prokopchuk1 wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> MISRA Rule 5.5 objects to a macro aliasing a function, which is what
> pirq_cleanup_check() does. The macro was originally intended to ensure
> the condition 'if (!pirq->evtchn)' is always checked before invoking
> the function, avoiding errors across call sites.
> 
> 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'.
> 
> While this complies with MISRA, it shifts the responsibility to
> developers to check 'if (!pirq->evtchn)' at call sites.
> 
> No functional changes.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
> ---
> Changes in v3:
> - added back wording from v1, originally written by Andrew.

Thanks. Just to mention, though - you copied it verbatim, including the
typo (simplifies). Can surely be adjusted while committing, if and when
somebody acks this. (I think it has become sufficiently clear that I'm
not going to.)

Jan