On Wed, Feb 07, 2018 at 06:34:37PM +0000, Suzuki K Poulose wrote:

On 07/02/18 10:38, Dave Martin wrote:

quoted

On Wed, Jan 31, 2018 at 06:27:58PM +0000, Suzuki K Poulose wrote:

quoted

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 <redacted>
Cc: Dave Martin <dave.martin@arm.com>
Cc: Will Deacon <redacted>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <redacted>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@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

[...]

quoted

quoted

  * 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.

quoted

quoted

+ *    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