[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] xen/irq: Delete the pirq_cleanup_check() macro


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Dmytro Prokopchuk1 <dmytro_prokopchuk1@xxxxxxxx>
  • Date: Wed, 30 Jul 2025 08:45:40 +0000
  • Accept-language: en-US, uk-UA, ru-RU
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=X1ZjLI47SoEIKGNXKKcCmCc2VfddhClpYpxPr3ELiPY=; b=wFGOCusT1oV9MOmgANAmbJsp+fNlx7/+PiXwLAFhZLjs+g3EzaMEunnnmaUWLkhouX+b9xS5DTXRgMy/C16hbAp4hGNyKtsuoUFqGlv18ZJISBOFy2yipRDQgNFjm13kfCG/ZiRWyV58FlpFxr0BEbA7/74l9Fk+Qxa65ZmV8WbxGbvNhamkYl3Zk4m4r7vP51BRCZYQB64T6i05imtbwwLm1vm1/+12BMzjOiVQ1Y7k1NF8yDyiUt9uCp1R4uiBysn67oR0oBIa7EOu6ya93dhX+0T7zk/Rhu8u6V1g3HHhIEp7/mk7bTNwIRqPkjxraa8cM+JMI2LFfQjKlrIvhw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=xPqzOlva2Htl+eHyy26MUGNBIvJUBbzfXaXJAqqetS9kpyE7dFfgbpljxkUQVk1NrhgrIUHI//rcB7Cm2VwDaNHOxhFEl0DuO4l5p8sjoaMueCo8XAUD6u/U/MnVLneJbwEfkj6hYoC5xWmN2Hsun5mVdzIfuYJMrcfo+hQS3It3S2QC58cJq6NoaG0UNlc7PUFMQTJ3hIBgerDIc8AthK6i+Hzcm7M4yhWXDLWaSIOYd2phIzl9sKfYdAicNpEvdGBgg4O+NKzji+PP8uYKGHlHcECDUWHm1Qr9T3soy/v0YdutaaLEagPbBN7H4bi02DfV/8zYUlyRUsCszqstjw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=epam.com;
  • Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Julien Grall <julien@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
  • Delivery-date: Wed, 30 Jul 2025 08:45:58 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHcANiB3YdmHa9oc0manxquZPhnqLRKWvQA
  • Thread-topic: [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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.