Minor improvements over handling dependency on GAS

RFC PATCH New Vladimir Murzin 2 participants 7 messages Nov 7, 2018

Subsystems (6)

Arm (32-bit) Kbuild Crypto API arm64 +2 more subsystems

lore

Share & tools

lore

Vladimir Murzin

author

2018-11-07 09:40 UTC #0 lore raw

cover Minor improvements over handling dependency on GAS

With recent changes in Kconfig processing it is now possible to expose dependency on specific tools and supported options via Kconfig rather than bury it deep in Makefile.

This small series try to address the case where the whole feature, for instance arm64/lse or arm/crypto, depends on GAS.

Vladimir Murzin (4):
kconfig: add as-instr macro to scripts/Kconfig.include
arm64: lse: expose dependency on gas via Kconfig
arm64: turn "broken gas inst" into real config option
ARM: crypto: expose dependency on gas via Kconfig

 arch/arm/crypto/Kconfig         | 31 +++++++++++++++++++++----------
 arch/arm/crypto/Makefile        | 31 ++++++-------------------------
 arch/arm64/Kconfig              |  4 ++++
 arch/arm64/Makefile             | 18 ++----------------
 arch/arm64/include/asm/atomic.h |  2 +-
 arch/arm64/include/asm/lse.h    |  6 +++---
 arch/arm64/kernel/cpufeature.c  |  4 ++--
 scripts/Kconfig.include         |  4 ++++
 8 files changed, 43 insertions(+), 57 deletions(-)

Signature · 1 line

1.9.1

Vladimir Murzin

author

2018-11-07 09:40 UTC #1 lore raw

RFC PATCH 1/4 kconfig: add as-instr macro to scripts/Kconfig.include

There are cases where the whole feature, for instance arm64/lse or arm/crypto, can depend on assembler. Current practice is to report buildtime that selected feature is not supported, which can be quite annoying...

It'd nicer if we can check assembler first and opt-in feature visibility in Kconfig.

Cc Masahiro Yamada <[email protected]>

Cc Will Deacon <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Ard Biesheuvel <[email protected]>

Signed-off-by Vladimir Murzin <[email protected]>

---
 scripts/Kconfig.include | 4 ++++
 1 file changed, 4 insertions(+)

scripts/Kconfig.include +4 -0

index dad5583..07c145c 100644
@@ -22,6 +22,10 @@ success = $(if-success,$(1),y,n)
 # Return y if the compiler supports <flag>, n otherwise
 cc-option = $(success,$(CC) -Werror $(1) -E -x c /dev/null -o /dev/null)
 
+# $(as-instr,<instr>)
+# Return y if the assembler supports <instr>, n otherwise
+as-instr = $(success,printf "%b\n" "$(1)" | $(CC) -Werror -c -x assembler -o /dev/null -)
+
 # $(ld-option,<flag>)
 # Return y if the linker supports <flag>, n otherwise
 ld-option = $(success,$(LD) -v $(1))

Signature · 1 line

1.9.1

Will Deacon

2018-11-07 14:55 UTC #2 lore raw

RFC PATCH 1/4 kconfig: add as-instr macro to scripts/Kconfig.include

Quoted 4 lines · Vladimir Murzin

There are cases where the whole feature, for instance arm64/lse or arm/crypto, can depend on assembler. Current practice is to report buildtime that selected feature is not supported, which can be quite annoying...

Why is it annoying? You still end up with a working kernel.

Quoted 11 lines

It'd nicer if we can check assembler first and opt-in feature visibility in Kconfig.
Cc Masahiro Yamada <[email protected]>
Cc Will Deacon <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Signed-off-by Vladimir Murzin <[email protected]>
---
 scripts/Kconfig.include | 4 ++++
 1 file changed, 4 insertions(+)

One issue I have with doing the check like this is that if somebody sends you a .config with e.g. ARM64_LSE_ATOMICS=y and you try to build a kernel using that .config and an old toolchain, the option is silently dropped.

I think the diagnostic is actually useful in this case.

