All of lore.kernel.org
 help / color / mirror / Atom feed
* [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	[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	[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	[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.