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

[oleksii.kurochko@xxxxxxxxx]

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, 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