Will

Vladimir Murzin

author

2018-11-07 16:50 UTC #3 lore raw

RFC PATCH 1/4 kconfig: add as-instr macro to scripts/Kconfig.include

Quoted 7 lines · Will Deacon

Quoted 4 lines · Vladimir Murzin
There are cases where the whole feature, for instance arm64/lse or arm/crypto, can depend on assembler. Current practice is to report buildtime that selected feature is not supported, which can be quite annoying...
Why is it annoying? You still end up with a working kernel.

.config doesn't really represent if option was built or not, annoying part is digging build logs (if anyone's saved them at all!) or relevant parts of dmesg (if option throws anything in there and which not always part of reports).

Quoted 13 lines

Quoted 4 lines
It'd nicer if we can check assembler first and opt-in feature visibility in Kconfig.
Quoted 8 lines
Cc Masahiro Yamada <[email protected]>
Cc Will Deacon <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Ard Biesheuvel <[email protected]>
Signed-off-by Vladimir Murzin <[email protected]>
---
 scripts/Kconfig.include | 4 ++++
 1 file changed, 4 insertions(+)

Quoted 3 lines

One issue I have with doing the check like this is that if somebody sends you a .config with e.g. ARM64_LSE_ATOMICS=y and you try to build a kernel using that .config and an old toolchain, the option is silently dropped.

I see... at least we have some tools like ./scripts/diffconfig

Quoted 2 lines

I think the diagnostic is actually useful in this case.

Fully agree on diagnostic side, any suggestions how it can be improved?

Cheers
Vladimir

Quoted 3 lines

Will

Vladimir Murzin

author

2018-11-07 09:40 UTC #4 lore raw

RFC PATCH 2/4 arm64: lse: expose dependency on gas via Kconfig

In reply to Vladimir Murzin

So we can simply hide LSE support if dependency is not satisfied.

Cc Will Deacon <[email protected]>

Signed-off-by Vladimir Murzin <[email protected]>

---
 arch/arm64/Kconfig              |  1 +
 arch/arm64/Makefile             | 13 ++-----------
 arch/arm64/include/asm/atomic.h |  2 +-
 arch/arm64/include/asm/lse.h    |  6 +++---
 arch/arm64/kernel/cpufeature.c  |  4 ++--
 5 files changed, 9 insertions(+), 17 deletions(-)

arch/arm64/Kconfig +1 -0

index 964f682..7978aee 100644
@@ -1072,6 +1072,7 @@ config ARM64_PAN
 config ARM64_LSE_ATOMICS
 	bool "Atomic instructions"
 	default y
+	depends on $(as-instr,.arch_extension lse)
 	help
 	  As part of the Large System Extensions, ARMv8.1 introduces new
 	  atomic instructions that are designed specifically to scale in

arch/arm64/Makefile +2 ~~-11~~

index b4e994c..3054757 100644
@@ -32,15 +32,6 @@ endif
 
 KBUILD_DEFCONFIG := defconfig
 
-# 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
-
 ifeq ($(CONFIG_ARM64), y)
 brokengasinst := $(call as-instr,1:\n.inst 0\n.rept . - 1b\n\nnop\n.endr\n,,-DCONFIG_BROKEN_GAS_INST=1)

@@ -49,9 +40,9 @@ $(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)
 KBUILD_CFLAGS	+= -fno-asynchronous-unwind-tables
-KBUILD_AFLAGS	+= $(lseinstr) $(brokengasinst)
+KBUILD_AFLAGS	+= $(brokengasinst)
 
 KBUILD_CFLAGS	+= $(call cc-option,-mabi=lp64)
 KBUILD_AFLAGS	+= $(call cc-option,-mabi=lp64)

arch/arm64/include/asm/atomic.h +1 -1

index 9bca54d..9d8d029 100644
@@ -30,7 +30,7 @@
 
 #define __ARM64_IN_ATOMIC_IMPL
 
