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

On 2026/07/30 1:23, Fuad Tabba wrote:
> On Mon, 20 Jul 2026 at 07:01, <[email protected]> wrote:
>>
>> 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.
> 
>   The code path is real, but I don't think it's reachable on current
> hardware: impdef_pmuv3_cpus lists both the P-cores and E-cores of
> M1/M2.

Indeed the code path is real, but Sashiko is wrong about its 
consequence. Accessing PMUv3 registers on CPUs with IMPDEF PMU but 
without hypervisor traps enabled is UNDEFINED, so host PMU registers 
won't be accessed. I'll change the patch message to clarify that in the 
next version.

Regards,
Akihiko Odaki

> 
>> Pre-existing issues:
>> - [High] Writes to control-plane system registers in CPU capability enable callbacks lack a mandatory `isb()` instruction.
> 
> No, exception return is a context synchronization event, which happens
> before the effect is consumed.

I agree. No change will be made for this false positive in the next version.

Regards,
Akihiko Odaki

> 
> 
> /fuad
> 
> 
>> --
>>
>> --- Patch [9]: [PATCH v9 09/16] arm64: errata: Require Apple IMPDEF PMUv3 traps on all CPUs ---
>> commit 3e98f085c1f1b418fb0ec6e336b981ba1df07b03
>> Author: Akihiko Odaki <[email protected]>
>>
>> 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/[email protected]?part=9
>>