On 17.04.2024 12:04, Oleksii Kurochko wrote:
> --- a/xen/arch/ppc/include/asm/page.h
> +++ b/xen/arch/ppc/include/asm/page.h
> @@ -4,7 +4,7 @@
>
> #include <xen/types.h>
>
> -#include <asm/bitops.h>
> +#include <xen/bitops.h>
> #include <asm/byteorder.h>
This wants to move up into the xen/*.h group then, like you have done ...
> --- a/xen/arch/ppc/mm-radix.c
> +++ b/xen/arch/ppc/mm-radix.c
> @@ -1,11 +1,11 @@
> /* SPDX-License-Identifier: GPL-2.0-or-later */
> +#include <xen/bitops.h>
> #include <xen/init.h>
> #include <xen/kernel.h>
> #include <xen/mm.h>
> #include <xen/types.h>
> #include <xen/lib.h>
>
> -#include <asm/bitops.h>
> #include <asm/byteorder.h>
> #include <asm/early_printk.h>
> #include <asm/page.h>
.. e.g. here.
> --- a/xen/include/xen/bitops.h
> +++ b/xen/include/xen/bitops.h
> @@ -65,10 +65,137 @@ static inline int generic_flsl(unsigned long x)
> * scope
> */
>
> +#define BITOP_MASK(nr) ((bitop_uint_t)1 << ((nr) % BITOP_BITS_PER_WORD))
> +
> +#define BITOP_WORD(nr) ((nr) / BITOP_BITS_PER_WORD)
> +
> /* --------------------- Please tidy above here --------------------- */
>
> #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.
> +#endif
> +
> +/**
> + * generic__test_and_set_bit - Set a bit and return its old value
> + * @nr: Bit to set
> + * @addr: Address to count from
> + *
> + * This operation is non-atomic and can be reordered.
> + * If two examples of this operation race, one can appear to succeed
> + * but actually fail. You must protect multiple accesses with a lock.
> + */
> +static always_inline bool
> +generic__test_and_set_bit(unsigned long nr, volatile void *addr)
> +{
> + bitop_uint_t mask = BITOP_MASK(nr);
> + volatile bitop_uint_t *p = ((volatile bitop_uint_t *)addr) +
> BITOP_WORD(nr);
The revision log suggests excess parentheses were dropped from such cast
expressions.
> --- a/xen/include/xen/types.h
> +++ b/xen/include/xen/types.h
> @@ -64,6 +64,11 @@ typedef __u64 __be64;
>
> typedef unsigned int __attribute__((__mode__(__pointer__))) uintptr_t;
>
> +#ifndef BITOP_TYPE
> + #define BITOP_BITS_PER_WORD 32
> + typedef uint32_t bitop_uint_t;
Personally I find this indentation odd / misleading. For pre-processor
directives the # preferrably remains first on a line (as was iirc
demanded by earlier C standards), followed by one or more blanks if so
desired. File-scope declarations imo should never be indented.
Jan
