Re: [RFC PATCH] KVM: arm64: Workaround for Ampere AC03_CPU_36 (exception taken to an incorrect EL)

[D Scott Phillips]

Marc Zyngier <maz@xxxxxxxxxx> writes:

> [reviewing both patches in one go, as it is way easier]
>
> On Fri, 05 Jan 2024 23:53:10 +0000,
> Oliver Upton <oliver.upton@xxxxxxxxx> wrote:
>> 
>> Hi Ilkka,
>> 
>> On Fri, Jan 05, 2024 at 01:32:51PM -0800, Ilkka Koskinen wrote:
>> > Due to erratum AC03_CPU_36 on AmpereOne, if an Asynchronous Exception
>> > (interrupts or SErrors) occurs to EL2, while EL2 software is modifying
>> > system register bits that control EL2 exception behavior, the processor
>> > may take an exception to an incorrect Exception Level.
>
> What needs to be described (both in the commit message and as part of
> the code) is under what circumstances this mis-routing happens.
>
> Is it that just clearing TGE while being at EL2 always results in the
> asynchronous exception being routed to the wrong exception level? Or
> is it a more subtle issue related to synchronisation?
>
> Also worth describing is to which other exception level is the
> exception delivered? EL1? EL3?

The mis-routing happens when the old value of HCR_EL2 allows IRQs in EL2
and the new value does not (a "C" entry in the target EL table), and
then an IRQ arrives. In that case the exception is routed to EL1.

>> > 
>> > The affected system registers are HCR_EL2 and SCTLR_EL2, which contain
>> > control bits for routing and enabling of EL2 exceptions.
>
> How does SCTLR_EL2 affects interrupt delivery? Is this related to
> FEAT_NMI and SCTLR_EL2.{NMI,SPINTMASK}? Because this is the only part
> of this register that has anything to do with interrupts.

It looks like sctlr isn't involved in the irq routing problem of
ac03_cpu_36. I'm working on pinning down whether there's some other
somewhat related bug with the sctlr bits, or if it's just a mistake that
it's listed in the write up at all.

>> > 
>> > The issue is triggered when HGE.TGE bit is cleared while having
>> > AMO/IMO/FMO bits cleared too. To avoid the exception getting taken
>> > at a wrong Exception Level, we set AMO/IMO/FMO.
>> 
>> We toggle HCR_EL2 for other things besides TLB invalidations, and the
>> changelog does not describe why they're apparently unaffected.
>> 
>> > Suggested-by: D Scott Phillips <scott@xxxxxxxxxxxxxxxxxxxxxx>
>> > Signed-off-by: Ilkka Koskinen <ilkka@xxxxxxxxxxxxxxxxxxxxxx>
>> 
>> This isn't an acceptable way to go about errata mitigations. Besides
>> extremely unusual circumstances, the pattern is to use a cpucap &&
>> alternatives to only enable the workaround on affected designs. We then
>> document the errata in the expected places (Kconfig and kernel
>> documentation) such that the folks saddled with maintaining this stuff
>> know how to handle it years down the line.
>
> +1. This hack will have to live forever, while the lack of
> documentation makes it totally unmaintainable. The KVM code *will*
> change in ways that cannot be anticipated today, and without
> exhaustive documentation, we will not be able to do a good job at
> maintaining this system alive by correctly mitigating the erratum.

Point taken on the documentation. I'll work on getting it updated with
the info above and get ac03_cpu_39 updated with the problem encodings.

Scott