-#if defined(CONFIG_ARM64_LSE_ATOMICS) && defined(CONFIG_AS_LSE)
+#ifdef CONFIG_ARM64_LSE_ATOMICS
 #include <asm/atomic_lse.h>
 #else
 #include <asm/atomic_ll_sc.h>

arch/arm64/include/asm/lse.h +3 -3

index 8262325..1fd31c7 100644
@@ -2,7 +2,7 @@
 #ifndef __ASM_LSE_H
 #define __ASM_LSE_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 @@
 	ALTERNATIVE(llsc, lse, ARM64_HAS_LSE_ATOMICS)
 
 #endif	/* __ASSEMBLER__ */
-#else	/* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
+#else	/* CONFIG_ARM64_LSE_ATOMICS */
 
 #ifdef __ASSEMBLER__

@@ -53,5 +53,5 @@
 #define ARM64_LSE_ATOMIC_INSN(llsc, lse)	llsc
 
 #endif	/* __ASSEMBLER__ */
-#endif	/* CONFIG_AS_LSE && CONFIG_ARM64_LSE_ATOMICS */
+#endif	/* CONFIG_ARM64_LSE_ATOMICS */
 #endif	/* __ASM_LSE_H */

arch/arm64/kernel/cpufeature.c +2 -2

index 74e9dcb..46f1bac 100644
@@ -1170,7 +1170,7 @@ static void cpu_clear_disr(const struct arm64_cpu_capabilities *__unused)
 		.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,

@@ -1181,7 +1181,7 @@ static void cpu_clear_disr(const struct arm64_cpu_capabilities *__unused)
 		.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,

Signature · 1 line

1.9.1

Vladimir Murzin

author

2018-11-07 09:40 UTC #5 lore raw

RFC PATCH 3/4 arm64: turn "broken gas inst" into real config option

In reply to Vladimir Murzin

So it is available everywhere and there is no need to keep CONFIG_ARM64 workaround ;)

Cc Marc Zyngier <[email protected]>

Signed-off-by Vladimir Murzin <[email protected]>

---
 arch/arm64/Kconfig  | 3 +++
 arch/arm64/Makefile | 9 ++-------
 2 files changed, 5 insertions(+), 7 deletions(-)

arch/arm64/Kconfig +3 -0

index 7978aee..86fc357 100644
@@ -287,6 +287,9 @@ config ARCH_SUPPORTS_UPROBES
 config ARCH_PROC_KCORE_TEXT
 	def_bool y
 
+config BROKEN_GAS_INST
+	def_bool y if !$(as-instr,1:\n.inst 0\n.rept . - 1b\n\nnop\n.endr\n)
+
 source "arch/arm64/Kconfig.platforms"
 
 menu "Bus support"

arch/arm64/Makefile +2 -7

index 3054757..9860d3a 100644
@@ -32,17 +32,12 @@ endif
 
 KBUILD_DEFCONFIG := defconfig
 
-ifeq ($(CONFIG_ARM64), y)
-brokengasinst := $(call as-instr,1:\n.inst 0\n.rept . - 1b\n\nnop\n.endr\n,,-DCONFIG_BROKEN_GAS_INST=1)
-
-  ifneq ($(brokengasinst),)
+ifeq ($(CONFIG_BROKEN_GAS_INST),y)
 $(warning Detected assembler with broken .inst; disassembly will be unreliable)
-  endif
 endif
 
-KBUILD_CFLAGS	+= -mgeneral-regs-only $(brokengasinst)
+KBUILD_CFLAGS	+= -mgeneral-regs-only
 KBUILD_CFLAGS	+= -fno-asynchronous-unwind-tables
-KBUILD_AFLAGS	+= $(brokengasinst)
 
 KBUILD_CFLAGS	+= $(call cc-option,-mabi=lp64)
 KBUILD_AFLAGS	+= $(call cc-option,-mabi=lp64)

Signature · 1 line

1.9.1

Vladimir Murzin

author

2018-11-07 09:40 UTC #6 lore raw

RFC PATCH 4/4 ARM: crypto: expose dependency on gas via Kconfig

