On Wed, Jul 31, 2024 at 09:23:53AM GMT, Alexandre Ghiti wrote:
> asm/cmpxchg.h will soon need riscv_has_extension_unlikely() macros and
> then needs to include asm/cpufeature.h which introduces a lot of header
> circular dependencies.
The includes of asm/cpufeature.h don't look well maintained. I don't think
it needs asm/errno.h and it should have linux/threads.h,
linux/percpu-defs.h, and linux/kconfig.h.
>
> So move the riscv_has_extension_XXX() macros into their own header which
> prevents such circular dependencies by including a restricted number of
> headers.
>
> Signed-off-by: Alexandre Ghiti <alexghiti@xxxxxxxxxxxx>
> ---
> arch/riscv/include/asm/cpufeature-macros.h | 66 ++++++++++++++++++++++
> arch/riscv/include/asm/cpufeature.h | 56 +-----------------
> 2 files changed, 67 insertions(+), 55 deletions(-)
> create mode 100644 arch/riscv/include/asm/cpufeature-macros.h
>
> diff --git a/arch/riscv/include/asm/cpufeature-macros.h b/arch/riscv/include/asm/cpufeature-macros.h
> new file mode 100644
> index 000000000000..c5f0bf75e026
> --- /dev/null
> +++ b/arch/riscv/include/asm/cpufeature-macros.h
> @@ -0,0 +1,66 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright 2022-2024 Rivos, Inc
> + */
> +
> +#ifndef _ASM_CPUFEATURE_MACROS_H
> +#define _ASM_CPUFEATURE_MACROS_H
> +
> +#include <asm/hwcap.h>
> +#include <asm/alternative-macros.h>
> +
> +#define STANDARD_EXT 0
> +
> +bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, unsigned int bit);
> +#define riscv_isa_extension_available(isa_bitmap, ext) \
> + __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
> +
> +static __always_inline bool __riscv_has_extension_likely(const unsigned long vendor,
> + const unsigned long ext)
> +{
> + asm goto(ALTERNATIVE("j %l[l_no]", "nop", %[vendor], %[ext], 1)
> + :
> + : [vendor] "i" (vendor), [ext] "i" (ext)
> + :
> + : l_no);
> +
> + return true;
> +l_no:
> + return false;
> +}
> +
> +static __always_inline bool __riscv_has_extension_unlikely(const unsigned long vendor,
> + const unsigned long ext)
> +{
> + asm goto(ALTERNATIVE("nop", "j %l[l_yes]", %[vendor], %[ext], 1)
> + :
> + : [vendor] "i" (vendor), [ext] "i" (ext)
> + :
> + : l_yes);
> +
> + return false;
> +l_yes:
> + return true;
> +}
> +
> +static __always_inline bool riscv_has_extension_unlikely(const unsigned long ext)
> +{
> + compiletime_assert(ext < RISCV_ISA_EXT_MAX, "ext must be < RISCV_ISA_EXT_MAX");
> +
> + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE))
> + return __riscv_has_extension_unlikely(STANDARD_EXT, ext);
> +
> + return __riscv_isa_extension_available(NULL, ext);
> +}
> +
> +static __always_inline bool riscv_has_extension_likely(const unsigned long ext)
> +{
> + compiletime_assert(ext < RISCV_ISA_EXT_MAX, "ext must be < RISCV_ISA_EXT_MAX");
> +
> + if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE))
> + return __riscv_has_extension_likely(STANDARD_EXT, ext);
> +
> + return __riscv_isa_extension_available(NULL, ext);
> +}
> +
> +#endif
nit: Add the /* _ASM_CPUFEATURE_MACROS_H */ here.
> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> index 45f9c1171a48..c991672bb401 100644
> --- a/arch/riscv/include/asm/cpufeature.h
> +++ b/arch/riscv/include/asm/cpufeature.h
> @@ -11,6 +11,7 @@
> #include <asm/hwcap.h>
> #include <asm/alternative-macros.h>
I think asm/alternative-macros.h can be dropped now.
> #include <asm/errno.h>
> +#include <asm/cpufeature-macros.h>
>
> /*
> * These are probed via a device_initcall(), via either the SBI or directly
> @@ -103,61 +104,6 @@ extern const size_t riscv_isa_ext_count;
> extern bool riscv_isa_fallback;
>
> unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
> -
> -#define STANDARD_EXT 0
> -
> -bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, unsigned int bit);
> -#define riscv_isa_extension_available(isa_bitmap, ext) \
> - __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
> -
> -static __always_inline bool __riscv_has_extension_likely(const unsigned long vendor,
> - const unsigned long ext)
> -{
> - asm goto(ALTERNATIVE("j %l[l_no]", "nop", %[vendor], %[ext], 1)
> - :
> - : [vendor] "i" (vendor), [ext] "i" (ext)
> - :
> - : l_no);
> -
> - return true;
> -l_no:
> - return false;
> -}
> -
> -static __always_inline bool __riscv_has_extension_unlikely(const unsigned long vendor,
> - const unsigned long ext)
> -{
> - asm goto(ALTERNATIVE("nop", "j %l[l_yes]", %[vendor], %[ext], 1)
> - :
> - : [vendor] "i" (vendor), [ext] "i" (ext)
> - :
> - : l_yes);
> -
> - return false;
> -l_yes:
> - return true;
> -}
> -
> -static __always_inline bool riscv_has_extension_unlikely(const unsigned long ext)
> -{
> - compiletime_assert(ext < RISCV_ISA_EXT_MAX, "ext must be < RISCV_ISA_EXT_MAX");
> -
> - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE))
> - return __riscv_has_extension_unlikely(STANDARD_EXT, ext);
> -
> - return __riscv_isa_extension_available(NULL, ext);
> -}
> -
> -static __always_inline bool riscv_has_extension_likely(const unsigned long ext)
> -{
> - compiletime_assert(ext < RISCV_ISA_EXT_MAX, "ext must be < RISCV_ISA_EXT_MAX");
> -
> - if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE))
> - return __riscv_has_extension_likely(STANDARD_EXT, ext);
> -
> - return __riscv_isa_extension_available(NULL, ext);
> -}
> -
> static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext)
> {
> compiletime_assert(ext < RISCV_ISA_EXT_MAX, "ext must be < RISCV_ISA_EXT_MAX");
> --
> 2.39.2
>
>
Otherwise,
Reviewed-by: Andrew Jones <ajones@xxxxxxxxxxxxxxxx>
Thanks,
drew
