[PATCH v2 0/6] arm64: cpufeature: Add sanity check for ASIDBits

[PATCH v2 0/6] arm64: cpufeature: Add sanity check for ASIDBits

[Suzuki K. Poulose]

This series adds support for verifying some of the cpufeatures
that are decided early in the boot process based on the boot
CPU and cannot be delayed (e.g, ASIDBits and may be VHE?). It also
adds one of the users of this early hook, check for ASIDBits.

The mm_context id is based on the ASIDBits width supported by the
boot CPU and is used early in the initialisation. So we need to make
sure that all the secondary CPUs supports the width reported by the
booting CPU, failing which the CPU will be prevented from turning
online.

Patches 1-4 : Rearranges the cpufeature infrastructure to add hooks
to perform checks for CPUfeatures that were used based on the boot
CPU values

Patch 5: Adds a helper to extract the ASID width on the current CPU
Patch 6: Adds the check to verify ASIDBits of a secondary CPU is compatible
with the boot CPU.

Changes since V1:
  - Split the series from handling of unsigned/signed bits, which were
    merged.
  - Move the checks to verify_local_cpu_capabilities(), making a way for
    further checks to be added (VHE ?) (Patch 1-4)
  - Added a kill_cpu_early() helper to smp.c
  - Added Patch 5.
  - Compare the represented ASIDBits value than the feature value.

Suzuki K. Poulose (6):
  arm64: Introduce kill_cpu_early
  arm64: Move kill_cpu_early to smp.c
  arm64: Enable CPU capability verification for !CONFIG_HOTPLUG_CPU
  arm64: Add hook for checking early CPU features
  arm64: Add helper for extracting ASIDBits
  arm64: Ensure the secondary CPUs have safe ASIDBits size

 arch/arm64/include/asm/cpufeature.h  |    6 ----
 arch/arm64/include/asm/mmu_context.h |    2 ++
 arch/arm64/include/asm/smp.h         |    1 +
 arch/arm64/kernel/cpufeature.c       |   48 ++++++++++++--------------------
 arch/arm64/kernel/smp.c              |   25 +++++++++++++++++
 arch/arm64/mm/context.c              |   50 +++++++++++++++++++++++++---------
 6 files changed, 82 insertions(+), 50 deletions(-)

-- 
1.7.9.5

[PATCH v2 1/6] arm64: Introduce kill_cpu_early

[Suzuki K. Poulose]

Or in other words, make fail_incapable_cpu() reusable.

We use fail_incapable_cpu() to kill a secondary CPU early during the
bringup, which doesn't have the system advertised capabilities.
This patch makes the routine more generic, to kill a secondary
booting CPU, getting rid of the dependency on capability struct.
This can be used by checks which are not necessarily attached to
a capability struct (e.g, cpu ASIDBits).

In that process, renames the function to kill_cpu_early() to better
match its functionality. This will be later moved to arch/arm64/kernel/smp.c.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/kernel/cpufeature.c |   24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 0669c63..a7e2262 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -826,15 +826,15 @@ static u64 __raw_read_system_reg(u32 sys_id)
 }
 
 /*
- * Park the CPU which doesn't have the capability as advertised
- * by the system.
+ * Kill the calling secondary CPU, early in bringup before it is turned
+ * online.
  */
-static void fail_incapable_cpu(char *cap_type,
-				 const struct arm64_cpu_capabilities *cap)
+void kill_cpu_early(void)
 {
 	int cpu = smp_processor_id();
 
-	pr_crit("CPU%d: missing %s : %s\n", cpu, cap_type, cap->desc);
+	pr_crit("CPU%d: will not boot\n", cpu);
+
 	/* Mark this CPU absent */
 	set_cpu_present(cpu, 0);
 
@@ -875,8 +875,11 @@ void verify_local_cpu_capabilities(void)
 		 * If the new CPU misses an advertised feature, we cannot proceed
 		 * further, park the cpu.
 		 */
-		if (!feature_matches(__raw_read_system_reg(caps[i].sys_reg), &caps[i]))
-			fail_incapable_cpu("arm64_features", &caps[i]);
+		if (!feature_matches(__raw_read_system_reg(caps[i].sys_reg), &caps[i])) {
+			pr_crit("CPU%d: missing feature: %s\n",
+					smp_processor_id(), caps[i].desc);
+			kill_cpu_early();
+		}
 		if (caps[i].enable)
 			caps[i].enable(NULL);
 	}
