[PATCH v2 0/5] arm64: Fix behavior of maxcpus=n

[PATCH v2 0/5] arm64: Fix behavior of maxcpus=n

[Suzuki K Poulose]

This series is an attempt at fixing the maxcpus=n behavior
on arm64. So far we have disabled hotplugging a CPU > n,
when maxcpus=n is in effect, due to following reasons.

 1) Possible cpu feature incompatibilities with the new CPU
    in heterogeneous systems.
 2) New CPU requiring an errata work around which was not detected
    (and the code patched in) at boot time.
 3) Failure to initialise the PMU in case the supported CPUs are
    not online while probing the PMU.

(1) has been mostly solved with our early CPU feature verification
support. This series tries to address (2) & (3).

(2) is solved by iterating over the known erratas and checking if
the new CPU requires an errata not set in the cpu_hwcaps, failing
which, we kill the CPU. We plan to fix this properly by retaining
the CPU errata work arounds and apply the required at runtime.

(3) is ignored and will not be fixed as there is no reliable way of
knowing if there would be a CPU that will be online to support the
PMU.

In the process, also restores the capability to check GIC interface settings
by the firmware on individual CPUs.

Tested on Juno with maxcpus=2 (enables A57 cores and A57-PMU) and
maxcpus=1 (disables both A57 cores and A57-PMU).

This series applies on 4.6-rc3 + "arm64: Support for systems without AArch32 state" [1].

The tree is available here :
	git://linux-arm.org/linux-skp.git maxcpus/v2

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/422979.html

Changes since V1:
 - Define default scope for capabilities.
 - WARN_ON !SCOPE_CPU for midr_check.

Marc Zyngier (2):
  arm64: Allow a capability to be checked on a single CPU
  irqchip/gic: Restore CPU interface checking

Suzuki K Poulose (3):
  arm64: cpufeature: Add scope for capability check
  arm64: Verify CPU errata work arounds on hotplugged CPU
  arm64: Fix behavior of maxcpus=N

 arch/arm64/include/asm/cpufeature.h |   12 +++++++-
 arch/arm64/kernel/cpu_errata.c      |   24 ++++++++++++++-
 arch/arm64/kernel/cpufeature.c      |   58 +++++++++++++++++++++++++----------
 arch/arm64/kernel/smp.c             |   18 +----------
 drivers/irqchip/irq-gic.c           |    5 ++-
 5 files changed, 78 insertions(+), 39 deletions(-)

-- 
1.7.9.5

[PATCH v2 1/5] arm64: cpufeature: Add scope for capability check

[Suzuki K Poulose]

Add scope parameter to the arm64_cpu_capabilities::matches(), so that
this can be reused for checking the capability on a given CPU vs the
system wide. The system uses the default scope associated with the
capability for initialising the CPU_HWCAPs and ELF_HWCAPs.

Cc: James Morse <james.morse@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Andre Przywara <andre.przywara@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>

---
Changes since V1:
 - Add a default scope for capabilities used by the system checks.
 - Add WARN_ON() for !SCOPE_CPU for midr checks
---
 arch/arm64/include/asm/cpufeature.h |    9 ++++++-
 arch/arm64/kernel/cpu_errata.c      |    4 ++-
 arch/arm64/kernel/cpufeature.c      |   46 ++++++++++++++++++++++-------------
 3 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index ca8fb4b..a5a6502 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -78,10 +78,17 @@ struct arm64_ftr_reg {
 	struct arm64_ftr_bits	*ftr_bits;
 };
 