In reply to Vladimir Murzin

So we can advertise only those entries which dependency is satisfied.

Cc Ard Biesheuvel <[email protected]>

Signed-off-by Vladimir Murzin <[email protected]>

---
 arch/arm/crypto/Kconfig  | 31 +++++++++++++++++++++----------
 arch/arm/crypto/Makefile | 31 ++++++-------------------------
 2 files changed, 27 insertions(+), 35 deletions(-)

arch/arm/crypto/Kconfig +21 ~~-10~~

index ef0c7fe..f437a91f 100644
@@ -9,6 +9,12 @@ menuconfig ARM_CRYPTO
 
 if ARM_CRYPTO
 
+config ARM_AS_HAS_CE
+	def_bool $(as-instr,.fpu crypto-neon-fp-armv8)
+
+config ARM_AS_HAS_CRC
+	def_bool $(as-instr,.arch armv8-a\n.arch_extension crc)
+
 config CRYPTO_SHA1_ARM
 	tristate "SHA1 digest algorithm (ARM-asm)"
 	select CRYPTO_SHA1

@@ -30,21 +36,21 @@ config CRYPTO_SHA1_ARM_NEON
 
 config CRYPTO_SHA1_ARM_CE
 	tristate "SHA1 digest algorithm (ARM v8 Crypto Extensions)"
-	depends on KERNEL_MODE_NEON
+	depends on KERNEL_MODE_NEON && ARM_AS_HAS_CE
 	select CRYPTO_SHA1_ARM
 	select CRYPTO_HASH
 	help
 	  SHA-1 secure hash standard (FIPS 180-1/DFIPS 180-2) implemented
-	  using special ARMv8 Crypto Extensions.
+	  using special ARMv8 Crypto Extensions (need binutils 2.23 or higher).
 
 config CRYPTO_SHA2_ARM_CE
 	tristate "SHA-224/256 digest algorithm (ARM v8 Crypto Extensions)"
-	depends on KERNEL_MODE_NEON
+	depends on KERNEL_MODE_NEON && ARM_AS_HAS_CE
 	select CRYPTO_SHA256_ARM
 	select CRYPTO_HASH
 	help
 	  SHA-256 secure hash standard (DFIPS 180-2) implemented
-	  using special ARMv8 Crypto Extensions.
+	  using special ARMv8 Crypto Extensions (need binutils 2.23 or higher).
 
 config CRYPTO_SHA256_ARM
 	tristate "SHA-224/256 digest algorithm (ARM-asm and NEON)"

@@ -87,16 +93,16 @@ config CRYPTO_AES_ARM_BS
 
 config CRYPTO_AES_ARM_CE
 	tristate "Accelerated AES using ARMv8 Crypto Extensions"
-	depends on KERNEL_MODE_NEON
+	depends on KERNEL_MODE_NEON && ARM_AS_HAS_CE
 	select CRYPTO_BLKCIPHER
 	select CRYPTO_SIMD
 	help
 	  Use an implementation of AES in CBC, CTR and XTS modes that uses
-	  ARMv8 Crypto Extensions
+	  ARMv8 Crypto Extensions (need binutils 2.23 or higher)
 
 config CRYPTO_GHASH_ARM_CE
 	tristate "PMULL-accelerated GHASH using NEON/ARMv8 Crypto Extensions"
-	depends on KERNEL_MODE_NEON
+	depends on KERNEL_MODE_NEON && ARM_AS_HAS_CE
 	select CRYPTO_HASH
 	select CRYPTO_CRYPTD
 	select CRYPTO_GF128MUL

@@ -104,17 +110,22 @@ config CRYPTO_GHASH_ARM_CE
 	  Use an implementation of GHASH (used by the GCM AEAD chaining mode)
 	  that uses the 64x64 to 128 bit polynomial multiplication (vmull.p64)
 	  that is part of the ARMv8 Crypto Extensions, or a slower variant that
