Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
---
xen/arch/x86/irq.c | 8 ++++----
xen/common/event_channel.c | 6 +++---
xen/drivers/passthrough/x86/hvm.c | 8 ++++----
xen/include/xen/irq.h | 2 +-
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 556134f85a..e70c7829b4 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -1325,7 +1325,7 @@ 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);
+ PIRQ_CLEANUP_CHECK(pirq, d);
radix_tree_delete(&d->arch.irq_pirq, irq);
}
@@ -1383,7 +1383,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 +2823,7 @@ 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);
+ PIRQ_CLEANUP_CHECK(info, d);
return err;
}
}
@@ -2858,7 +2858,7 @@ int unmap_domain_pirq_emuirq(struct domain *d, int pirq)
if ( info )
{
info->arch.hvm.emuirq = IRQ_UNBOUND;
- pirq_cleanup_check(info, d);
+ 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 c8c1bfa615..2efb5f5c78 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -672,7 +672,7 @@ static int evtchn_bind_pirq(evtchn_bind_pirq_t *bind)
if ( rc != 0 )
{
info->evtchn = 0;
- pirq_cleanup_check(info, d);
+ PIRQ_CLEANUP_CHECK(info, d);
goto out;
}
@@ -743,9 +743,9 @@ int evtchn_close(struct domain *d1, int port1, bool guest)
unmap_domain_pirq_emuirq(d1, pirq->pirq) < 0 )
/*
* The successful path of unmap_domain_pirq_emuirq() will have
- * called pirq_cleanup_check() already.
+ * called PIRQ_CLEANUP_CHECK() already.
*/
- pirq_cleanup_check(pirq, d1);
+ 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..1c545ed89d 100644
--- a/xen/drivers/passthrough/x86/hvm.c
+++ b/xen/drivers/passthrough/x86/hvm.c
@@ -296,7 +296,7 @@ int pt_irq_create_bind(
pirq_dpci->gmsi.gflags = gflags;
/*
* 'pt_irq_create_bind' can be called after 'pt_irq_destroy_bind'.
- * The 'pirq_cleanup_check' which would free the structure is only
+ * The 'PIRQ_CLEANUP_CHECK' which would free the structure is only
* called if the event channel for the PIRQ is active. However
* OS-es that use event channels usually bind PIRQs to eventds
* and unbind them before calling 'pt_irq_destroy_bind' - with the
@@ -329,7 +329,7 @@ int pt_irq_create_bind(
pirq_dpci->gmsi.gvec = 0;
pirq_dpci->dom = NULL;
pirq_dpci->flags = 0;
- pirq_cleanup_check(info, d);
+ PIRQ_CLEANUP_CHECK(info, d);
write_unlock(&d->event_lock);
return rc;
}
@@ -536,7 +536,7 @@ int pt_irq_create_bind(
hvm_irq_dpci->link_cnt[link]--;
}
pirq_dpci->flags = 0;
- pirq_cleanup_check(info, d);
+ PIRQ_CLEANUP_CHECK(info, d);
write_unlock(&d->event_lock);
xfree(girq);
xfree(digl);
@@ -693,7 +693,7 @@ int pt_irq_destroy_bind(
*/
pt_pirq_softirq_reset(pirq_dpci);
- pirq_cleanup_check(pirq, d);
+ 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..958d0b1aca 100644
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -185,7 +185,7 @@ 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) \
+#define PIRQ_CLEANUP_CHECK(pirq, d) \
(!(pirq)->evtchn ? pirq_cleanup_check(pirq, d) : (void)0)
extern void pirq_guest_eoi(struct pirq *pirq);
--
2.43.0On 29/07/2025 10:24 pm, Dmytro Prokopchuk1 wrote:
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index c8c1bfa615..2efb5f5c78 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -672,7 +672,7 @@ static int evtchn_bind_pirq(evtchn_bind_pirq_t *bind)
> if ( rc != 0 )
> {
> info->evtchn = 0;
> - pirq_cleanup_check(info, d);
> + PIRQ_CLEANUP_CHECK(info, d);
Well, this is awkward. This is dead code, but only when you realise ...
> diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
> index 95034c0d6b..958d0b1aca 100644
> --- a/xen/include/xen/irq.h
> +++ b/xen/include/xen/irq.h
> @@ -185,7 +185,7 @@ 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) \
> +#define PIRQ_CLEANUP_CHECK(pirq, d) \
> (!(pirq)->evtchn ? pirq_cleanup_check(pirq, d) : (void)0)
>
... what this is really doing.
Looking at this overall diff, it really is outrageous that we're hiding
a conditional call like this.
We should just remove the macro, and expand
if ( !pirq->evtchn )
pirq_cleanup_check(pirq, d);
in most of the callsites. The overall diff will be smaller (no need to
change the comments), and the end result is proper regular normal C.
I can draft a patch to that effect.
~Andrew
On Tue, 29 Jul 2025, Andrew Cooper wrote:
> On 29/07/2025 10:24 pm, Dmytro Prokopchuk1 wrote:
> > diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> > index c8c1bfa615..2efb5f5c78 100644
> > --- a/xen/common/event_channel.c
> > +++ b/xen/common/event_channel.c
> > @@ -672,7 +672,7 @@ static int evtchn_bind_pirq(evtchn_bind_pirq_t *bind)
> > if ( rc != 0 )
> > {
> > info->evtchn = 0;
> > - pirq_cleanup_check(info, d);
> > + PIRQ_CLEANUP_CHECK(info, d);
>
> Well, this is awkward. This is dead code, but only when you realise ...
>
> > diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
> > index 95034c0d6b..958d0b1aca 100644
> > --- a/xen/include/xen/irq.h
> > +++ b/xen/include/xen/irq.h
> > @@ -185,7 +185,7 @@ 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) \
> > +#define PIRQ_CLEANUP_CHECK(pirq, d) \
> > (!(pirq)->evtchn ? pirq_cleanup_check(pirq, d) : (void)0)
> >
>
> ... what this is really doing.
>
> Looking at this overall diff, it really is outrageous that we're hiding
> a conditional call like this.
>
> We should just remove the macro, and expand
>
> if ( !pirq->evtchn )
> pirq_cleanup_check(pirq, d);
>
> in most of the callsites. The overall diff will be smaller (no need to
> change the comments), and the end result is proper regular normal C.
Yes, would look much better. +1 from me.
© 2016 - 2025 Red Hat, Inc.