On 06.05.2024 10:16, Oleksii wrote: > On Mon, 2024-05-06 at 08:33 +0200, Jan Beulich wrote: >> On 03.05.2024 19:15, Oleksii wrote: >>> On Thu, 2024-04-25 at 17:35 +0200, Jan Beulich wrote: >>>>> #include <asm/bitops.h> >>>>> >>>>> +#ifndef arch_check_bitop_size >>>>> +#define arch_check_bitop_size(addr) >>>> >>>> Can this really do nothing? Passing the address of an object >>>> smaller >>>> than >>>> bitop_uint_t will read past the object in the generic__*_bit() >>>> functions. >>> It seems RISC-V isn' happy with the following generic definition: >>> extern void __bitop_bad_size(void); >>> >>> /* --------------------- Please tidy above here ---------------- >>> ---- >>> - */ >>> >>> #include <asm/bitops.h> >>> >>> #ifndef arch_check_bitop_size >>> >>> #define bitop_bad_size(addr) sizeof(*(addr)) < >>> sizeof(bitop_uint_t) >>> >>> #define arch_check_bitop_size(addr) \ >>> if ( bitop_bad_size(addr) ) __bitop_bad_size(); >>> >>> #endif /* arch_check_bitop_size */ >> >> I'm afraid you've re-discovered something that was also found during >> the >> original Arm porting effort. As nice and logical as it would (seem >> to) be >> to have bitop_uint_t match machine word size, there are places ... >> >>> The following errors occurs. bitop_uint_t for RISC-V is defined as >>> unsigned long for now: >> >> ... where we assume such operations can be done on 32-bit quantities. > Based on RISC-V spec machine word is 32-bit, so then I can just drop > re-definition of bitop_uint_t in riscv/asm/types.h and use the > definition of bitop_uint_t in xen/types.h. > Also it will be needed to update __AMO() macros in <riscv>/asm/bitops.h > in the following way: > #if BITOP_BITS_PER_WORD == 64 > #define __AMO(op) "amo" #op ".d" > #elif BITOP_BITS_PER_WORD == 32 > #define __AMO(op) "amo" #op ".w" > #else > #error "Unexpected BITS_PER_LONG" > #endif > Note: BITS_PER_LONG was changed to BITOP_BITS_PER_WORD ! > > Only one question remains for me. Given that there are some operations > whichcan be performed on 32-bit quantities, it seems to me that bitop_uint_t > can only be uint32_t. > Am I correct? If yes, do we need to have ability to redefine > bitop_uint_t and BITOP_BITS_PER_WORD in xen/types.h: > #ifndef BITOP_TYPE > #define BITOP_BITS_PER_WORD 32 > > typedef uint32_t bitop_uint_t; > #endif
Probably not, right now. Hence me having said "As nice and logical as it would (seem to) be" in the earlier reply. Jan
