[PATCH 0/2] arm64: cpucap handling cleanups

[PATCH 0/2] arm64: cpucap handling cleanups

[Mark Rutland]

The recent changes to remove cpus_have_const_cap() left a period between
detecting cpucaps and patching alternatives where it is necessary to use
cpus_have_cap() to check that a cpucap has been detected. During this
period we do a few things, and there's a risk that code using a mixture
of cpus_have_cap() and alternative_has_cap_*() sees an inconsistent
state.

These patches rework the cpucap code to minimize the period between
detecting cpucaps and patching the relevant alternatives, deferring
other work until after the alternatives have been patched. This removes
the need for some code to use cpus_have_cap(), and allows them to use
common helper functions that may use alternative_has_cap_*() internally,
making them consistent with other kernel code.

This cleanup was requested by Will across:

  https://lore.kernel.org/linux-arm-kernel/20231127154818.GA8453@willie-the-truck/
  https://lore.kernel.org/linux-arm-kernel/20231127163103.GA8627@willie-the-truck/
  https://lore.kernel.org/linux-arm-kernel/20231127164127.GB8627@willie-the-truck/
  https://lore.kernel.org/linux-arm-kernel/20231128110339.GA9717@willie-the-truck/

The patches are based on the arm64 for-next/fixes branch, whose HEAD
commit is currently:

  f5259997f3e8d6ed ("arm64: Avoid enabling KPTI unnecessarily")

... which patch 1 is partially a cleanup for.

I've given both patches some testing in a few configurations (e.g.
with and without nVHE forced on the command line) to check that
detection and patching works as expected.

Mark.

Mark Rutland (2):
  arm64: Cleanup system cpucap handling
  arm64: Align boot cpucap handling with system cpucap handling

 arch/arm64/include/asm/cpufeature.h |   1 +
 arch/arm64/kernel/cpufeature.c      | 105 +++++++++++++++-------------
 arch/arm64/kernel/fpsimd.c          |   4 +-
 arch/arm64/kernel/smp.c             |  12 +---
 4 files changed, 64 insertions(+), 58 deletions(-)

-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] arm64: Cleanup system cpucap handling

[Mark Rutland]

Recent changes to remove cpus_have_const_cap() introduced new users of
cpus_have_cap() in the period between detecting system cpucaps and
patching alternatives. It would be preferable to defer these until after
the relevant cpucaps have been patched so that these can use the usual
feature check helper functions, which is clearer and has less risk of
accidental usage of code relying upon an alternative which has not yet
been patched.

This patch reworks the system-wide cpucap detection and patching to
minimize this transient period:

* The detection, enablement, and patching of system cpucaps is moved
  into a new setup_system_capabilities() function so that these can be
  grouped together more clearly, with no other functions called in the
  period between detection and patching. This is called from
  setup_system_features() before the subsequent checks that depend on
  the cpucaps.

  The logging of TTBR0 PAN and cpucaps with a mask is also moved here to
  keep these as close as possible to update_cpu_capabilities().

  At the same time, comments are corrected and improved to make the
  intent clearer.

* As hyp_mode_check() only tests system register values (not hwcaps) and
  must be called prior to patching, the call to hyp_mode_check() is
  moved before the call to setup_system_features().

* In setup_system_features(), the use of system_uses_ttbr0_pan() is
  restored, now that this occurs after alternatives are patched. This is
  a partial revert of commit:

    53d62e995d9eaed1 ("arm64: Avoid cpus_have_const_cap() for ARM64_HAS_PAN")

* In sve_setup() and sme_setup(), the use of system_supports_sve() and
  system_supports_sme() respectively are restored, now that these occur
  after alternatives are patched. This is a partial revert of commit:

    a76521d160284a1e ("arm64: Avoid cpus_have_const_cap() for ARM64_{SVE,SME,SME2,FA64}")

* In kpti_install_ng_mappings(), cpus_have_cap(ARM64_UNMAP_KERNEL_AT_EL0)
  is replaced with arm64_kernel_unmapped_at_el0(), now that this occurs
  after alternatives are patched. This is a cleanup to commit:

    f5259997f3e8d6ed ("arm64: Avoid enabling KPTI unnecessarily")

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/cpufeature.c | 48 ++++++++++++++++++++--------------
 arch/arm64/kernel/fpsimd.c     |  4 +--
 arch/arm64/kernel/smp.c        |  3 +--
 3 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 91d2d67149691..a065c259cfcf4 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1840,7 +1840,7 @@ static int __init __kpti_install_ng_mappings(void *__unused)
 static void __init kpti_install_ng_mappings(void)
 {
 	/* Check whether KPTI is going to be used */
-	if (!cpus_have_cap(ARM64_UNMAP_KERNEL_AT_EL0))
+	if (!arm64_kernel_unmapped_at_el0())
 		return;
 
 	/*
@@ -3338,23 +3338,40 @@ unsigned long cpu_get_elf_hwcap2(void)
 	return elf_hwcap[1];
 }
 
-void __init setup_system_features(void)
+static void __init setup_system_capabilities(void)
 {
-	int i;
 	/*
-	 * The system-wide safe feature feature register values have been
-	 * finalized. Finalize and log the available system capabilities.
+	 * The system-wide safe feature register values have been finalized.
+	 * Detect, enable, and patch alternatives for the available system
+	 * cpucaps.
 	 */
 	update_cpu_capabilities(SCOPE_SYSTEM);
-	if (IS_ENABLED(CONFIG_ARM64_SW_TTBR0_PAN) &&
-	    !cpus_have_cap(ARM64_HAS_PAN))
-		pr_info("emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
+	enable_cpu_capabilities(SCOPE_ALL & ~SCOPE_BOOT_CPU);
+	apply_alternatives_all();
 
 	/*
-	 * Enable all the available capabilities which have not been enabled
-	 * already.
+	 * Log any cpucaps with a cpumask as these aren't logged by
+	 * update_cpu_capabilities().
 	 */
-	enable_cpu_capabilities(SCOPE_ALL & ~SCOPE_BOOT_CPU);
+	for (int i = 0; i < ARM64_NCAPS; i++) {
+		const struct arm64_cpu_capabilities *caps = cpucap_ptrs[i];
+
+		if (caps && caps->cpus && caps->desc &&
+			cpumask_any(caps->cpus) < nr_cpu_ids)
+			pr_info("detected: %s on CPU%*pbl\n",
+				caps->desc, cpumask_pr_args(caps->cpus));
+	}
+
+	/*
+	 * TTBR0 PAN doesn't have its own cpucap, so log it manually.
+	 */
+	if (system_uses_ttbr0_pan())
+		pr_info("emulated: Privileged Access Never (PAN) using TTBR0_EL1 switching\n");
+}
+
+void __init setup_system_features(void)
+{
+	setup_system_capabilities();
 
 	kpti_install_ng_mappings();
 
@@ -3367,15 +3384,6 @@ void __init setup_system_features(void)
 	if (!cache_type_cwg())
 		pr_warn("No Cache Writeback Granule information, assuming %d\n",
 			ARCH_DMA_MINALIGN);
-
-	for (i = 0; i < ARM64_NCAPS; i++) {
-		const struct arm64_cpu_capabilities *caps = cpucap_ptrs[i];
-
-		if (caps && caps->cpus && caps->desc &&
-			cpumask_any(caps->cpus) < nr_cpu_ids)
-			pr_info("detected: %s on CPU%*pbl\n",
-				caps->desc, cpumask_pr_args(caps->cpus));
-	}
 }
 
 void __init setup_user_features(void)
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 1559c706d32d1..bc9384517db3d 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1171,7 +1171,7 @@ void __init sve_setup(void)
 	unsigned long b;
 	int max_bit;
 
-	if (!cpus_have_cap(ARM64_SVE))
+	if (!system_supports_sve())
 		return;
 
 	/*
@@ -1301,7 +1301,7 @@ void __init sme_setup(void)
 	struct vl_info *info = &vl_info[ARM64_VEC_SME];
 	int min_bit, max_bit;
 
-	if (!cpus_have_cap(ARM64_SME))
+	if (!system_supports_sme())
 		return;
 
 	/*
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index defbab84e9e5c..85384dc9a89d6 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -439,9 +439,8 @@ static void __init hyp_mode_check(void)
 void __init smp_cpus_done(unsigned int max_cpus)
 {
 	pr_info("SMP: Total of %d processors activated.\n", num_online_cpus());
-	setup_system_features();
 	hyp_mode_check();
-	apply_alternatives_all();
+	setup_system_features();
 	setup_user_features();
 	mark_linear_text_alias_ro();
 }
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] arm64: Align boot cpucap handling with system cpucap handling

[Mark Rutland]

Currently the detection+enablement of boot cpucaps is separate from the
patching of boot cpucap alternatives, which means there's a period where
cpus_have_cap($CAP) and alternative_has_cap($CAP) may be mismatched.

It would be preferable to manage the boot cpucaps in the same way as the
system cpucaps, both for clarity and to minimize the risk of accidental
usage of code relying upon an alternative which has not yet been
patched.

This patch aligns the handling of boot cpucaps with the handling of
system cpucaps:

* The existing setup_boot_cpu_capabilities() function is moved to be
  closer to the setup_system_capabilities() and setup_system_features()
  functions so that they're more clearly related and more likely to be
  updated together in future.

* The patching of boot cpucap alternatives is moved into
  setup_boot_cpu_capabilities(), immediately after boot cpucaps are
  detected and enabled.

* A new setup_boot_cpu_features() function is added to mirror
  setup_system_features(); this handles initialization of cpucap data
  structures and calls setup_boot_cpu_capabilities(). This makes
  init_cpu_features() a closer mirror to update_cpu_features(), and
  makes smp_prepare_boot_cpu() a closer mirror to smp_cpus_done().

Importantly, while these changes alter the structure of the code, they
retain the existing order of calls to:

  init_cpu_features(); // prefix initializing feature regs
  init_cpucap_indirect_list();
  detect_system_supports_pseudo_nmi();
  update_cpu_capabilities(SCOPE_BOOT_CPU | SCOPE_LOCAL_CPU);
  enable_cpu_capabilities(SCOPE_BOOT_CPU);
  apply_boot_alternatives();

... and hence there should be no functional change as a result of this
patch; this is purely a structural cleanup.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/cpufeature.h |  1 +
 arch/arm64/kernel/cpufeature.c      | 57 +++++++++++++++--------------
 arch/arm64/kernel/smp.c             |  9 +----
 3 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index f6d416fe49b09..2ea24c5cb900e 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -617,6 +617,7 @@ static inline bool id_aa64pfr1_mte(u64 pfr1)
 	return val >= ID_AA64PFR1_EL1_MTE_MTE2;
 }
 
+void __init setup_boot_cpu_features(void);
 void __init setup_system_features(void);
 void __init setup_user_features(void);
 
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index a065c259cfcf4..008164b7a2d49 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1081,25 +1081,6 @@ void __init init_cpu_features(struct cpuinfo_arm64 *info)
 
 	if (id_aa64pfr1_mte(info->reg_id_aa64pfr1))
 		init_cpu_ftr_reg(SYS_GMID_EL1, info->reg_gmid);
-
-	/*
-	 * Initialize the indirect array of CPU capabilities pointers before we
-	 * handle the boot CPU below.
-	 */
-	init_cpucap_indirect_list();
-
-	/*
-	 * Detect broken pseudo-NMI. Must be called _before_ the call to
-	 * setup_boot_cpu_capabilities() since it interacts with
-	 * can_use_gic_priorities().
-	 */
-	detect_system_supports_pseudo_nmi();
-
-	/*
-	 * Detect and enable early CPU capabilities based on the boot CPU,
-	 * after we have initialised the CPU feature infrastructure.
-	 */
-	setup_boot_cpu_capabilities();
 }
 
 static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new)