-	  uses the vmull.p8 instruction that is part of the basic NEON ISA.
+	  uses the vmull.p8 instruction that is part of the basic NEON ISA (need
+	  binutils 2.23 or higher).
 
 config CRYPTO_CRCT10DIF_ARM_CE
 	tristate "CRCT10DIF digest algorithm using PMULL instructions"
-	depends on KERNEL_MODE_NEON && CRC_T10DIF
+	depends on KERNEL_MODE_NEON && CRC_T10DIF  && ARM_AS_HAS_CE
 	select CRYPTO_HASH
+	help
+	  Need binutils 2.23 or higher
 
 config CRYPTO_CRC32_ARM_CE
 	tristate "CRC32(C) digest algorithm using CRC and/or PMULL instructions"
-	depends on KERNEL_MODE_NEON && CRC32
+	depends on KERNEL_MODE_NEON && CRC32 && ARM_AS_HAS_CRC
 	select CRYPTO_HASH
+	help
+	  Need binutils 2.23 or higher
 
 config CRYPTO_CHACHA20_NEON
 	tristate "NEON accelerated ChaCha20 symmetric cipher"

arch/arm/crypto/Makefile +6 ~~-25~~

index bd5bcee..e897327 100644
@@ -11,32 +11,13 @@ obj-$(CONFIG_CRYPTO_SHA256_ARM) += sha256-arm.o
 obj-$(CONFIG_CRYPTO_SHA512_ARM) += sha512-arm.o
 obj-$(CONFIG_CRYPTO_CHACHA20_NEON) += chacha20-neon.o
 
-ce-obj-$(CONFIG_CRYPTO_AES_ARM_CE) += aes-arm-ce.o
-ce-obj-$(CONFIG_CRYPTO_SHA1_ARM_CE) += sha1-arm-ce.o
-ce-obj-$(CONFIG_CRYPTO_SHA2_ARM_CE) += sha2-arm-ce.o
-ce-obj-$(CONFIG_CRYPTO_GHASH_ARM_CE) += ghash-arm-ce.o
-ce-obj-$(CONFIG_CRYPTO_CRCT10DIF_ARM_CE) += crct10dif-arm-ce.o
-crc-obj-$(CONFIG_CRYPTO_CRC32_ARM_CE) += crc32-arm-ce.o
+obj-$(CONFIG_CRYPTO_AES_ARM_CE) += aes-arm-ce.o
+obj-$(CONFIG_CRYPTO_SHA1_ARM_CE) += sha1-arm-ce.o
+obj-$(CONFIG_CRYPTO_SHA2_ARM_CE) += sha2-arm-ce.o
+obj-$(CONFIG_CRYPTO_GHASH_ARM_CE) += ghash-arm-ce.o
+obj-$(CONFIG_CRYPTO_CRCT10DIF_ARM_CE) += crct10dif-arm-ce.o
 
-ifneq ($(crc-obj-y)$(crc-obj-m),)
-ifeq ($(call as-instr,.arch armv8-a\n.arch_extension crc,y,n),y)
-ce-obj-y += $(crc-obj-y)
-ce-obj-m += $(crc-obj-m)
-else
-$(warning These CRC Extensions modules need binutils 2.23 or higher)
-$(warning $(crc-obj-y) $(crc-obj-m))
-endif
-endif
-
-ifneq ($(ce-obj-y)$(ce-obj-m),)
-ifeq ($(call as-instr,.fpu crypto-neon-fp-armv8,y,n),y)
-obj-y += $(ce-obj-y)
-obj-m += $(ce-obj-m)
-else
-$(warning These ARMv8 Crypto Extensions modules need binutils 2.23 or higher)
-$(warning $(ce-obj-y) $(ce-obj-m))
-endif
-endif
+obj-$(CONFIG_CRYPTO_CRC32_ARM_CE) += crc32-arm-ce.o
 
 aes-arm-y	:= aes-cipher-core.o aes-cipher-glue.o
 aes-arm-bs-y	:= aes-neonbs-core.o aes-neonbs-glue.o

Signature · 1 line

1.9.1