Re: [PATCH v7 04/19] xen: introduce generic non-atomic test_*bit()

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Fri, 2024-04-05 at 08:11 +0200, Jan Beulich wrote:
> On 04.04.2024 18:24, Oleksii wrote:
> > On Thu, 2024-04-04 at 18:12 +0200, Jan Beulich wrote:
> > > On 04.04.2024 17:45, Oleksii wrote:
> > > > On Thu, 2024-04-04 at 15:22 +0200, Jan Beulich wrote:
> > > > > On 03.04.2024 12:19, Oleksii Kurochko wrote:
> > > > > > --- a/xen/include/xen/bitops.h
> > > > > > +++ b/xen/include/xen/bitops.h
> > > > > > @@ -65,10 +65,164 @@ static inline int
> > > > > > generic_flsl(unsigned
> > > > > > long
> > > > > > x)
> > > > > >   * scope
> > > > > >   */
> > > > > >  
> > > > > > +#define BITOP_BITS_PER_WORD 32
> > > > > > +/* typedef uint32_t bitop_uint_t; */
> > > > > > +#define bitop_uint_t uint32_t
> > > > > 
> > > > > So no arch overrides permitted anymore at all?
> > > > Not really, I agree that it is ugly, but I expected that arch
> > > > will
> > > > use
> > > > undef to override.
> > > 
> > > Which would be fine in principle, just that Misra wants us to
> > > avoid
> > > #undef-s
> > > (iirc).
> > Could you please give me a recommendation how to do that better?
> > 
> > The reason why I put this defintions before inclusion of
> > asm/bitops.h
> > as RISC-V specific code uses these definitions inside it, so they
> > should be defined before asm/bitops.h; other option is to define
> > these
> > definitions inside asm/bitops.h for each architecture.
> 
> Earlier on you had it that other way already (in a different header,
> but the principle is the same): Move the generic definitions
> immediately
> past inclusion of asm/bitops.h and frame them with #ifndef.
It can be done in this way:
xen/bitops.h:
   ...
   #include <asm/bitops.h>
   
   #ifndef BITOP_TYPE
   #define BITOP_BITS_PER_WORD 32
   /* typedef uint32_t bitop_uint_t; */
   #define bitop_uint_t uint32_t
   #endif
   ...
   
But then RISC-V will fail as it is using bitop_uint_t inside
asm/bitops.h.
So, at least, for RISC-V it will be needed to add asm/bitops.h:
   #define BITOP_BITS_PER_WORD 32
   /* typedef uint32_t bitop_uint_t; */
   #define bitop_uint_t uint32_t
   

It seems to me that this breaks the idea of having these macro
definitions generic, as RISC-V will redefine BITOP_BITS_PER_WORD and
bitop_uint_t with the same values as the generic ones.
   
~ Oleksii

> 
> Jan