@@ -3275,14 +3256,6 @@ void check_local_cpu_capabilities(void)
 		verify_local_cpu_capabilities();
 }
 
-static void __init setup_boot_cpu_capabilities(void)
-{
-	/* Detect capabilities with either SCOPE_BOOT_CPU or SCOPE_LOCAL_CPU */
-	update_cpu_capabilities(SCOPE_BOOT_CPU | SCOPE_LOCAL_CPU);
-	/* Enable the SCOPE_BOOT_CPU capabilities alone right away */
-	enable_cpu_capabilities(SCOPE_BOOT_CPU);
-}
-
 bool this_cpu_has_cap(unsigned int n)
 {
 	if (!WARN_ON(preemptible()) && n < ARM64_NCAPS) {
@@ -3338,6 +3311,36 @@ unsigned long cpu_get_elf_hwcap2(void)
 	return elf_hwcap[1];
 }
 
+static void __init setup_boot_cpu_capabilities(void)
+{
+	/*
+	 * The boot CPU's feature register values have been recorded. Detect
+	 * boot cpucaps and local cpucaps for the boot CPU, then enable and
+	 * patch alternatives for the available boot cpucaps.
+	 */
+	update_cpu_capabilities(SCOPE_BOOT_CPU | SCOPE_LOCAL_CPU);
+	enable_cpu_capabilities(SCOPE_BOOT_CPU);
+	apply_boot_alternatives();
+}
+
+void __init setup_boot_cpu_features(void)
+{
+	/*
+	 * Initialize the indirect array of CPU capabilities pointers before we
+	 * handle the boot CPU.
+	 */
+	init_cpucap_indirect_list();
+
+	/*
+	 * Detect broken pseudo-NMI. Must be called _before_ the call to
+	 * setup_boot_cpu_capabilities() since it interacts with
+	 * can_use_gic_priorities().
+	 */
+	detect_system_supports_pseudo_nmi();
+
+	setup_boot_cpu_capabilities();
+}
+
 static void __init setup_system_capabilities(void)
 {
 	/*
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 85384dc9a89d6..4ced34f62dab5 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -453,14 +453,9 @@ void __init smp_prepare_boot_cpu(void)
 	 * freed shortly, so we must move over to the runtime per-cpu area.
 	 */
 	set_my_cpu_offset(per_cpu_offset(smp_processor_id()));
-	cpuinfo_store_boot_cpu();
 
-	/*
-	 * We now know enough about the boot CPU to apply the
-	 * alternatives that cannot wait until interrupt handling
-	 * and/or scheduling is enabled.
-	 */
-	apply_boot_alternatives();
+	cpuinfo_store_boot_cpu();
+	setup_boot_cpu_features();
 
 	/* Conditionally switch to GIC PMR for interrupt masking */
 	if (system_uses_irq_prio_masking())
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] arm64: cpucap handling cleanups

[Will Deacon]

On Tue, 12 Dec 2023 17:09:08 +0000, Mark Rutland wrote:
> The recent changes to remove cpus_have_const_cap() left a period between
> detecting cpucaps and patching alternatives where it is necessary to use
> cpus_have_cap() to check that a cpucap has been detected. During this
> period we do a few things, and there's a risk that code using a mixture
> of cpus_have_cap() and alternative_has_cap_*() sees an inconsistent
> state.
> 
> [...]

Applied to arm64 (for-next/cpufeature), thanks!

Basing the series on for-next/fixes makes it fiddly to manage, so I
dropped the kpti_install_ng_mappings() hunk from patch 1 and will
re-introduce it when I merge the branches together.

[1/2] arm64: Cleanup system cpucap handling
      https://git.kernel.org/arm64/c/63a2d92e1461
[2/2] arm64: Align boot cpucap handling with system cpucap handling
      https://git.kernel.org/arm64/c/eb15d707c252

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] arm64: cpucap handling cleanups

