On Wed, Feb 07, 2018 at 06:34:37PM +0000, Suzuki K Poulose wrote: > On 07/02/18 10:38, Dave Martin wrote: > >On Wed, Jan 31, 2018 at 06:27:58PM +0000, Suzuki K Poulose wrote: > >>The kernel detects and uses some of the features based on the boot > >>CPU and expects that all the following CPUs conform to it. e.g, > >>with VHE and the boot CPU running at EL2, the kernel decides to > >>keep the kernel running at EL2. If another CPU is brought up without > >>this capability, we use custom hooks (via check_early_cpu_features()) > >>to handle it. To handle such capabilities add support for detecting > >>and enabling capabilities based on the boot CPU. > >> > >>A bit is added to indicate if the capability should be detected > >>early on the boot CPU. The infrastructure then ensures that such > >>capabilities are probed and "enabled" early on in the boot CPU > >>and, enabled on the subsequent CPUs. > >> > >>Cc: Julien Thierry <julien.thie...@arm.com> > >>Cc: Dave Martin <dave.mar...@arm.com> > >>Cc: Will Deacon <will.dea...@arm.com> > >>Cc: Mark Rutland <mark.rutl...@arm.com> > >>Cc: Marc Zyngier <marc.zyng...@arm.com> > >>Signed-off-by: Suzuki K Poulose <suzuki.poul...@arm.com> > >>--- > >> arch/arm64/include/asm/cpufeature.h | 48 > >> +++++++++++++++++++++++++++++-------- > >> arch/arm64/kernel/cpufeature.c | 48 > >> +++++++++++++++++++++++++++---------- > >> 2 files changed, 74 insertions(+), 22 deletions(-) > >> > >>diff --git a/arch/arm64/include/asm/cpufeature.h > >>b/arch/arm64/include/asm/cpufeature.h
[...] > >> * 3) Verification: When a CPU is brought online (e.g, by user or by the > >> kernel), > >> * the kernel should make sure that it is safe to use the CPU, by > >> verifying > >>@@ -139,11 +148,22 @@ extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0; > >> * > >> * As explained in (2) above, capabilities could be finalised at > >> different > >> * points in the execution. Each CPU is verified against the > >> "finalised" > >>- * capabilities and if there is a conflict, the kernel takes an action, > >>based > >>- * on the severity (e.g, a CPU could be prevented from booting or cause > >>a > >>- * kernel panic). The CPU is allowed to "affect" the state of the > >>capability, > >>- * if it has not been finalised already. See section 5 for more details > >>on > >>- * conflicts. > >>+ * capabilities. > >>+ * > >>+ * x------------------------------------------------------------------- x > >>+ * | Verification: | Boot CPU | SMP CPUs by kernel | CPUs by user | > >>+ * |--------------------------------------------------------------------| > >>+ * | Primary boot CPU | | | | > >>+ * | capability | n | y | y | > >>+ * |--------------------------------------------------------------------| > >>+ * | All others | n | n | y | > >>+ * x--------------------------------------------------------------------x > > > >Minor clarify nit: it's not obvious that "n" means "no conflict" and "y" > >means "conflict". > > > >Could we have blank cell versus "X" (with a note saying what that > >means), or "ok" versus "CONFLICT"? > > This is not strictly about conflicts, but about what each CPU get > verified against. Since there are multiple stages of "finalisation" You're right: I meant something like "potential conflict", but I hadn't read the previous paragraph carefully enough and didn't explain what I meant very well. > for the capabilities, the table shows how the CPUs get verified. > > Would it help if I changed the description above the table to : > > * As explained in (2) above, capabilities could be finalised at different > * points in the execution. Each CPU is verified against the "finalised" > * capabilities. The following table shows, the capabilities verified > * against each CPU in the system. > * > * x------------------------------------------------------------------- x > * | Verified against: | Boot CPU | SMP CPUs by kernel | CPUs by user | I still find it a bit cryptic. Would it be simpler just to write this out in prose, with reference to the actual capability types? I feel that things have to be abbreviated a bit to fit nicely into the table otherwise. What about: * As explained in (2) above, capabilities could be finalised at different * points in the execution, depending on the capability type. Each newly booted * CPU is verified against those capabilities that have been finalised by the * time that CPU boots: * * * SCOPE_BOOT_CPU: all CPUs are verified against the capability except * for the primary boot CPU. * * * SCOPE_LOCAL_CPU, SCOPE_SYSTEM: all CPUs hotplugged on by the user * after kernel boot are verified against the capability. > >>+ * If there is a conflict, the kernel takes an action, based on the > >>severity > >>+ * (e.g, a CPU could be prevented from booting or cause a kernel panic). > >>+ * The CPU is allowed to "affect" the state of the capability, if it > >>has not > >>+ * been finalised already. See section 5 for more details on conflicts. > >> * > >> * 4) Action: As mentioned in (2), the kernel can take an action for each > >> detected > >> * capability, on all CPUs on the system. This is always initiated > >> only after [...] Cheers ---Dave