Re: [PATCH v2 1/6] arm64: cpufeature: Allow early detect of specific features

From: Suzuki K Poulose
Date: Mon Jan 22 2018 - 09:45:11 EST


On 22/01/18 12:21, Julien Thierry wrote:


On 22/01/18 12:05, Suzuki K Poulose wrote:
On 17/01/18 11:54, Julien Thierry wrote:
From: Daniel Thompson <daniel.thompson@xxxxxxxxxx>

Currently it is not possible to detect features of the boot CPU
until the other CPUs have been brought up.

This prevents us from reacting to features of the boot CPU until
fairly late in the boot process. To solve this we allow a subset
of features (that are likely to be common to all clusters) to be
detected based on the boot CPU alone.

Signed-off-by: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
[julien.thierry@xxxxxxx: check non-boot cpu missing early features, avoid
ÂÂÂÂÂÂÂÂÂÂÂÂ duplicates between early features and normal
ÂÂÂÂÂÂÂÂÂÂÂÂ features]
Signed-off-by: Julien Thierry <julien.thierry@xxxxxxx>
Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
Cc: Will Deacon <will.deacon@xxxxxxx>
Cc: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
---
 arch/arm64/kernel/cpufeature.c | 69 ++++++++++++++++++++++++++++--------------
 1 file changed, 47 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index a73a592..6698404 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -52,6 +52,8 @@
 DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
 EXPORT_SYMBOL(cpu_hwcaps);

+static void __init setup_early_feature_capabilities(void);
+
 /*
ÂÂ * Flag to indicate if we have computed the system wide
ÂÂ * capabilities based on the boot time active CPUs. This
@@ -542,6 +544,8 @@ void __init init_cpu_features(struct cpuinfo_arm64 *info)
ÂÂÂÂÂÂÂÂÂ init_cpu_ftr_reg(SYS_ZCR_EL1, info->reg_zcr);
ÂÂÂÂÂÂÂÂÂ sve_init_vq_map();
ÂÂÂÂÂ }
+
+ÂÂÂ setup_early_feature_capabilities();
 }

 static void update_cpu_ftr_reg(struct arm64_ftr_reg *reg, u64 new)
@@ -846,7 +850,7 @@ static bool has_no_fpsimd(const struct arm64_cpu_capabilities *entry, int __unus
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ ID_AA64PFR0_FP_SHIFT) < 0;
 }

-static const struct arm64_cpu_capabilities arm64_features[] = {
+static const struct arm64_cpu_capabilities arm64_early_features[] = {
ÂÂÂÂÂ {
ÂÂÂÂÂÂÂÂÂ .desc = "GIC system register CPU interface",
ÂÂÂÂÂÂÂÂÂ .capability = ARM64_HAS_SYSREG_GIC_CPUIF,
@@ -857,6 +861,10 @@ static bool has_no_fpsimd(const struct arm64_cpu_capabilities *entry, int __unus
ÂÂÂÂÂÂÂÂÂ .sign = FTR_UNSIGNED,
ÂÂÂÂÂÂÂÂÂ .min_field_value = 1,
ÂÂÂÂÂ },
+ÂÂÂ {}
+};
+


Julien,

One potential problem with this is that we don't have a way
to make this work on a "theoretical" system with and without
GIC system reg interface. i.e, if we don't have the CONFIG
enabled for using ICC system regs for IRQ flags, the kernel
could still panic. I understand this is not a "normal" configuration
but, may be we could make the panic option based on whether
we actually use the system regs early enough ?


I see, however I'm not sure what happens in the GIC drivers if we have a CPU running with a GICv3 and other CPUs with something else... But of course this is not technically limited by the arm64 capabilities handling.

What behaviour would you be looking for? A way to prevent the CPU to be brought up instead of panicking?


If we have the CONFIG enabled for using system regs, we can continue
to panic the system. Otherwise, we should ignore the mismatch early,
as we don't use the system register access unless all boot time active
CPUs have it.

In a nutshell, this is an early feature only if the CONFIG is enabled,
otherwise should fall back to the normal behavior.

Cheers
Suzuki