@@ -884,8 +887,11 @@ void verify_local_cpu_capabilities(void)
 	for (i = 0, caps = arm64_hwcaps; caps[i].desc; i++) {
 		if (!cpus_have_hwcap(&caps[i]))
 			continue;
-		if (!feature_matches(__raw_read_system_reg(caps[i].sys_reg), &caps[i]))
-			fail_incapable_cpu("arm64_hwcaps", &caps[i]);
+		if (!feature_matches(__raw_read_system_reg(caps[i].sys_reg), &caps[i])) {
+			pr_crit("CPU%d: missing HWCAP: %s\n",
+					smp_processor_id(), caps[i].desc);
+			kill_cpu_early();
+		}
 	}
 }
 
-- 
1.7.9.5

[PATCH v2 2/6] arm64: Move kill_cpu_early to smp.c

[Suzuki K. Poulose]

This patch moves kill_cpu_early to smp.c, where it fits better.
No functional changes, except for adding the necessary checks
for CONFIG_HOTPLUG_CPU.

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

diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
index d9c3d6a..d23d482 100644
--- a/arch/arm64/include/asm/smp.h
+++ b/arch/arm64/include/asm/smp.h
@@ -68,5 +68,6 @@ extern int __cpu_disable(void);
 
 extern void __cpu_die(unsigned int cpu);
 extern void cpu_die(void);
+extern void kill_cpu_early(void);
 
 #endif /* ifndef __ASM_SMP_H */
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index a7e2262..a17fde9 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -826,28 +826,6 @@ static u64 __raw_read_system_reg(u32 sys_id)
 }
 
 /*
- * Kill the calling secondary CPU, early in bringup before it is turned
- * online.
- */
-void kill_cpu_early(void)
-{
-	int cpu = smp_processor_id();
-
-	pr_crit("CPU%d: will not boot\n", cpu);
-
-	/* Mark this CPU absent */
-	set_cpu_present(cpu, 0);
-
-	/* Check if we can park ourselves */
-	if (cpu_ops[cpu] && cpu_ops[cpu]->cpu_die)
-		cpu_ops[cpu]->cpu_die(cpu);
-	asm(
-	"1:	wfe\n"
-	"	wfi\n"
-	"	b	1b");
-}
-
-/*
  * Run through the enabled system capabilities and enable() it on this CPU.
  * The capabilities were decided based on the available CPUs at the boot time.
  * Any new CPU should match the system wide status of the capability. If the
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index b1adc51..ba33615 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -313,6 +313,31 @@ void cpu_die(void)
 }
 #endif
 
+/*
+ * Kill the calling secondary CPU, early in bringup before it is turned
+ * online.
+ */
+void kill_cpu_early(void)
+{
+	int cpu = smp_processor_id();
+
+	pr_crit("CPU%d: will not boot\n", cpu);
+
+	/* Mark this CPU absent */
+	set_cpu_present(cpu, 0);
+
+#ifdef CONFIG_HOTPLUG_CPU
+	/* Check if we can park ourselves */
+	if (cpu_ops[cpu] && cpu_ops[cpu]->cpu_die)
+		cpu_ops[cpu]->cpu_die(cpu);
+#endif
+
+	asm(
+	"1:	wfe\n"
+	"	wfi\n"
+	"	b	1b");
+}
+
 static void __init hyp_mode_check(void)
 {
 	if (is_hyp_mode_available())
-- 
1.7.9.5

[PATCH v2 3/6] arm64: Enable CPU capability verification for !CONFIG_HOTPLUG_CPU

[Suzuki K. Poulose]

We verify the capabilities of the secondary CPUs only when
hotplug is enabled. The boot time activated CPUs do not
go through the verification by checking whether the system
wide capabilities were initialised or not.

This patch removes the capability check dependency on CONFIG_HOTPLUG_CPU,
to make sure that all the secondary CPUs go through the check.
The boot time activated CPUs will still skip the system wide
capability check. The plan is to hook in a check for CPU features
used by the kernel at early boot up, based on the Boot CPU values.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@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 |    6 ------
 arch/arm64/kernel/cpufeature.c      |   10 ----------
 2 files changed, 16 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 8f271b8..26024c4 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -164,13 +164,7 @@ void update_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
 			    const char *info);
 void check_local_cpu_errata(void);
 
