Re: [PATCH v3 10/34] xen/riscv: introduce bitops.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.01.2024 12:37, Oleksii wrote:
>>>>
>>>>>> Also you want to make sure asm-generic/bitops/bitops-bits.h
>>>>>> is
>>>>>> really in use here, or else an arch overriding / not using
>>>>>> that
>>>>>> header may end up screwed.
>>>>> I am not really understand what do you mean. Could you please
>>>>> explain a
>>>>> little bit more.
>>>>
>>>> Whichever type you use here, it needs to be in sync with
>>>> BITOP_BITS_PER_WORD. Hence you want to include the _local_
>>>> bitops-
>>>> bits.h
>>>> here, such that in case of an inconsistent override by an arch
>>>> the
>>>> compiler would complain about the two differring #define-s. (IOW
>>>> an
>>>> arch overriding BITOP_BITS_PER_WORD cannot re-use this header as-
>>>> is.)
>>>>
>>>> The same may, btw, be true for others of the new headers you add
>>>> -
>>>> the
>>>> same #include would therefore be needed there as well.
>>> Now it clear to me.
>>>
>>>
>>> It seems like BITOP_BITS_PER_WORD, BITOP_MASK, BITOP_WORD, and
>>> BITS_PER_BYTE are defined in {arm, ppc,
>>> riscv}/include/asm/bitops.h.
>>> I expected that any architecture planning to use asm-
>>> generic/bitops/bitops-bits.h would include it at the beginning of
>>> <arch>/include/asm/bitops.h, similar to what is done for RISC-V:
>>>    #ifndef _ASM_RISCV_BITOPS_H
>>>    #define _ASM_RISCV_BITOPS_H
>>>    
>>>    #include <asm/system.h>
>>>    
>>>    #include <asm-generic/bitops/bitops-bits.h>
>>>    ...
>>>
>>> But in this case, to allow architecture overrides macros, it is
>>> necessary to update asm-generic/bitops/bitops-bits.h:
>>>     #ifndef BITOP_BITS_PER_WORD
>>>     #define BITOP_BITS_PER_WORD     32
>>>     #endif
>>>    ...
>>> Therefore,  if an architecture needs to override something, it will
>>> add
>>> #define ... before #include <asm-generic/bitops/bitops-bits.h>.
>>>
>>> Does it make sense?
>>
>> Sure. But then the arch also needs to provide a corresponding typedef
>> (and bitops-bits.h the fallback one), for use wherever you use any of
>> those #define-s.
> Which one typedef is needed to provide?
> <asm-generic/bitops/bitops-bits.h> contains only macros.

A new one, to replace where right now you use "unsigned int" and I
initially said you need to use "uint32_t" instead. With what you said
earlier, uint32_t won't work there (anymore).

Jan