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

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

[Julien Grall]

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(-)

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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Stefano Stabellini]

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
> 

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