diff mbox series

xen/arm: gic: Remove pointless assertion against enum gic_sgi

Message ID 20200118153924.10878-1-julien@xen.org (mailing list archive)
State New, archived
Headers show
Series xen/arm: gic: Remove pointless assertion against enum gic_sgi | expand

Commit Message

Julien Grall Jan. 18, 2020, 3:39 p.m. UTC
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
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@xen.org>
CC: Andrii Anisov <andrii_anisov@epam.com>
---
 xen/arch/arm/gic.c        | 12 ++++++------
 xen/include/asm-arm/gic.h |  7 ++++---
 2 files changed, 10 insertions(+), 9 deletions(-)

Comments

Stefano Stabellini Jan. 21, 2020, 9:19 p.m. UTC | #1
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@xen.org>
> CC: Andrii Anisov <andrii_anisov@epam.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  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
>
diff mbox series

Patch

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 */