Re: [Xen-devel] [PATCH] xen/arm: gic: Remove pointless assertion against enum gic_sgi

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Sat, 18 Jan 2020, Julien Grall wrote:
> The Arm Compiler will complain that the assertions ASSERT(sgi < 16) is
> always true. This is because sgi is a item of the enum gic_sgi and
> should always contain less than 16 SGIs.
> 
> Rather than using ASSERTs, introduce a new item in the num that could be
                                                         ^ enum

> checked against a build time.
> 
> Take the opportunity to remove the specific assigned values for each
> items. This is fine because enum always starts at zero and values will
> be assigned by increment of one. None of our code also rely on hardcoded
> value.
> 
> Signed-off-by: Julien Grall <julien@xxxxxxx>
> CC: Andrii Anisov <andrii_anisov@xxxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>  xen/arch/arm/gic.c        | 12 ++++++------
>  xen/include/asm-arm/gic.h |  7 ++++---
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 113655a789..d623c57cb9 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -44,6 +44,12 @@ DEFINE_PER_CPU(uint64_t, lr_mask);
>  
>  const struct gic_hw_operations *gic_hw_ops;
>  
> +static void __init __maybe_unused build_assertions(void)
> +{
> +    /* Check our enum gic_sgi only covers SGIs */
> +    BUILD_BUG_ON(GIC_SGI_MAX > NR_GIC_SGI);
> +}
> +
>  void register_gic_ops(const struct gic_hw_operations *ops)
>  {
>      gic_hw_ops = ops;
> @@ -294,8 +300,6 @@ void __init gic_init(void)
>  
>  void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi)
>  {
> -    ASSERT(sgi < 16); /* There are only 16 SGIs */
> -
>      gic_hw_ops->send_SGI(sgi, SGI_TARGET_LIST, cpumask);
>  }
>  
> @@ -306,15 +310,11 @@ void send_SGI_one(unsigned int cpu, enum gic_sgi sgi)
>  
>  void send_SGI_self(enum gic_sgi sgi)
>  {
> -    ASSERT(sgi < 16); /* There are only 16 SGIs */
> -
>      gic_hw_ops->send_SGI(sgi, SGI_TARGET_SELF, NULL);
>  }
>  
>  void send_SGI_allbutself(enum gic_sgi sgi)
>  {
> -   ASSERT(sgi < 16); /* There are only 16 SGIs */
> -
>     gic_hw_ops->send_SGI(sgi, SGI_TARGET_OTHERS, NULL);
>  }
>  
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 793d324b33..ba870523bb 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -277,9 +277,10 @@ extern void gic_restore_state(struct vcpu *v);
>  
>  /* SGI (AKA IPIs) */
>  enum gic_sgi {
> -    GIC_SGI_EVENT_CHECK = 0,
> -    GIC_SGI_DUMP_STATE  = 1,
> -    GIC_SGI_CALL_FUNCTION = 2,
> +    GIC_SGI_EVENT_CHECK,
> +    GIC_SGI_DUMP_STATE,
> +    GIC_SGI_CALL_FUNCTION,
> +    GIC_SGI_MAX,
>  };
>  
>  /* SGI irq mode types */
> -- 
> 2.17.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel