| Message ID | 20170613161323.25196-2-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: Extend the usage of typesafe MFN | expand |
On 13/06/17 17:13, Julien Grall wrote: > INVALID_{G,M}FN are defined using static inline helpers _{g,m}fn. > This means, they cannot be used to initialize a build time static variable: > > In file included from mm.c:24:0: > xen/xen/include/xen/mm.h:59:26: error: initializer element is not constant > #define INVALID_MFN _mfn(~0UL) > > Signed-off-by: Julien Grall <julien.grall@arm.com> > --- > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: George Dunlap <George.Dunlap@eu.citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Tim Deegan <tim@xen.org> > Cc: Wei Liu <wei.liu2@citrix.com> > > I know that this solution will not work for non-debug build. I would > like input from the community on way to fix it nicely. Hmm - a proper typedef gets inserted. I presume the compiler objects to (unsigned long){ ~0UL } to initialise a scalar? It might be better to move the definition of INVALID_$FOO into the TYPE_SAFE() declaration so we can create an appropriate initialiser for each both builds. ~Andrew
>>> On 13.06.17 at 18:20, <andrew.cooper3@citrix.com> wrote: > On 13/06/17 17:13, Julien Grall wrote: >> INVALID_{G,M}FN are defined using static inline helpers _{g,m}fn. >> This means, they cannot be used to initialize a build time static variable: >> >> In file included from mm.c:24:0: >> xen/xen/include/xen/mm.h:59:26: error: initializer element is not constant >> #define INVALID_MFN _mfn(~0UL) >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> >> --- >> Cc: Andrew Cooper <andrew.cooper3@citrix.com> >> Cc: George Dunlap <George.Dunlap@eu.citrix.com> >> Cc: Ian Jackson <ian.jackson@eu.citrix.com> >> Cc: Jan Beulich <jbeulich@suse.com> >> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> >> Cc: Stefano Stabellini <sstabellini@kernel.org> >> Cc: Tim Deegan <tim@xen.org> >> Cc: Wei Liu <wei.liu2@citrix.com> >> >> I know that this solution will not work for non-debug build. I would >> like input from the community on way to fix it nicely. > > Hmm - a proper typedef gets inserted. I presume the compiler objects to > (unsigned long){ ~0UL } to initialise a scalar? > > It might be better to move the definition of INVALID_$FOO into the > TYPE_SAFE() declaration so we can create an appropriate initialiser for > each both builds. Except that you can't put a #define in a macro definition, and producing a static const wouldn't help Julien's case of wanting it in the initializer of a static variable. So best I can come up with right now would be to introduce a separate #define TYPE_SAFE_CONSTANT(name, val) \ (name##_t){ val } and #define TYPE_SAFE_CONSTANT(name, val) \ (name##_t)(val) for the !NDEBUG / NDEBUG cases respectively and use it as #define INVALID_MFN TYPE_SAFE_CONSTANT(mfn, ~0UL) Jan
At 17:13 +0100 on 13 Jun (1497373980), Julien Grall wrote: > INVALID_{G,M}FN are defined using static inline helpers _{g,m}fn. > This means, they cannot be used to initialize a build time static variable: > > In file included from mm.c:24:0: > xen/xen/include/xen/mm.h:59:26: error: initializer element is not constant > #define INVALID_MFN _mfn(~0UL) > > Signed-off-by: Julien Grall <julien.grall@arm.com> Acked-by: Tim Deegan <tim@xen.org> > I know that this solution will not work for non-debug build. I would > like input from the community on way to fix it nicely. It seems to WFM: https://godbolt.org/g/vEVNY3 Tim.
Hi Tim, On 06/14/2017 09:49 AM, Tim Deegan wrote: > At 17:13 +0100 on 13 Jun (1497373980), Julien Grall wrote: >> INVALID_{G,M}FN are defined using static inline helpers _{g,m}fn. >> This means, they cannot be used to initialize a build time static variable: >> >> In file included from mm.c:24:0: >> xen/xen/include/xen/mm.h:59:26: error: initializer element is not constant >> #define INVALID_MFN _mfn(~0UL) >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> > > Acked-by: Tim Deegan <tim@xen.org> > >> I know that this solution will not work for non-debug build. I would >> like input from the community on way to fix it nicely. > > It seems to WFM: https://godbolt.org/g/vEVNY3 To be honest, I thought it would not work and a bit surprised that it actually works. I am happy to keep like that or introduced a new define (such as TYPE_SAFE_CONSTANT suggested by Jan). Cheers,
>>> On 14.06.17 at 11:40, <julien.grall@arm.com> wrote: > On 06/14/2017 09:49 AM, Tim Deegan wrote: >> At 17:13 +0100 on 13 Jun (1497373980), Julien Grall wrote: >>> INVALID_{G,M}FN are defined using static inline helpers _{g,m}fn. >>> This means, they cannot be used to initialize a build time static variable: >>> >>> In file included from mm.c:24:0: >>> xen/xen/include/xen/mm.h:59:26: error: initializer element is not constant >>> #define INVALID_MFN _mfn(~0UL) >>> >>> Signed-off-by: Julien Grall <julien.grall@arm.com> >> >> Acked-by: Tim Deegan <tim@xen.org> >> >>> I know that this solution will not work for non-debug build. I would >>> like input from the community on way to fix it nicely. >> >> It seems to WFM: https://godbolt.org/g/vEVNY3 > > To be honest, I thought it would not work and a bit surprised that it > actually works. Me too - it didn't occur to me that the extension "Compound Literals" also, contrary to its name, covers scalar types. But it's indeed documented, so we don't need to be afraid of it going away. Remains the question whether our oldest supported gcc allows it. The oldest one I can easily try (4.3.something) works fine. Jan
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h index ee50d4cd7b..f0daae6b5c 100644 --- a/xen/include/xen/mm.h +++ b/xen/include/xen/mm.h @@ -56,7 +56,7 @@ TYPE_SAFE(unsigned long, mfn); #define PRI_mfn "05lx" -#define INVALID_MFN _mfn(~0UL) +#define INVALID_MFN (mfn_t){ ~0UL } #ifndef mfn_t #define mfn_t /* Grep fodder: mfn_t, _mfn() and mfn_x() are defined above */ @@ -85,7 +85,7 @@ static inline bool_t mfn_eq(mfn_t x, mfn_t y) TYPE_SAFE(unsigned long, gfn); #define PRI_gfn "05lx" -#define INVALID_GFN _gfn(~0UL) +#define INVALID_GFN (gfn_t){ ~0UL } #ifndef gfn_t #define gfn_t /* Grep fodder: gfn_t, _gfn() and gfn_x() are defined above */
INVALID_{G,M}FN are defined using static inline helpers _{g,m}fn. This means, they cannot be used to initialize a build time static variable: In file included from mm.c:24:0: xen/xen/include/xen/mm.h:59:26: error: initializer element is not constant #define INVALID_MFN _mfn(~0UL) Signed-off-by: Julien Grall <julien.grall@arm.com> --- Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Tim Deegan <tim@xen.org> Cc: Wei Liu <wei.liu2@citrix.com> I know that this solution will not work for non-debug build. I would like input from the community on way to fix it nicely. --- xen/include/xen/mm.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)