Re: [kvm-unit-tests PATCH 1/8] s390x: lib: Extend bitops

Claudio Imbrenda
2021-08-13T08:50:44Z

On Fri, 13 Aug 2021 07:36:08 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:

> Bit setting and clearing is never bad to have.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> lib/s390x/asm/bitops.h | 102
> +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102
> insertions(+)
>
> diff --git a/lib/s390x/asm/bitops.h b/lib/s390x/asm/bitops.h
> index 792881ec..f5612855 100644
> --- a/lib/s390x/asm/bitops.h
> +++ b/lib/s390x/asm/bitops.h
> @@ -17,6 +17,78 @@
>
> #define BITS_PER_LONG 64
>
> +static inline unsigned long *bitops_word(unsigned long nr,
> + const volatile unsigned
> long *ptr) +{
> + unsigned long addr;
> +
> + addr = (unsigned long)ptr + ((nr ^ (nr & (BITS_PER_LONG -
> 1))) >> 3);
> + return (unsigned long *)addr;

why not just

return ptr + (nr / BITS_PER_LONG);

> +}
> +
> +static inline unsigned long bitops_mask(unsigned long nr)
> +{
> + return 1UL << (nr & (BITS_PER_LONG - 1));
> +}
> +
> +static inline uint64_t laog(volatile unsigned long *ptr, uint64_t
> mask) +{
> + uint64_t old;
> +
> + /* load and or 64bit concurrent and interlocked */
> + asm volatile(
> + " laog %[old],%[mask],%[ptr]\n"
> + : [old] "=d" (old), [ptr] "+Q" (*ptr)
> + : [mask] "d" (mask)
> + : "memory", "cc" );
> + return old;
> +}

do we really need the artillery (asm) here?
is there a reason why we can't do this in C?

> +static inline uint64_t lang(volatile unsigned long *ptr, uint64_t
> mask) +{
> + uint64_t old;
> +
> + /* load and and 64bit concurrent and interlocked */
> + asm volatile(
> + " lang %[old],%[mask],%[ptr]\n"
> + : [old] "=d" (old), [ptr] "+Q" (*ptr)
> + : [mask] "d" (mask)
> + : "memory", "cc" );
> + return old;
> +}

(same here as above)

> +
> +static inline void set_bit(unsigned long nr,
> + const volatile unsigned long *ptr)
> +{
> + uint64_t mask = bitops_mask(nr);
> + uint64_t *addr = bitops_word(nr, ptr);
> +
> + laog(addr, mask);
> +}
> +
> +static inline void set_bit_inv(unsigned long nr,
> + const volatile unsigned long *ptr)
> +{
> + return set_bit(nr ^ (BITS_PER_LONG - 1), ptr);
> +}
> +
> +static inline void clear_bit(unsigned long nr,
> + const volatile unsigned long *ptr)
> +{
> + uint64_t mask = bitops_mask(nr);
> + uint64_t *addr = bitops_word(nr, ptr);
> +
> + lang(addr, ~mask);
> +}
> +
> +static inline void clear_bit_inv(unsigned long nr,
> + const volatile unsigned long *ptr)
> +{
> + return clear_bit(nr ^ (BITS_PER_LONG - 1), ptr);
> +}
> +
> +/* non-atomic bit manipulation functions */
> +
> static inline bool test_bit(unsigned long nr,
> const volatile unsigned long *ptr)
> {
> @@ -33,4 +105,34 @@ static inline bool test_bit_inv(unsigned long nr,
> return test_bit(nr ^ (BITS_PER_LONG - 1), ptr);
> }
>
> +static inline void __set_bit(unsigned long nr,
> + const volatile unsigned long *ptr)
> +{
> + uint64_t mask = bitops_mask(nr);
> + uint64_t *addr = bitops_word(nr, ptr);
> +
> + *addr |= mask;
> +}
> +
> +static inline void __set_bit_inv(unsigned long nr,
> + const volatile unsigned long *ptr)
> +{
> + return __set_bit(nr ^ (BITS_PER_LONG - 1), ptr);
> +}
> +
> +static inline void __clear_bit(unsigned long nr,
> + const volatile unsigned long *ptr)
> +{
> + uint64_t mask = bitops_mask(nr);
> + uint64_t *addr = bitops_word(nr, ptr);
> +
> + *addr &= ~mask;
> +}
> +
> +static inline void __clear_bit_inv(unsigned long nr,
> + const volatile unsigned long *ptr)
> +{
> + return __clear_bit(nr ^ (BITS_PER_LONG - 1), ptr);
> +}
> +
> #endif