On Tue, Jan 30, 2018 at 03:38:44PM +0000, Suzuki K Poulose wrote:

On 30/01/18 15:16, Dave Martin wrote:

quoted

On Fri, Jan 26, 2018 at 03:57:44PM +0000, Suzuki K Poulose wrote:

quoted

On 26/01/18 14:16, Dave Martin wrote:

quoted

On Tue, Jan 23, 2018 at 12:28:06PM +0000, Suzuki K Poulose wrote:

quoted

Add helpers for detecting an errata on list of midr ranges
of affected CPUs.

This doesn't describe what the patch does: instead, helpers are being
added for checking whether an MIDR falls in one of multiple affected
model(s) and or revision(s).

Doing this makes sense, but is it really worth it?

Well, we need th MIDR list helpers anyway for other things:
  - White list of CPUs where we know KPTI is not needed
  - Black list of CPUs where DBM shouldn't be enabled.

So all we do is add a new type which could reduce the number of entries.

quoted

We might save 100-200 bytes in the kernel image for now, but a common
workaround for errata on multiple unrelated cpus is surely a rare case.

Only if there are many such lists, or if the lists become large does
this start to seem a clear win.

quoted

Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/include/asm/cpufeature.h |  1 +
 arch/arm64/kernel/cpu_errata.c      | 40 ++++++++++++++++++++++---------------
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h

[...]

quoted

quoted

quoted

quoted

-	{
-		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,
-		ERRATA_MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
-		.enable = enable_psci_bp_hardening,
-	},
-	{
-		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,
-		ERRATA_MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),
+		ERRATA_MIDR_RANGE_LIST(cortex_bp_harden_cpus),

Could we just use a macro to generate multiple structs, instead of
inventing a new type of struct?

We could. Somehow, I don't think we are over engineering much here.

There is a flipside to this: I commented elsewhere that not allowing
mutiple match criteria per capability struct complicates verification
for late CPUs and/or makes it more costly.

Your changes here do implement support for multiple match criteria,
albeit only for the specific case of MIDR matching.

It could be worth generalising this in the future, but that's
probably not for this series.

It is not that complex, right now. See below.

quoted

OTOH, if MIDR matching is the only scenario where we have duplicate
cap structs with different match criteria and this patch allows all
those duplicates to be removed, then is there still a need to walk
the whole list in verify_local_cpu_features(), as introduced in
67948af41f2e ("arm64: capabilities: Handle duplicate entries for a
capability")?  Or can that now be simplified?

I have added support for this in my v2. So here is what I have done :

1) Continue to use midr_list for capability entries that just matches
MIDRS and share the same enable() call back.

 and

2) Add support for wrapper entries where a capability is determined
by two or more entries with different matches()/enable() call backs.

And that can get rid of the changes introduced in commit 67948af41f2e
("arm64: capabilities: Handle duplicate entries for a capability").

OK, cool.  I was presuming that might be too much work to be justified
here, but if not, great.

Cheers
---Dave