On Fri, 2024-04-05 at 10:05 +0200, Jan Beulich wrote: > On 05.04.2024 09:56, Oleksii wrote: > > 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. > > I don't follow. Right now patch 7 has > > #undef BITOP_BITS_PER_WORD > #undef bitop_uint_t > > #define BITOP_BITS_PER_WORD BITS_PER_LONG > #define bitop_uint_t unsigned long > > You'd drop the #undef-s and keep the #define-s. You want to override > them > both, after all. > > A problem would arise for _another_ arch wanting to use these > (default) > types in its asm/bitops.h. Which then could still be solved by having > a > types-only header. This problem arise now for Arm and PPC which use BITOP_BITS_PER_WORD inside it. Then it is needed to define BITOP_BITS_PER_WORD=32 in asm/bitops.h for Arm and PPC. If it is okay, then I will happy to follow this approach.
> Recall the discussion on the last summit of us meaning > to switch to such a model anyway (perhaps it being xen/types/bitops.h > and > asm/types/bitops.h then), in a broader fashion? IOW for now you could > use > the simple approach as long as no other arch needs the types in its > asm/bitops.h. Later we would introduce the types-only headers, thus > catering for possible future uses. Do we really need asm/types/bitops.h? Can't we just do the following in asm/bitops.h: #ifndef BITOP_TYPE #include <xen/types/bitops.h> #endif ~ Oleksii
