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

[PATCH 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.

(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 the A57 cores and A57-PMU).

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

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

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      |   49 +++++++++++++++++++++++------------
 arch/arm64/kernel/smp.c             |   10 -------
 drivers/irqchip/irq-gic.c           |    5 ++--
 5 files changed, 67 insertions(+), 33 deletions(-)

-- 
1.7.9.5

[PATCH 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. By default, the system uses
'system' wide values for setting 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>
---
 arch/arm64/include/asm/cpufeature.h |    8 +++++++-
 arch/arm64/kernel/cpu_errata.c      |    2 +-
 arch/arm64/kernel/cpufeature.c      |   37 +++++++++++++++++++----------------
 3 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index ca8fb4b..5f10344 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -78,10 +78,16 @@ 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 *);
+	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..2fd5780 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -22,7 +22,7 @@
 #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 __unused)
 {
 	return MIDR_IS_CPU_MODEL_RANGE(read_cpuid_id(), entry->midr_model,
 				       entry->midr_range_min,
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index c31be96..d3ee1df 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();
 }
@@ -830,7 +836,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, SCOPE_SYSTEM))
 			cap_set_elf_hwcap(hwcaps);
 }
 
@@ -838,7 +844,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, SCOPE_SYSTEM))
 			continue;
 
 		if (!cpus_have_cap(caps->capability) && caps->desc)
@@ -929,28 +935,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 +1024,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 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 5f10344..733b016 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -108,6 +108,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 d3ee1df..3ace45f 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -995,6 +995,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 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 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 |    2 +-
 arch/arm64/kernel/cpu_errata.c      |   22 ++++++++++++++++++++++
 arch/arm64/kernel/cpufeature.c      |    1 +
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 733b016..d5b7386 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -191,7 +191,7 @@ void __init setup_cpu_features(void);
 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 2fd5780..d35b398 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -101,6 +101,28 @@ 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)) {
+			int cpu = smp_processor_id();
+
+			if (caps->desc)
+				pr_crit("CPU%d: Detected - %s\n", cpu, caps->desc);
+			pr_crit("CPU%d: Requires an errata work around not detected"
+					" at boot time\n", cpu);
+			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 3ace45f..2f6ce1d 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -983,6 +983,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 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>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/kernel/smp.c |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index b2d5f4e..0988ccc 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -659,21 +659,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 	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 at 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 +677,6 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 			continue;
 
 		set_cpu_present(cpu, true);
-		max_cpus--;
 	}
 }
 
-- 
1.7.9.5

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

[James Morse]

Hi Suzuki,

On 06/04/16 12:24, Suzuki K Poulose wrote:
> 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.

> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index b2d5f4e..0988ccc 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -659,21 +659,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
>  	if (max_cpus > ncores)
>  		max_cpus = ncores;

This "if (max_cpus > ncores)" is the only user of max_cpus left in this
function, and 'ncores' isn't used for anything else. Dead code?