-#ifdef CONFIG_HOTPLUG_CPU
 void verify_local_cpu_capabilities(void);
-#else
-static inline void verify_local_cpu_capabilities(void)
-{
-}
-#endif
 
 u64 read_system_reg(u32 id);
 
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index a17fde9..dde1f56 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -767,8 +767,6 @@ static void enable_cpu_capabilities(const struct arm64_cpu_capabilities *caps)
 			on_each_cpu(caps[i].enable, NULL, true);
 }
 
-#ifdef CONFIG_HOTPLUG_CPU
-
 /*
  * Flag to indicate if we have computed the system wide
  * capabilities based on the boot time active CPUs. This
@@ -873,14 +871,6 @@ void verify_local_cpu_capabilities(void)
 	}
 }
 
-#else	/* !CONFIG_HOTPLUG_CPU */
-
-static inline void set_sys_caps_initialised(void)
-{
-}
-
-#endif	/* CONFIG_HOTPLUG_CPU */
-
 static void setup_feature_capabilities(void)
 {
 	update_cpu_capabilities(arm64_features, "detected feature:");
-- 
1.7.9.5

[PATCH v2 4/6] arm64: Add hook for checking early CPU features

[Suzuki K. Poulose]

Adds a hook for checking whether a secondary CPU has the
features used already by the kernel during early boot, based
on the boot CPU.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/kernel/cpufeature.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index dde1f56..8dd05283 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -824,6 +824,14 @@ static u64 __raw_read_system_reg(u32 sys_id)
 }
 
 /*
+ * Check for CPU features that are used in early boot
+ * based on the Boot CPU value.
+ */
+static void check_early_cpu_features(void)
+{
+}
+
+/*
  * Run through the enabled system capabilities and enable() it on this CPU.
  * The capabilities were decided based on the available CPUs at the boot time.
  * Any new CPU should match the system wide status of the capability. If the
@@ -836,6 +844,8 @@ void verify_local_cpu_capabilities(void)
 	int i;
 	const struct arm64_cpu_capabilities *caps;
 
+	check_early_cpu_features();
+
 	/*
 	 * If we haven't computed the system capabilities, there is nothing
 	 * to verify.
-- 
1.7.9.5

[PATCH v2 5/6] arm64: Add helper for extracting ASIDBits

[Suzuki K. Poulose]

Add a helper to extract ASIDBits on the current cpu

Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/mm/context.c |   36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
index e87f53f..643bf4b 100644
--- a/arch/arm64/mm/context.c
+++ b/arch/arm64/mm/context.c
@@ -40,6 +40,28 @@ static cpumask_t tlb_flush_pending;
 #define ASID_FIRST_VERSION	(1UL << asid_bits)
 #define NUM_USER_ASIDS		ASID_FIRST_VERSION
 
+/* Get the ASIDBits supported by the current CPU */
+static u32 get_cpu_asid_bits(void)
+{
+	u32 asid;
+	int fld = cpuid_feature_extract_field(read_cpuid(ID_AA64MMFR0_EL1),
+						ID_AA64MMFR0_ASID_SHIFT);
+
+	switch (fld) {
+	default:
+		pr_warn("CPU%d: Unknown ASID size (%d); assuming 8-bit\n",
+					smp_processor_id(),  fld);
+		/* Fallthrough */
+	case 0:
+		asid = 8;
+		break;
+	case 2:
+		asid = 16;
+	}
+
+	return asid;
+}
+
 static void flush_context(unsigned int cpu)
 {
 	int i;
@@ -187,19 +209,7 @@ switch_mm_fastpath:
 
 static int asids_init(void)
 {
-	int fld = cpuid_feature_extract_field(read_cpuid(ID_AA64MMFR0_EL1), 4);
-
-	switch (fld) {
-	default:
-		pr_warn("Unknown ASID size (%d); assuming 8-bit\n", fld);
-		/* Fallthrough */
-	case 0:
-		asid_bits = 8;
-		break;
-	case 2:
-		asid_bits = 16;
-	}
-
+	asid_bits = get_cpu_asid_bits();
 	/* If we end up with more CPUs than ASIDs, expect things to crash */
 	WARN_ON(NUM_USER_ASIDS < num_possible_cpus());
 	atomic64_set(&asid_generation, ASID_FIRST_VERSION);
-- 
1.7.9.5

[PATCH v2 6/6] arm64: Ensure the secondary CPUs have safe ASIDBits size

[Suzuki K. Poulose]

The ID_AA64MMFR0_EL1:ASIDBits determines the size of the mm context
id and is used in the early boot to make decisions. The value is
picked up from the Boot CPU and cannot be delayed until other CPUs
are up. If a secondary CPU has a smaller size than that of the Boot
CPU, things will break horribly and the usual SANITY check is not good
enough to prevent the system from crashing. Prevent this by failing CPUs with
ASID smaller than that of the boot CPU.

Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/include/asm/mmu_context.h |    2 ++
 arch/arm64/kernel/cpufeature.c       |    2 ++
 arch/arm64/mm/context.c              |   14 ++++++++++++++
 3 files changed, 18 insertions(+)

diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index 2416578..bd8a0b9 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -147,4 +147,6 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
 #define deactivate_mm(tsk,mm)	do { } while (0)
 #define activate_mm(prev,next)	switch_mm(prev, next, NULL)
 
+void verify_cpu_asid_bits(void);
+
 #endif
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 8dd05283..76bb988 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -24,6 +24,7 @@
 #include <asm/cpu.h>
 #include <asm/cpufeature.h>
 #include <asm/cpu_ops.h>
+#include <asm/mmu_context.h>
 #include <asm/processor.h>
 #include <asm/sysreg.h>
 
@@ -829,6 +830,7 @@ static u64 __raw_read_system_reg(u32 sys_id)
  */
 static void check_early_cpu_features(void)
 {
+	verify_cpu_asid_bits();
 }
 
 /*
diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
index 643bf4b..3d75c2f 100644
--- a/arch/arm64/mm/context.c
+++ b/arch/arm64/mm/context.c
@@ -24,6 +24,7 @@
 
 #include <asm/cpufeature.h>
 #include <asm/mmu_context.h>
+#include <asm/smp.h>
 #include <asm/tlbflush.h>
 
 static u32 asid_bits;
@@ -62,6 +63,19 @@ static u32 get_cpu_asid_bits(void)
 	return asid;
 }
 
+/* Check if the current cpu's ASIDBits is compatible with asid_bits */
+void verify_cpu_asid_bits(void)
+{
+	u32 asid = get_cpu_asid_bits();
+
+	if (asid < asid_bits) {
+		/* Kill the CPU */
+		pr_crit("CPU%d: has smaller ASIDBits(%u) than the one in use (%u)\n",
+				smp_processor_id(), asid, asid_bits);
+		kill_cpu_early();
+	}
+}
+
 static void flush_context(unsigned int cpu)
 {
 	int i;
-- 
1.7.9.5

Re: [PATCH v2 0/6] arm64: cpufeature: Add sanity check for ASIDBits

[Suzuki K. Poulose]

On 01/12/15 15:12, Suzuki K. Poulose wrote:
> This series adds support for verifying some of the cpufeatures
> that are decided early in the boot process based on the boot
> CPU and cannot be delayed (e.g, ASIDBits and may be VHE?). It also
> adds one of the users of this early hook, check for ASIDBits.

Catalin,

This is based on 4.4-rc3 and adds the ASIDBits check, which was missed
in the cpufeature infrastructure series.

Thanks
Suzuki

Re: [PATCH v2 2/6] arm64: Move kill_cpu_early to smp.c

[Mark Rutland]

Hi Suzuki,

On Tue, Dec 01, 2015 at 03:12:07PM +0000, Suzuki K. Poulose wrote:
> This patch moves kill_cpu_early to smp.c, where it fits better.
> No functional changes, except for adding the necessary checks
> for CONFIG_HOTPLUG_CPU.

This is mostly a code move, and the comments below were true for the
original, too.

> +/*
> + * Kill the calling secondary CPU, early in bringup before it is turned
> + * online.
> + */
> +void kill_cpu_early(void)
> +{
> +	int cpu = smp_processor_id();
> +
> +	pr_crit("CPU%d: will not boot\n", cpu);
> +
> +	/* Mark this CPU absent */
> +	set_cpu_present(cpu, 0);
> +
> +#ifdef CONFIG_HOTPLUG_CPU
> +	/* Check if we can park ourselves */
> +	if (cpu_ops[cpu] && cpu_ops[cpu]->cpu_die)
> +		cpu_ops[cpu]->cpu_die(cpu);
> +#endif

Is there no way we can synchronise against this from another CPU, to be
sure that this CPU is actually gone?

> +
> +	asm(
> +	"1:	wfe\n"
> +	"	wfi\n"
> +	"	b	1b");
> +}

This can be:

for (;;) {
	wfe();
	wfi();
}

Regardless of that, we now have a CPU stuck in the kernel, despite
beleiving it to be !present (and therefore !online).

This is problematic for anything where we need to offline or stop
secondary CPUs. For instance, we need to inhibit kexec here (as we will
also need to in case CPUs were stuck in the spinning due to spin-table).

Thanks,
Mark.

Re: [PATCH v2 2/6] arm64: Move kill_cpu_early to smp.c

[Suzuki K. Poulose]

On 01/12/15 15:28, Mark Rutland wrote:
> Hi Suzuki,

>> This patch moves kill_cpu_early to smp.c, where it fits better.
>> No functional changes, except for adding the necessary checks
>> for CONFIG_HOTPLUG_CPU.
>
> This is mostly a code move, and the comments below were true for the
> original, too.

Right, just that there was a dependency on struct arm64_cpu_capabilities
earlier, which was removed in the previous patch in the series.

>> +
>> +#ifdef CONFIG_HOTPLUG_CPU
>> +	/* Check if we can park ourselves */
>> +	if (cpu_ops[cpu] && cpu_ops[cpu]->cpu_die)
>> +		cpu_ops[cpu]->cpu_die(cpu);
>> +#endif
>
> Is there no way we can synchronise against this from another CPU, to be
> sure that this CPU is actually gone?

Unfortunately, no. It cannot be guaranteed whether the CPU died gracefully
or is held up in the kernel. All the other CPU can find is whether the
CPU successfully turned online or not (using a wait_for_completion_timeout).

>> +
>> +	asm(
>> +	"1:	wfe\n"
>> +	"	wfi\n"
>> +	"	b	1b");
>> +}
>
> This can be:
>
> for (;;) {
> 	wfe();
> 	wfi();
> }

Nice, I will change it.

>
> Regardless of that, we now have a CPU stuck in the kernel, despite
> beleiving it to be !present (and therefore !online).

Right, the CPU could be spinning in the kernel.

>
> This is problematic for anything where we need to offline or stop
> secondary CPUs. For instance, we need to inhibit kexec here (as we will
> also need to in case CPUs were stuck in the spinning due to spin-table).

Correct, I didn't think about kexec. May be we could indicate the result
back (that we are looping in kernel) in secondary_data and that could solve
the synchronisation part ?


Suzuki

Re: [PATCH v2 2/6] arm64: Move kill_cpu_early to smp.c

[Mark Rutland]

> >>+#ifdef CONFIG_HOTPLUG_CPU
> >>+	/* Check if we can park ourselves */
> >>+	if (cpu_ops[cpu] && cpu_ops[cpu]->cpu_die)
> >>+		cpu_ops[cpu]->cpu_die(cpu);
> >>+#endif
> >
> >Is there no way we can synchronise against this from another CPU, to be
> >sure that this CPU is actually gone?
> 
> Unfortunately, no. It cannot be guaranteed whether the CPU died gracefully
> or is held up in the kernel. All the other CPU can find is whether the
> CPU successfully turned online or not (using a wait_for_completion_timeout).

That's true if we only consider the kernel, but the firmware can help
us.

For PSCI 0.2+ we can query AFFINITY_INFO to discover whether a CPU is
whether or not it is in the firmware (i.e. whether or not it is
potentially in the kernel), so we can certainly query this in some
cases.

We already do this in the usual hotplug-off case; see cpu_kill.

> >>+
> >>+	asm(
> >>+	"1:	wfe\n"
> >>+	"	wfi\n"
> >>+	"	b	1b");
> >>+}
> >
> >This can be:
> >
> >for (;;) {
> >	wfe();
> >	wfi();
> >}
> 
> Nice, I will change it.
> 
> >
> >Regardless of that, we now have a CPU stuck in the kernel, despite
> >beleiving it to be !present (and therefore !online).
> 
> Right, the CPU could be spinning in the kernel.
> 
> >
> >This is problematic for anything where we need to offline or stop
> >secondary CPUs. For instance, we need to inhibit kexec here (as we will
> >also need to in case CPUs were stuck in the spinning due to spin-table).
> 
> Correct, I didn't think about kexec. May be we could indicate the result
> back (that we are looping in kernel) in secondary_data and that could solve
> the synchronisation part ?

I think we need to have two flags, a cpu-must-die flag in secondary
data, and a global stuck-in-the-kernel flag.

The CPU wanting to die could set its cpu-must-die flag, signal the
completion, then cpu_die(). The CPU awaiting the completion would then
check cpu-must-die, and if so, cpu_kill() that CPU. If not set, we had a
successful onlining.

We need stuck-in-the-kernel flag to account for CPUs which didn't manage
to turn the MMU on (which are either in the spin-table, or failed when
they were individually onlined).

Thanks,
Mark.

Re: [PATCH v2 2/6] arm64: Move kill_cpu_early to smp.c

[Suzuki K. Poulose]

On 01/12/15 16:31, Mark Rutland wrote:
> For PSCI 0.2+ we can query AFFINITY_INFO to discover whether a CPU is
> whether or not it is in the firmware (i.e. whether or not it is
> potentially in the kernel), so we can certainly query this in some
> cases.
>
> We already do this in the usual hotplug-off case; see cpu_kill.

OK, good to know.

>> Correct, I didn't think about kexec. May be we could indicate the result
>> back (that we are looping in kernel) in secondary_data and that could solve
>> the synchronisation part ?
>
> I think we need to have two flags, a cpu-must-die flag in secondary
> data, and a global stuck-in-the-kernel flag.
>
> The CPU wanting to die could set its cpu-must-die flag, signal the
> completion, then cpu_die(). The CPU awaiting the completion would then
> check cpu-must-die, and if so, cpu_kill() that CPU. If not set, we had a
> successful onlining.

Correct.

>
> We need stuck-in-the-kernel flag to account for CPUs which didn't manage
> to turn the MMU on (which are either in the spin-table, or failed when
> they were individually onlined).

Did you mean to say "turn the MMU off" ?

Cheers
Suzuki

Re: [PATCH v2 2/6] arm64: Move kill_cpu_early to smp.c

[Mark Rutland]

On Tue, Dec 01, 2015 at 05:38:54PM +0000, Suzuki K. Poulose wrote:
> On 01/12/15 16:31, Mark Rutland wrote:

[...]

> >We need stuck-in-the-kernel flag to account for CPUs which didn't manage
> >to turn the MMU on (which are either in the spin-table, or failed when
> >they were individually onlined).
> 
> Did you mean to say "turn the MMU off" ?

No, I mean CPUs which were unable to turn the MMU on in the first place.
Perhaps they entered the spin-table but were never individually onlined,
perhaps they didn't support the kernel page size, etc.

When CPUs exit the kernel via PSCI they never switch the MMU off within
the kernel.

Thanks,
Mark.

Re: [PATCH v2 2/6] arm64: Move kill_cpu_early to smp.c

[Suzuki K. Poulose]

On 01/12/15 17:52, Mark Rutland wrote:
> On Tue, Dec 01, 2015 at 05:38:54PM +0000, Suzuki K. Poulose wrote:
>> On 01/12/15 16:31, Mark Rutland wrote:
>
> [...]
>
>>> We need stuck-in-the-kernel flag to account for CPUs which didn't manage
>>> to turn the MMU on (which are either in the spin-table, or failed when
>>> they were individually onlined).
>>
>> Did you mean to say "turn the MMU off" ?
>
> No, I mean CPUs which were unable to turn the MMU on in the first place.
> Perhaps they entered the spin-table but were never individually onlined,
> perhaps they didn't support the kernel page size, etc.
>
> When CPUs exit the kernel via PSCI they never switch the MMU off within
> the kernel.

OK. So the flag will also be used for CPUs which are stuck-in-the-kernel
with MMU turned on. e.g, a CPU (using spin-table) we try to bring down
in kill_cpu_early(). Correct ?

Thanks
Suzuki

Re: [PATCH v2 2/6] arm64: Move kill_cpu_early to smp.c

[Mark Rutland]

On Tue, Dec 01, 2015 at 06:10:17PM +0000, Suzuki K. Poulose wrote:
> On 01/12/15 17:52, Mark Rutland wrote:
> >On Tue, Dec 01, 2015 at 05:38:54PM +0000, Suzuki K. Poulose wrote:
> >>On 01/12/15 16:31, Mark Rutland wrote:
> >
> >[...]
> >
> >>>We need stuck-in-the-kernel flag to account for CPUs which didn't manage
> >>>to turn the MMU on (which are either in the spin-table, or failed when
> >>>they were individually onlined).
> >>
> >>Did you mean to say "turn the MMU off" ?
> >
> >No, I mean CPUs which were unable to turn the MMU on in the first place.
> >Perhaps they entered the spin-table but were never individually onlined,
> >perhaps they didn't support the kernel page size, etc.
> >
> >When CPUs exit the kernel via PSCI they never switch the MMU off within
> >the kernel.
> 
> OK. So the flag will also be used for CPUs which are stuck-in-the-kernel
> with MMU turned on. e.g, a CPU (using spin-table) we try to bring down
> in kill_cpu_early(). Correct ?

Yes.

We'd also pad it such that nothing else shares the same writeback
granule, and when writing to it with the MMU off we can invalidate the
stale cached copy.

Thanks,
Mark.

Re: [PATCH v2 2/6] arm64: Move kill_cpu_early to smp.c

[Suzuki K. Poulose]

On 01/12/15 18:50, Mark Rutland wrote:
> On Tue, Dec 01, 2015 at 06:10:17PM +0000, Suzuki K. Poulose wrote:
>> On 01/12/15 17:52, Mark Rutland wrote:
>>> On Tue, Dec 01, 2015 at 05:38:54PM +0000, Suzuki K. Poulose wrote:
>>>> On 01/12/15 16:31, Mark Rutland wrote:
>> OK. So the flag will also be used for CPUs which are stuck-in-the-kernel
>> with MMU turned on. e.g, a CPU (using spin-table) we try to bring down
>> in kill_cpu_early(). Correct ?
>
> Yes.
>
> We'd also pad it such that nothing else shares the same writeback
> granule, and when writing to it with the MMU off we can invalidate the
> stale cached copy.

I have started working on this approach. But the changes are a bit more invasive
and looks more like suited for 4.5. We could push this series(which doesn't change
the current behavior as it is in 4.4-rc3, except for the code movement) to fix
the ASID sanity check and introduce the synchronisation part in 4.5.
What do you think ?

Cheers
Suzuki

Re: [PATCH v2 2/6] arm64: Move kill_cpu_early to smp.c

[Mark Rutland]

On Thu, Dec 03, 2015 at 04:36:51PM +0000, Suzuki K. Poulose wrote:
> On 01/12/15 18:50, Mark Rutland wrote:
> >On Tue, Dec 01, 2015 at 06:10:17PM +0000, Suzuki K. Poulose wrote:
> >>On 01/12/15 17:52, Mark Rutland wrote:
> >>>On Tue, Dec 01, 2015 at 05:38:54PM +0000, Suzuki K. Poulose wrote:
> >>>>On 01/12/15 16:31, Mark Rutland wrote:
> >>OK. So the flag will also be used for CPUs which are stuck-in-the-kernel
> >>with MMU turned on. e.g, a CPU (using spin-table) we try to bring down
> >>in kill_cpu_early(). Correct ?
> >
> >Yes.
> >
> >We'd also pad it such that nothing else shares the same writeback
> >granule, and when writing to it with the MMU off we can invalidate the
> >stale cached copy.
> 
> I have started working on this approach. But the changes are a bit more invasive
> and looks more like suited for 4.5. We could push this series(which doesn't change
> the current behavior as it is in 4.4-rc3, except for the code movement) to fix
> the ASID sanity check and introduce the synchronisation part in 4.5.
> What do you think ?

I'm happy with that. I agree this patch as-is doesn't make matters
worse.

Thanks,
Mark.