[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v7 04/19] xen: introduce generic non-atomic test_*bit()
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. 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. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |