All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: Generate cpucaps.h
@ 2021-04-28 12:12 Mark Brown
  2021-04-29 12:32 ` Catalin Marinas
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Mark Brown @ 2021-04-28 12:12 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: Suzuki K Poulose, linux-arm-kernel, Mark Brown

The arm64 code allocates an internal constant to every CPU feature it can
detect, distinct from the public hwcap numbers we use to expose some
features to userspace. Currently this is maintained manually which is an
irritating source of conflicts when working on new features, to avoid this
replace the header with a simple text file listing the names we've assigned
and sort it to minimise conflicts.

As part of doing this we also do the Kbuild hookup required to hook up
an arch tools directory and to generate header files in there.

This will result in a renumbering and reordering of the existing constants,
since they are all internal only the values should not be important. The
reordering will impact the order in which some steps in enumeration handle
features but the algorithm is not intended to depend on this and I haven't
seen any issues when testing. Due to the UAO cpucap having been removed in
the past we end up with ARM64_NCAPS being 1 smaller than it was before.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/Makefile              |  3 ++
 arch/arm64/include/asm/Kbuild    |  2 +
 arch/arm64/include/asm/cpucaps.h | 74 --------------------------------
 arch/arm64/tools/Makefile        | 22 ++++++++++
 arch/arm64/tools/cpucaps         | 65 ++++++++++++++++++++++++++++
 arch/arm64/tools/gen-cpucaps.awk | 40 +++++++++++++++++
 6 files changed, 132 insertions(+), 74 deletions(-)
 delete mode 100644 arch/arm64/include/asm/cpucaps.h
 create mode 100644 arch/arm64/tools/Makefile
 create mode 100644 arch/arm64/tools/cpucaps
 create mode 100755 arch/arm64/tools/gen-cpucaps.awk

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 7ef44478560d..b52481f0605d 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -175,6 +175,9 @@ vdso_install:
 	$(if $(CONFIG_COMPAT_VDSO), \
 		$(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 $@)
 
+archprepare:
+	$(Q)$(MAKE) $(build)=arch/arm64/tools kapi
+
 # We use MRPROPER_FILES and CLEAN_FILES now
 archclean:
 	$(Q)$(MAKE) $(clean)=$(boot)
diff --git a/arch/arm64/include/asm/Kbuild b/arch/arm64/include/asm/Kbuild
index 73aa25843f65..64202010b700 100644
--- a/arch/arm64/include/asm/Kbuild
+++ b/arch/arm64/include/asm/Kbuild
@@ -4,3 +4,5 @@ generic-y += mcs_spinlock.h
 generic-y += qrwlock.h
 generic-y += qspinlock.h
 generic-y += user.h
+
+generated-y += cpucaps.h
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
deleted file mode 100644
index b0c5eda0498f..000000000000
--- a/arch/arm64/include/asm/cpucaps.h
+++ /dev/null
@@ -1,74 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * arch/arm64/include/asm/cpucaps.h
- *
- * Copyright (C) 2016 ARM Ltd.
- */
-#ifndef __ASM_CPUCAPS_H
-#define __ASM_CPUCAPS_H
-
-#define ARM64_WORKAROUND_CLEAN_CACHE		0
-#define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE	1
-#define ARM64_WORKAROUND_845719			2
-#define ARM64_HAS_SYSREG_GIC_CPUIF		3
-#define ARM64_HAS_PAN				4
-#define ARM64_HAS_LSE_ATOMICS			5
-#define ARM64_WORKAROUND_CAVIUM_23154		6
-#define ARM64_WORKAROUND_834220			7
-#define ARM64_HAS_NO_HW_PREFETCH		8
-#define ARM64_HAS_VIRT_HOST_EXTN		11
-#define ARM64_WORKAROUND_CAVIUM_27456		12
-#define ARM64_HAS_32BIT_EL0			13
-#define ARM64_SPECTRE_V3A			14
-#define ARM64_HAS_CNP				15
-#define ARM64_HAS_NO_FPSIMD			16
-#define ARM64_WORKAROUND_REPEAT_TLBI		17
-#define ARM64_WORKAROUND_QCOM_FALKOR_E1003	18
-#define ARM64_WORKAROUND_858921			19
-#define ARM64_WORKAROUND_CAVIUM_30115		20
-#define ARM64_HAS_DCPOP				21
-#define ARM64_SVE				22
-#define ARM64_UNMAP_KERNEL_AT_EL0		23
-#define ARM64_SPECTRE_V2			24
-#define ARM64_HAS_RAS_EXTN			25
-#define ARM64_WORKAROUND_843419			26
-#define ARM64_HAS_CACHE_IDC			27
-#define ARM64_HAS_CACHE_DIC			28
-#define ARM64_HW_DBM				29
-#define ARM64_SPECTRE_V4			30
-#define ARM64_MISMATCHED_CACHE_TYPE		31
-#define ARM64_HAS_STAGE2_FWB			32
-#define ARM64_HAS_CRC32				33
-#define ARM64_SSBS				34
-#define ARM64_WORKAROUND_1418040		35
-#define ARM64_HAS_SB				36
-#define ARM64_WORKAROUND_SPECULATIVE_AT		37
-#define ARM64_HAS_ADDRESS_AUTH_ARCH		38
-#define ARM64_HAS_ADDRESS_AUTH_IMP_DEF		39
-#define ARM64_HAS_GENERIC_AUTH_ARCH		40
-#define ARM64_HAS_GENERIC_AUTH_IMP_DEF		41
-#define ARM64_HAS_IRQ_PRIO_MASKING		42
-#define ARM64_HAS_DCPODP			43
-#define ARM64_WORKAROUND_1463225		44
-#define ARM64_WORKAROUND_CAVIUM_TX2_219_TVM	45
-#define ARM64_WORKAROUND_CAVIUM_TX2_219_PRFM	46
-#define ARM64_WORKAROUND_1542419		47
-#define ARM64_HAS_E0PD				48
-#define ARM64_HAS_RNG				49
-#define ARM64_HAS_AMU_EXTN			50
-#define ARM64_HAS_ADDRESS_AUTH			51
-#define ARM64_HAS_GENERIC_AUTH			52
-#define ARM64_HAS_32BIT_EL1			53
-#define ARM64_BTI				54
-#define ARM64_HAS_ARMv8_4_TTL			55
-#define ARM64_HAS_TLB_RANGE			56
-#define ARM64_MTE				57
-#define ARM64_WORKAROUND_1508412		58
-#define ARM64_HAS_LDAPR				59
-#define ARM64_KVM_PROTECTED_MODE		60
-#define ARM64_WORKAROUND_NVIDIA_CARMEL_CNP	61
-#define ARM64_HAS_EPAN				62
-
-#define ARM64_NCAPS				63
-
-#endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/tools/Makefile b/arch/arm64/tools/Makefile
new file mode 100644
index 000000000000..932b4fe5c768
--- /dev/null
+++ b/arch/arm64/tools/Makefile
@@ -0,0 +1,22 @@
+# SPDX-License-Identifier: GPL-2.0
+
+gen := arch/$(ARCH)/include/generated
+kapi := $(gen)/asm
+
+kapi-hdrs-y := $(kapi)/cpucaps.h
+
+targets += $(addprefix ../../../,$(gen-y) $(kapi-hdrs-y))
+
+PHONY += kapi
+
+kapi:   $(kapi-hdrs-y) $(gen-y)
+
+# Create output directory if not already present
+_dummy := $(shell [ -d '$(kapi)' ] || mkdir -p '$(kapi)')
+
+quiet_cmd_gen_cpucaps = GEN     $@
+      cmd_gen_cpucaps = mkdir -p $(dir $@) && \
+                     $(AWK) -f $(filter-out $(PHONY),$^) > $@
+
+$(kapi)/cpucaps.h: $(src)/gen-cpucaps.awk $(src)/cpucaps FORCE
+	$(call if_changed,gen_cpucaps)
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
new file mode 100644
index 000000000000..21fbdda7086e
--- /dev/null
+++ b/arch/arm64/tools/cpucaps
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Internal CPU capabilities constants, keep this list sorted
+
+BTI
+HAS_32BIT_EL0
+HAS_32BIT_EL1
+HAS_ADDRESS_AUTH
+HAS_ADDRESS_AUTH_ARCH
+HAS_ADDRESS_AUTH_IMP_DEF
+HAS_AMU_EXTN
+HAS_ARMv8_4_TTL
+HAS_CACHE_DIC
+HAS_CACHE_IDC
+HAS_CNP
+HAS_CRC32
+HAS_DCPODP
+HAS_DCPOP
+HAS_E0PD
+HAS_EPAN
+HAS_GENERIC_AUTH
+HAS_GENERIC_AUTH_ARCH
+HAS_GENERIC_AUTH_IMP_DEF
+HAS_IRQ_PRIO_MASKING
+HAS_LDAPR
+HAS_LSE_ATOMICS
+HAS_NO_FPSIMD
+HAS_NO_HW_PREFETCH
+HAS_PAN
+HAS_RAS_EXTN
+HAS_RNG
+HAS_SB
+HAS_STAGE2_FWB
+HAS_SYSREG_GIC_CPUIF
+HAS_TLB_RANGE
+HAS_VIRT_HOST_EXTN
+HW_DBM
+KVM_PROTECTED_MODE
+MISMATCHED_CACHE_TYPE
+MTE
+SPECTRE_V2
+SPECTRE_V3A
+SPECTRE_V4
+SSBS
+SVE
+UNMAP_KERNEL_AT_EL0
+WORKAROUND_834220
+WORKAROUND_843419
+WORKAROUND_845719
+WORKAROUND_858921
+WORKAROUND_1418040
+WORKAROUND_1463225
+WORKAROUND_1508412
+WORKAROUND_1542419
+WORKAROUND_CAVIUM_23154
+WORKAROUND_CAVIUM_27456
+WORKAROUND_CAVIUM_30115
+WORKAROUND_CAVIUM_TX2_219_PRFM
+WORKAROUND_CAVIUM_TX2_219_TVM
+WORKAROUND_CLEAN_CACHE
+WORKAROUND_DEVICE_LOAD_ACQUIRE
+WORKAROUND_NVIDIA_CARMEL_CNP
+WORKAROUND_QCOM_FALKOR_E1003
+WORKAROUND_REPEAT_TLBI
+WORKAROUND_SPECULATIVE_AT
diff --git a/arch/arm64/tools/gen-cpucaps.awk b/arch/arm64/tools/gen-cpucaps.awk
new file mode 100755
index 000000000000..18737a1ce044
--- /dev/null
+++ b/arch/arm64/tools/gen-cpucaps.awk
@@ -0,0 +1,40 @@
+#!/bin/awk -f
+# SPDX-License-Identifier: GPL-2.0
+# gen-cpucaps.awk: arm64 cpucaps header generator
+#
+# Usage: awk -f gen-cpucaps.awk cpucaps.txt
+
+# Log an error and terminate
+function fatal(msg) {
+	print "Error at line " NR ": " msg > "/dev/stderr"
+	exit 1
+}
+
+# skip blank lines and comment lines
+/^$/ { next }
+/^#/ { next }
+
+BEGIN {
+	print "#ifndef __ASM_CPUCAPS_H"
+	print "#define __ASM_CPUCAPS_H"
+	print ""
+	print "/* Generated file - do not edit */"
+	cap_num = 0
+	print ""
+}
+
+/^[vA-Z0-9_]+$/ {
+	printf("#define ARM64_%-30s\t%d\n", $0, cap_num++)
+	next
+}
+
+END {
+	printf("#define ARM64_NCAPS\t\t\t\t%d\n", cap_num)
+	print ""
+	print "#endif"
+}
+
+# Any lines not handled by previous rules are unexpected
+{
+	fatal("unhandled statement")
+}
-- 
2.20.1


_______________________________________________
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] 13+ messages in thread

* Re: [PATCH] arm64: Generate cpucaps.h
  2021-04-28 12:12 [PATCH] arm64: Generate cpucaps.h Mark Brown
@ 2021-04-29 12:32 ` Catalin Marinas
  2021-05-04 11:43 ` Mark Rutland
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Catalin Marinas @ 2021-04-29 12:32 UTC (permalink / raw)
  To: Mark Brown; +Cc: Will Deacon, Suzuki K Poulose, linux-arm-kernel

On Wed, Apr 28, 2021 at 01:12:31PM +0100, Mark Brown wrote:
> The arm64 code allocates an internal constant to every CPU feature it can
> detect, distinct from the public hwcap numbers we use to expose some
> features to userspace. Currently this is maintained manually which is an
> irritating source of conflicts when working on new features, to avoid this
> replace the header with a simple text file listing the names we've assigned
> and sort it to minimise conflicts.
> 
> As part of doing this we also do the Kbuild hookup required to hook up
> an arch tools directory and to generate header files in there.
> 
> This will result in a renumbering and reordering of the existing constants,
> since they are all internal only the values should not be important. The
> reordering will impact the order in which some steps in enumeration handle
> features but the algorithm is not intended to depend on this and I haven't
> seen any issues when testing. Due to the UAO cpucap having been removed in
> the past we end up with ARM64_NCAPS being 1 smaller than it was before.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/Makefile              |  3 ++
>  arch/arm64/include/asm/Kbuild    |  2 +
>  arch/arm64/include/asm/cpucaps.h | 74 --------------------------------
>  arch/arm64/tools/Makefile        | 22 ++++++++++
>  arch/arm64/tools/cpucaps         | 65 ++++++++++++++++++++++++++++
>  arch/arm64/tools/gen-cpucaps.awk | 40 +++++++++++++++++
>  6 files changed, 132 insertions(+), 74 deletions(-)
>  delete mode 100644 arch/arm64/include/asm/cpucaps.h
>  create mode 100644 arch/arm64/tools/Makefile
>  create mode 100644 arch/arm64/tools/cpucaps
>  create mode 100755 arch/arm64/tools/gen-cpucaps.awk

It's a good cleanup, it saves us from always sorting out conflicts
around ARM64_NCAPS. I'll either take it during the merging window with
the for-next/fixes folded in or push it at -rc1 as there's a change in
cpucaps.h not currently in for-next/core.

Thanks.

-- 
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] 13+ messages in thread

* Re: [PATCH] arm64: Generate cpucaps.h
  2021-04-28 12:12 [PATCH] arm64: Generate cpucaps.h Mark Brown
  2021-04-29 12:32 ` Catalin Marinas
@ 2021-05-04 11:43 ` Mark Rutland
  2021-05-10 12:55 ` Catalin Marinas
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Mark Rutland @ 2021-05-04 11:43 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Suzuki K Poulose, linux-arm-kernel

On Wed, Apr 28, 2021 at 01:12:31PM +0100, Mark Brown wrote:
> The arm64 code allocates an internal constant to every CPU feature it can
> detect, distinct from the public hwcap numbers we use to expose some
> features to userspace. Currently this is maintained manually which is an
> irritating source of conflicts when working on new features, to avoid this
> replace the header with a simple text file listing the names we've assigned
> and sort it to minimise conflicts.
> 
> As part of doing this we also do the Kbuild hookup required to hook up
> an arch tools directory and to generate header files in there.
> 
> This will result in a renumbering and reordering of the existing constants,
> since they are all internal only the values should not be important. The
> reordering will impact the order in which some steps in enumeration handle
> features but the algorithm is not intended to depend on this and I haven't
> seen any issues when testing. Due to the UAO cpucap having been removed in
> the past we end up with ARM64_NCAPS being 1 smaller than it was before.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>

Neat! This looks good to me per my understanding of awk.

I have this s quick spin, and tested and out-of-tree build and common
awk implementations (gawk, mawk, nawk), and everything worked as
expected. So FWIW:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

> ---
>  arch/arm64/Makefile              |  3 ++
>  arch/arm64/include/asm/Kbuild    |  2 +
>  arch/arm64/include/asm/cpucaps.h | 74 --------------------------------
>  arch/arm64/tools/Makefile        | 22 ++++++++++
>  arch/arm64/tools/cpucaps         | 65 ++++++++++++++++++++++++++++
>  arch/arm64/tools/gen-cpucaps.awk | 40 +++++++++++++++++
>  6 files changed, 132 insertions(+), 74 deletions(-)
>  delete mode 100644 arch/arm64/include/asm/cpucaps.h
>  create mode 100644 arch/arm64/tools/Makefile
>  create mode 100644 arch/arm64/tools/cpucaps
>  create mode 100755 arch/arm64/tools/gen-cpucaps.awk
> 
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 7ef44478560d..b52481f0605d 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -175,6 +175,9 @@ vdso_install:
>  	$(if $(CONFIG_COMPAT_VDSO), \
>  		$(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 $@)
>  
> +archprepare:
> +	$(Q)$(MAKE) $(build)=arch/arm64/tools kapi
> +
>  # We use MRPROPER_FILES and CLEAN_FILES now
>  archclean:
>  	$(Q)$(MAKE) $(clean)=$(boot)
> diff --git a/arch/arm64/include/asm/Kbuild b/arch/arm64/include/asm/Kbuild
> index 73aa25843f65..64202010b700 100644
> --- a/arch/arm64/include/asm/Kbuild
> +++ b/arch/arm64/include/asm/Kbuild
> @@ -4,3 +4,5 @@ generic-y += mcs_spinlock.h
>  generic-y += qrwlock.h
>  generic-y += qspinlock.h
>  generic-y += user.h
> +
> +generated-y += cpucaps.h
> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
> deleted file mode 100644
> index b0c5eda0498f..000000000000
> --- a/arch/arm64/include/asm/cpucaps.h
> +++ /dev/null
> @@ -1,74 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * arch/arm64/include/asm/cpucaps.h
> - *
> - * Copyright (C) 2016 ARM Ltd.
> - */
> -#ifndef __ASM_CPUCAPS_H
> -#define __ASM_CPUCAPS_H
> -
> -#define ARM64_WORKAROUND_CLEAN_CACHE		0
> -#define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE	1
> -#define ARM64_WORKAROUND_845719			2
> -#define ARM64_HAS_SYSREG_GIC_CPUIF		3
> -#define ARM64_HAS_PAN				4
> -#define ARM64_HAS_LSE_ATOMICS			5
> -#define ARM64_WORKAROUND_CAVIUM_23154		6
> -#define ARM64_WORKAROUND_834220			7
> -#define ARM64_HAS_NO_HW_PREFETCH		8
> -#define ARM64_HAS_VIRT_HOST_EXTN		11
> -#define ARM64_WORKAROUND_CAVIUM_27456		12
> -#define ARM64_HAS_32BIT_EL0			13
> -#define ARM64_SPECTRE_V3A			14
> -#define ARM64_HAS_CNP				15
> -#define ARM64_HAS_NO_FPSIMD			16
> -#define ARM64_WORKAROUND_REPEAT_TLBI		17
> -#define ARM64_WORKAROUND_QCOM_FALKOR_E1003	18
> -#define ARM64_WORKAROUND_858921			19
> -#define ARM64_WORKAROUND_CAVIUM_30115		20
> -#define ARM64_HAS_DCPOP				21
> -#define ARM64_SVE				22
> -#define ARM64_UNMAP_KERNEL_AT_EL0		23
> -#define ARM64_SPECTRE_V2			24
> -#define ARM64_HAS_RAS_EXTN			25
> -#define ARM64_WORKAROUND_843419			26
> -#define ARM64_HAS_CACHE_IDC			27
> -#define ARM64_HAS_CACHE_DIC			28
> -#define ARM64_HW_DBM				29
> -#define ARM64_SPECTRE_V4			30
> -#define ARM64_MISMATCHED_CACHE_TYPE		31
> -#define ARM64_HAS_STAGE2_FWB			32
> -#define ARM64_HAS_CRC32				33
> -#define ARM64_SSBS				34
> -#define ARM64_WORKAROUND_1418040		35
> -#define ARM64_HAS_SB				36
> -#define ARM64_WORKAROUND_SPECULATIVE_AT		37
> -#define ARM64_HAS_ADDRESS_AUTH_ARCH		38
> -#define ARM64_HAS_ADDRESS_AUTH_IMP_DEF		39
> -#define ARM64_HAS_GENERIC_AUTH_ARCH		40
> -#define ARM64_HAS_GENERIC_AUTH_IMP_DEF		41
> -#define ARM64_HAS_IRQ_PRIO_MASKING		42
> -#define ARM64_HAS_DCPODP			43
> -#define ARM64_WORKAROUND_1463225		44
> -#define ARM64_WORKAROUND_CAVIUM_TX2_219_TVM	45
> -#define ARM64_WORKAROUND_CAVIUM_TX2_219_PRFM	46
> -#define ARM64_WORKAROUND_1542419		47
> -#define ARM64_HAS_E0PD				48
> -#define ARM64_HAS_RNG				49
> -#define ARM64_HAS_AMU_EXTN			50
> -#define ARM64_HAS_ADDRESS_AUTH			51
> -#define ARM64_HAS_GENERIC_AUTH			52
> -#define ARM64_HAS_32BIT_EL1			53
> -#define ARM64_BTI				54
> -#define ARM64_HAS_ARMv8_4_TTL			55
> -#define ARM64_HAS_TLB_RANGE			56
> -#define ARM64_MTE				57
> -#define ARM64_WORKAROUND_1508412		58
> -#define ARM64_HAS_LDAPR				59
> -#define ARM64_KVM_PROTECTED_MODE		60
> -#define ARM64_WORKAROUND_NVIDIA_CARMEL_CNP	61
> -#define ARM64_HAS_EPAN				62
> -
> -#define ARM64_NCAPS				63
> -
> -#endif /* __ASM_CPUCAPS_H */
> diff --git a/arch/arm64/tools/Makefile b/arch/arm64/tools/Makefile
> new file mode 100644
> index 000000000000..932b4fe5c768
> --- /dev/null
> +++ b/arch/arm64/tools/Makefile
> @@ -0,0 +1,22 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +gen := arch/$(ARCH)/include/generated
> +kapi := $(gen)/asm
> +
> +kapi-hdrs-y := $(kapi)/cpucaps.h
> +
> +targets += $(addprefix ../../../,$(gen-y) $(kapi-hdrs-y))
> +
> +PHONY += kapi
> +
> +kapi:   $(kapi-hdrs-y) $(gen-y)
> +
> +# Create output directory if not already present
> +_dummy := $(shell [ -d '$(kapi)' ] || mkdir -p '$(kapi)')
> +
> +quiet_cmd_gen_cpucaps = GEN     $@
> +      cmd_gen_cpucaps = mkdir -p $(dir $@) && \
> +                     $(AWK) -f $(filter-out $(PHONY),$^) > $@
> +
> +$(kapi)/cpucaps.h: $(src)/gen-cpucaps.awk $(src)/cpucaps FORCE
> +	$(call if_changed,gen_cpucaps)
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> new file mode 100644
> index 000000000000..21fbdda7086e
> --- /dev/null
> +++ b/arch/arm64/tools/cpucaps
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Internal CPU capabilities constants, keep this list sorted
> +
> +BTI
> +HAS_32BIT_EL0
> +HAS_32BIT_EL1
> +HAS_ADDRESS_AUTH
> +HAS_ADDRESS_AUTH_ARCH
> +HAS_ADDRESS_AUTH_IMP_DEF
> +HAS_AMU_EXTN
> +HAS_ARMv8_4_TTL
> +HAS_CACHE_DIC
> +HAS_CACHE_IDC
> +HAS_CNP
> +HAS_CRC32
> +HAS_DCPODP
> +HAS_DCPOP
> +HAS_E0PD
> +HAS_EPAN
> +HAS_GENERIC_AUTH
> +HAS_GENERIC_AUTH_ARCH
> +HAS_GENERIC_AUTH_IMP_DEF
> +HAS_IRQ_PRIO_MASKING
> +HAS_LDAPR
> +HAS_LSE_ATOMICS
> +HAS_NO_FPSIMD
> +HAS_NO_HW_PREFETCH
> +HAS_PAN
> +HAS_RAS_EXTN
> +HAS_RNG
> +HAS_SB
> +HAS_STAGE2_FWB
> +HAS_SYSREG_GIC_CPUIF
> +HAS_TLB_RANGE
> +HAS_VIRT_HOST_EXTN
> +HW_DBM
> +KVM_PROTECTED_MODE
> +MISMATCHED_CACHE_TYPE
> +MTE
> +SPECTRE_V2
> +SPECTRE_V3A
> +SPECTRE_V4
> +SSBS
> +SVE
> +UNMAP_KERNEL_AT_EL0
> +WORKAROUND_834220
> +WORKAROUND_843419
> +WORKAROUND_845719
> +WORKAROUND_858921
> +WORKAROUND_1418040
> +WORKAROUND_1463225
> +WORKAROUND_1508412
> +WORKAROUND_1542419
> +WORKAROUND_CAVIUM_23154
> +WORKAROUND_CAVIUM_27456
> +WORKAROUND_CAVIUM_30115
> +WORKAROUND_CAVIUM_TX2_219_PRFM
> +WORKAROUND_CAVIUM_TX2_219_TVM
> +WORKAROUND_CLEAN_CACHE
> +WORKAROUND_DEVICE_LOAD_ACQUIRE
> +WORKAROUND_NVIDIA_CARMEL_CNP
> +WORKAROUND_QCOM_FALKOR_E1003
> +WORKAROUND_REPEAT_TLBI
> +WORKAROUND_SPECULATIVE_AT
> diff --git a/arch/arm64/tools/gen-cpucaps.awk b/arch/arm64/tools/gen-cpucaps.awk
> new file mode 100755
> index 000000000000..18737a1ce044
> --- /dev/null
> +++ b/arch/arm64/tools/gen-cpucaps.awk
> @@ -0,0 +1,40 @@
> +#!/bin/awk -f
> +# SPDX-License-Identifier: GPL-2.0
> +# gen-cpucaps.awk: arm64 cpucaps header generator
> +#
> +# Usage: awk -f gen-cpucaps.awk cpucaps.txt
> +
> +# Log an error and terminate
> +function fatal(msg) {
> +	print "Error at line " NR ": " msg > "/dev/stderr"
> +	exit 1
> +}
> +
> +# skip blank lines and comment lines
> +/^$/ { next }
> +/^#/ { next }
> +
> +BEGIN {
> +	print "#ifndef __ASM_CPUCAPS_H"
> +	print "#define __ASM_CPUCAPS_H"
> +	print ""
> +	print "/* Generated file - do not edit */"
> +	cap_num = 0
> +	print ""
> +}
> +
> +/^[vA-Z0-9_]+$/ {
> +	printf("#define ARM64_%-30s\t%d\n", $0, cap_num++)
> +	next
> +}
> +
> +END {
> +	printf("#define ARM64_NCAPS\t\t\t\t%d\n", cap_num)
> +	print ""
> +	print "#endif"
> +}
> +
> +# Any lines not handled by previous rules are unexpected
> +{
> +	fatal("unhandled statement")
> +}
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> 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] 13+ messages in thread

* Re: [PATCH] arm64: Generate cpucaps.h
  2021-04-28 12:12 [PATCH] arm64: Generate cpucaps.h Mark Brown
  2021-04-29 12:32 ` Catalin Marinas
  2021-05-04 11:43 ` Mark Rutland
@ 2021-05-10 12:55 ` Catalin Marinas
  2021-05-13  5:05 ` Anshuman Khandual
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Catalin Marinas @ 2021-05-10 12:55 UTC (permalink / raw)
  To: Will Deacon, Mark Brown; +Cc: Suzuki K Poulose, linux-arm-kernel

On Wed, 28 Apr 2021 13:12:31 +0100, Mark Brown wrote:
> The arm64 code allocates an internal constant to every CPU feature it can
> detect, distinct from the public hwcap numbers we use to expose some
> features to userspace. Currently this is maintained manually which is an
> irritating source of conflicts when working on new features, to avoid this
> replace the header with a simple text file listing the names we've assigned
> and sort it to minimise conflicts.
> 
> [...]

Applied to arm64 (for-next/fixes), thanks! While not actually a fix, I'm
merging it at -rc1 to avoid conflicts with changes to cpucaps.h during
the merging window.

[1/1] arm64: Generate cpucaps.h
      https://git.kernel.org/arm64/c/0c6c2d3615ef

-- 
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] 13+ messages in thread

* Re: [PATCH] arm64: Generate cpucaps.h
  2021-04-28 12:12 [PATCH] arm64: Generate cpucaps.h Mark Brown
                   ` (2 preceding siblings ...)
  2021-05-10 12:55 ` Catalin Marinas
@ 2021-05-13  5:05 ` Anshuman Khandual
  2021-05-13 11:51   ` Mark Rutland
  2021-05-13 12:45   ` Mark Brown
  2021-05-13 11:30 ` Geert Uytterhoeven
  2021-09-21 18:08 ` dann frazier
  5 siblings, 2 replies; 13+ messages in thread
From: Anshuman Khandual @ 2021-05-13  5:05 UTC (permalink / raw)
  To: Mark Brown, Catalin Marinas, Will Deacon
  Cc: Suzuki K Poulose, linux-arm-kernel


On 4/28/21 5:42 PM, Mark Brown wrote:
> The arm64 code allocates an internal constant to every CPU feature it can
> detect, distinct from the public hwcap numbers we use to expose some
> features to userspace. Currently this is maintained manually which is an
> irritating source of conflicts when working on new features, to avoid this
> replace the header with a simple text file listing the names we've assigned
> and sort it to minimise conflicts.
> 
> As part of doing this we also do the Kbuild hookup required to hook up
> an arch tools directory and to generate header files in there.
> 
> This will result in a renumbering and reordering of the existing constants,
> since they are all internal only the values should not be important. The
> reordering will impact the order in which some steps in enumeration handle
> features but the algorithm is not intended to depend on this and I haven't
> seen any issues when testing. Due to the UAO cpucap having been removed in
> the past we end up with ARM64_NCAPS being 1 smaller than it was before.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/Makefile              |  3 ++
>  arch/arm64/include/asm/Kbuild    |  2 +
>  arch/arm64/include/asm/cpucaps.h | 74 --------------------------------
>  arch/arm64/tools/Makefile        | 22 ++++++++++
>  arch/arm64/tools/cpucaps         | 65 ++++++++++++++++++++++++++++
>  arch/arm64/tools/gen-cpucaps.awk | 40 +++++++++++++++++
>  6 files changed, 132 insertions(+), 74 deletions(-)
>  delete mode 100644 arch/arm64/include/asm/cpucaps.h
>  create mode 100644 arch/arm64/tools/Makefile
>  create mode 100644 arch/arm64/tools/cpucaps
>  create mode 100755 arch/arm64/tools/gen-cpucaps.awk
> 
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 7ef44478560d..b52481f0605d 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -175,6 +175,9 @@ vdso_install:
>  	$(if $(CONFIG_COMPAT_VDSO), \
>  		$(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 $@)
>  
> +archprepare:
> +	$(Q)$(MAKE) $(build)=arch/arm64/tools kapi

Small nit. So going forward this new directory (arch/arm64/tools/), will
be used to hold similar scripts that generate header or similar things ?
At first the directory name 'tool' was bit confusing but seems like other
archs (arm, m68k, mips, powerpc, s390, sh) have this directory as well.

> +
>  # We use MRPROPER_FILES and CLEAN_FILES now
>  archclean:
>  	$(Q)$(MAKE) $(clean)=$(boot)
> diff --git a/arch/arm64/include/asm/Kbuild b/arch/arm64/include/asm/Kbuild
> index 73aa25843f65..64202010b700 100644
> --- a/arch/arm64/include/asm/Kbuild
> +++ b/arch/arm64/include/asm/Kbuild
> @@ -4,3 +4,5 @@ generic-y += mcs_spinlock.h
>  generic-y += qrwlock.h
>  generic-y += qspinlock.h
>  generic-y += user.h
> +
> +generated-y += cpucaps.h
> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
> deleted file mode 100644
> index b0c5eda0498f..000000000000
> --- a/arch/arm64/include/asm/cpucaps.h
> +++ /dev/null
> @@ -1,74 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * arch/arm64/include/asm/cpucaps.h
> - *
> - * Copyright (C) 2016 ARM Ltd.
> - */
> -#ifndef __ASM_CPUCAPS_H
> -#define __ASM_CPUCAPS_H
> -
> -#define ARM64_WORKAROUND_CLEAN_CACHE		0
> -#define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE	1
> -#define ARM64_WORKAROUND_845719			2
> -#define ARM64_HAS_SYSREG_GIC_CPUIF		3
> -#define ARM64_HAS_PAN				4
> -#define ARM64_HAS_LSE_ATOMICS			5
> -#define ARM64_WORKAROUND_CAVIUM_23154		6
> -#define ARM64_WORKAROUND_834220			7
> -#define ARM64_HAS_NO_HW_PREFETCH		8
> -#define ARM64_HAS_VIRT_HOST_EXTN		11
> -#define ARM64_WORKAROUND_CAVIUM_27456		12
> -#define ARM64_HAS_32BIT_EL0			13
> -#define ARM64_SPECTRE_V3A			14
> -#define ARM64_HAS_CNP				15
> -#define ARM64_HAS_NO_FPSIMD			16
> -#define ARM64_WORKAROUND_REPEAT_TLBI		17
> -#define ARM64_WORKAROUND_QCOM_FALKOR_E1003	18
> -#define ARM64_WORKAROUND_858921			19
> -#define ARM64_WORKAROUND_CAVIUM_30115		20
> -#define ARM64_HAS_DCPOP				21
> -#define ARM64_SVE				22
> -#define ARM64_UNMAP_KERNEL_AT_EL0		23
> -#define ARM64_SPECTRE_V2			24
> -#define ARM64_HAS_RAS_EXTN			25
> -#define ARM64_WORKAROUND_843419			26
> -#define ARM64_HAS_CACHE_IDC			27
> -#define ARM64_HAS_CACHE_DIC			28
> -#define ARM64_HW_DBM				29
> -#define ARM64_SPECTRE_V4			30
> -#define ARM64_MISMATCHED_CACHE_TYPE		31
> -#define ARM64_HAS_STAGE2_FWB			32
> -#define ARM64_HAS_CRC32				33
> -#define ARM64_SSBS				34
> -#define ARM64_WORKAROUND_1418040		35
> -#define ARM64_HAS_SB				36
> -#define ARM64_WORKAROUND_SPECULATIVE_AT		37
> -#define ARM64_HAS_ADDRESS_AUTH_ARCH		38
> -#define ARM64_HAS_ADDRESS_AUTH_IMP_DEF		39
> -#define ARM64_HAS_GENERIC_AUTH_ARCH		40
> -#define ARM64_HAS_GENERIC_AUTH_IMP_DEF		41
> -#define ARM64_HAS_IRQ_PRIO_MASKING		42
> -#define ARM64_HAS_DCPODP			43
> -#define ARM64_WORKAROUND_1463225		44
> -#define ARM64_WORKAROUND_CAVIUM_TX2_219_TVM	45
> -#define ARM64_WORKAROUND_CAVIUM_TX2_219_PRFM	46
> -#define ARM64_WORKAROUND_1542419		47
> -#define ARM64_HAS_E0PD				48
> -#define ARM64_HAS_RNG				49
> -#define ARM64_HAS_AMU_EXTN			50
> -#define ARM64_HAS_ADDRESS_AUTH			51
> -#define ARM64_HAS_GENERIC_AUTH			52
> -#define ARM64_HAS_32BIT_EL1			53
> -#define ARM64_BTI				54
> -#define ARM64_HAS_ARMv8_4_TTL			55
> -#define ARM64_HAS_TLB_RANGE			56
> -#define ARM64_MTE				57
> -#define ARM64_WORKAROUND_1508412		58
> -#define ARM64_HAS_LDAPR				59
> -#define ARM64_KVM_PROTECTED_MODE		60
> -#define ARM64_WORKAROUND_NVIDIA_CARMEL_CNP	61
> -#define ARM64_HAS_EPAN				62
> -
> -#define ARM64_NCAPS				63
> -
> -#endif /* __ASM_CPUCAPS_H */
> diff --git a/arch/arm64/tools/Makefile b/arch/arm64/tools/Makefile
> new file mode 100644
> index 000000000000..932b4fe5c768
> --- /dev/null
> +++ b/arch/arm64/tools/Makefile
> @@ -0,0 +1,22 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +gen := arch/$(ARCH)/include/generated
> +kapi := $(gen)/asm
> +
> +kapi-hdrs-y := $(kapi)/cpucaps.h
> +
> +targets += $(addprefix ../../../,$(gen-y) $(kapi-hdrs-y))
> +
> +PHONY += kapi
> +
> +kapi:   $(kapi-hdrs-y) $(gen-y)
> +
> +# Create output directory if not already present
> +_dummy := $(shell [ -d '$(kapi)' ] || mkdir -p '$(kapi)')
> +
> +quiet_cmd_gen_cpucaps = GEN     $@
> +      cmd_gen_cpucaps = mkdir -p $(dir $@) && \
> +                     $(AWK) -f $(filter-out $(PHONY),$^) > $@
> +
> +$(kapi)/cpucaps.h: $(src)/gen-cpucaps.awk $(src)/cpucaps FORCE
> +	$(call if_changed,gen_cpucaps)
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> new file mode 100644
> index 000000000000..21fbdda7086e
> --- /dev/null
> +++ b/arch/arm64/tools/cpucaps
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Internal CPU capabilities constants, keep this list sorted
> +
> +BTI
> +HAS_32BIT_EL0
> +HAS_32BIT_EL1
> +HAS_ADDRESS_AUTH
> +HAS_ADDRESS_AUTH_ARCH
> +HAS_ADDRESS_AUTH_IMP_DEF
> +HAS_AMU_EXTN
> +HAS_ARMv8_4_TTL
> +HAS_CACHE_DIC
> +HAS_CACHE_IDC
> +HAS_CNP
> +HAS_CRC32
> +HAS_DCPODP
> +HAS_DCPOP
> +HAS_E0PD
> +HAS_EPAN
> +HAS_GENERIC_AUTH
> +HAS_GENERIC_AUTH_ARCH
> +HAS_GENERIC_AUTH_IMP_DEF
> +HAS_IRQ_PRIO_MASKING
> +HAS_LDAPR
> +HAS_LSE_ATOMICS
> +HAS_NO_FPSIMD
> +HAS_NO_HW_PREFETCH
> +HAS_PAN
> +HAS_RAS_EXTN
> +HAS_RNG
> +HAS_SB
> +HAS_STAGE2_FWB
> +HAS_SYSREG_GIC_CPUIF
> +HAS_TLB_RANGE
> +HAS_VIRT_HOST_EXTN
> +HW_DBM
> +KVM_PROTECTED_MODE
> +MISMATCHED_CACHE_TYPE
> +MTE
> +SPECTRE_V2
> +SPECTRE_V3A
> +SPECTRE_V4
> +SSBS
> +SVE
> +UNMAP_KERNEL_AT_EL0
> +WORKAROUND_834220
> +WORKAROUND_843419
> +WORKAROUND_845719
> +WORKAROUND_858921
> +WORKAROUND_1418040
> +WORKAROUND_1463225
> +WORKAROUND_1508412
> +WORKAROUND_1542419
> +WORKAROUND_CAVIUM_23154
> +WORKAROUND_CAVIUM_27456
> +WORKAROUND_CAVIUM_30115
> +WORKAROUND_CAVIUM_TX2_219_PRFM
> +WORKAROUND_CAVIUM_TX2_219_TVM
> +WORKAROUND_CLEAN_CACHE
> +WORKAROUND_DEVICE_LOAD_ACQUIRE
> +WORKAROUND_NVIDIA_CARMEL_CNP
> +WORKAROUND_QCOM_FALKOR_E1003
> +WORKAROUND_REPEAT_TLBI
> +WORKAROUND_SPECULATIVE_AT
> diff --git a/arch/arm64/tools/gen-cpucaps.awk b/arch/arm64/tools/gen-cpucaps.awk
> new file mode 100755
> index 000000000000..18737a1ce044
> --- /dev/null
> +++ b/arch/arm64/tools/gen-cpucaps.awk
> @@ -0,0 +1,40 @@
> +#!/bin/awk -f
> +# SPDX-License-Identifier: GPL-2.0
> +# gen-cpucaps.awk: arm64 cpucaps header generator
> +#
> +# Usage: awk -f gen-cpucaps.awk cpucaps.txt
> +
> +# Log an error and terminate
> +function fatal(msg) {
> +	print "Error at line " NR ": " msg > "/dev/stderr"
> +	exit 1
> +}
> +
> +# skip blank lines and comment lines
> +/^$/ { next }
> +/^#/ { next }
> +
> +BEGIN {
> +	print "#ifndef __ASM_CPUCAPS_H"
> +	print "#define __ASM_CPUCAPS_H"
> +	print ""
> +	print "/* Generated file - do not edit */"
> +	cap_num = 0
> +	print ""
> +}
> +
> +/^[vA-Z0-9_]+$/ {

Small nit. Should this be length restricted at the least ?
Like each CAP entries should not exceed a certain length.

> +	printf("#define ARM64_%-30s\t%d\n", $0, cap_num++)
> +	next
> +}
> +
> +END {
> +	printf("#define ARM64_NCAPS\t\t\t\t%d\n", cap_num)
> +	print ""
> +	print "#endif"

Small nit. Should it be print "#endif /* __ASM_CPUCAPS_H */" instead
in order to be complete.

> +}
> +
> +# Any lines not handled by previous rules are unexpected
> +{
> +	fatal("unhandled statement")
> +}
> 

Did not see any problem with the patch after using it with multiple
'awk' implementations. FWIW, with/without changes requested earlier.

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Tested-by: Anshuman Khandual <anshuman.khandual@arm.com>

_______________________________________________
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] 13+ messages in thread

* Re: [PATCH] arm64: Generate cpucaps.h
  2021-04-28 12:12 [PATCH] arm64: Generate cpucaps.h Mark Brown
                   ` (3 preceding siblings ...)
  2021-05-13  5:05 ` Anshuman Khandual
@ 2021-05-13 11:30 ` Geert Uytterhoeven
  2021-05-13 13:03   ` Mark Brown
  2021-09-21 18:08 ` dann frazier
  5 siblings, 1 reply; 13+ messages in thread
From: Geert Uytterhoeven @ 2021-05-13 11:30 UTC (permalink / raw)
  To: Mark Brown; +Cc: Catalin Marinas, Will Deacon, Suzuki K Poulose, Linux ARM

Hi Mark,

On Wed, Apr 28, 2021 at 2:25 PM Mark Brown <broonie@kernel.org> wrote:
> The arm64 code allocates an internal constant to every CPU feature it can
> detect, distinct from the public hwcap numbers we use to expose some
> features to userspace. Currently this is maintained manually which is an
> irritating source of conflicts when working on new features, to avoid this
> replace the header with a simple text file listing the names we've assigned
> and sort it to minimise conflicts.
>
> As part of doing this we also do the Kbuild hookup required to hook up
> an arch tools directory and to generate header files in there.
>
> This will result in a renumbering and reordering of the existing constants,
> since they are all internal only the values should not be important. The
> reordering will impact the order in which some steps in enumeration handle
> features but the algorithm is not intended to depend on this and I haven't
> seen any issues when testing. Due to the UAO cpucap having been removed in
> the past we end up with ARM64_NCAPS being 1 smaller than it was before.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/Makefile              |  3 ++
>  arch/arm64/include/asm/Kbuild    |  2 +
>  arch/arm64/include/asm/cpucaps.h | 74 --------------------------------
>  arch/arm64/tools/Makefile        | 22 ++++++++++
>  arch/arm64/tools/cpucaps         | 65 ++++++++++++++++++++++++++++
>  arch/arm64/tools/gen-cpucaps.awk | 40 +++++++++++++++++

I guess the reason you're doing it like this, and not using a simple C
enum, is that some of these definitions are used from asm files?
Do we use similar mechanisms in other places?
Would introducing a generic way to generate headers with definitions
from a C enum be worthwhile?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
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] 13+ messages in thread

* Re: [PATCH] arm64: Generate cpucaps.h
  2021-05-13  5:05 ` Anshuman Khandual
@ 2021-05-13 11:51   ` Mark Rutland
  2021-05-13 12:45   ` Mark Brown
  1 sibling, 0 replies; 13+ messages in thread
From: Mark Rutland @ 2021-05-13 11:51 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Mark Brown, Catalin Marinas, Will Deacon, Suzuki K Poulose,
	linux-arm-kernel

On Thu, May 13, 2021 at 10:35:04AM +0530, Anshuman Khandual wrote:
> 
> On 4/28/21 5:42 PM, Mark Brown wrote:
> > The arm64 code allocates an internal constant to every CPU feature it can
> > detect, distinct from the public hwcap numbers we use to expose some
> > features to userspace. Currently this is maintained manually which is an
> > irritating source of conflicts when working on new features, to avoid this
> > replace the header with a simple text file listing the names we've assigned
> > and sort it to minimise conflicts.
> > 
> > As part of doing this we also do the Kbuild hookup required to hook up
> > an arch tools directory and to generate header files in there.
> > 
> > This will result in a renumbering and reordering of the existing constants,
> > since they are all internal only the values should not be important. The
> > reordering will impact the order in which some steps in enumeration handle
> > features but the algorithm is not intended to depend on this and I haven't
> > seen any issues when testing. Due to the UAO cpucap having been removed in
> > the past we end up with ARM64_NCAPS being 1 smaller than it was before.
> > 
> > Signed-off-by: Mark Brown <broonie@kernel.org>
> > ---
> >  arch/arm64/Makefile              |  3 ++
> >  arch/arm64/include/asm/Kbuild    |  2 +
> >  arch/arm64/include/asm/cpucaps.h | 74 --------------------------------
> >  arch/arm64/tools/Makefile        | 22 ++++++++++
> >  arch/arm64/tools/cpucaps         | 65 ++++++++++++++++++++++++++++
> >  arch/arm64/tools/gen-cpucaps.awk | 40 +++++++++++++++++
> >  6 files changed, 132 insertions(+), 74 deletions(-)
> >  delete mode 100644 arch/arm64/include/asm/cpucaps.h
> >  create mode 100644 arch/arm64/tools/Makefile
> >  create mode 100644 arch/arm64/tools/cpucaps
> >  create mode 100755 arch/arm64/tools/gen-cpucaps.awk
> > 
> > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> > index 7ef44478560d..b52481f0605d 100644
> > --- a/arch/arm64/Makefile
> > +++ b/arch/arm64/Makefile
> > @@ -175,6 +175,9 @@ vdso_install:
> >  	$(if $(CONFIG_COMPAT_VDSO), \
> >  		$(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 $@)
> >  
> > +archprepare:
> > +	$(Q)$(MAKE) $(build)=arch/arm64/tools kapi
> 
> Small nit. So going forward this new directory (arch/arm64/tools/), will
> be used to hold similar scripts that generate header or similar things ?
> At first the directory name 'tool' was bit confusing but seems like other
> archs (arm, m68k, mips, powerpc, s390, sh) have this directory as well.

Yup; there's more stuff we might want to put here in future, e.g.

* Generation of (or consistency-checks) for the kernel HWCAP glue
* Generation of the sysreg headers
* Generation of the insn code
* Generation of metadata (or consistency checks) for
  RELIABLE_STACKTRACE, LIVEPATCH, etc

[...]

> > +BEGIN {
> > +	print "#ifndef __ASM_CPUCAPS_H"
> > +	print "#define __ASM_CPUCAPS_H"
> > +	print ""
> > +	print "/* Generated file - do not edit */"
> > +	cap_num = 0
> > +	print ""
> > +}
> > +
> > +/^[vA-Z0-9_]+$/ {
> 
> Small nit. Should this be length restricted at the least ?
> Like each CAP entries should not exceed a certain length.

I don't think we should enforce a length limit in the script, as we'd
have to select an arbitrary limit, and given we should catch exceedingly
long names during review, I think it's more likely to be a hindrance
than a help (e.g. if we ever bump up against that limit).

> > +	printf("#define ARM64_%-30s\t%d\n", $0, cap_num++)
> > +	next
> > +}
> > +
> > +END {
> > +	printf("#define ARM64_NCAPS\t\t\t\t%d\n", cap_num)
> > +	print ""
> > +	print "#endif"
> 
> Small nit. Should it be print "#endif /* __ASM_CPUCAPS_H */" instead
> in order to be complete.

Agreed; that would be nice.

Thanks,
Mark.

_______________________________________________
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] 13+ messages in thread

* Re: [PATCH] arm64: Generate cpucaps.h
  2021-05-13  5:05 ` Anshuman Khandual
  2021-05-13 11:51   ` Mark Rutland
@ 2021-05-13 12:45   ` Mark Brown
  1 sibling, 0 replies; 13+ messages in thread
From: Mark Brown @ 2021-05-13 12:45 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Catalin Marinas, Will Deacon, Suzuki K Poulose, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1427 bytes --]

On Thu, May 13, 2021 at 10:35:04AM +0530, Anshuman Khandual wrote:

> Small nit. So going forward this new directory (arch/arm64/tools/), will
> be used to hold similar scripts that generate header or similar things ?

Yes, anything we need to run in the build process.  Some of the other
architectures have some tools to verify generated binaries have
particular properties as well for example.

> At first the directory name 'tool' was bit confusing but seems like other
> archs (arm, m68k, mips, powerpc, s390, sh) have this directory as well.

Yes, it's the standard pattern.

> > +/^[vA-Z0-9_]+$/ {

> Small nit. Should this be length restricted at the least ?
> Like each CAP entries should not exceed a certain length.

If you want to send a patch by all means, I don't think it's useful
though - it doesn't matter to the software and the main issue is taste
which is going to get considered regardless of what software says, if
people are happy with an identifer that hits some limit they're just
going to raise the limit.

> Small nit. Should it be print "#endif /* __ASM_CPUCAPS_H */" instead
> in order to be complete.

I guess.  Given that this is merged that'd need to be an incremental
patch.

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
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] 13+ messages in thread

* Re: [PATCH] arm64: Generate cpucaps.h
  2021-05-13 11:30 ` Geert Uytterhoeven
@ 2021-05-13 13:03   ` Mark Brown
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2021-05-13 13:03 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Catalin Marinas, Will Deacon, Suzuki K Poulose, Linux ARM


[-- Attachment #1.1: Type: text/plain, Size: 644 bytes --]

On Thu, May 13, 2021 at 01:30:53PM +0200, Geert Uytterhoeven wrote:

> I guess the reason you're doing it like this, and not using a simple C
> enum, is that some of these definitions are used from asm files?

Yes, there's references in asm files.

> Do we use similar mechanisms in other places?

I'd be surprised if we weren't, though off the top of my head the other
examples in arm64 are the sysregs and the hwcaps which are both ABI and
don't really benefit in the same way.

> Would introducing a generic way to generate headers with definitions
> from a C enum be worthwhile?

There must be some other arch which could use one at least.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
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] 13+ messages in thread

* Re: [PATCH] arm64: Generate cpucaps.h
  2021-04-28 12:12 [PATCH] arm64: Generate cpucaps.h Mark Brown
                   ` (4 preceding siblings ...)
  2021-05-13 11:30 ` Geert Uytterhoeven
@ 2021-09-21 18:08 ` dann frazier
  2021-09-21 18:35   ` Mark Brown
  5 siblings, 1 reply; 13+ messages in thread
From: dann frazier @ 2021-09-21 18:08 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Suzuki K Poulose, linux-arm-kernel

On Wed, Apr 28, 2021 at 01:12:31PM +0100, Mark Brown wrote:
> The arm64 code allocates an internal constant to every CPU feature it can
> detect, distinct from the public hwcap numbers we use to expose some
> features to userspace. Currently this is maintained manually which is an
> irritating source of conflicts when working on new features, to avoid this
> replace the header with a simple text file listing the names we've assigned
> and sort it to minimise conflicts.

Thanks for doing this Mark, speaking as someone who has had to
backport a lot of these patches to a distro kernel :)

> As part of doing this we also do the Kbuild hookup required to hook up
> an arch tools directory and to generate header files in there.
> 
> This will result in a renumbering and reordering of the existing constants,
> since they are all internal only the values should not be important. The
> reordering will impact the order in which some steps in enumeration handle
> features but the algorithm is not intended to depend on this and I haven't
> seen any issues when testing.

Unfortunately I believe I've hit a regression[*] due to such an
ordering dependency. UNMAP_KERNEL_AT_EL0 currently needs to be
processed after WORKAROUND_CAVIUM_27456. ThunderX systems are
incompatible with KPTI, so unmap_kernel_at_el0() bails if
WORKAROUND_CAVIUM_27456 is set. Because of the sorting,
WORKAROUND_CAVIUM_27456 will not yet have been considered when
unmap_kernel_at_el0() checks for it, so the kernel tries to
run w/ KPTI - and quickly falls over.

I've verified that reordering cpucaps to move WORKAROUND_CAVIUM_27456
just above UNMAP_KERNEL_AT_EL0 restores the old behavior. I'm not sure
of the right way to address this - perhaps unmap_kernel_at_el0() could
check cavium_erratum_27456_cpus[] directly instead of keying on the
ARM64_WORKAROUND_CAVIUM_27456 cap?

  -dann

[*] https://bugs.launchpad.net/bugs/1942633

> Due to the UAO cpucap having been removed in
> the past we end up with ARM64_NCAPS being 1 smaller than it was before.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/Makefile              |  3 ++
>  arch/arm64/include/asm/Kbuild    |  2 +
>  arch/arm64/include/asm/cpucaps.h | 74 --------------------------------
>  arch/arm64/tools/Makefile        | 22 ++++++++++
>  arch/arm64/tools/cpucaps         | 65 ++++++++++++++++++++++++++++
>  arch/arm64/tools/gen-cpucaps.awk | 40 +++++++++++++++++
>  6 files changed, 132 insertions(+), 74 deletions(-)
>  delete mode 100644 arch/arm64/include/asm/cpucaps.h
>  create mode 100644 arch/arm64/tools/Makefile
>  create mode 100644 arch/arm64/tools/cpucaps
>  create mode 100755 arch/arm64/tools/gen-cpucaps.awk
> 
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 7ef44478560d..b52481f0605d 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -175,6 +175,9 @@ vdso_install:
>  	$(if $(CONFIG_COMPAT_VDSO), \
>  		$(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso32 $@)
>  
> +archprepare:
> +	$(Q)$(MAKE) $(build)=arch/arm64/tools kapi
> +
>  # We use MRPROPER_FILES and CLEAN_FILES now
>  archclean:
>  	$(Q)$(MAKE) $(clean)=$(boot)
> diff --git a/arch/arm64/include/asm/Kbuild b/arch/arm64/include/asm/Kbuild
> index 73aa25843f65..64202010b700 100644
> --- a/arch/arm64/include/asm/Kbuild
> +++ b/arch/arm64/include/asm/Kbuild
> @@ -4,3 +4,5 @@ generic-y += mcs_spinlock.h
>  generic-y += qrwlock.h
>  generic-y += qspinlock.h
>  generic-y += user.h
> +
> +generated-y += cpucaps.h
> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
> deleted file mode 100644
> index b0c5eda0498f..000000000000
> --- a/arch/arm64/include/asm/cpucaps.h
> +++ /dev/null
> @@ -1,74 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * arch/arm64/include/asm/cpucaps.h
> - *
> - * Copyright (C) 2016 ARM Ltd.
> - */
> -#ifndef __ASM_CPUCAPS_H
> -#define __ASM_CPUCAPS_H
> -
> -#define ARM64_WORKAROUND_CLEAN_CACHE		0
> -#define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE	1
> -#define ARM64_WORKAROUND_845719			2
> -#define ARM64_HAS_SYSREG_GIC_CPUIF		3
> -#define ARM64_HAS_PAN				4
> -#define ARM64_HAS_LSE_ATOMICS			5
> -#define ARM64_WORKAROUND_CAVIUM_23154		6
> -#define ARM64_WORKAROUND_834220			7
> -#define ARM64_HAS_NO_HW_PREFETCH		8
> -#define ARM64_HAS_VIRT_HOST_EXTN		11
> -#define ARM64_WORKAROUND_CAVIUM_27456		12
> -#define ARM64_HAS_32BIT_EL0			13
> -#define ARM64_SPECTRE_V3A			14
> -#define ARM64_HAS_CNP				15
> -#define ARM64_HAS_NO_FPSIMD			16
> -#define ARM64_WORKAROUND_REPEAT_TLBI		17
> -#define ARM64_WORKAROUND_QCOM_FALKOR_E1003	18
> -#define ARM64_WORKAROUND_858921			19
> -#define ARM64_WORKAROUND_CAVIUM_30115		20
> -#define ARM64_HAS_DCPOP				21
> -#define ARM64_SVE				22
> -#define ARM64_UNMAP_KERNEL_AT_EL0		23
> -#define ARM64_SPECTRE_V2			24
> -#define ARM64_HAS_RAS_EXTN			25
> -#define ARM64_WORKAROUND_843419			26
> -#define ARM64_HAS_CACHE_IDC			27
> -#define ARM64_HAS_CACHE_DIC			28
> -#define ARM64_HW_DBM				29
> -#define ARM64_SPECTRE_V4			30
> -#define ARM64_MISMATCHED_CACHE_TYPE		31
> -#define ARM64_HAS_STAGE2_FWB			32
> -#define ARM64_HAS_CRC32				33
> -#define ARM64_SSBS				34
> -#define ARM64_WORKAROUND_1418040		35
> -#define ARM64_HAS_SB				36
> -#define ARM64_WORKAROUND_SPECULATIVE_AT		37
> -#define ARM64_HAS_ADDRESS_AUTH_ARCH		38
> -#define ARM64_HAS_ADDRESS_AUTH_IMP_DEF		39
> -#define ARM64_HAS_GENERIC_AUTH_ARCH		40
> -#define ARM64_HAS_GENERIC_AUTH_IMP_DEF		41
> -#define ARM64_HAS_IRQ_PRIO_MASKING		42
> -#define ARM64_HAS_DCPODP			43
> -#define ARM64_WORKAROUND_1463225		44
> -#define ARM64_WORKAROUND_CAVIUM_TX2_219_TVM	45
> -#define ARM64_WORKAROUND_CAVIUM_TX2_219_PRFM	46
> -#define ARM64_WORKAROUND_1542419		47
> -#define ARM64_HAS_E0PD				48
> -#define ARM64_HAS_RNG				49
> -#define ARM64_HAS_AMU_EXTN			50
> -#define ARM64_HAS_ADDRESS_AUTH			51
> -#define ARM64_HAS_GENERIC_AUTH			52
> -#define ARM64_HAS_32BIT_EL1			53
> -#define ARM64_BTI				54
> -#define ARM64_HAS_ARMv8_4_TTL			55
> -#define ARM64_HAS_TLB_RANGE			56
> -#define ARM64_MTE				57
> -#define ARM64_WORKAROUND_1508412		58
> -#define ARM64_HAS_LDAPR				59
> -#define ARM64_KVM_PROTECTED_MODE		60
> -#define ARM64_WORKAROUND_NVIDIA_CARMEL_CNP	61
> -#define ARM64_HAS_EPAN				62
> -
> -#define ARM64_NCAPS				63
> -
> -#endif /* __ASM_CPUCAPS_H */
> diff --git a/arch/arm64/tools/Makefile b/arch/arm64/tools/Makefile
> new file mode 100644
> index 000000000000..932b4fe5c768
> --- /dev/null
> +++ b/arch/arm64/tools/Makefile
> @@ -0,0 +1,22 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +gen := arch/$(ARCH)/include/generated
> +kapi := $(gen)/asm
> +
> +kapi-hdrs-y := $(kapi)/cpucaps.h
> +
> +targets += $(addprefix ../../../,$(gen-y) $(kapi-hdrs-y))
> +
> +PHONY += kapi
> +
> +kapi:   $(kapi-hdrs-y) $(gen-y)
> +
> +# Create output directory if not already present
> +_dummy := $(shell [ -d '$(kapi)' ] || mkdir -p '$(kapi)')
> +
> +quiet_cmd_gen_cpucaps = GEN     $@
> +      cmd_gen_cpucaps = mkdir -p $(dir $@) && \
> +                     $(AWK) -f $(filter-out $(PHONY),$^) > $@
> +
> +$(kapi)/cpucaps.h: $(src)/gen-cpucaps.awk $(src)/cpucaps FORCE
> +	$(call if_changed,gen_cpucaps)
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> new file mode 100644
> index 000000000000..21fbdda7086e
> --- /dev/null
> +++ b/arch/arm64/tools/cpucaps
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Internal CPU capabilities constants, keep this list sorted
> +
> +BTI
> +HAS_32BIT_EL0
> +HAS_32BIT_EL1
> +HAS_ADDRESS_AUTH
> +HAS_ADDRESS_AUTH_ARCH
> +HAS_ADDRESS_AUTH_IMP_DEF
> +HAS_AMU_EXTN
> +HAS_ARMv8_4_TTL
> +HAS_CACHE_DIC
> +HAS_CACHE_IDC
> +HAS_CNP
> +HAS_CRC32
> +HAS_DCPODP
> +HAS_DCPOP
> +HAS_E0PD
> +HAS_EPAN
> +HAS_GENERIC_AUTH
> +HAS_GENERIC_AUTH_ARCH
> +HAS_GENERIC_AUTH_IMP_DEF
> +HAS_IRQ_PRIO_MASKING
> +HAS_LDAPR
> +HAS_LSE_ATOMICS
> +HAS_NO_FPSIMD
> +HAS_NO_HW_PREFETCH
> +HAS_PAN
> +HAS_RAS_EXTN
> +HAS_RNG
> +HAS_SB
> +HAS_STAGE2_FWB
> +HAS_SYSREG_GIC_CPUIF
> +HAS_TLB_RANGE
> +HAS_VIRT_HOST_EXTN
> +HW_DBM
> +KVM_PROTECTED_MODE
> +MISMATCHED_CACHE_TYPE
> +MTE
> +SPECTRE_V2
> +SPECTRE_V3A
> +SPECTRE_V4
> +SSBS
> +SVE
> +UNMAP_KERNEL_AT_EL0
> +WORKAROUND_834220
> +WORKAROUND_843419
> +WORKAROUND_845719
> +WORKAROUND_858921
> +WORKAROUND_1418040
> +WORKAROUND_1463225
> +WORKAROUND_1508412
> +WORKAROUND_1542419
> +WORKAROUND_CAVIUM_23154
> +WORKAROUND_CAVIUM_27456
> +WORKAROUND_CAVIUM_30115
> +WORKAROUND_CAVIUM_TX2_219_PRFM
> +WORKAROUND_CAVIUM_TX2_219_TVM
> +WORKAROUND_CLEAN_CACHE
> +WORKAROUND_DEVICE_LOAD_ACQUIRE
> +WORKAROUND_NVIDIA_CARMEL_CNP
> +WORKAROUND_QCOM_FALKOR_E1003
> +WORKAROUND_REPEAT_TLBI
> +WORKAROUND_SPECULATIVE_AT
> diff --git a/arch/arm64/tools/gen-cpucaps.awk b/arch/arm64/tools/gen-cpucaps.awk
> new file mode 100755
> index 000000000000..18737a1ce044
> --- /dev/null
> +++ b/arch/arm64/tools/gen-cpucaps.awk
> @@ -0,0 +1,40 @@
> +#!/bin/awk -f
> +# SPDX-License-Identifier: GPL-2.0
> +# gen-cpucaps.awk: arm64 cpucaps header generator
> +#
> +# Usage: awk -f gen-cpucaps.awk cpucaps.txt
> +
> +# Log an error and terminate
> +function fatal(msg) {
> +	print "Error at line " NR ": " msg > "/dev/stderr"
> +	exit 1
> +}
> +
> +# skip blank lines and comment lines
> +/^$/ { next }
> +/^#/ { next }
> +
> +BEGIN {
> +	print "#ifndef __ASM_CPUCAPS_H"
> +	print "#define __ASM_CPUCAPS_H"
> +	print ""
> +	print "/* Generated file - do not edit */"
> +	cap_num = 0
> +	print ""
> +}
> +
> +/^[vA-Z0-9_]+$/ {
> +	printf("#define ARM64_%-30s\t%d\n", $0, cap_num++)
> +	next
> +}
> +
> +END {
> +	printf("#define ARM64_NCAPS\t\t\t\t%d\n", cap_num)
> +	print ""
> +	print "#endif"
> +}
> +
> +# Any lines not handled by previous rules are unexpected
> +{
> +	fatal("unhandled statement")
> +}

_______________________________________________
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] 13+ messages in thread

* Re: [PATCH] arm64: Generate cpucaps.h
  2021-09-21 18:08 ` dann frazier
@ 2021-09-21 18:35   ` Mark Brown
  2021-09-21 21:09     ` Suzuki K Poulose
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2021-09-21 18:35 UTC (permalink / raw)
  To: dann frazier
  Cc: Catalin Marinas, Will Deacon, Suzuki K Poulose, linux-arm-kernel


[-- Attachment #1.1: Type: text/plain, Size: 1683 bytes --]

On Tue, Sep 21, 2021 at 12:08:11PM -0600, dann frazier wrote:
> On Wed, Apr 28, 2021 at 01:12:31PM +0100, Mark Brown wrote:

> > This will result in a renumbering and reordering of the existing constants,
> > since they are all internal only the values should not be important. The
> > reordering will impact the order in which some steps in enumeration handle
> > features but the algorithm is not intended to depend on this and I haven't
> > seen any issues when testing.

> Unfortunately I believe I've hit a regression[*] due to such an
> ordering dependency. UNMAP_KERNEL_AT_EL0 currently needs to be
> processed after WORKAROUND_CAVIUM_27456. ThunderX systems are
> incompatible with KPTI, so unmap_kernel_at_el0() bails if
> WORKAROUND_CAVIUM_27456 is set. Because of the sorting,
> WORKAROUND_CAVIUM_27456 will not yet have been considered when
> unmap_kernel_at_el0() checks for it, so the kernel tries to
> run w/ KPTI - and quickly falls over.

> I've verified that reordering cpucaps to move WORKAROUND_CAVIUM_27456
> just above UNMAP_KERNEL_AT_EL0 restores the old behavior. I'm not sure
> of the right way to address this - perhaps unmap_kernel_at_el0() could
> check cavium_erratum_27456_cpus[] directly instead of keying on the
> ARM64_WORKAROUND_CAVIUM_27456 cap?

Ugh, right.  Another option would be to do something like rename to
CAVIUM_WORKAROUND_27456 which would be inconsistent naming and but would
sort things in a way that gets the checks to run in the order we're
relying on but either works for me.  Neither is great.

It'd be really good to get more coverage of the enterprise systems in
KernelCI or similar so we can catch issues like this more easily.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
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] 13+ messages in thread

* Re: [PATCH] arm64: Generate cpucaps.h
  2021-09-21 18:35   ` Mark Brown
@ 2021-09-21 21:09     ` Suzuki K Poulose
  2021-09-22 13:45       ` dann frazier
  0 siblings, 1 reply; 13+ messages in thread
From: Suzuki K Poulose @ 2021-09-21 21:09 UTC (permalink / raw)
  To: Mark Brown, dann frazier; +Cc: Catalin Marinas, Will Deacon, linux-arm-kernel

On 21/09/2021 19:35, Mark Brown wrote:
> On Tue, Sep 21, 2021 at 12:08:11PM -0600, dann frazier wrote:
>> On Wed, Apr 28, 2021 at 01:12:31PM +0100, Mark Brown wrote:
> 
>>> This will result in a renumbering and reordering of the existing constants,
>>> since they are all internal only the values should not be important. The
>>> reordering will impact the order in which some steps in enumeration handle
>>> features but the algorithm is not intended to depend on this and I haven't
>>> seen any issues when testing.
> 
>> Unfortunately I believe I've hit a regression[*] due to such an
>> ordering dependency. UNMAP_KERNEL_AT_EL0 currently needs to be
>> processed after WORKAROUND_CAVIUM_27456. ThunderX systems are
>> incompatible with KPTI, so unmap_kernel_at_el0() bails if
>> WORKAROUND_CAVIUM_27456 is set. Because of the sorting,
>> WORKAROUND_CAVIUM_27456 will not yet have been considered when
>> unmap_kernel_at_el0() checks for it, so the kernel tries to
>> run w/ KPTI - and quickly falls over.
> 
>> I've verified that reordering cpucaps to move WORKAROUND_CAVIUM_27456
>> just above UNMAP_KERNEL_AT_EL0 restores the old behavior. I'm not sure
>> of the right way to address this - perhaps unmap_kernel_at_el0() could
>> check cavium_erratum_27456_cpus[] directly instead of keying on the
>> ARM64_WORKAROUND_CAVIUM_27456 cap?

Given that these capabilities are LOCAL_CPU scope, and the systems in
question never have heterogeneous CPUs, you could replace

  cpus_have_const_cap() =>  this_cpu_has_cap()

in unmap_kernel_at_el0().

If they were heterogeneous, we would fail anyway, no matter what the
order was.

Suzuki

_______________________________________________
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] 13+ messages in thread

* Re: [PATCH] arm64: Generate cpucaps.h
  2021-09-21 21:09     ` Suzuki K Poulose
@ 2021-09-22 13:45       ` dann frazier
  0 siblings, 0 replies; 13+ messages in thread
From: dann frazier @ 2021-09-22 13:45 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: Mark Brown, Catalin Marinas, Will Deacon, linux-arm-kernel

On Tue, Sep 21, 2021 at 10:09:12PM +0100, Suzuki K Poulose wrote:
> On 21/09/2021 19:35, Mark Brown wrote:
> > On Tue, Sep 21, 2021 at 12:08:11PM -0600, dann frazier wrote:
> > > On Wed, Apr 28, 2021 at 01:12:31PM +0100, Mark Brown wrote:
> > 
> > > > This will result in a renumbering and reordering of the existing constants,
> > > > since they are all internal only the values should not be important. The
> > > > reordering will impact the order in which some steps in enumeration handle
> > > > features but the algorithm is not intended to depend on this and I haven't
> > > > seen any issues when testing.
> > 
> > > Unfortunately I believe I've hit a regression[*] due to such an
> > > ordering dependency. UNMAP_KERNEL_AT_EL0 currently needs to be
> > > processed after WORKAROUND_CAVIUM_27456. ThunderX systems are
> > > incompatible with KPTI, so unmap_kernel_at_el0() bails if
> > > WORKAROUND_CAVIUM_27456 is set. Because of the sorting,
> > > WORKAROUND_CAVIUM_27456 will not yet have been considered when
> > > unmap_kernel_at_el0() checks for it, so the kernel tries to
> > > run w/ KPTI - and quickly falls over.
> > 
> > > I've verified that reordering cpucaps to move WORKAROUND_CAVIUM_27456
> > > just above UNMAP_KERNEL_AT_EL0 restores the old behavior. I'm not sure
> > > of the right way to address this - perhaps unmap_kernel_at_el0() could
> > > check cavium_erratum_27456_cpus[] directly instead of keying on the
> > > ARM64_WORKAROUND_CAVIUM_27456 cap?
> 
> Given that these capabilities are LOCAL_CPU scope, and the systems in
> question never have heterogeneous CPUs, you could replace
> 
>  cpus_have_const_cap() =>  this_cpu_has_cap()
> 
> in unmap_kernel_at_el0().
> 
> If they were heterogeneous, we would fail anyway, no matter what the
> order was.

That seems to me to be the simplest approach, and I've verified it
addresses the problem. I'll prepare a patch. Thanks Suzuki.

  -dann

_______________________________________________
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] 13+ messages in thread

end of thread, other threads:[~2021-09-22 13:48 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-28 12:12 [PATCH] arm64: Generate cpucaps.h Mark Brown
2021-04-29 12:32 ` Catalin Marinas
2021-05-04 11:43 ` Mark Rutland
2021-05-10 12:55 ` Catalin Marinas
2021-05-13  5:05 ` Anshuman Khandual
2021-05-13 11:51   ` Mark Rutland
2021-05-13 12:45   ` Mark Brown
2021-05-13 11:30 ` Geert Uytterhoeven
2021-05-13 13:03   ` Mark Brown
2021-09-21 18:08 ` dann frazier
2021-09-21 18:35   ` Mark Brown
2021-09-21 21:09     ` Suzuki K Poulose
2021-09-22 13:45       ` dann frazier

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.