| Message ID | bb47caf6c275d8aea307b96e79828831eab4a703.1703255175.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable build of full Xen for RISC-V | expand |
On 22.12.2023 16:13, Oleksii Kurochko wrote: > --- a/xen/arch/riscv/include/asm/fence.h > +++ b/xen/arch/riscv/include/asm/fence.h > @@ -1,4 +1,4 @@ > -/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* SPDX-License-Identifier: GPL-2.0-only */ > #ifndef _ASM_RISCV_FENCE_H > #define _ASM_RISCV_FENCE_H > > @@ -11,3 +11,12 @@ > #endif > > #endif /* _ASM_RISCV_FENCE_H */ > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ Surely all of this wants doing in the previous patch, where the header is introduced? > --- a/xen/common/Kconfig > +++ b/xen/common/Kconfig > @@ -47,6 +47,9 @@ config ARCH_MAP_DOMAIN_PAGE > config GENERIC_BUG_FRAME > bool > > +config GENERIC_FIND_NEXT_BIT > + bool There's no need for this, as ... > --- a/xen/lib/Makefile > +++ b/xen/lib/Makefile > @@ -3,6 +3,7 @@ obj-$(CONFIG_X86) += x86/ > lib-y += bsearch.o > lib-y += ctors.o > lib-y += ctype.o > +lib-$(CONFIG_GENERIC_FIND_NEXT_BIT) += find-next-bit.o ... you're moving this to lib/. Or have you encountered any issue with building this uniformly, and you forgot to mention this in the description? > --- /dev/null > +++ b/xen/lib/find-next-bit.c > @@ -0,0 +1,281 @@ > +/* find_next_bit.c: fallback find next bit implementation > + * > + * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved. > + * Written by David Howells (dhowells@redhat.com) > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version > + * 2 of the License, or (at your option) any later version. > + */ > +#include <xen/bitops.h> > + > +#include <asm/byteorder.h> > + > +#ifndef find_next_bit > +/* > + * Find the next set bit in a memory region. > + */ > +unsigned long find_next_bit(const unsigned long *addr, unsigned long size, > + unsigned long offset) > +{ > + const unsigned long *p = addr + BIT_WORD(offset); > + unsigned long result = offset & ~(BITS_PER_LONG-1); > + unsigned long tmp; > + > + if (offset >= size) > + return size; > + size -= result; > + offset %= BITS_PER_LONG; > + if (offset) { > + tmp = *(p++); > + tmp &= (~0UL << offset); > + if (size < BITS_PER_LONG) > + goto found_first; > + if (tmp) > + goto found_middle; > + size -= BITS_PER_LONG; > + result += BITS_PER_LONG; > + } > + while (size & ~(BITS_PER_LONG-1)) { > + if ((tmp = *(p++))) > + goto found_middle; > + result += BITS_PER_LONG; > + size -= BITS_PER_LONG; > + } > + if (!size) > + return result; > + tmp = *p; > + > +found_first: > + tmp &= (~0UL >> (BITS_PER_LONG - size)); > + if (tmp == 0UL) /* Are any bits set? */ > + return result + size; /* Nope. */ > +found_middle: > + return result + __ffs(tmp); > +} > +EXPORT_SYMBOL(find_next_bit); > +#endif > + > +#ifndef find_next_zero_bit > +/* > + * This implementation of find_{first,next}_zero_bit was stolen from > + * Linus' asm-alpha/bitops.h. > + */ > +unsigned long find_next_zero_bit(const unsigned long *addr, unsigned long size, > + unsigned long offset) > +{ > + const unsigned long *p = addr + BIT_WORD(offset); > + unsigned long result = offset & ~(BITS_PER_LONG-1); > + unsigned long tmp; > + > + if (offset >= size) > + return size; > + size -= result; > + offset %= BITS_PER_LONG; > + if (offset) { > + tmp = *(p++); > + tmp |= ~0UL >> (BITS_PER_LONG - offset); > + if (size < BITS_PER_LONG) > + goto found_first; > + if (~tmp) > + goto found_middle; > + size -= BITS_PER_LONG; > + result += BITS_PER_LONG; > + } > + while (size & ~(BITS_PER_LONG-1)) { > + if (~(tmp = *(p++))) > + goto found_middle; > + result += BITS_PER_LONG; > + size -= BITS_PER_LONG; > + } > + if (!size) > + return result; > + tmp = *p; > + > +found_first: > + tmp |= ~0UL << size; > + if (tmp == ~0UL) /* Are any bits zero? */ > + return result + size; /* Nope. */ > +found_middle: > + return result + ffz(tmp); > +} > +EXPORT_SYMBOL(find_next_zero_bit); > +#endif > + > +#ifndef find_first_bit > +/* > + * Find the first set bit in a memory region. > + */ > +unsigned long find_first_bit(const unsigned long *addr, unsigned long size) > +{ > + const unsigned long *p = addr; > + unsigned long result = 0; > + unsigned long tmp; > + > + while (size & ~(BITS_PER_LONG-1)) { > + if ((tmp = *(p++))) > + goto found; > + result += BITS_PER_LONG; > + size -= BITS_PER_LONG; > + } > + if (!size) > + return result; > + > + tmp = (*p) & (~0UL >> (BITS_PER_LONG - size)); > + if (tmp == 0UL) /* Are any bits set? */ > + return result + size; /* Nope. */ > +found: > + return result + __ffs(tmp); > +} > +EXPORT_SYMBOL(find_first_bit); > +#endif > + > +#ifndef find_first_zero_bit > +/* > + * Find the first cleared bit in a memory region. > + */ > +unsigned long find_first_zero_bit(const unsigned long *addr, unsigned long size) > +{ > + const unsigned long *p = addr; > + unsigned long result = 0; > + unsigned long tmp; > + > + while (size & ~(BITS_PER_LONG-1)) { > + if (~(tmp = *(p++))) > + goto found; > + result += BITS_PER_LONG; > + size -= BITS_PER_LONG; > + } > + if (!size) > + return result; > + > + tmp = (*p) | (~0UL << size); > + if (tmp == ~0UL) /* Are any bits zero? */ > + return result + size; /* Nope. */ > +found: > + return result + ffz(tmp); > +} > +EXPORT_SYMBOL(find_first_zero_bit); > +#endif > + > +#ifdef __BIG_ENDIAN > + > +/* include/linux/byteorder does not support "unsigned long" type */ > +static inline unsigned long ext2_swabp(const unsigned long * x) > +{ > +#if BITS_PER_LONG == 64 > + return (unsigned long) __swab64p((u64 *) x); > +#elif BITS_PER_LONG == 32 > + return (unsigned long) __swab32p((u32 *) x); > +#else > +#error BITS_PER_LONG not defined > +#endif > +} > + > +/* include/linux/byteorder doesn't support "unsigned long" type */ > +static inline unsigned long ext2_swab(const unsigned long y) > +{ > +#if BITS_PER_LONG == 64 > + return (unsigned long) __swab64((u64) y); > +#elif BITS_PER_LONG == 32 > + return (unsigned long) __swab32((u32) y); > +#else > +#error BITS_PER_LONG not defined > +#endif > +} > + > +#ifndef find_next_zero_bit_le > +unsigned long find_next_zero_bit_le(const void *addr, unsigned > + long size, unsigned long offset) > +{ > + const unsigned long *p = addr; > + unsigned long result = offset & ~(BITS_PER_LONG - 1); > + unsigned long tmp; > + > + if (offset >= size) > + return size; > + p += BIT_WORD(offset); > + size -= result; > + offset &= (BITS_PER_LONG - 1UL); > + if (offset) { > + tmp = ext2_swabp(p++); > + tmp |= (~0UL >> (BITS_PER_LONG - offset)); > + if (size < BITS_PER_LONG) > + goto found_first; > + if (~tmp) > + goto found_middle; > + size -= BITS_PER_LONG; > + result += BITS_PER_LONG; > + } > + > + while (size & ~(BITS_PER_LONG - 1)) { > + if (~(tmp = *(p++))) > + goto found_middle_swap; > + result += BITS_PER_LONG; > + size -= BITS_PER_LONG; > + } > + if (!size) > + return result; > + tmp = ext2_swabp(p); > +found_first: > + tmp |= ~0UL << size; > + if (tmp == ~0UL) /* Are any bits zero? */ > + return result + size; /* Nope. Skip ffz */ > +found_middle: > + return result + ffz(tmp); > + > +found_middle_swap: > + return result + ffz(ext2_swab(tmp)); > +} > +EXPORT_SYMBOL(find_next_zero_bit_le); > +#endif > + > +#ifndef find_next_bit_le > +unsigned long find_next_bit_le(const void *addr, unsigned > + long size, unsigned long offset) > +{ > + const unsigned long *p = addr; > + unsigned long result = offset & ~(BITS_PER_LONG - 1); > + unsigned long tmp; > + > + if (offset >= size) > + return size; > + p += BIT_WORD(offset); > + size -= result; > + offset &= (BITS_PER_LONG - 1UL); > + if (offset) { > + tmp = ext2_swabp(p++); > + tmp &= (~0UL << offset); > + if (size < BITS_PER_LONG) > + goto found_first; > + if (tmp) > + goto found_middle; > + size -= BITS_PER_LONG; > + result += BITS_PER_LONG; > + } > + > + while (size & ~(BITS_PER_LONG - 1)) { > + tmp = *(p++); > + if (tmp) > + goto found_middle_swap; > + result += BITS_PER_LONG; > + size -= BITS_PER_LONG; > + } > + if (!size) > + return result; > + tmp = ext2_swabp(p); > +found_first: > + tmp &= (~0UL >> (BITS_PER_LONG - size)); > + if (tmp == 0UL) /* Are any bits set? */ > + return result + size; /* Nope. */ > +found_middle: > + return result + __ffs(tmp); > + > +found_middle_swap: > + return result + __ffs(ext2_swab(tmp)); > +} > +EXPORT_SYMBOL(find_next_bit_le); > +#endif > + > +#endif /* __BIG_ENDIAN */ I was going to ask that you convince git to actually present a proper diff, to make visible what changes. But other than the description says you don't really move the file, you copy it. Judging from further titles there's also nowhere you'd make Arm actually use this now generic code. Jan
On Tue, 2024-01-23 at 12:14 +0100, Jan Beulich wrote: > On 22.12.2023 16:13, Oleksii Kurochko wrote: > > --- a/xen/arch/riscv/include/asm/fence.h > > +++ b/xen/arch/riscv/include/asm/fence.h > > @@ -1,4 +1,4 @@ > > -/* SPDX-License-Identifier: GPL-2.0-or-later */ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > #ifndef _ASM_RISCV_FENCE_H > > #define _ASM_RISCV_FENCE_H > > > > @@ -11,3 +11,12 @@ > > #endif > > > > #endif /* _ASM_RISCV_FENCE_H */ > > + > > +/* > > + * Local variables: > > + * mode: C > > + * c-file-style: "BSD" > > + * c-basic-offset: 4 > > + * indent-tabs-mode: nil > > + * End: > > + */ > > Surely all of this wants doing in the previous patch, where the > header > is introduced? Yes, it should be in the previous patch. I'll do the proper rebase. > > > --- a/xen/common/Kconfig > > +++ b/xen/common/Kconfig > > @@ -47,6 +47,9 @@ config ARCH_MAP_DOMAIN_PAGE > > config GENERIC_BUG_FRAME > > bool > > > > +config GENERIC_FIND_NEXT_BIT > > + bool > > There's no need for this, as ... > > > --- a/xen/lib/Makefile > > +++ b/xen/lib/Makefile > > @@ -3,6 +3,7 @@ obj-$(CONFIG_X86) += x86/ > > lib-y += bsearch.o > > lib-y += ctors.o > > lib-y += ctype.o > > +lib-$(CONFIG_GENERIC_FIND_NEXT_BIT) += find-next-bit.o > > ... you're moving this to lib/. Or have you encountered any issue > with building this uniformly, and you forgot to mention this in > the description? I didn't check. My intention was to provide opportunity to check if an architecture want to use generic version or not. Otherwise, I expected that we will have multiple definiotion of the funcion. But considering that they are all defined under #ifdef...#endif we can remove the declaration of the config GENERIC_FIND_NEXT_BIT. > > > --- /dev/null > > +++ b/xen/lib/find-next-bit.c > > @@ -0,0 +1,281 @@ > > +/* find_next_bit.c: fallback find next bit implementation > > + * > > + * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved. > > + * Written by David Howells (dhowells@redhat.com) > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License > > + * as published by the Free Software Foundation; either version > > + * 2 of the License, or (at your option) any later version. > > + */ > > +#include <xen/bitops.h> > > + > > +#include <asm/byteorder.h> > > + > > +#ifndef find_next_bit > > +/* > > + * Find the next set bit in a memory region. > > + */ > > +unsigned long find_next_bit(const unsigned long *addr, unsigned > > long size, > > + unsigned long offset) > > +{ > > + const unsigned long *p = addr + BIT_WORD(offset); > > + unsigned long result = offset & ~(BITS_PER_LONG-1); > > + unsigned long tmp; > > + > > + if (offset >= size) > > + return size; > > + size -= result; > > + offset %= BITS_PER_LONG; > > + if (offset) { > > + tmp = *(p++); > > + tmp &= (~0UL << offset); > > + if (size < BITS_PER_LONG) > > + goto found_first; > > + if (tmp) > > + goto found_middle; > > + size -= BITS_PER_LONG; > > + result += BITS_PER_LONG; > > + } > > + while (size & ~(BITS_PER_LONG-1)) { > > + if ((tmp = *(p++))) > > + goto found_middle; > > + result += BITS_PER_LONG; > > + size -= BITS_PER_LONG; > > + } > > + if (!size) > > + return result; > > + tmp = *p; > > + > > +found_first: > > + tmp &= (~0UL >> (BITS_PER_LONG - size)); > > + if (tmp == 0UL) /* Are any bits set? */ > > + return result + size; /* Nope. */ > > +found_middle: > > + return result + __ffs(tmp); > > +} > > +EXPORT_SYMBOL(find_next_bit); > > +#endif > > + > > +#ifndef find_next_zero_bit > > +/* > > + * This implementation of find_{first,next}_zero_bit was stolen > > from > > + * Linus' asm-alpha/bitops.h. > > + */ > > +unsigned long find_next_zero_bit(const unsigned long *addr, > > unsigned long size, > > + unsigned long offset) > > +{ > > + const unsigned long *p = addr + BIT_WORD(offset); > > + unsigned long result = offset & ~(BITS_PER_LONG-1); > > + unsigned long tmp; > > + > > + if (offset >= size) > > + return size; > > + size -= result; > > + offset %= BITS_PER_LONG; > > + if (offset) { > > + tmp = *(p++); > > + tmp |= ~0UL >> (BITS_PER_LONG - offset); > > + if (size < BITS_PER_LONG) > > + goto found_first; > > + if (~tmp) > > + goto found_middle; > > + size -= BITS_PER_LONG; > > + result += BITS_PER_LONG; > > + } > > + while (size & ~(BITS_PER_LONG-1)) { > > + if (~(tmp = *(p++))) > > + goto found_middle; > > + result += BITS_PER_LONG; > > + size -= BITS_PER_LONG; > > + } > > + if (!size) > > + return result; > > + tmp = *p; > > + > > +found_first: > > + tmp |= ~0UL << size; > > + if (tmp == ~0UL) /* Are any bits zero? */ > > + return result + size; /* Nope. */ > > +found_middle: > > + return result + ffz(tmp); > > +} > > +EXPORT_SYMBOL(find_next_zero_bit); > > +#endif > > + > > +#ifndef find_first_bit > > +/* > > + * Find the first set bit in a memory region. > > + */ > > +unsigned long find_first_bit(const unsigned long *addr, unsigned > > long size) > > +{ > > + const unsigned long *p = addr; > > + unsigned long result = 0; > > + unsigned long tmp; > > + > > + while (size & ~(BITS_PER_LONG-1)) { > > + if ((tmp = *(p++))) > > + goto found; > > + result += BITS_PER_LONG; > > + size -= BITS_PER_LONG; > > + } > > + if (!size) > > + return result; > > + > > + tmp = (*p) & (~0UL >> (BITS_PER_LONG - size)); > > + if (tmp == 0UL) /* Are any bits set? */ > > + return result + size; /* Nope. */ > > +found: > > + return result + __ffs(tmp); > > +} > > +EXPORT_SYMBOL(find_first_bit); > > +#endif > > + > > +#ifndef find_first_zero_bit > > +/* > > + * Find the first cleared bit in a memory region. > > + */ > > +unsigned long find_first_zero_bit(const unsigned long *addr, > > unsigned long size) > > +{ > > + const unsigned long *p = addr; > > + unsigned long result = 0; > > + unsigned long tmp; > > + > > + while (size & ~(BITS_PER_LONG-1)) { > > + if (~(tmp = *(p++))) > > + goto found; > > + result += BITS_PER_LONG; > > + size -= BITS_PER_LONG; > > + } > > + if (!size) > > + return result; > > + > > + tmp = (*p) | (~0UL << size); > > + if (tmp == ~0UL) /* Are any bits zero? */ > > + return result + size; /* Nope. */ > > +found: > > + return result + ffz(tmp); > > +} > > +EXPORT_SYMBOL(find_first_zero_bit); > > +#endif > > + > > +#ifdef __BIG_ENDIAN > > + > > +/* include/linux/byteorder does not support "unsigned long" type > > */ > > +static inline unsigned long ext2_swabp(const unsigned long * x) > > +{ > > +#if BITS_PER_LONG == 64 > > + return (unsigned long) __swab64p((u64 *) x); > > +#elif BITS_PER_LONG == 32 > > + return (unsigned long) __swab32p((u32 *) x); > > +#else > > +#error BITS_PER_LONG not defined > > +#endif > > +} > > + > > +/* include/linux/byteorder doesn't support "unsigned long" type */ > > +static inline unsigned long ext2_swab(const unsigned long y) > > +{ > > +#if BITS_PER_LONG == 64 > > + return (unsigned long) __swab64((u64) y); > > +#elif BITS_PER_LONG == 32 > > + return (unsigned long) __swab32((u32) y); > > +#else > > +#error BITS_PER_LONG not defined > > +#endif > > +} > > + > > +#ifndef find_next_zero_bit_le > > +unsigned long find_next_zero_bit_le(const void *addr, unsigned > > + long size, unsigned long offset) > > +{ > > + const unsigned long *p = addr; > > + unsigned long result = offset & ~(BITS_PER_LONG - 1); > > + unsigned long tmp; > > + > > + if (offset >= size) > > + return size; > > + p += BIT_WORD(offset); > > + size -= result; > > + offset &= (BITS_PER_LONG - 1UL); > > + if (offset) { > > + tmp = ext2_swabp(p++); > > + tmp |= (~0UL >> (BITS_PER_LONG - offset)); > > + if (size < BITS_PER_LONG) > > + goto found_first; > > + if (~tmp) > > + goto found_middle; > > + size -= BITS_PER_LONG; > > + result += BITS_PER_LONG; > > + } > > + > > + while (size & ~(BITS_PER_LONG - 1)) { > > + if (~(tmp = *(p++))) > > + goto found_middle_swap; > > + result += BITS_PER_LONG; > > + size -= BITS_PER_LONG; > > + } > > + if (!size) > > + return result; > > + tmp = ext2_swabp(p); > > +found_first: > > + tmp |= ~0UL << size; > > + if (tmp == ~0UL) /* Are any bits zero? */ > > + return result + size; /* Nope. Skip ffz */ > > +found_middle: > > + return result + ffz(tmp); > > + > > +found_middle_swap: > > + return result + ffz(ext2_swab(tmp)); > > +} > > +EXPORT_SYMBOL(find_next_zero_bit_le); > > +#endif > > + > > +#ifndef find_next_bit_le > > +unsigned long find_next_bit_le(const void *addr, unsigned > > + long size, unsigned long offset) > > +{ > > + const unsigned long *p = addr; > > + unsigned long result = offset & ~(BITS_PER_LONG - 1); > > + unsigned long tmp; > > + > > + if (offset >= size) > > + return size; > > + p += BIT_WORD(offset); > > + size -= result; > > + offset &= (BITS_PER_LONG - 1UL); > > + if (offset) { > > + tmp = ext2_swabp(p++); > > + tmp &= (~0UL << offset); > > + if (size < BITS_PER_LONG) > > + goto found_first; > > + if (tmp) > > + goto found_middle; > > + size -= BITS_PER_LONG; > > + result += BITS_PER_LONG; > > + } > > + > > + while (size & ~(BITS_PER_LONG - 1)) { > > + tmp = *(p++); > > + if (tmp) > > + goto found_middle_swap; > > + result += BITS_PER_LONG; > > + size -= BITS_PER_LONG; > > + } > > + if (!size) > > + return result; > > + tmp = ext2_swabp(p); > > +found_first: > > + tmp &= (~0UL >> (BITS_PER_LONG - size)); > > + if (tmp == 0UL) /* Are any bits set? */ > > + return result + size; /* Nope. */ > > +found_middle: > > + return result + __ffs(tmp); > > + > > +found_middle_swap: > > + return result + __ffs(ext2_swab(tmp)); > > +} > > +EXPORT_SYMBOL(find_next_bit_le); > > +#endif > > + > > +#endif /* __BIG_ENDIAN */ > > I was going to ask that you convince git to actually present a proper > diff, to make visible what changes. But other than the description > says > you don't really move the file, you copy it. Judging from further > titles > there's also nowhere you'd make Arm actually use this now generic > code. I wanted to do it separately, outside this patch series to simplify review and not have Arm specific changes in RISC-V patch series. Regarding a proper diff, you would like me to make git shows that it was copy from Arm and it is not newly created file. Am I understand you correctly? ~ Oleksii
On 23.01.2024 13:34, Oleksii wrote: > On Tue, 2024-01-23 at 12:14 +0100, Jan Beulich wrote: >> On 22.12.2023 16:13, Oleksii Kurochko wrote: >>> --- a/xen/common/Kconfig >>> +++ b/xen/common/Kconfig >>> @@ -47,6 +47,9 @@ config ARCH_MAP_DOMAIN_PAGE >>> config GENERIC_BUG_FRAME >>> bool >>> >>> +config GENERIC_FIND_NEXT_BIT >>> + bool >> >> There's no need for this, as ... >> >>> --- a/xen/lib/Makefile >>> +++ b/xen/lib/Makefile >>> @@ -3,6 +3,7 @@ obj-$(CONFIG_X86) += x86/ >>> lib-y += bsearch.o >>> lib-y += ctors.o >>> lib-y += ctype.o >>> +lib-$(CONFIG_GENERIC_FIND_NEXT_BIT) += find-next-bit.o >> >> ... you're moving this to lib/. Or have you encountered any issue >> with building this uniformly, and you forgot to mention this in >> the description? > I didn't check. My intention was to provide opportunity to check if an > architecture want to use generic version or not. Otherwise, I expected > that we will have multiple definiotion of the funcion. > > But considering that they are all defined under #ifdef...#endif we can > remove the declaration of the config GENERIC_FIND_NEXT_BIT. What #ifdef / #endif would matter here? Whats in lib/ is intended to be generic anyway. And what is in the resulting lib.a won't be used by an arch if it has an arch-specific implementation. Problems could arise if an arch had an inline function colliding with the out-of-line one. But that's about the old case where I could see a need to make the building of one of the objects conditional. And you'll note that withing this Makefile there are pretty few conditionals. >>> --- /dev/null >>> +++ b/xen/lib/find-next-bit.c >>>[...] >> >> I was going to ask that you convince git to actually present a proper >> diff, to make visible what changes. But other than the description >> says >> you don't really move the file, you copy it. Judging from further >> titles >> there's also nowhere you'd make Arm actually use this now generic >> code. > I wanted to do it separately, outside this patch series to simplify > review and not have Arm specific changes in RISC-V patch series. Then do it the other way around: Make a separate _prereq_ change truly moving the file. > Regarding a proper diff, you would like me to make git shows that it > was copy from Arm and it is not newly created file. Am I understand you > correctly? Not quite, I think. Git has move detection (and we've seen that in action in other patches of yours). So when truly moving a file, what (if anything) is changed is easily visible. Jan
On Tue, 2024-01-23 at 14:37 +0100, Jan Beulich wrote: > On 23.01.2024 13:34, Oleksii wrote: > > On Tue, 2024-01-23 at 12:14 +0100, Jan Beulich wrote: > > > On 22.12.2023 16:13, Oleksii Kurochko wrote: > > > > --- a/xen/common/Kconfig > > > > +++ b/xen/common/Kconfig > > > > @@ -47,6 +47,9 @@ config ARCH_MAP_DOMAIN_PAGE > > > > config GENERIC_BUG_FRAME > > > > bool > > > > > > > > +config GENERIC_FIND_NEXT_BIT > > > > + bool > > > > > > There's no need for this, as ... > > > > > > > --- a/xen/lib/Makefile > > > > +++ b/xen/lib/Makefile > > > > @@ -3,6 +3,7 @@ obj-$(CONFIG_X86) += x86/ > > > > lib-y += bsearch.o > > > > lib-y += ctors.o > > > > lib-y += ctype.o > > > > +lib-$(CONFIG_GENERIC_FIND_NEXT_BIT) += find-next-bit.o > > > > > > ... you're moving this to lib/. Or have you encountered any issue > > > with building this uniformly, and you forgot to mention this in > > > the description? > > I didn't check. My intention was to provide opportunity to check if > > an > > architecture want to use generic version or not. Otherwise, I > > expected > > that we will have multiple definiotion of the funcion. > > > > But considering that they are all defined under #ifdef...#endif we > > can > > remove the declaration of the config GENERIC_FIND_NEXT_BIT. > > What #ifdef / #endif would matter here? Whats in lib/ is intended to > be > generic anyway. And what is in the resulting lib.a won't be used by > an > arch if it has an arch-specific implementation. If what is implemented in lib.a won't be used by an arch if it has an arch-specific implementation then, for sure, I have to drop CONFIG_GENERIC_FIND_NEXT_BIT. But I am not really understand if lib.a is linked with Xen, then it should be an issue then if some arch implement find-next-bit function we will have to multiple definitions ( one in lib.a and one arch specific ). Probably, I have to look at how it is done. > Problems could arise if > an arch had an inline function colliding with the out-of-line one. > But > that's about the old case where I could see a need to make the > building > of one of the objects conditional. And you'll note that withing this > Makefile there are pretty few conditionals. Could you please clarify What does it mean "out-of-line" ? > > > > > --- /dev/null > > > > +++ b/xen/lib/find-next-bit.c > > > > [...] > > > > > > I was going to ask that you convince git to actually present a > > > proper > > > diff, to make visible what changes. But other than the > > > description > > > says > > > you don't really move the file, you copy it. Judging from further > > > titles > > > there's also nowhere you'd make Arm actually use this now generic > > > code. > > I wanted to do it separately, outside this patch series to simplify > > review and not have Arm specific changes in RISC-V patch series. > > Then do it the other way around: Make a separate _prereq_ change > truly > moving the file. So this one patch should be separated by 2? One which moves find-next- bit.c from Arm to xen/lib, and second where xen/lib/Makefile is updated. > > > Regarding a proper diff, you would like me to make git shows that > > it > > was copy from Arm and it is not newly created file. Am I understand > > you > > correctly? > > Not quite, I think. Git has move detection (and we've seen that in > action in other patches of yours). So when truly moving a file, what > (if anything) is changed is easily visible. I think I am still a little bit confused. But I think the answer on my question above can clarify that. ~ Oleksii
On 24.01.2024 10:34, Oleksii wrote: > On Tue, 2024-01-23 at 14:37 +0100, Jan Beulich wrote: >> On 23.01.2024 13:34, Oleksii wrote: >>> On Tue, 2024-01-23 at 12:14 +0100, Jan Beulich wrote: >>>> On 22.12.2023 16:13, Oleksii Kurochko wrote: >>>>> --- a/xen/common/Kconfig >>>>> +++ b/xen/common/Kconfig >>>>> @@ -47,6 +47,9 @@ config ARCH_MAP_DOMAIN_PAGE >>>>> config GENERIC_BUG_FRAME >>>>> bool >>>>> >>>>> +config GENERIC_FIND_NEXT_BIT >>>>> + bool >>>> >>>> There's no need for this, as ... >>>> >>>>> --- a/xen/lib/Makefile >>>>> +++ b/xen/lib/Makefile >>>>> @@ -3,6 +3,7 @@ obj-$(CONFIG_X86) += x86/ >>>>> lib-y += bsearch.o >>>>> lib-y += ctors.o >>>>> lib-y += ctype.o >>>>> +lib-$(CONFIG_GENERIC_FIND_NEXT_BIT) += find-next-bit.o >>>> >>>> ... you're moving this to lib/. Or have you encountered any issue >>>> with building this uniformly, and you forgot to mention this in >>>> the description? >>> I didn't check. My intention was to provide opportunity to check if >>> an >>> architecture want to use generic version or not. Otherwise, I >>> expected >>> that we will have multiple definiotion of the funcion. >>> >>> But considering that they are all defined under #ifdef...#endif we >>> can >>> remove the declaration of the config GENERIC_FIND_NEXT_BIT. >> >> What #ifdef / #endif would matter here? Whats in lib/ is intended to >> be >> generic anyway. And what is in the resulting lib.a won't be used by >> an >> arch if it has an arch-specific implementation. > If what is implemented in lib.a won't be used by an arch if it has an > arch-specific implementation then, for sure, I have to drop > CONFIG_GENERIC_FIND_NEXT_BIT. > But I am not really understand if lib.a is linked with Xen, then it > should be an issue then if some arch implement find-next-bit function > we will have to multiple definitions ( one in lib.a and one arch > specific ). Probably, I have to look at how it is done. You're aware how linking works? Objects are pulled out of archives only if there's no other definition for a to-be-resolved symbol provided by a particular object in the archive. >> Problems could arise if >> an arch had an inline function colliding with the out-of-line one. >> But >> that's about the old case where I could see a need to make the >> building >> of one of the objects conditional. And you'll note that withing this >> Makefile there are pretty few conditionals. > Could you please clarify What does it mean "out-of-line" ? "not inline" >>>>> --- /dev/null >>>>> +++ b/xen/lib/find-next-bit.c >>>>> [...] >>>> >>>> I was going to ask that you convince git to actually present a >>>> proper >>>> diff, to make visible what changes. But other than the >>>> description >>>> says >>>> you don't really move the file, you copy it. Judging from further >>>> titles >>>> there's also nowhere you'd make Arm actually use this now generic >>>> code. >>> I wanted to do it separately, outside this patch series to simplify >>> review and not have Arm specific changes in RISC-V patch series. >> >> Then do it the other way around: Make a separate _prereq_ change >> truly >> moving the file. > So this one patch should be separated by 2? One which moves find-next- > bit.c from Arm to xen/lib, and second where xen/lib/Makefile is > updated. No, that would break the Arm build. I suggested breaking out this patch from the series, and then doing what the description says: Actually move the file. I don't think I suggested splitting the patch. Even the breaking out of the series was only because you said "I wanted to do it separately, outside this patch series". Jan
On Wed, 2024-01-24 at 12:24 +0100, Jan Beulich wrote: > On 24.01.2024 10:34, Oleksii wrote: > > On Tue, 2024-01-23 at 14:37 +0100, Jan Beulich wrote: > > > On 23.01.2024 13:34, Oleksii wrote: > > > > On Tue, 2024-01-23 at 12:14 +0100, Jan Beulich wrote: > > > > > On 22.12.2023 16:13, Oleksii Kurochko wrote: > > > > > > --- a/xen/common/Kconfig > > > > > > +++ b/xen/common/Kconfig > > > > > > @@ -47,6 +47,9 @@ config ARCH_MAP_DOMAIN_PAGE > > > > > > config GENERIC_BUG_FRAME > > > > > > bool > > > > > > > > > > > > +config GENERIC_FIND_NEXT_BIT > > > > > > + bool > > > > > > > > > > There's no need for this, as ... > > > > > > > > > > > --- a/xen/lib/Makefile > > > > > > +++ b/xen/lib/Makefile > > > > > > @@ -3,6 +3,7 @@ obj-$(CONFIG_X86) += x86/ > > > > > > lib-y += bsearch.o > > > > > > lib-y += ctors.o > > > > > > lib-y += ctype.o > > > > > > +lib-$(CONFIG_GENERIC_FIND_NEXT_BIT) += find-next-bit.o > > > > > > > > > > ... you're moving this to lib/. Or have you encountered any > > > > > issue > > > > > with building this uniformly, and you forgot to mention this > > > > > in > > > > > the description? > > > > I didn't check. My intention was to provide opportunity to > > > > check if > > > > an > > > > architecture want to use generic version or not. Otherwise, I > > > > expected > > > > that we will have multiple definiotion of the funcion. > > > > > > > > But considering that they are all defined under #ifdef...#endif > > > > we > > > > can > > > > remove the declaration of the config GENERIC_FIND_NEXT_BIT. > > > > > > What #ifdef / #endif would matter here? Whats in lib/ is intended > > > to > > > be > > > generic anyway. And what is in the resulting lib.a won't be used > > > by > > > an > > > arch if it has an arch-specific implementation. > > If what is implemented in lib.a won't be used by an arch if it has > > an > > arch-specific implementation then, for sure, I have to drop > > CONFIG_GENERIC_FIND_NEXT_BIT. > > But I am not really understand if lib.a is linked with Xen, then it > > should be an issue then if some arch implement find-next-bit > > function > > we will have to multiple definitions ( one in lib.a and one arch > > specific ). Probably, I have to look at how it is done. > > You're aware how linking works? Objects are pulled out of archives > only > if there's no other definition for a to-be-resolved symbol provided > by > a particular object in the archive. I wasn't aware about the case of the archive. Thanks for the explanation. > > > > Problems could arise if > > > an arch had an inline function colliding with the out-of-line > > > one. > > > But > > > that's about the old case where I could see a need to make the > > > building > > > of one of the objects conditional. And you'll note that withing > > > this > > > Makefile there are pretty few conditionals. > > Could you please clarify What does it mean "out-of-line" ? > > "not inline" > > > > > > > --- /dev/null > > > > > > +++ b/xen/lib/find-next-bit.c > > > > > > [...] > > > > > > > > > > I was going to ask that you convince git to actually present > > > > > a > > > > > proper > > > > > diff, to make visible what changes. But other than the > > > > > description > > > > > says > > > > > you don't really move the file, you copy it. Judging from > > > > > further > > > > > titles > > > > > there's also nowhere you'd make Arm actually use this now > > > > > generic > > > > > code. > > > > I wanted to do it separately, outside this patch series to > > > > simplify > > > > review and not have Arm specific changes in RISC-V patch > > > > series. > > > > > > Then do it the other way around: Make a separate _prereq_ change > > > truly > > > moving the file. > > So this one patch should be separated by 2? One which moves find- > > next- > > bit.c from Arm to xen/lib, and second where xen/lib/Makefile is > > updated. > > No, that would break the Arm build. I suggested breaking out this > patch from the series, and then doing what the description says: > Actually move the file. I don't think I suggested splitting the > patch. Even the breaking out of the series was only because you > said "I wanted to do it separately, outside this patch series". What I meant was that I would like to have a patch which introduces generic version of find-next-bit in the current patch series and provide a separate patch outside of the current patch series which switches Arm to use generic version. ~ Oleksii
On 24.01.2024 16:04, Oleksii wrote: > On Wed, 2024-01-24 at 12:24 +0100, Jan Beulich wrote: >> On 24.01.2024 10:34, Oleksii wrote: >>> On Tue, 2024-01-23 at 14:37 +0100, Jan Beulich wrote: >>>> On 23.01.2024 13:34, Oleksii wrote: >>>>> On Tue, 2024-01-23 at 12:14 +0100, Jan Beulich wrote: >>>>>> On 22.12.2023 16:13, Oleksii Kurochko wrote: >>>>>>> --- /dev/null >>>>>>> +++ b/xen/lib/find-next-bit.c >>>>>>> [...] >>>>>> >>>>>> I was going to ask that you convince git to actually present >>>>>> a >>>>>> proper >>>>>> diff, to make visible what changes. But other than the >>>>>> description >>>>>> says >>>>>> you don't really move the file, you copy it. Judging from >>>>>> further >>>>>> titles >>>>>> there's also nowhere you'd make Arm actually use this now >>>>>> generic >>>>>> code. >>>>> I wanted to do it separately, outside this patch series to >>>>> simplify >>>>> review and not have Arm specific changes in RISC-V patch >>>>> series. >>>> >>>> Then do it the other way around: Make a separate _prereq_ change >>>> truly >>>> moving the file. >>> So this one patch should be separated by 2? One which moves find- >>> next- >>> bit.c from Arm to xen/lib, and second where xen/lib/Makefile is >>> updated. >> >> No, that would break the Arm build. I suggested breaking out this >> patch from the series, and then doing what the description says: >> Actually move the file. I don't think I suggested splitting the >> patch. Even the breaking out of the series was only because you >> said "I wanted to do it separately, outside this patch series". > What I meant was that I would like to have a patch which introduces > generic version of find-next-bit in the current patch series and > provide a separate patch outside of the current patch series which > switches Arm to use generic version. I understand that this is what you meant. Yet I don't like the duplication of code, even if it's only temporary. The more that iirc git can show proper history for moved files, while there'll be a disconnect when you first add a 2nd copy and later purge the original. If you want this change separate from the series - fine. But then, as said, please as a prereq patch. Jan
On Tue, 2024-01-23 at 14:37 +0100, Jan Beulich wrote: > On 23.01.2024 13:34, Oleksii wrote: > > On Tue, 2024-01-23 at 12:14 +0100, Jan Beulich wrote: > > > On 22.12.2023 16:13, Oleksii Kurochko wrote: > > > > --- a/xen/common/Kconfig > > > > +++ b/xen/common/Kconfig > > > > @@ -47,6 +47,9 @@ config ARCH_MAP_DOMAIN_PAGE > > > > config GENERIC_BUG_FRAME > > > > bool > > > > > > > > +config GENERIC_FIND_NEXT_BIT > > > > + bool > > > > > > There's no need for this, as ... > > > > > > > --- a/xen/lib/Makefile > > > > +++ b/xen/lib/Makefile > > > > @@ -3,6 +3,7 @@ obj-$(CONFIG_X86) += x86/ > > > > lib-y += bsearch.o > > > > lib-y += ctors.o > > > > lib-y += ctype.o > > > > +lib-$(CONFIG_GENERIC_FIND_NEXT_BIT) += find-next-bit.o > > > > > > ... you're moving this to lib/. Or have you encountered any issue > > > with building this uniformly, and you forgot to mention this in > > > the description? > > I didn't check. My intention was to provide opportunity to check if > > an > > architecture want to use generic version or not. Otherwise, I > > expected > > that we will have multiple definiotion of the funcion. > > > > But considering that they are all defined under #ifdef...#endif we > > can > > remove the declaration of the config GENERIC_FIND_NEXT_BIT. > > What #ifdef / #endif would matter here? Whats in lib/ is intended to > be > generic anyway. And what is in the resulting lib.a won't be used by > an > arch if it has an arch-specific implementation. Problems could arise > if > an arch had an inline function colliding with the out-of-line one. > But > that's about the old case where I could see a need to make the > building > of one of the objects conditional. And you'll note that withing this > Makefile there are pretty few conditionals. We will have such issue with PPC: ... static inline unsigned long find_next_bit(const unsigned long *addr, unsigned long size, unsigned long offset) ... It looks like an introduction of new config for find_next_bit is needed. Does a better option exist? Would making find_next_bit non inline non inline for PPC better? ~ Oleksii
On 26.01.2024 10:44, Oleksii wrote: > On Tue, 2024-01-23 at 14:37 +0100, Jan Beulich wrote: >> On 23.01.2024 13:34, Oleksii wrote: >>> On Tue, 2024-01-23 at 12:14 +0100, Jan Beulich wrote: >>>> On 22.12.2023 16:13, Oleksii Kurochko wrote: >>>>> --- a/xen/common/Kconfig >>>>> +++ b/xen/common/Kconfig >>>>> @@ -47,6 +47,9 @@ config ARCH_MAP_DOMAIN_PAGE >>>>> config GENERIC_BUG_FRAME >>>>> bool >>>>> >>>>> +config GENERIC_FIND_NEXT_BIT >>>>> + bool >>>> >>>> There's no need for this, as ... >>>> >>>>> --- a/xen/lib/Makefile >>>>> +++ b/xen/lib/Makefile >>>>> @@ -3,6 +3,7 @@ obj-$(CONFIG_X86) += x86/ >>>>> lib-y += bsearch.o >>>>> lib-y += ctors.o >>>>> lib-y += ctype.o >>>>> +lib-$(CONFIG_GENERIC_FIND_NEXT_BIT) += find-next-bit.o >>>> >>>> ... you're moving this to lib/. Or have you encountered any issue >>>> with building this uniformly, and you forgot to mention this in >>>> the description? >>> I didn't check. My intention was to provide opportunity to check if >>> an >>> architecture want to use generic version or not. Otherwise, I >>> expected >>> that we will have multiple definiotion of the funcion. >>> >>> But considering that they are all defined under #ifdef...#endif we >>> can >>> remove the declaration of the config GENERIC_FIND_NEXT_BIT. >> >> What #ifdef / #endif would matter here? Whats in lib/ is intended to >> be >> generic anyway. And what is in the resulting lib.a won't be used by >> an >> arch if it has an arch-specific implementation. Problems could arise >> if >> an arch had an inline function colliding with the out-of-line one. >> But >> that's about the old case where I could see a need to make the >> building >> of one of the objects conditional. And you'll note that withing this >> Makefile there are pretty few conditionals. > We will have such issue with PPC: > ... > static inline unsigned long find_next_bit(const unsigned long *addr, > unsigned long size, > unsigned long offset) > ... > > It looks like an introduction of new config for find_next_bit is > needed. > > Does a better option exist? Would making find_next_bit non inline non > inline for PPC better? Isn't that generic code anyway? If so, that also wants replacing by the generic library function(s). Shawn - I have to admit I have a hard time seeing why this was introduced as inline functions in the first place. Jan
On Fri, 2024-01-26 at 10:48 +0100, Jan Beulich wrote: > On 26.01.2024 10:44, Oleksii wrote: > > On Tue, 2024-01-23 at 14:37 +0100, Jan Beulich wrote: > > > On 23.01.2024 13:34, Oleksii wrote: > > > > On Tue, 2024-01-23 at 12:14 +0100, Jan Beulich wrote: > > > > > On 22.12.2023 16:13, Oleksii Kurochko wrote: > > > > > > --- a/xen/common/Kconfig > > > > > > +++ b/xen/common/Kconfig > > > > > > @@ -47,6 +47,9 @@ config ARCH_MAP_DOMAIN_PAGE > > > > > > config GENERIC_BUG_FRAME > > > > > > bool > > > > > > > > > > > > +config GENERIC_FIND_NEXT_BIT > > > > > > + bool > > > > > > > > > > There's no need for this, as ... > > > > > > > > > > > --- a/xen/lib/Makefile > > > > > > +++ b/xen/lib/Makefile > > > > > > @@ -3,6 +3,7 @@ obj-$(CONFIG_X86) += x86/ > > > > > > lib-y += bsearch.o > > > > > > lib-y += ctors.o > > > > > > lib-y += ctype.o > > > > > > +lib-$(CONFIG_GENERIC_FIND_NEXT_BIT) += find-next-bit.o > > > > > > > > > > ... you're moving this to lib/. Or have you encountered any > > > > > issue > > > > > with building this uniformly, and you forgot to mention this > > > > > in > > > > > the description? > > > > I didn't check. My intention was to provide opportunity to > > > > check if > > > > an > > > > architecture want to use generic version or not. Otherwise, I > > > > expected > > > > that we will have multiple definiotion of the funcion. > > > > > > > > But considering that they are all defined under #ifdef...#endif > > > > we > > > > can > > > > remove the declaration of the config GENERIC_FIND_NEXT_BIT. > > > > > > What #ifdef / #endif would matter here? Whats in lib/ is intended > > > to > > > be > > > generic anyway. And what is in the resulting lib.a won't be used > > > by > > > an > > > arch if it has an arch-specific implementation. Problems could > > > arise > > > if > > > an arch had an inline function colliding with the out-of-line > > > one. > > > But > > > that's about the old case where I could see a need to make the > > > building > > > of one of the objects conditional. And you'll note that withing > > > this > > > Makefile there are pretty few conditionals. > > We will have such issue with PPC: > > ... > > static inline unsigned long find_next_bit(const unsigned long > > *addr, > > unsigned long size, > > unsigned long offset) > > ... > > > > It looks like an introduction of new config for find_next_bit is > > needed. > > > > Does a better option exist? Would making find_next_bit non inline > > non > > inline for PPC better? > > Isn't that generic code anyway? If so, that also wants replacing by > the generic library function(s). Shawn - I have to admit I have a > hard time seeing why this was introduced as inline functions in the > first place. You are right, it is generic one too. I'll replace it too. ~ Oleksii
diff --git a/xen/arch/riscv/include/asm/fence.h b/xen/arch/riscv/include/asm/fence.h index b3f6b1c20a..5d9a851ae8 100644 --- a/xen/arch/riscv/include/asm/fence.h +++ b/xen/arch/riscv/include/asm/fence.h @@ -1,4 +1,4 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* SPDX-License-Identifier: GPL-2.0-only */ #ifndef _ASM_RISCV_FENCE_H #define _ASM_RISCV_FENCE_H @@ -11,3 +11,12 @@ #endif #endif /* _ASM_RISCV_FENCE_H */ + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/common/Kconfig b/xen/common/Kconfig index 310ad4229c..56ce34ffd4 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -47,6 +47,9 @@ config ARCH_MAP_DOMAIN_PAGE config GENERIC_BUG_FRAME bool +config GENERIC_FIND_NEXT_BIT + bool + config HAS_ALTERNATIVE bool diff --git a/xen/lib/Makefile b/xen/lib/Makefile index 2d9ebb945f..66237a45c5 100644 --- a/xen/lib/Makefile +++ b/xen/lib/Makefile @@ -3,6 +3,7 @@ obj-$(CONFIG_X86) += x86/ lib-y += bsearch.o lib-y += ctors.o lib-y += ctype.o +lib-$(CONFIG_GENERIC_FIND_NEXT_BIT) += find-next-bit.o lib-y += list-sort.o lib-y += memchr.o lib-y += memchr_inv.o diff --git a/xen/lib/find-next-bit.c b/xen/lib/find-next-bit.c new file mode 100644 index 0000000000..ca6f82277e --- /dev/null +++ b/xen/lib/find-next-bit.c @@ -0,0 +1,281 @@ +/* find_next_bit.c: fallback find next bit implementation + * + * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved. + * Written by David Howells (dhowells@redhat.com) + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + */ +#include <xen/bitops.h> + +#include <asm/byteorder.h> + +#ifndef find_next_bit +/* + * Find the next set bit in a memory region. + */ +unsigned long find_next_bit(const unsigned long *addr, unsigned long size, + unsigned long offset) +{ + const unsigned long *p = addr + BIT_WORD(offset); + unsigned long result = offset & ~(BITS_PER_LONG-1); + unsigned long tmp; + + if (offset >= size) + return size; + size -= result; + offset %= BITS_PER_LONG; + if (offset) { + tmp = *(p++); + tmp &= (~0UL << offset); + if (size < BITS_PER_LONG) + goto found_first; + if (tmp) + goto found_middle; + size -= BITS_PER_LONG; + result += BITS_PER_LONG; + } + while (size & ~(BITS_PER_LONG-1)) { + if ((tmp = *(p++))) + goto found_middle; + result += BITS_PER_LONG; + size -= BITS_PER_LONG; + } + if (!size) + return result; + tmp = *p; + +found_first: + tmp &= (~0UL >> (BITS_PER_LONG - size)); + if (tmp == 0UL) /* Are any bits set? */ + return result + size; /* Nope. */ +found_middle: + return result + __ffs(tmp); +} +EXPORT_SYMBOL(find_next_bit); +#endif + +#ifndef find_next_zero_bit +/* + * This implementation of find_{first,next}_zero_bit was stolen from + * Linus' asm-alpha/bitops.h. + */ +unsigned long find_next_zero_bit(const unsigned long *addr, unsigned long size, + unsigned long offset) +{ + const unsigned long *p = addr + BIT_WORD(offset); + unsigned long result = offset & ~(BITS_PER_LONG-1); + unsigned long tmp; + + if (offset >= size) + return size; + size -= result; + offset %= BITS_PER_LONG; + if (offset) { + tmp = *(p++); + tmp |= ~0UL >> (BITS_PER_LONG - offset); + if (size < BITS_PER_LONG) + goto found_first; + if (~tmp) + goto found_middle; + size -= BITS_PER_LONG; + result += BITS_PER_LONG; + } + while (size & ~(BITS_PER_LONG-1)) { + if (~(tmp = *(p++))) + goto found_middle; + result += BITS_PER_LONG; + size -= BITS_PER_LONG; + } + if (!size) + return result; + tmp = *p; + +found_first: + tmp |= ~0UL << size; + if (tmp == ~0UL) /* Are any bits zero? */ + return result + size; /* Nope. */ +found_middle: + return result + ffz(tmp); +} +EXPORT_SYMBOL(find_next_zero_bit); +#endif + +#ifndef find_first_bit +/* + * Find the first set bit in a memory region. + */ +unsigned long find_first_bit(const unsigned long *addr, unsigned long size) +{ + const unsigned long *p = addr; + unsigned long result = 0; + unsigned long tmp; + + while (size & ~(BITS_PER_LONG-1)) { + if ((tmp = *(p++))) + goto found; + result += BITS_PER_LONG; + size -= BITS_PER_LONG; + } + if (!size) + return result; + + tmp = (*p) & (~0UL >> (BITS_PER_LONG - size)); + if (tmp == 0UL) /* Are any bits set? */ + return result + size; /* Nope. */ +found: + return result + __ffs(tmp); +} +EXPORT_SYMBOL(find_first_bit); +#endif + +#ifndef find_first_zero_bit +/* + * Find the first cleared bit in a memory region. + */ +unsigned long find_first_zero_bit(const unsigned long *addr, unsigned long size) +{ + const unsigned long *p = addr; + unsigned long result = 0; + unsigned long tmp; + + while (size & ~(BITS_PER_LONG-1)) { + if (~(tmp = *(p++))) + goto found; + result += BITS_PER_LONG; + size -= BITS_PER_LONG; + } + if (!size) + return result; + + tmp = (*p) | (~0UL << size); + if (tmp == ~0UL) /* Are any bits zero? */ + return result + size; /* Nope. */ +found: + return result + ffz(tmp); +} +EXPORT_SYMBOL(find_first_zero_bit); +#endif + +#ifdef __BIG_ENDIAN + +/* include/linux/byteorder does not support "unsigned long" type */ +static inline unsigned long ext2_swabp(const unsigned long * x) +{ +#if BITS_PER_LONG == 64 + return (unsigned long) __swab64p((u64 *) x); +#elif BITS_PER_LONG == 32 + return (unsigned long) __swab32p((u32 *) x); +#else +#error BITS_PER_LONG not defined +#endif +} + +/* include/linux/byteorder doesn't support "unsigned long" type */ +static inline unsigned long ext2_swab(const unsigned long y) +{ +#if BITS_PER_LONG == 64 + return (unsigned long) __swab64((u64) y); +#elif BITS_PER_LONG == 32 + return (unsigned long) __swab32((u32) y); +#else +#error BITS_PER_LONG not defined +#endif +} + +#ifndef find_next_zero_bit_le +unsigned long find_next_zero_bit_le(const void *addr, unsigned + long size, unsigned long offset) +{ + const unsigned long *p = addr; + unsigned long result = offset & ~(BITS_PER_LONG - 1); + unsigned long tmp; + + if (offset >= size) + return size; + p += BIT_WORD(offset); + size -= result; + offset &= (BITS_PER_LONG - 1UL); + if (offset) { + tmp = ext2_swabp(p++); + tmp |= (~0UL >> (BITS_PER_LONG - offset)); + if (size < BITS_PER_LONG) + goto found_first; + if (~tmp) + goto found_middle; + size -= BITS_PER_LONG; + result += BITS_PER_LONG; + } + + while (size & ~(BITS_PER_LONG - 1)) { + if (~(tmp = *(p++))) + goto found_middle_swap; + result += BITS_PER_LONG; + size -= BITS_PER_LONG; + } + if (!size) + return result; + tmp = ext2_swabp(p); +found_first: + tmp |= ~0UL << size; + if (tmp == ~0UL) /* Are any bits zero? */ + return result + size; /* Nope. Skip ffz */ +found_middle: + return result + ffz(tmp); + +found_middle_swap: + return result + ffz(ext2_swab(tmp)); +} +EXPORT_SYMBOL(find_next_zero_bit_le); +#endif + +#ifndef find_next_bit_le +unsigned long find_next_bit_le(const void *addr, unsigned + long size, unsigned long offset) +{ + const unsigned long *p = addr; + unsigned long result = offset & ~(BITS_PER_LONG - 1); + unsigned long tmp; + + if (offset >= size) + return size; + p += BIT_WORD(offset); + size -= result; + offset &= (BITS_PER_LONG - 1UL); + if (offset) { + tmp = ext2_swabp(p++); + tmp &= (~0UL << offset); + if (size < BITS_PER_LONG) + goto found_first; + if (tmp) + goto found_middle; + size -= BITS_PER_LONG; + result += BITS_PER_LONG; + } + + while (size & ~(BITS_PER_LONG - 1)) { + tmp = *(p++); + if (tmp) + goto found_middle_swap; + result += BITS_PER_LONG; + size -= BITS_PER_LONG; + } + if (!size) + return result; + tmp = ext2_swabp(p); +found_first: + tmp &= (~0UL >> (BITS_PER_LONG - size)); + if (tmp == 0UL) /* Are any bits set? */ + return result + size; /* Nope. */ +found_middle: + return result + __ffs(tmp); + +found_middle_swap: + return result + __ffs(ext2_swab(tmp)); +} +EXPORT_SYMBOL(find_next_bit_le); +#endif + +#endif /* __BIG_ENDIAN */
find-next-bit.c is common for Arm64 and RISC-V64 so it is moved to xen/lib. Suggested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in V3: - new patch. --- xen/arch/riscv/include/asm/fence.h | 11 +- xen/common/Kconfig | 3 + xen/lib/Makefile | 1 + xen/lib/find-next-bit.c | 281 +++++++++++++++++++++++++++++ 4 files changed, 295 insertions(+), 1 deletion(-) create mode 100644 xen/lib/find-next-bit.c