+/* scope of capability check */
+enum {
+	SCOPE_SYSTEM,
+	SCOPE_CPU,
+};
+
 struct arm64_cpu_capabilities {
 	const char *desc;
 	u16 capability;
-	bool (*matches)(const struct arm64_cpu_capabilities *);
+	int def_scope;			/* default scope */
+	bool (*matches)(const struct arm64_cpu_capabilities *caps, int scope);
 	void (*enable)(void *);		/* Called on all active CPUs */
 	union {
 		struct {	/* To be used for erratum handling only */
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 06afd04..e171a14 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -22,14 +22,16 @@
 #include <asm/cpufeature.h>
 
 static bool __maybe_unused
-is_affected_midr_range(const struct arm64_cpu_capabilities *entry)
+is_affected_midr_range(const struct arm64_cpu_capabilities *entry, int scope)
 {
+	WARN_ON(scope != SCOPE_CPU);
 	return MIDR_IS_CPU_MODEL_RANGE(read_cpuid_id(), entry->midr_model,
 				       entry->midr_range_min,
 				       entry->midr_range_max);
 }
 
 #define MIDR_RANGE(model, min, max) \
+	.def_scope = SCOPE_CPU, \
 	.matches = is_affected_midr_range, \
 	.midr_model = model, \
 	.midr_range_min = min, \
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 8c46621..db392c5 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -71,7 +71,9 @@ DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
 
 /* meta feature for alternatives */
 static bool __maybe_unused
-cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry);
+cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused);
+
+static u64 __raw_read_system_reg(u32 sys_id);
 
 static struct arm64_ftr_bits ftr_id_aa64isar0[] = {
 	ARM64_FTR_BITS(FTR_STRICT, FTR_EXACT, 32, 32, 0),
@@ -633,19 +635,23 @@ feature_matches(u64 reg, const struct arm64_cpu_capabilities *entry)
 }
 
 static bool
-has_cpuid_feature(const struct arm64_cpu_capabilities *entry)
+has_cpuid_feature(const struct arm64_cpu_capabilities *entry, int scope)
 {
 	u64 val;
 
-	val = read_system_reg(entry->sys_reg);
+	if (scope == SCOPE_SYSTEM)
+		val = read_system_reg(entry->sys_reg);
+	else
+		val = __raw_read_system_reg(entry->sys_reg);
+
 	return feature_matches(val, entry);
 }
 
-static bool has_useable_gicv3_cpuif(const struct arm64_cpu_capabilities *entry)
+static bool has_useable_gicv3_cpuif(const struct arm64_cpu_capabilities *entry, int scope)
 {
 	bool has_sre;
 
-	if (!has_cpuid_feature(entry))
+	if (!has_cpuid_feature(entry, scope))
 		return false;
 
 	has_sre = gic_enable_sre();
@@ -656,7 +662,7 @@ static bool has_useable_gicv3_cpuif(const struct arm64_cpu_capabilities *entry)
 	return has_sre;
 }
 
-static bool has_no_hw_prefetch(const struct arm64_cpu_capabilities *entry)
+static bool has_no_hw_prefetch(const struct arm64_cpu_capabilities *entry, int __unused)
 {
 	u32 midr = read_cpuid_id();
 	u32 rv_min, rv_max;
@@ -668,7 +674,7 @@ static bool has_no_hw_prefetch(const struct arm64_cpu_capabilities *entry)
 	return MIDR_IS_CPU_MODEL_RANGE(midr, MIDR_THUNDERX, rv_min, rv_max);
 }
 
-static bool runs_at_el2(const struct arm64_cpu_capabilities *entry)
+static bool runs_at_el2(const struct arm64_cpu_capabilities *entry, int __unused)
 {
 	return is_kernel_in_hyp_mode();
 }
@@ -677,6 +683,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 	{
 		.desc = "GIC system register CPU interface",
 		.capability = ARM64_HAS_SYSREG_GIC_CPUIF,
+		.def_scope = SCOPE_SYSTEM,
 		.matches = has_useable_gicv3_cpuif,
 		.sys_reg = SYS_ID_AA64PFR0_EL1,
 		.field_pos = ID_AA64PFR0_GIC_SHIFT,
@@ -687,6 +694,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 	{
 		.desc = "Privileged Access Never",
 		.capability = ARM64_HAS_PAN,
+		.def_scope = SCOPE_SYSTEM,
 		.matches = has_cpuid_feature,
 		.sys_reg = SYS_ID_AA64MMFR1_EL1,
 		.field_pos = ID_AA64MMFR1_PAN_SHIFT,
@@ -699,6 +707,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 	{
 		.desc = "LSE atomic instructions",
 		.capability = ARM64_HAS_LSE_ATOMICS,
+		.def_scope = SCOPE_SYSTEM,
 		.matches = has_cpuid_feature,
 		.sys_reg = SYS_ID_AA64ISAR0_EL1,
 		.field_pos = ID_AA64ISAR0_ATOMICS_SHIFT,
@@ -709,12 +718,14 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 	{
 		.desc = "Software prefetching using PRFM",
 		.capability = ARM64_HAS_NO_HW_PREFETCH,
+		.def_scope = SCOPE_SYSTEM,
 		.matches = has_no_hw_prefetch,
 	},
 #ifdef CONFIG_ARM64_UAO
 	{
 		.desc = "User Access Override",
 		.capability = ARM64_HAS_UAO,
+		.def_scope = SCOPE_SYSTEM,
 		.matches = has_cpuid_feature,
 		.sys_reg = SYS_ID_AA64MMFR2_EL1,
 		.field_pos = ID_AA64MMFR2_UAO_SHIFT,
@@ -725,17 +736,20 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 #ifdef CONFIG_ARM64_PAN
 	{
 		.capability = ARM64_ALT_PAN_NOT_UAO,
+		.def_scope = SCOPE_SYSTEM,
 		.matches = cpufeature_pan_not_uao,
 	},
 #endif /* CONFIG_ARM64_PAN */
 	{
 		.desc = "Virtualization Host Extensions",
 		.capability = ARM64_HAS_VIRT_HOST_EXTN,
+		.def_scope = SCOPE_SYSTEM,
 		.matches = runs_at_el2,
 	},
 	{
 		.desc = "32-bit EL0 Support",
 		.capability = ARM64_HAS_32BIT_EL0,
+		.def_scope = SCOPE_SYSTEM,
 		.matches = has_cpuid_feature,
 		.sys_reg = SYS_ID_AA64PFR0_EL1,
 		.sign = FTR_UNSIGNED,
@@ -748,6 +762,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 #define HWCAP_CAP(reg, field, s, min_value, type, cap)	\
 	{							\
 		.desc = #cap,					\
+		.def_scope = SCOPE_SYSTEM,			\
 		.matches = has_cpuid_feature,			\
 		.sys_reg = reg,					\
 		.field_pos = field,				\
@@ -830,7 +845,7 @@ static bool cpus_have_elf_hwcap(const struct arm64_cpu_capabilities *cap)
 static void __init setup_elf_hwcaps(const struct arm64_cpu_capabilities *hwcaps)
 {
 	for (; hwcaps->matches; hwcaps++)
-		if (hwcaps->matches(hwcaps))
+		if (hwcaps->matches(hwcaps, hwcaps->def_scope))
 			cap_set_elf_hwcap(hwcaps);
 }
 
@@ -838,7 +853,7 @@ void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
 			    const char *info)
 {
 	for (; caps->matches; caps++) {
-		if (!caps->matches(caps))
+		if (!caps->matches(caps, caps->def_scope))
 			continue;
 
 		if (!cpus_have_cap(caps->capability) && caps->desc)
@@ -929,28 +944,25 @@ static void
 verify_local_elf_hwcaps(const struct arm64_cpu_capabilities *caps)
 {
 
-	for (; caps->matches; caps++) {
-		if (!cpus_have_elf_hwcap(caps))
-			continue;
-		if (!feature_matches(__raw_read_system_reg(caps->sys_reg), caps)) {
+	for (; caps->matches; caps++)
+		if (cpus_have_elf_hwcap(caps) && !caps->matches(caps, SCOPE_CPU)) {
 			pr_crit("CPU%d: missing HWCAP: %s\n",
 					smp_processor_id(), caps->desc);
 			cpu_die_early();
 		}
-	}
 }
 
 static void
 verify_local_cpu_features(const struct arm64_cpu_capabilities *caps)
 {
 	for (; caps->matches; caps++) {
-		if (!cpus_have_cap(caps->capability) || !caps->sys_reg)
+		if (!cpus_have_cap(caps->capability))
 			continue;
 		/*
 		 * If the new CPU misses an advertised feature, we cannot proceed
 		 * further, park the cpu.
 		 */
-		if (!feature_matches(__raw_read_system_reg(caps->sys_reg), caps)) {
+		if (!caps->matches(caps, SCOPE_CPU)) {
 			pr_crit("CPU%d: missing feature: %s\n",
 					smp_processor_id(), caps->desc);
 			cpu_die_early();
@@ -1021,7 +1033,7 @@ void __init setup_cpu_features(void)
 }
 
 static bool __maybe_unused
-cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry)
+cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused)
 {
 	return (cpus_have_cap(ARM64_HAS_PAN) && !cpus_have_cap(ARM64_HAS_UAO));
 }
-- 
1.7.9.5

[PATCH v2 2/5] arm64: Allow a capability to be checked on a single CPU

[Suzuki K Poulose]

From: Marc Zyngier <marc.zyngier@arm.com>

Now that the capabilities are only available once all the CPUs
have booted, we're unable to check for a particular feature
in any subsystem that gets initialized before then.

In order to support this, introduce a local_cpu_has_cap() function
that tests for the presence of a given capability independently
of the whole framework.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/include/asm/cpufeature.h |    2 ++
 arch/arm64/kernel/cpufeature.c      |   11 +++++++++++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index a5a6502..c535aa7 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -109,6 +109,8 @@ struct arm64_cpu_capabilities {
 
 extern DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
 
+bool this_cpu_has_cap(unsigned int cap);
+
 static inline bool cpu_have_feature(unsigned int num)
 {
 	return elf_hwcap & (1UL << num);
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index db392c5..10aa58b 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1004,6 +1004,17 @@ static void __init setup_feature_capabilities(void)
 	enable_cpu_capabilities(arm64_features);
 }
 
+bool this_cpu_has_cap(unsigned int cap)
+{
+	const struct arm64_cpu_capabilities *caps = arm64_features;
+
+	for (caps = arm64_features; caps->desc; caps++)
+		if (caps->capability == cap && caps->matches)
+			return caps->matches(caps, SCOPE_CPU);
+
+	return false;
+}
+
 void __init setup_cpu_features(void)
 {
 	u32 cwg;
-- 
1.7.9.5

[PATCH v2 3/5] irqchip/gic: Restore CPU interface checking

[Suzuki K Poulose]

From: Marc Zyngier <marc.zyngier@arm.com>

When introducing the whole CPU feature detection framework,
we lost the capability to detect a mismatched GIC configuration
(using the GICv2 MMIO interface, but having the system register
interface enabled).

In order to solve this, use the new this_cpu_has_cap() helper.
Also move the check to the CPU interface path in order to catch
systems where the first CPU has been correctly configured,
but the secondaries are not.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/irqchip/irq-gic.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 282344b..095bb5b 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -55,7 +55,7 @@
 
 static void gic_check_cpu_features(void)
 {
-	WARN_TAINT_ONCE(cpus_have_cap(ARM64_HAS_SYSREG_GIC_CPUIF),
+	WARN_TAINT_ONCE(this_cpu_has_cap(ARM64_HAS_SYSREG_GIC_CPUIF),
 			TAINT_CPU_OUT_OF_SPEC,
 			"GICv3 system registers enabled, broken firmware!\n");
 }
@@ -490,6 +490,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 		 * Get what the GIC says our CPU mask is.
 		 */
 		BUG_ON(cpu >= NR_GIC_CPU_IF);
+		gic_check_cpu_features();
 		cpu_mask = gic_get_cpumask(gic);
 		gic_cpu_map[cpu] = cpu_mask;
 
@@ -1021,8 +1022,6 @@ static void __init __gic_init_bases(unsigned int gic_nr, int irq_start,
 
 	BUG_ON(gic_nr >= CONFIG_ARM_GIC_MAX_NR);
 
-	gic_check_cpu_features();
-
 	gic = &gic_data[gic_nr];
 
 	/* Initialize irq_chip */
-- 
1.7.9.5

[PATCH v2 4/5] arm64: Verify CPU errata work arounds on hotplugged CPU

[Suzuki K Poulose]

CPU Errata work arounds are detected and applied to the
kernel code at boot time and the data is then freed up.
If a new hotplugged CPU requires a work around which
was not applied at boot time, there is nothing we can
do but simply fail the booting.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Andre Przywara <andre.przywara@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/include/asm/cpufeature.h |    1 +
 arch/arm64/kernel/cpu_errata.c      |   20 ++++++++++++++++++++
 arch/arm64/kernel/cpufeature.c      |    1 +
 3 files changed, 22 insertions(+)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index c535aa7..0796975 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -193,6 +193,7 @@ void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
 			    const char *info);
 void check_local_cpu_errata(void);
 
+void verify_local_cpu_errata(void);
 void verify_local_cpu_capabilities(void);
 
 u64 read_system_reg(u32 id);
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index e171a14..9bcb32b 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -103,6 +103,26 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 	}
 };
 
+/*
+ * The CPU Errata work arounds are detected and applied at boot time
+ * and the related information is freed soon after. If the new CPU requires
+ * an errata not detected at boot, fail this CPU.
+ */
+void verify_local_cpu_errata(void)
+{
+	const struct arm64_cpu_capabilities *caps = arm64_errata;
+
+	for (; caps->matches; caps++)
+		if (!cpus_have_cap(caps->capability) &&
+			caps->matches(caps, SCOPE_CPU)) {
+			pr_crit("CPU%d: Requires work around for %s, not detected"
+					" at boot time\n",
+				smp_processor_id(),
+				caps->desc ? : "an erratum");
+			cpu_die_early();
+		}
+}
+
 void check_local_cpu_errata(void)
 {
 	update_cpu_capabilities(arm64_errata, "enabling workaround for");
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 10aa58b..898ff17 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -992,6 +992,7 @@ void verify_local_cpu_capabilities(void)
 	if (!sys_caps_initialised)
 		return;
 
+	verify_local_cpu_errata();
 	verify_local_cpu_features(arm64_features);
 	verify_local_elf_hwcaps(arm64_elf_hwcaps);
 	if (system_supports_32bit_el0())
-- 
1.7.9.5

[PATCH v2 5/5] arm64: Fix behavior of maxcpus=N

[Suzuki K Poulose]

maxcpu=n sets the number of CPUs activated at boot time to a max of n,
but allowing the remaining CPUs to be brought up later if the user
decides to do so. However, on arm64 due to various reasons, we disallowed
hotplugging CPUs beyond n, by marking them not present. Now that
we have checks in place to make sure the hotplugged CPUs have compatible
features with system and requires no new errata, relax the restriction.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: James Morse <james.morse@arm.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/kernel/smp.c |   18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index b2d5f4e..fa9dbaa 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -647,33 +647,18 @@ void __init smp_init_cpus(void)
 void __init smp_prepare_cpus(unsigned int max_cpus)
 {
 	int err;
-	unsigned int cpu, ncores = num_possible_cpus();
+	unsigned int cpu;
 
 	init_cpu_topology();
 
 	smp_store_cpu_info(smp_processor_id());
 
 	/*
-	 * are we trying to boot more cores than exist?
-	 */
-	if (max_cpus > ncores)
-		max_cpus = ncores;
-
-	/* Don't bother if we're effectively UP */
-	if (max_cpus <= 1)
-		return;
-
-	/*
 	 * Initialise the present map (which describes the set of CPUs
 	 * actually populated at the present time) and release the
 	 * secondaries from the bootloader.
-	 *
-	 * Make sure we online@most (max_cpus - 1) additional CPUs.
 	 */
-	max_cpus--;
 	for_each_possible_cpu(cpu) {
-		if (max_cpus == 0)
-			break;
 
 		if (cpu == smp_processor_id())
 			continue;
@@ -686,7 +671,6 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 			continue;
 
 		set_cpu_present(cpu, true);
-		max_cpus--;
 	}
 }
 
-- 
1.7.9.5

[PATCH v2 1/5] arm64: cpufeature: Add scope for capability check

[Will Deacon]

On Mon, Apr 18, 2016 at 05:35:30PM +0100, Suzuki K Poulose wrote:
> Add scope parameter to the arm64_cpu_capabilities::matches(), so that
> this can be reused for checking the capability on a given CPU vs the
> system wide. The system uses the default scope associated with the
> capability for initialising the CPU_HWCAPs and ELF_HWCAPs.
> 
> Cc: James Morse <james.morse@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Andre Przywara <andre.przywara@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> 
> ---
> Changes since V1:
>  - Add a default scope for capabilities used by the system checks.
>  - Add WARN_ON() for !SCOPE_CPU for midr checks
> ---
>  arch/arm64/include/asm/cpufeature.h |    9 ++++++-
>  arch/arm64/kernel/cpu_errata.c      |    4 ++-
>  arch/arm64/kernel/cpufeature.c      |   46 ++++++++++++++++++++++-------------
>  3 files changed, 40 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index ca8fb4b..a5a6502 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -78,10 +78,17 @@ struct arm64_ftr_reg {
>  	struct arm64_ftr_bits	*ftr_bits;
>  };
>  
> +/* scope of capability check */
> +enum {
> +	SCOPE_SYSTEM,
> +	SCOPE_CPU,
> +};

I think I actually prefer the GLOBAL/LOCAL naming, since SYSTEM is going
to be the scope you want when talking about all CPUs. Or maybe just
rename SCOPE_CPU to SCOPE_LOCAL_CPU.

We might want a preemptible() check when probing SCOPE_CPU properties,
too.

> +
>  struct arm64_cpu_capabilities {
>  	const char *desc;
>  	u16 capability;
> -	bool (*matches)(const struct arm64_cpu_capabilities *);
> +	int def_scope;			/* default scope */
> +	bool (*matches)(const struct arm64_cpu_capabilities *caps, int scope);
>  	void (*enable)(void *);		/* Called on all active CPUs */
>  	union {
>  		struct {	/* To be used for erratum handling only */
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 06afd04..e171a14 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -22,14 +22,16 @@
>  #include <asm/cpufeature.h>
>  
>  static bool __maybe_unused
> -is_affected_midr_range(const struct arm64_cpu_capabilities *entry)
> +is_affected_midr_range(const struct arm64_cpu_capabilities *entry, int scope)
>  {
> +	WARN_ON(scope != SCOPE_CPU);
>  	return MIDR_IS_CPU_MODEL_RANGE(read_cpuid_id(), entry->midr_model,
>  				       entry->midr_range_min,
>  				       entry->midr_range_max);
>  }
>  
>  #define MIDR_RANGE(model, min, max) \
> +	.def_scope = SCOPE_CPU, \
>  	.matches = is_affected_midr_range, \
>  	.midr_model = model, \
>  	.midr_range_min = min, \
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 8c46621..db392c5 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -71,7 +71,9 @@ DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
>  
>  /* meta feature for alternatives */
>  static bool __maybe_unused
> -cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry);
> +cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused);
> +
> +static u64 __raw_read_system_reg(u32 sys_id);

Can we not reorder the functions in this file to avoid the internal forward
declarations?

Will

[PATCH v2 1/5] arm64: cpufeature: Add scope for capability check

[Suzuki K Poulose]

On 20/04/16 12:28, Will Deacon wrote:
> On Mon, Apr 18, 2016 at 05:35:30PM +0100, Suzuki K Poulose wrote:
>> Add scope parameter to the arm64_cpu_capabilities::matches(), so that
>> this can be reused for checking the capability on a given CPU vs the
>> system wide. The system uses the default scope associated with the
>> capability for initialising the CPU_HWCAPs and ELF_HWCAPs.

>> +/* scope of capability check */
>> +enum {
>> +	SCOPE_SYSTEM,
>> +	SCOPE_CPU,
>> +};
>
> I think I actually prefer the GLOBAL/LOCAL naming, since SYSTEM is going
> to be the scope you want when talking about all CPUs. Or maybe just
> rename SCOPE_CPU to SCOPE_LOCAL_CPU.

OK

>
> We might want a preemptible() check when probing SCOPE_CPU properties,
> too.

Good point. The current users are all calling them from the CPU init phase,
where it is not preemptible. But it would be good to add a check to make sure
nobody violates this condition. Also, will add a comment for
"this_cpu_has_cap()" API to call it under !preemptible() state.
  

>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index 8c46621..db392c5 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -71,7 +71,9 @@ DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
>>
>>   /* meta feature for alternatives */
>>   static bool __maybe_unused
>> -cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry);
>> +cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused);
>> +
>> +static u64 __raw_read_system_reg(u32 sys_id);
>
> Can we not reorder the functions in this file to avoid the internal forward
> declarations?

We can. I had that in my initial version, but the patch looked a bit more complicated
with the code movement. I will bring it back and get rid of the declaration.


Cheers
Suzuki