| Message ID | 8bc35da4a9fd44d2dcf9677dcc99334eb7142581.1710517542.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable build of full Xen for RISC-V | expand |
On 15.03.2024 19:06, Oleksii Kurochko wrote: > --- a/xen/lib/find-next-bit.c > +++ b/xen/lib/find-next-bit.c > @@ -9,6 +9,7 @@ > * 2 of the License, or (at your option) any later version. > */ > #include <xen/bitops.h> > +#include <xen/linux-compat.h> > > #include <asm/byteorder.h> Hmm, no, a library source would better not include this header. Surely the ffz() can be taken care of locally here? Jan
On Wed, 2024-03-20 at 16:42 +0100, Jan Beulich wrote: > On 15.03.2024 19:06, Oleksii Kurochko wrote: > > --- a/xen/lib/find-next-bit.c > > +++ b/xen/lib/find-next-bit.c > > @@ -9,6 +9,7 @@ > > * 2 of the License, or (at your option) any later version. > > */ > > #include <xen/bitops.h> > > +#include <xen/linux-compat.h> > > > > #include <asm/byteorder.h> > > Hmm, no, a library source would better not include this header. > Surely > the ffz() can be taken care of locally here? Considering that ffz() is only used here, then it makes sense to drop ffz() from xen/linux-compat.h> and defines it here as: #define ffz(x) ffsl(~(x)) - 1 ~ Oleksii
On Wed, 2024-03-20 at 16:42 +0100, Jan Beulich wrote: > On 15.03.2024 19:06, Oleksii Kurochko wrote: > > --- a/xen/lib/find-next-bit.c > > +++ b/xen/lib/find-next-bit.c > > @@ -9,6 +9,7 @@ > > * 2 of the License, or (at your option) any later version. > > */ > > #include <xen/bitops.h> > > +#include <xen/linux-compat.h> > > > > #include <asm/byteorder.h> > > Hmm, no, a library source would better not include this header. > Surely > the ffz() can be taken care of locally here? Except that __ffs() from xen/linux-compat.h is used in find-next-bit.c, so it seems that it is still need to include the header. ~ Oleksii
On 29.03.2024 19:23, Oleksii wrote: > On Wed, 2024-03-20 at 16:42 +0100, Jan Beulich wrote: >> On 15.03.2024 19:06, Oleksii Kurochko wrote: >>> --- a/xen/lib/find-next-bit.c >>> +++ b/xen/lib/find-next-bit.c >>> @@ -9,6 +9,7 @@ >>> * 2 of the License, or (at your option) any later version. >>> */ >>> #include <xen/bitops.h> >>> +#include <xen/linux-compat.h> >>> >>> #include <asm/byteorder.h> >> >> Hmm, no, a library source would better not include this header. >> Surely >> the ffz() can be taken care of locally here? > Except that __ffs() from xen/linux-compat.h is used in find-next-bit.c, > so it seems that it is still need to include the header. Hmm, no - that __ffs() use, if we mean that to become a Linux compat thing only, should then be replaced (or covered locally), too, imo. Jan
diff --git a/xen/arch/arm/include/asm/arm64/bitops.h b/xen/arch/arm/include/asm/arm64/bitops.h index 5f5d97faa0..2deb134388 100644 --- a/xen/arch/arm/include/asm/arm64/bitops.h +++ b/xen/arch/arm/include/asm/arm64/bitops.h @@ -1,27 +1,6 @@ #ifndef _ARM_ARM64_BITOPS_H #define _ARM_ARM64_BITOPS_H -/* Based on linux/include/asm-generic/bitops/builtin-__ffs.h */ -/** - * __ffs - find first bit in word. - * @word: The word to search - * - * Undefined if no bit exists, so code should check against 0 first. - */ -static /*__*/always_inline unsigned long __ffs(unsigned long word) -{ - return __builtin_ctzl(word); -} - -/* Based on linux/include/asm-generic/bitops/ffz.h */ -/* - * ffz - find first zero in word. - * @word: The word to search - * - * Undefined if no zero exists, so code should check against ~0UL first. - */ -#define ffz(x) __ffs(~(x)) - static inline int arch_flsl(unsigned long x) { uint64_t ret; diff --git a/xen/arch/ppc/include/asm/bitops.h b/xen/arch/ppc/include/asm/bitops.h index 16447a4be6..fd157f3632 100644 --- a/xen/arch/ppc/include/asm/bitops.h +++ b/xen/arch/ppc/include/asm/bitops.h @@ -171,15 +171,6 @@ static inline int __test_and_clear_bit(int nr, volatile void *addr) return (old & mask) != 0; } -/* Based on linux/include/asm-generic/bitops/ffz.h */ -/* - * ffz - find first zero in word. - * @word: The word to search - * - * Undefined if no zero exists, so code should check against ~0UL first. - */ -#define ffz(x) __ffs(~(x)) - /** * hweightN - returns the hamming weight of a N-bit word * @x: the word to weigh @@ -191,16 +182,4 @@ static inline int __test_and_clear_bit(int nr, volatile void *addr) #define hweight16(x) __builtin_popcount((uint16_t)(x)) #define hweight8(x) __builtin_popcount((uint8_t)(x)) -/* Based on linux/include/asm-generic/bitops/builtin-__ffs.h */ -/** - * __ffs - find first bit in word. - * @word: The word to search - * - * Undefined if no bit exists, so code should check against 0 first. - */ -static always_inline unsigned long __ffs(unsigned long word) -{ - return __builtin_ctzl(word); -} - #endif /* _ASM_PPC_BITOPS_H */ diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c index b1c40c2c0a..6904962467 100644 --- a/xen/drivers/passthrough/arm/smmu-v3.c +++ b/xen/drivers/passthrough/arm/smmu-v3.c @@ -72,12 +72,14 @@ */ #include <xen/acpi.h> +#include <xen/bitops.h> #include <xen/config.h> #include <xen/delay.h> #include <xen/errno.h> #include <xen/err.h> #include <xen/irq.h> #include <xen/lib.h> +#include <xen/linux-compat.h> #include <xen/list.h> #include <xen/mm.h> #include <xen/rbtree.h> diff --git a/xen/include/xen/linux-compat.h b/xen/include/xen/linux-compat.h index 62ba71485c..de059bdf12 100644 --- a/xen/include/xen/linux-compat.h +++ b/xen/include/xen/linux-compat.h @@ -19,4 +19,7 @@ typedef int64_t __s64; typedef paddr_t phys_addr_t; +#define __ffs(x) (ffsl(x) - 1) +#define ffz(x) __ffs(~(x)) + #endif /* __XEN_LINUX_COMPAT_H__ */ diff --git a/xen/lib/find-next-bit.c b/xen/lib/find-next-bit.c index ca6f82277e..b151f7f994 100644 --- a/xen/lib/find-next-bit.c +++ b/xen/lib/find-next-bit.c @@ -9,6 +9,7 @@ * 2 of the License, or (at your option) any later version. */ #include <xen/bitops.h> +#include <xen/linux-compat.h> #include <asm/byteorder.h>
The mentioned macros exist only because of Linux compatible purpose. The patch defines __ffs() in terms of Xen bitops and it is safe to define in this way ( as __ffs() - 1 ) as considering that __ffs() was defined as __builtin_ctzl(x), which has undefined behavior when x=0, so it is assumed that such cases are not encountered in the current code. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in V6: - new patch for the patch series. --- xen/arch/arm/include/asm/arm64/bitops.h | 21 --------------------- xen/arch/ppc/include/asm/bitops.h | 21 --------------------- xen/drivers/passthrough/arm/smmu-v3.c | 2 ++ xen/include/xen/linux-compat.h | 3 +++ xen/lib/find-next-bit.c | 1 + 5 files changed, 6 insertions(+), 42 deletions(-)