> -	/* 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 at 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 +677,6 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
>  			continue;
>  
>  		set_cpu_present(cpu, true);
> -		max_cpus--;
>  	}
>  }
>  
> 


Thanks,

James

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

[Suzuki K Poulose]

On 07/04/16 10:18, James Morse wrote:
> Hi Suzuki,
>
> On 06/04/16 12:24, Suzuki K Poulose wrote:
>> 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.
>
>> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
>> index b2d5f4e..0988ccc 100644
>> --- a/arch/arm64/kernel/smp.c
>> +++ b/arch/arm64/kernel/smp.c
>> @@ -659,21 +659,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
>>   	if (max_cpus > ncores)
>>   		max_cpus = ncores;
>
> This "if (max_cpus > ncores)" is the only user of max_cpus left in this
> function, and 'ncores' isn't used for anything else. Dead code?

Yes, it is. We already do for_each_possible_cpu() and ncores was set to num_possible_cpus().
So, the ncores was kind of superfluous to begin with. Thanks for pointing it
out, I will remove it.

Cheers
Suzuki

[UPDATED] [PATCH 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>
---

Changes:
  - Remove dead code (ncores) - [James Morse]

---
 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 at 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

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

[Will Deacon]

Hi Suzuki,

On Wed, Apr 06, 2016 at 12:24:10PM +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. By default, the system uses
> 'system' wide values for setting 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>
> ---
>  arch/arm64/include/asm/cpufeature.h |    8 +++++++-
>  arch/arm64/kernel/cpu_errata.c      |    2 +-
>  arch/arm64/kernel/cpufeature.c      |   37 +++++++++++++++++++----------------
>  3 files changed, 28 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index ca8fb4b..5f10344 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -78,10 +78,16 @@ 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 *);
> +	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..2fd5780 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -22,7 +22,7 @@
>  #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 __unused)

Maybe it would be better to WARN if somebody passes SCOPE_SYSTEM, rather
than silently treat it as per-cpu?

Will

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

[Will Deacon]

On Wed, Apr 06, 2016 at 12:24:13PM +0100, Suzuki K Poulose wrote:
> 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.

Hmm, wouldn't it be better not to free the alternative text for errata
workarounds instead?

Will

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

[Suzuki K Poulose]

On 14/04/16 18:38, Will Deacon wrote:
> Hi Suzuki,
>
> On Wed, Apr 06, 2016 at 12:24:10PM +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. By default, the system uses
>> 'system' wide values for setting the CPU_HWCAPs and ELF_HWCAPs.

>>   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 __unused)
>
> Maybe it would be better to WARN if somebody passes SCOPE_SYSTEM, rather
> than silently treat it as per-cpu?

Should we worry about errata's which may not necessarily depend on per CPU or
a local capability (GIC) ? If not, we could add a WARN after passing down LOCAL
scope for errata. Right now we always do SCOPE_SYSTEM from update_cpu_capabilities(), even for
cpu_errata table. There is no specific reason for that.

Cheers
Suzuki




>
> Will
>

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

[Suzuki K Poulose]

On 14/04/16 18:39, Will Deacon wrote:
> On Wed, Apr 06, 2016 at 12:24:13PM +0100, Suzuki K Poulose wrote:
>> 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.
>
> Hmm, wouldn't it be better not to free the alternative text for errata
> workarounds instead?

We could. I don't have a strong opinion. If there are no objections, I could
adopt it.

Cheers
Suzuki

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

[Will Deacon]

On Thu, Apr 14, 2016 at 06:47:46PM +0100, Suzuki K Poulose wrote:
> On 14/04/16 18:38, Will Deacon wrote:
> >Hi Suzuki,
> >
> >On Wed, Apr 06, 2016 at 12:24:10PM +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. By default, the system uses
> >>'system' wide values for setting the CPU_HWCAPs and ELF_HWCAPs.
> 
> >>  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 __unused)
> >
> >Maybe it would be better to WARN if somebody passes SCOPE_SYSTEM, rather
> >than silently treat it as per-cpu?
> 
> Should we worry about errata's which may not necessarily depend on per CPU or
> a local capability (GIC) ?

Why would they be calling is_affected_midr_range?

> If not, we could add a WARN after passing down LOCAL
> scope for errata.

But if we don't care about errata that aren't local, then why would we warn
on LOCAL?

> Right now we always do SCOPE_SYSTEM from update_cpu_capabilities(), even for
> cpu_errata table. There is no specific reason for that.

I'm totally confused. Can you define SCOPE_SYSTEM and SCOPE_LOCAL for me,
please?

Will

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

[Suzuki K Poulose]

On 15/04/16 13:55, Will Deacon wrote:
> On Thu, Apr 14, 2016 at 06:47:46PM +0100, Suzuki K Poulose wrote:
>> On 14/04/16 18:38, Will Deacon wrote:
>>> Hi Suzuki,
>>>
>>> On Wed, Apr 06, 2016 at 12:24:10PM +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. By default, the system uses
>>>> 'system' wide values for setting the CPU_HWCAPs and ELF_HWCAPs.
>>
>>>>   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 __unused)
>>>
>>> Maybe it would be better to WARN if somebody passes SCOPE_SYSTEM, rather
>>> than silently treat it as per-cpu?
>>
>> Should we worry about errata's which may not necessarily depend on per CPU or
>> a local capability (GIC) ?

So all of these are arm64_cpu_capabilities, i.e, cpu_errata, ELF_HWCAPS and cpu_hwcaps.
Each of them have a matches() routine, which without this series, checks if the system
as a whole has the capability. Now the matches() could use its own checks using system
wide values (read_system_reg(), used by everything except cpu_errata) or CPU register
values(right now, all of them use MIDR).

The tables are processed by a common routine, update_cpu_capabilities(). The cpu_errata
table is processed for each booting CPU, while the cpu_hwcaps is processed only once,
after all the CPUs have booted.

SCOPE is really a hint to the matches() to tell us the capability of the entire system (SCOPE_SYSTEM)
vs that of a single CPU(SCOPE_LOCAL). This is needed, because matches() may do more than checking
the feature register values (e.g, has_useable_gicv3_cpuif() ), which makes it difficult
to the check if a given CPU has a capability. So we initialise the system capabilities
by passing SCOPE_SYSTEM to the matches() from update_cpu_capabilities(). And we use
SCOPE_LOCAL when we do the early verification check for CPUs that turn online after the
system initialised the bits or this_cpu_has_cap() introduced in this series.
At the moment scope doesn't really matter to cpu_errata, as all of them are using
MIDR checks (btw, it is done for all CPUs unlike, cpu_hwcaps or ELF Hwcaps

>
> Why would they be calling is_affected_midr_range?

They won't be. My point is, since we use a shared structures and shared routines to
process the table, the scope parameter comes from the top, while we go through the
table. The entire table of capabilities::matches() will be called with a specific scope
in either case as explained above.


>> If not, we could add a WARN after passing down LOCAL
>> scope for errata.
>
> But if we don't care about errata that aren't local, then why would we warn
> on LOCAL?

We don't have to.
  

>> Right now we always do SCOPE_SYSTEM from update_cpu_capabilities(), even for
>> cpu_errata table. There is no specific reason for that.
>
> I'm totally confused. Can you define SCOPE_SYSTEM and SCOPE_LOCAL for me,
> please?

Hope it is clear now.

Suzuki

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

[Suzuki K Poulose]

On 14/04/16 18:49, Suzuki K Poulose wrote:
> On 14/04/16 18:39, Will Deacon wrote:
>> On Wed, Apr 06, 2016 at 12:24:13PM +0100, Suzuki K Poulose wrote:
>>> 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.
>>
>> Hmm, wouldn't it be better not to free the alternative text for errata
>> workarounds instead?
>
> We could. I don't have a strong opinion. If there are no objections, I could
> adopt it.

The alternatives have been merged to .init by :

commit 9aa4ec1571da6236 ("arm64: mm: fold alternatives into .init")

Mark,

Do you have any concerns on applying alternatives for CPUs turned online later ?
We have to revert the patch above to get going

Suzuki

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

[Will Deacon]

On Fri, Apr 15, 2016 at 03:10:27PM +0100, Suzuki K Poulose wrote:
> On 14/04/16 18:49, Suzuki K Poulose wrote:
> >On 14/04/16 18:39, Will Deacon wrote:
> >>On Wed, Apr 06, 2016 at 12:24:13PM +0100, Suzuki K Poulose wrote:
> >>>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.
> >>
> >>Hmm, wouldn't it be better not to free the alternative text for errata
> >>workarounds instead?
> >
> >We could. I don't have a strong opinion. If there are no objections, I could
> >adopt it.
> 
> The alternatives have been merged to .init by :
> 
> commit 9aa4ec1571da6236 ("arm64: mm: fold alternatives into .init")
> 
> Mark,
> 
> Do you have any concerns on applying alternatives for CPUs turned online later ?
> We have to revert the patch above to get going

Well, note that I'm only suggesting that we keep the alternatives for
CPU errata workarounds.

Will

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

[Suzuki K Poulose]

On 15/04/16 15:12, Will Deacon wrote:
> On Fri, Apr 15, 2016 at 03:10:27PM +0100, Suzuki K Poulose wrote:
>> On 14/04/16 18:49, Suzuki K Poulose wrote:
>>> On 14/04/16 18:39, Will Deacon wrote:
>>>> On Wed, Apr 06, 2016 at 12:24:13PM +0100, Suzuki K Poulose wrote:
>>>>> 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.
>>>>
>>>> Hmm, wouldn't it be better not to free the alternative text for errata
>>>> workarounds instead?
>>>
>>> We could. I don't have a strong opinion. If there are no objections, I could
>>> adopt it.
>>
>> The alternatives have been merged to .init by :
>>
>> commit 9aa4ec1571da6236 ("arm64: mm: fold alternatives into .init")
>>
>> Mark,
>>
>> Do you have any concerns on applying alternatives for CPUs turned online later ?
>> We have to revert the patch above to get going
>
> Well, note that I'm only suggesting that we keep the alternatives for
> CPU errata workarounds.

So to confirmt, do you mean for now we could do :

  - Not release the alternatives for errata work arounds.
  - And retain the check for new erratas, failing the CPUs.

And in a follow up series,

  - Add support for applying errata work arounds later, as we find
    a new CPU which requires it
  - and remove the constraint above ?


Suzuki