| Message ID | 5332A496.8060506@linaro.org |
|---|---|
| State | Deferred, archived |
| Headers | show |
On Wed, 26 Mar 2014, Julien Grall wrote: > > > On 26/03/14 09:03, Ian Campbell wrote: > > Does ASSERT(sgi < GIC_SGI_MAX) not compiler without warnings? > > I reworked this patch (see below): > > commit a07ef8994c10ae45c0fa42040e802006f7a510c6 > Author: Julien Grall <julien.grall@linaro.org> > Date: Wed Mar 19 20:51:51 2014 +0000 > > xen/arm: gic: Introduce GIC_SGI_MAX > > All the functions that send an SGI takes an enum. Therefore checking everytime > if the value is in the range is not correct. > > Introduce GIC_SGI_MAX to check the enum will never reach more than 16 values and > use it to check if the developper will use a wrong SGI by mistake. > > This is fix the compilation with Clang 3.5: > > gic.c:515:15: error: comparison of constant 16 with expression of type 'enum gic_sgi' is always true [-Werror,-Wtautological-constant-out-of-range-compare] > ASSERT(sgi < 16); /* There are only 16 SGIs */ > ~~~ ^ ~~ > xen/xen/include/xen/lib.h:43:26: note: expanded from macro 'ASSERT' > do { if ( unlikely(!(p)) ) assert_failed(#p); } while (0) > ^ > xen/xen/include/xen/compiler.h:11:41: note: expanded from macro 'unlikely' > #define unlikely(x) __builtin_expect((x),0) > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@citrix.com> > Cc: Tim Deegan <tim@xen.org> I think that this is fine. Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > Changes in v2: > - Replace ASSERT(sgi != GIC_SGI_MAX) by ASSERT(sgi < GIC_SGI_MAX) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index b03f2c2..2b9cdc5 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -473,7 +473,8 @@ void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi) > unsigned int mask = 0; > cpumask_t online_mask; > > - ASSERT(sgi < 16); /* There are only 16 SGIs */ > + BUILD_BUG_ON(GIC_SGI_MAX >= 16); > + ASSERT(sgi < GIC_SGI_MAX); > > cpumask_and(&online_mask, cpumask, &cpu_online_map); > mask = gic_cpu_mask(&online_mask); > @@ -493,7 +494,7 @@ 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 */ > + ASSERT(sgi < GIC_SGI_MAX); > > dsb(sy); > > @@ -503,7 +504,7 @@ void send_SGI_self(enum gic_sgi sgi) > > void send_SGI_allbutself(enum gic_sgi sgi) > { > - ASSERT(sgi < 16); /* There are only 16 SGIs */ > + ASSERT(sgi < GIC_SGI_MAX); > > dsb(sy); > > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index 76586ab..a4e513f 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -198,6 +198,8 @@ enum gic_sgi { > GIC_SGI_EVENT_CHECK = 0, > GIC_SGI_DUMP_STATE = 1, > GIC_SGI_CALL_FUNCTION = 2, > + /* GIC_SGI_MAX must be the last type of the enum */ > + GIC_SGI_MAX, > }; > extern void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi); > extern void send_SGI_one(unsigned int cpu, enum gic_sgi sgi); > > -- > Julien Grall >
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index b03f2c2..2b9cdc5 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -473,7 +473,8 @@ void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi) unsigned int mask = 0; cpumask_t online_mask; - ASSERT(sgi < 16); /* There are only 16 SGIs */ + BUILD_BUG_ON(GIC_SGI_MAX >= 16); + ASSERT(sgi < GIC_SGI_MAX); cpumask_and(&online_mask, cpumask, &cpu_online_map); mask = gic_cpu_mask(&online_mask); @@ -493,7 +494,7 @@ 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 */ + ASSERT(sgi < GIC_SGI_MAX); dsb(sy); @@ -503,7 +504,7 @@ void send_SGI_self(enum gic_sgi sgi) void send_SGI_allbutself(enum gic_sgi sgi) { - ASSERT(sgi < 16); /* There are only 16 SGIs */ + ASSERT(sgi < GIC_SGI_MAX); dsb(sy); diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h index 76586ab..a4e513f 100644 --- a/xen/include/asm-arm/gic.h +++ b/xen/include/asm-arm/gic.h @@ -198,6 +198,8 @@ enum gic_sgi { GIC_SGI_EVENT_CHECK = 0, GIC_SGI_DUMP_STATE = 1, GIC_SGI_CALL_FUNCTION = 2, + /* GIC_SGI_MAX must be the last type of the enum */ + GIC_SGI_MAX, }; extern void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi); extern void send_SGI_one(unsigned int cpu, enum gic_sgi sgi);