Re: [PATCH v13 02/10] xen/riscv: introduce bitops.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.06.2024 19:27, oleksii.kurochko@xxxxxxxxx wrote:
> On Wed, 2024-06-26 at 10:31 +0200, Jan Beulich wrote:
>> On 25.06.2024 15:51, Oleksii Kurochko wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/riscv/include/asm/bitops.h
>>> @@ -0,0 +1,137 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/* Copyright (C) 2012 Regents of the University of California */
>>> +
>>> +#ifndef _ASM_RISCV_BITOPS_H
>>> +#define _ASM_RISCV_BITOPS_H
>>> +
>>> +#include <asm/system.h>
>>> +
>>> +#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 BITOP_BITS_PER_WORD"
>>> +#endif
>>> +
>>> +/* Based on linux/arch/include/asm/bitops.h */
>>> +
>>> +/*
>>> + * Non-atomic bit manipulation.
>>> + *
>>> + * Implemented using atomics to be interrupt safe. Could
>>> alternatively
>>> + * implement with local interrupt masking.
>>> + */
>>> +#define __set_bit(n, p)      set_bit(n, p)
>>> +#define __clear_bit(n, p)    clear_bit(n, p)
>>> +
>>> +#define test_and_op_bit_ord(op, mod, nr, addr, ord)     \
>>> +({                                                      \
>>> +    bitop_uint_t res, mask;                             \
>>> +    mask = BITOP_MASK(nr);                              \
>>> +    asm volatile (                                      \
>>> +        __AMO(op) #ord " %0, %2, %1"                    \
>>> +        : "=r" (res), "+A" (addr[BITOP_WORD(nr)])       \
>>> +        : "r" (mod(mask))                               \
>>> +        : "memory");                                    \
>>> +    ((res & mask) != 0);                                \
>>> +})
>>> +
>>> +#define op_bit_ord(op, mod, nr, addr, ord)      \
>>> +    asm volatile (                              \
>>> +        __AMO(op) #ord " zero, %1, %0"          \
>>> +        : "+A" (addr[BITOP_WORD(nr)])           \
>>> +        : "r" (mod(BITOP_MASK(nr)))             \
>>> +        : "memory");
>>> +
>>> +#define test_and_op_bit(op, mod, nr, addr)    \
>>> +    test_and_op_bit_ord(op, mod, nr, addr, .aqrl)
>>> +#define op_bit(op, mod, nr, addr) \
>>> +    op_bit_ord(op, mod, nr, addr, )
>>> +
>>> +/* Bitmask modifiers */
>>> +#define NOP(x)    (x)
>>> +#define NOT(x)    (~(x))
>>
>> Since elsewhere you said we would use Zbb in bitops, I wanted to come
>> back
>> on that: Up to here all we use is AMO.
>>
>> And further down there's no asm() anymore. What were you referring
>> to?
> RISC-V doesn't have a CLZ instruction in the base
> ISA.  As a consequence, __builtin_ffs() emits a library call to ffs()
> on GCC,

Oh, so we'd need to implement that libgcc function, along the lines of
Arm32 implementing quite a few of them to support shifts on 64-bit
quantities as well as division and modulo.

Jan

> or a de Bruijn sequence on Clang.
> 
> The optional Zbb extension adds a CLZ instruction, after which
> __builtin_ffs() emits a very simple sequence.
> 
> ~ Oleksii