[Mark Rutland]

On Wed, Dec 13, 2023 at 05:25:18PM +0000, Will Deacon wrote:
> On Tue, 12 Dec 2023 17:09:08 +0000, Mark Rutland wrote:
> > The recent changes to remove cpus_have_const_cap() left a period between
> > detecting cpucaps and patching alternatives where it is necessary to use
> > cpus_have_cap() to check that a cpucap has been detected. During this
> > period we do a few things, and there's a risk that code using a mixture
> > of cpus_have_cap() and alternative_has_cap_*() sees an inconsistent
> > state.
> > 
> > [...]
> 
> Applied to arm64 (for-next/cpufeature), thanks!
> 
> Basing the series on for-next/fixes makes it fiddly to manage, so I
> dropped the kpti_install_ng_mappings() hunk from patch 1 and will
> re-introduce it when I merge the branches together.

Thanks, and sorry for the hassle!

The changes and resolution in that branch look good to me (and build and boot
cleanly); I'll go throw that on my test system and see if anything blows up
overnight.

Mark.

> [1/2] arm64: Cleanup system cpucap handling
>       https://git.kernel.org/arm64/c/63a2d92e1461
> [2/2] arm64: Align boot cpucap handling with system cpucap handling
>       https://git.kernel.org/arm64/c/eb15d707c252
> 
> Cheers,
> -- 
> Will
> 
> https://fixes.arm64.dev
> https://next.arm64.dev
> https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel