Re: [PATCH v9 09/16] arm64: errata: Require Apple IMPDEF PMUv3 traps on all CPUs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] Permitting late CPUs with IMPDEF PMUv3 to boot without enabling hypervisor traps allows untrapped guest access to host PMU registers.

Pre-existing issues:
- [High] Writes to control-plane system registers in CPU capability enable callbacks lack a mandatory `isb()` instruction.
--

--- Patch [9]: [PATCH v9 09/16] arm64: errata: Require Apple IMPDEF PMUv3 traps on all CPUs ---
commit 3e98f085c1f1b418fb0ec6e336b981ba1df07b03
Author: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

arm64: errata: Require Apple IMPDEF PMUv3 traps on all CPUs

This commit changes the ARM64_WORKAROUND_PMUV3_IMPDEF_TRAPS erratum to an
all-early-CPU capability. It ensures the capability is only advertised if all
early CPUs match, aiming to reject late CPUs that miss the capability while
permitting late CPUs to have it when the system does not.

> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index 1995e1198648e..a5ae628204a12 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c

[ ... ]

> @@ -1005,7 +1005,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>  	{
>  		.desc = "Apple IMPDEF PMUv3 Traps",
>  		.capability = ARM64_WORKAROUND_PMUV3_IMPDEF_TRAPS,
> -		.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
> +		.type = ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE,

[Severity: Critical]
By changing the capability type to ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE,
does this allow a late CPU with IMPDEF PMUv3 to boot without enabling the
hypervisor trap if the system lacks the capability system-wide?

If an asymmetric system boots without the PMUv3 IMPDEF capability system-wide
and a late Apple CPU with the feature is onlined, verify_local_cpu_caps()
will permit the CPU. However, because system_has_cap is false, it skips
caps->cpu_enable() which leaves HACR_EL2 bit 56 clear.

Since KVM relies on this hardware trap to intercept these instructions and
inject an UNDEF exception, could this missing trap allow a malicious guest
scheduled on the late CPU to execute IMPDEF PMU instructions natively and
manipulate host physical PMU state?

>  		.matches = has_impdef_pmuv3,
>  		.cpu_enable = cpu_enable_impdef_pmuv3_traps,

[Severity: High]
This is a pre-existing issue, but do the writes to control-plane system
registers in capability enable callbacks require an isb() synchronization
barrier?

Currently, callbacks like cpu_enable_impdef_pmuv3_traps update SYS_HACR_EL2
using sysreg_clear_set_s, and then return immediately without a trailing
isb(). Other callbacks like cpu_enable_trap_ctr_access and
cpu_enable_cache_maint_trap update sctlr_el1 in a similar way.

Under the ARM64 subsystem rules, every write to a control-plane system
register must be followed by an isb() as the very next instruction. Since
sysreg_clear_set_s does not issue this barrier, returning immediately places
the return branch instruction against an architecturally undefined pipeline
state. Should these callbacks issue an explicit isb() before returning?

>  	},

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-hybrid-v9-0-2b713ca1b5dc@rsg.ci.i.u-tokyo.ac.jp?part=9