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(-)
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.
To achieve MISRA compliance, the condition is expanded inline at all
call sites, using plain C constructs. This ensures clarity while
retaining the original behavior. 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>
---
Link to v1:
https://patchew.org/Xen/20250729223110.3404441-1-andrew.cooper3@citrix.com/
Changes in v2:
- updated commit message
---
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.0On 16.08.2025 18:31, 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. > > To achieve MISRA compliance, the condition is expanded inline at all > call sites, That's not correct. Andrew's description specifically said "..., 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." Imo this wording wants adding back, with adjustments as necessary to make it fit the rest of the sentence. (Likely doable on commit, if no other reason for a v3 arises.) Jan > using plain C constructs. This ensures clarity while > retaining the original behavior. 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>
© 2016 - 2025 Red Hat, Inc.