* [PATCH 0/2] arm64: Simplify gas LSE support detection
@ 2020-01-09 17:49 Catalin Marinas
2020-01-09 17:49 ` [PATCH 1/2] kbuild: Add support for 'as-instr' to be used in Kconfig files Catalin Marinas
2020-01-09 17:49 ` [PATCH 2/2] arm64: Move the LSE gas support detection to Kconfig Catalin Marinas
0 siblings, 2 replies; 9+ messages in thread
From: Catalin Marinas @ 2020-01-09 17:49 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Will Deacon
The first patch was developed as part the of the MTE support [1] and
acked by the kbuild maintainers [2].
The second patch removes the Makefile logic to define CONFIG_AS_LSE
since the $(as-instr) check can now be handled in Kconfig.
[1] https://lore.kernel.org/linux-arm-kernel/20191211184027.20130-3-catalin.marinas@arm.com/
[2] https://lore.kernel.org/linux-arm-kernel/CAK7LNARR=DjdnZdu=L+0H8ALr4XJNpVbcRTOz_sVZdZpcM0pdQ@mail.gmail.com/
Catalin Marinas (2):
kbuild: Add support for 'as-instr' to be used in Kconfig files
arm64: Move the LSE gas support detection to Kconfig
arch/arm64/Kconfig | 4 ++++
arch/arm64/Makefile | 13 ++-----------
arch/arm64/include/asm/atomic_ll_sc.h | 2 +-
arch/arm64/include/asm/lse.h | 6 +++---
arch/arm64/kernel/cpufeature.c | 4 ++--
scripts/Kconfig.include | 4 ++++
6 files changed, 16 insertions(+), 17 deletions(-)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] kbuild: Add support for 'as-instr' to be used in Kconfig files
2020-01-09 17:49 [PATCH 0/2] arm64: Simplify gas LSE support detection Catalin Marinas
@ 2020-01-09 17:49 ` Catalin Marinas
2020-01-09 17:49 ` [PATCH 2/2] arm64: Move the LSE gas support detection to Kconfig Catalin Marinas
1 sibling, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2020-01-09 17:49 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Will Deacon
Similar to 'cc-option' or 'ld-option', it is occasionally necessary to
check whether the assembler supports certain ISA extensions. In the
arm64 code we currently do this in Makefile with an additional define:
lseinstr := $(call as-instr,.arch_extension lse,-DCONFIG_AS_LSE=1)
Add the 'as-instr' option so that it can be used in Kconfig directly:
def_bool $(as-instr,.arch_extension lse)
Acked-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
scripts/Kconfig.include | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
index d4adfbe42690..9d07e59cbdf7 100644
--- a/scripts/Kconfig.include
+++ b/scripts/Kconfig.include
@@ -31,6 +31,10 @@ cc-option = $(success,$(CC) -Werror $(CLANG_FLAGS) $(1) -E -x c /dev/null -o /de
# Return y if the linker supports <flag>, n otherwise
ld-option = $(success,$(LD) -v $(1))
+# $(as-instr,<instr>)
+# Return y if the assembler supports <instr>, n otherwise
+as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -c -x assembler -o /dev/null -)
+
# check if $(CC) and $(LD) exist
$(error-if,$(failure,command -v $(CC)),compiler '$(CC)' not found)
$(error-if,$(failure,command -v $(LD)),linker '$(LD)' not found)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] arm64: Move the LSE gas support detection to Kconfig
2020-01-09 17:49 [PATCH 0/2] arm64: Simplify gas LSE support detection Catalin Marinas
2020-01-09 17:49 ` [PATCH 1/2] kbuild: Add support for 'as-instr' to be used in Kconfig files Catalin Marinas
@ 2020-01-09 17:49 ` Catalin Marinas
2020-01-10 11:54 ` Vladimir Murzin
1 sibling, 1 reply; 9+ messages in thread
From: Catalin Marinas @ 2020-01-09 17:49 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Will Deacon
As the Kconfig syntax gained support for $(as-instr) tests, move the LSE
gas support detection from Makefile to the main arm64 Kconfig and remove
the additional CONFIG_AS_LSE definition and check.
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/Kconfig | 4 ++++
arch/arm64/Makefile | 13 ++-----------
arch/arm64/include/asm/atomic_ll_sc.h | 2 +-
arch/arm64/include/asm/lse.h | 6 +++---
arch/arm64/kernel/cpufeature.c | 4 ++--
5 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index b1b4476ddb83..2a0521f0f156 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1362,8 +1362,12 @@ config ARM64_PAN
The feature is detected at runtime, and will remain as a 'nop'
instruction if the cpu does not implement the feature.
+config ARM64_AS_HAS_LSE
+ def_bool $(as-instr,.arch_extension lse)
+
config ARM64_LSE_ATOMICS
bool "Atomic instructions"
+ depends on ARM64_AS_HAS_LSE
depends on JUMP_LABEL
default y
help
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 1fbe24d4fdb6..cca6de192d42 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -30,15 +30,6 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419
endif
endif
-# Check for binutils support for specific extensions
-lseinstr := $(call as-instr,.arch_extension lse,-DCONFIG_AS_LSE=1)
-
-ifeq ($(CONFIG_ARM64_LSE_ATOMICS), y)
- ifeq ($(lseinstr),)
-$(warning LSE atomics not supported by binutils)
- endif
-endif
-
cc_has_k_constraint := $(call try-run,echo \
'int main(void) { \
asm volatile("and w0, w0, %w0" :: "K" (4294967295)); \
@@ -53,11 +44,11 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable)
endif
endif
-KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) \
+KBUILD_CFLAGS += -mgeneral-regs-only $(brokengasinst) \
$(compat_vdso) $(cc_has_k_constraint)
KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
KBUILD_CFLAGS += $(call cc-disable-warning, psabi)
-KBUILD_AFLAGS += $(lseinstr) $(brokengasinst) $(compat_vdso)
+KBUILD_AFLAGS += $(brokengasinst) $(compat_vdso)
KBUILD_CFLAGS += $(call cc-option,-mabi=lp64)
KBUILD_AFLAGS += $(call cc-option,-mabi=lp64)
diff --git a/arch/arm64/include/asm/atomic_ll_sc.h b/arch/arm64/include/asm/atomic_ll_sc.h
index 7b012148bfd6..13869b76b58c 100644
--- a/arch/arm64/include/asm/atomic_ll_sc.h
+++ b/arch/arm64/include/asm/atomic_ll_sc.h
@@ -12,7 +12,7 @@
#include <linux/stringify.h>
-#if IS_ENABLED(CONFIG_ARM64_LSE_ATOMICS) && IS_ENABLED(CONFIG_AS_LSE)
+#ifdef CONFIG_ARM64_LSE_ATOMICS
#define __LL_SC_FALLBACK(asm_ops) \
" b 3f\n" \
" .subsection 1\n" \
diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h
index 80b388278149..4e1009fff686 100644
--- a/arch/arm64/include/asm/lse.h
+++ b/arch/arm64/include/asm/lse.h
@@ -4,7 +4,7 @@
#include <asm/atomic_ll_sc.h>
-#if defined(CONFIG_AS_LSE) && defined(CONFIG_ARM64_LSE_ATOMICS)
+#ifdef CONFIG_ARM64_LSE_ATOMICS
#include <linux/compiler_types.h>
#include <linux/export.h>
@@ -36,7 +36,7 @@ static inline bool system_uses_lse_atomics(void)
#define ARM64_LSE_ATOMIC_INSN(llsc, lse) \
ALTERNATIVE(llsc, lse, ARM64_HAS_LSE_ATOMICS)
-#else /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
+#else /* CONFIG_ARM64_LSE_ATOMICS */
static inline bool system_uses_lse_atomics(void) { return false; }
@@ -44,5 +44,5 @@ static inline bool system_uses_lse_atomics(void) { return false; }
#define ARM64_LSE_ATOMIC_INSN(llsc, lse) llsc
-#endif /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
+#endif /* CONFIG_ARM64_LSE_ATOMICS */
#endif /* __ASM_LSE_H */
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 04cf64e9f0c9..2595c2886d3f 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1291,7 +1291,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
.cpu_enable = cpu_enable_pan,
},
#endif /* CONFIG_ARM64_PAN */
-#if defined(CONFIG_AS_LSE) && defined(CONFIG_ARM64_LSE_ATOMICS)
+#ifdef CONFIG_ARM64_LSE_ATOMICS
{
.desc = "LSE atomic instructions",
.capability = ARM64_HAS_LSE_ATOMICS,
@@ -1302,7 +1302,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
.sign = FTR_UNSIGNED,
.min_field_value = 2,
},
-#endif /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
+#endif /* CONFIG_ARM64_LSE_ATOMICS */
{
.desc = "Software prefetching using PRFM",
.capability = ARM64_HAS_NO_HW_PREFETCH,
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] arm64: Move the LSE gas support detection to Kconfig
2020-01-09 17:49 ` [PATCH 2/2] arm64: Move the LSE gas support detection to Kconfig Catalin Marinas
@ 2020-01-10 11:54 ` Vladimir Murzin
2020-01-10 12:08 ` Will Deacon
0 siblings, 1 reply; 9+ messages in thread
From: Vladimir Murzin @ 2020-01-10 11:54 UTC (permalink / raw)
To: Catalin Marinas, linux-arm-kernel; +Cc: Will Deacon
On 1/9/20 5:49 PM, Catalin Marinas wrote:
> As the Kconfig syntax gained support for $(as-instr) tests, move the LSE
> gas support detection from Makefile to the main arm64 Kconfig and remove
> the additional CONFIG_AS_LSE definition and check.
>
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
> arch/arm64/Kconfig | 4 ++++
> arch/arm64/Makefile | 13 ++-----------
> arch/arm64/include/asm/atomic_ll_sc.h | 2 +-
> arch/arm64/include/asm/lse.h | 6 +++---
> arch/arm64/kernel/cpufeature.c | 4 ++--
> 5 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index b1b4476ddb83..2a0521f0f156 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1362,8 +1362,12 @@ config ARM64_PAN
> The feature is detected at runtime, and will remain as a 'nop'
> instruction if the cpu does not implement the feature.
>
> +config ARM64_AS_HAS_LSE
> + def_bool $(as-instr,.arch_extension lse)
> +
> config ARM64_LSE_ATOMICS
> bool "Atomic instructions"
> + depends on ARM64_AS_HAS_LSE
> depends on JUMP_LABEL
> default y
> help
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 1fbe24d4fdb6..cca6de192d42 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -30,15 +30,6 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419
> endif
> endif
>
> -# Check for binutils support for specific extensions
> -lseinstr := $(call as-instr,.arch_extension lse,-DCONFIG_AS_LSE=1)
> -
> -ifeq ($(CONFIG_ARM64_LSE_ATOMICS), y)
> - ifeq ($(lseinstr),)
> -$(warning LSE atomics not supported by binutils)
> - endif
> -endif
> -
> cc_has_k_constraint := $(call try-run,echo \
> 'int main(void) { \
> asm volatile("and w0, w0, %w0" :: "K" (4294967295)); \
> @@ -53,11 +44,11 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable)
> endif
> endif
>
> -KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) \
> +KBUILD_CFLAGS += -mgeneral-regs-only $(brokengasinst) \
> $(compat_vdso) $(cc_has_k_constraint)
> KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> KBUILD_CFLAGS += $(call cc-disable-warning, psabi)
> -KBUILD_AFLAGS += $(lseinstr) $(brokengasinst) $(compat_vdso)
> +KBUILD_AFLAGS += $(brokengasinst) $(compat_vdso)
>
> KBUILD_CFLAGS += $(call cc-option,-mabi=lp64)
> KBUILD_AFLAGS += $(call cc-option,-mabi=lp64)
> diff --git a/arch/arm64/include/asm/atomic_ll_sc.h b/arch/arm64/include/asm/atomic_ll_sc.h
> index 7b012148bfd6..13869b76b58c 100644
> --- a/arch/arm64/include/asm/atomic_ll_sc.h
> +++ b/arch/arm64/include/asm/atomic_ll_sc.h
> @@ -12,7 +12,7 @@
>
> #include <linux/stringify.h>
>
> -#if IS_ENABLED(CONFIG_ARM64_LSE_ATOMICS) && IS_ENABLED(CONFIG_AS_LSE)
> +#ifdef CONFIG_ARM64_LSE_ATOMICS
> #define __LL_SC_FALLBACK(asm_ops) \
> " b 3f\n" \
> " .subsection 1\n" \
> diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h
> index 80b388278149..4e1009fff686 100644
> --- a/arch/arm64/include/asm/lse.h
> +++ b/arch/arm64/include/asm/lse.h
> @@ -4,7 +4,7 @@
>
> #include <asm/atomic_ll_sc.h>
>
> -#if defined(CONFIG_AS_LSE) && defined(CONFIG_ARM64_LSE_ATOMICS)
> +#ifdef CONFIG_ARM64_LSE_ATOMICS
>
> #include <linux/compiler_types.h>
> #include <linux/export.h>
> @@ -36,7 +36,7 @@ static inline bool system_uses_lse_atomics(void)
> #define ARM64_LSE_ATOMIC_INSN(llsc, lse) \
> ALTERNATIVE(llsc, lse, ARM64_HAS_LSE_ATOMICS)
>
> -#else /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
> +#else /* CONFIG_ARM64_LSE_ATOMICS */
>
> static inline bool system_uses_lse_atomics(void) { return false; }
>
> @@ -44,5 +44,5 @@ static inline bool system_uses_lse_atomics(void) { return false; }
>
> #define ARM64_LSE_ATOMIC_INSN(llsc, lse) llsc
>
> -#endif /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
> +#endif /* CONFIG_ARM64_LSE_ATOMICS */
> #endif /* __ASM_LSE_H */
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 04cf64e9f0c9..2595c2886d3f 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1291,7 +1291,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> .cpu_enable = cpu_enable_pan,
> },
> #endif /* CONFIG_ARM64_PAN */
> -#if defined(CONFIG_AS_LSE) && defined(CONFIG_ARM64_LSE_ATOMICS)
> +#ifdef CONFIG_ARM64_LSE_ATOMICS
> {
> .desc = "LSE atomic instructions",
> .capability = ARM64_HAS_LSE_ATOMICS,
> @@ -1302,7 +1302,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> .sign = FTR_UNSIGNED,
> .min_field_value = 2,
> },
> -#endif /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
> +#endif /* CONFIG_ARM64_LSE_ATOMICS */
> {
> .desc = "Software prefetching using PRFM",
> .capability = ARM64_HAS_NO_HW_PREFETCH,
>
I was not lucky with the similar patch [1], anyway
Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>
[1] https://www.spinics.net/lists/linux-crypto/msg36059.html
Cheers
Vladimir
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] arm64: Move the LSE gas support detection to Kconfig
2020-01-10 11:54 ` Vladimir Murzin
@ 2020-01-10 12:08 ` Will Deacon
2020-01-10 15:30 ` Catalin Marinas
0 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2020-01-10 12:08 UTC (permalink / raw)
To: Vladimir Murzin; +Cc: Catalin Marinas, linux-arm-kernel
On Fri, Jan 10, 2020 at 11:54:38AM +0000, Vladimir Murzin wrote:
> On 1/9/20 5:49 PM, Catalin Marinas wrote:
> > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > index 04cf64e9f0c9..2595c2886d3f 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -1291,7 +1291,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> > .cpu_enable = cpu_enable_pan,
> > },
> > #endif /* CONFIG_ARM64_PAN */
> > -#if defined(CONFIG_AS_LSE) && defined(CONFIG_ARM64_LSE_ATOMICS)
> > +#ifdef CONFIG_ARM64_LSE_ATOMICS
> > {
> > .desc = "LSE atomic instructions",
> > .capability = ARM64_HAS_LSE_ATOMICS,
> > @@ -1302,7 +1302,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> > .sign = FTR_UNSIGNED,
> > .min_field_value = 2,
> > },
> > -#endif /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
> > +#endif /* CONFIG_ARM64_LSE_ATOMICS */
> > {
> > .desc = "Software prefetching using PRFM",
> > .capability = ARM64_HAS_NO_HW_PREFETCH,
> >
>
> I was not lucky with the similar patch [1], anyway
>
> Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>
>
>
> [1] https://www.spinics.net/lists/linux-crypto/msg36059.html
It's the loss of the warning that I object to, since I think it's a useful
diagnostic to have. Is there some way we can keep that, but using the new
kbuild logic?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] arm64: Move the LSE gas support detection to Kconfig
2020-01-10 12:08 ` Will Deacon
@ 2020-01-10 15:30 ` Catalin Marinas
2020-01-10 15:45 ` Will Deacon
0 siblings, 1 reply; 9+ messages in thread
From: Catalin Marinas @ 2020-01-10 15:30 UTC (permalink / raw)
To: Will Deacon; +Cc: Vladimir Murzin, linux-arm-kernel
On Fri, Jan 10, 2020 at 12:08:26PM +0000, Will Deacon wrote:
> On Fri, Jan 10, 2020 at 11:54:38AM +0000, Vladimir Murzin wrote:
> > On 1/9/20 5:49 PM, Catalin Marinas wrote:
> > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > > index 04cf64e9f0c9..2595c2886d3f 100644
> > > --- a/arch/arm64/kernel/cpufeature.c
> > > +++ b/arch/arm64/kernel/cpufeature.c
> > > @@ -1291,7 +1291,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> > > .cpu_enable = cpu_enable_pan,
> > > },
> > > #endif /* CONFIG_ARM64_PAN */
> > > -#if defined(CONFIG_AS_LSE) && defined(CONFIG_ARM64_LSE_ATOMICS)
> > > +#ifdef CONFIG_ARM64_LSE_ATOMICS
> > > {
> > > .desc = "LSE atomic instructions",
> > > .capability = ARM64_HAS_LSE_ATOMICS,
> > > @@ -1302,7 +1302,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> > > .sign = FTR_UNSIGNED,
> > > .min_field_value = 2,
> > > },
> > > -#endif /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
> > > +#endif /* CONFIG_ARM64_LSE_ATOMICS */
> > > {
> > > .desc = "Software prefetching using PRFM",
> > > .capability = ARM64_HAS_NO_HW_PREFETCH,
> > >
> >
> > I was not lucky with the similar patch [1], anyway
> >
> > Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>
> >
> >
> > [1] https://www.spinics.net/lists/linux-crypto/msg36059.html
>
> It's the loss of the warning that I object to, since I think it's a useful
> diagnostic to have. Is there some way we can keep that, but using the new
> kbuild logic?
With the kbuild logic, you can't select CONFIG_ARM64_LSE_ATOMICS because
CONFIG_AS_HAS_LSE is n (when gas doesn't support LSE). I consider this a
good indication to the user trying to enable it without the need for a
warning.
The alternative is to let the user state their preference with a config
option without any dependencies:
config ARM64_WANT_LSE_ATOMICS
bool "..."
depends on JUMP_LABEL
default y
config ARM64_LSE_ATOMICS
def_bool CONFIG_ARM64_WANT_LSE_ATOMICS
depends on CONFIG_AS_HAS_LSE
and in the Makefile, warn if CONFIG_ARM64_WANT_LSE_ATOMICS &&
!CONFIG_ARM64_LSE_ATOMICS.
You can even get the warning directly from kbuild if you select
ARM64_LSE_ATOMICS from ARM64_WANT_* (unmet dependency).
I personally don't think it's worth the hassle. We don't warn if the
compiler doesn't support jump labels, why would we do this for LSE.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] arm64: Move the LSE gas support detection to Kconfig
2020-01-10 15:30 ` Catalin Marinas
@ 2020-01-10 15:45 ` Will Deacon
2020-01-13 11:36 ` Catalin Marinas
0 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2020-01-10 15:45 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Vladimir Murzin, linux-arm-kernel
On Fri, Jan 10, 2020 at 03:30:14PM +0000, Catalin Marinas wrote:
> On Fri, Jan 10, 2020 at 12:08:26PM +0000, Will Deacon wrote:
> > On Fri, Jan 10, 2020 at 11:54:38AM +0000, Vladimir Murzin wrote:
> > > On 1/9/20 5:49 PM, Catalin Marinas wrote:
> > > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > > > index 04cf64e9f0c9..2595c2886d3f 100644
> > > > --- a/arch/arm64/kernel/cpufeature.c
> > > > +++ b/arch/arm64/kernel/cpufeature.c
> > > > @@ -1291,7 +1291,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> > > > .cpu_enable = cpu_enable_pan,
> > > > },
> > > > #endif /* CONFIG_ARM64_PAN */
> > > > -#if defined(CONFIG_AS_LSE) && defined(CONFIG_ARM64_LSE_ATOMICS)
> > > > +#ifdef CONFIG_ARM64_LSE_ATOMICS
> > > > {
> > > > .desc = "LSE atomic instructions",
> > > > .capability = ARM64_HAS_LSE_ATOMICS,
> > > > @@ -1302,7 +1302,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> > > > .sign = FTR_UNSIGNED,
> > > > .min_field_value = 2,
> > > > },
> > > > -#endif /* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
> > > > +#endif /* CONFIG_ARM64_LSE_ATOMICS */
> > > > {
> > > > .desc = "Software prefetching using PRFM",
> > > > .capability = ARM64_HAS_NO_HW_PREFETCH,
> > > >
> > >
> > > I was not lucky with the similar patch [1], anyway
> > >
> > > Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>
> > >
> > >
> > > [1] https://www.spinics.net/lists/linux-crypto/msg36059.html
> >
> > It's the loss of the warning that I object to, since I think it's a useful
> > diagnostic to have. Is there some way we can keep that, but using the new
> > kbuild logic?
>
> With the kbuild logic, you can't select CONFIG_ARM64_LSE_ATOMICS because
> CONFIG_AS_HAS_LSE is n (when gas doesn't support LSE). I consider this a
> good indication to the user trying to enable it without the need for a
> warning.
>
> The alternative is to let the user state their preference with a config
> option without any dependencies:
>
> config ARM64_WANT_LSE_ATOMICS
> bool "..."
> depends on JUMP_LABEL
> default y
>
> config ARM64_LSE_ATOMICS
> def_bool CONFIG_ARM64_WANT_LSE_ATOMICS
> depends on CONFIG_AS_HAS_LSE
>
> and in the Makefile, warn if CONFIG_ARM64_WANT_LSE_ATOMICS &&
> !CONFIG_ARM64_LSE_ATOMICS.
>
> You can even get the warning directly from kbuild if you select
> ARM64_LSE_ATOMICS from ARM64_WANT_* (unmet dependency).
>
> I personally don't think it's worth the hassle. We don't warn if the
> compiler doesn't support jump labels, why would we do this for LSE.
The thing I dislike about it is that if somebody sends you a .config with
LSE enabled, and your compiler doesn't support it, then it silently get
disabled. There are two differences with jump labels:
1. Most compilers support jump labels
2. LSE atomics also depend on the CPU features before they get enabled
at runtime.
Mainly because of (2), I think silently disabling LSE at build time is
really confusing when you're trying to figure out why it's not in use.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] arm64: Move the LSE gas support detection to Kconfig
2020-01-10 15:45 ` Will Deacon
@ 2020-01-13 11:36 ` Catalin Marinas
2020-01-14 17:19 ` Will Deacon
0 siblings, 1 reply; 9+ messages in thread
From: Catalin Marinas @ 2020-01-13 11:36 UTC (permalink / raw)
To: Will Deacon; +Cc: Vladimir Murzin, linux-arm-kernel
On Fri, Jan 10, 2020 at 03:45:43PM +0000, Will Deacon wrote:
> On Fri, Jan 10, 2020 at 03:30:14PM +0000, Catalin Marinas wrote:
> > On Fri, Jan 10, 2020 at 12:08:26PM +0000, Will Deacon wrote:
> > > On Fri, Jan 10, 2020 at 11:54:38AM +0000, Vladimir Murzin wrote:
> > > > I was not lucky with the similar patch [1], anyway
> > > >
> > > > Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>
> > > >
> > > > [1] https://www.spinics.net/lists/linux-crypto/msg36059.html
> > >
> > > It's the loss of the warning that I object to, since I think it's a useful
> > > diagnostic to have. Is there some way we can keep that, but using the new
> > > kbuild logic?
[...]
> The thing I dislike about it is that if somebody sends you a .config with
> LSE enabled, and your compiler doesn't support it, then it silently get
> disabled.
I was thinking the other way. Someone sending me a .config file and I
can't tell whether LSE was built into their kernel or not unless they
also send the build log (I haven't seen anyone do this). At least for my
local compiler, I have some control over it and can compare the
resulting .config file. It would have been nice if oldconfig warned of
features being disabled but I guess it would become too noisy.
To accommodate the two scenarios, what about this diff on top of patch 2
(I can fold it in in v2 and feel free to suggest better config names):
-------8<-------------------------
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 2a0521f0f156..cf3b6d2a67cf 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1362,12 +1362,13 @@ config ARM64_PAN
The feature is detected at runtime, and will remain as a 'nop'
instruction if the cpu does not implement the feature.
-config ARM64_AS_HAS_LSE
- def_bool $(as-instr,.arch_extension lse)
-
config ARM64_LSE_ATOMICS
+ bool
+ default ARM64_USE_LSE_ATOMICS
+ depends on $(as-instr,.arch_extension lse)
+
+config ARM64_USE_LSE_ATOMICS
bool "Atomic instructions"
- depends on ARM64_AS_HAS_LSE
depends on JUMP_LABEL
default y
help
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index cca6de192d42..6dd8ecacc428 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -30,6 +30,12 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419
endif
endif
+ifeq ($(CONFIG_ARM64_USE_LSE_ATOMICS), y)
+ ifneq ($(CONFIG_ARM64_LSE_ATOMICS), y)
+$(warning LSE atomics not supported by binutils)
+ endif
+endif
+
cc_has_k_constraint := $(call try-run,echo \
'int main(void) { \
asm volatile("and w0, w0, %w0" :: "K" (4294967295)); \
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] arm64: Move the LSE gas support detection to Kconfig
2020-01-13 11:36 ` Catalin Marinas
@ 2020-01-14 17:19 ` Will Deacon
0 siblings, 0 replies; 9+ messages in thread
From: Will Deacon @ 2020-01-14 17:19 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Vladimir Murzin, linux-arm-kernel
On Mon, Jan 13, 2020 at 11:36:14AM +0000, Catalin Marinas wrote:
> On Fri, Jan 10, 2020 at 03:45:43PM +0000, Will Deacon wrote:
> > On Fri, Jan 10, 2020 at 03:30:14PM +0000, Catalin Marinas wrote:
> > > On Fri, Jan 10, 2020 at 12:08:26PM +0000, Will Deacon wrote:
> > > > On Fri, Jan 10, 2020 at 11:54:38AM +0000, Vladimir Murzin wrote:
> > > > > I was not lucky with the similar patch [1], anyway
> > > > >
> > > > > Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>
> > > > >
> > > > > [1] https://www.spinics.net/lists/linux-crypto/msg36059.html
> > > >
> > > > It's the loss of the warning that I object to, since I think it's a useful
> > > > diagnostic to have. Is there some way we can keep that, but using the new
> > > > kbuild logic?
> [...]
> > The thing I dislike about it is that if somebody sends you a .config with
> > LSE enabled, and your compiler doesn't support it, then it silently get
> > disabled.
>
> I was thinking the other way. Someone sending me a .config file and I
> can't tell whether LSE was built into their kernel or not unless they
> also send the build log (I haven't seen anyone do this). At least for my
> local compiler, I have some control over it and can compare the
> resulting .config file. It would have been nice if oldconfig warned of
> features being disabled but I guess it would become too noisy.
>
> To accommodate the two scenarios, what about this diff on top of patch 2
> (I can fold it in in v2 and feel free to suggest better config names):
>
> -------8<-------------------------
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 2a0521f0f156..cf3b6d2a67cf 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1362,12 +1362,13 @@ config ARM64_PAN
> The feature is detected at runtime, and will remain as a 'nop'
> instruction if the cpu does not implement the feature.
>
> -config ARM64_AS_HAS_LSE
> - def_bool $(as-instr,.arch_extension lse)
> -
> config ARM64_LSE_ATOMICS
> + bool
> + default ARM64_USE_LSE_ATOMICS
> + depends on $(as-instr,.arch_extension lse)
> +
> +config ARM64_USE_LSE_ATOMICS
> bool "Atomic instructions"
> - depends on ARM64_AS_HAS_LSE
> depends on JUMP_LABEL
> default y
> help
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index cca6de192d42..6dd8ecacc428 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -30,6 +30,12 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419
> endif
> endif
>
> +ifeq ($(CONFIG_ARM64_USE_LSE_ATOMICS), y)
> + ifneq ($(CONFIG_ARM64_LSE_ATOMICS), y)
> +$(warning LSE atomics not supported by binutils)
> + endif
> +endif
> +
This looks good to me; I'll apply v2 when you post it.
Cheers,
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-01-14 17:19 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-09 17:49 [PATCH 0/2] arm64: Simplify gas LSE support detection Catalin Marinas
2020-01-09 17:49 ` [PATCH 1/2] kbuild: Add support for 'as-instr' to be used in Kconfig files Catalin Marinas
2020-01-09 17:49 ` [PATCH 2/2] arm64: Move the LSE gas support detection to Kconfig Catalin Marinas
2020-01-10 11:54 ` Vladimir Murzin
2020-01-10 12:08 ` Will Deacon
2020-01-10 15:30 ` Catalin Marinas
2020-01-10 15:45 ` Will Deacon
2020-01-13 11:36 ` Catalin Marinas
2020-01-14 17:19 ` Will Deacon
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.