Re: [PATCH] arm64: errata: Mitigate AmpereOne erratum AC03_CPU_57 and AC04_CPU_29

[D Scott Phillips <scott@os.amperecomputing.com>]

Marc Zyngier <maz@kernel.org> writes:

> Hi Scott,
>
> On Fri, 10 Jul 2026 23:21:28 +0100,
> D Scott Phillips <scott@os.amperecomputing.com> wrote:
>> 
>> On AmpereOne, deactivating a physical interrupt through ICC_DIR_EL1 or
>> ICC_EOIRx_EL1 (depending on EOImode) which is not active, but is the
>> highest priority pending interrupt causes the cpu to lose the interrupt
>> pending state and also prevents the delivery of future interrupts.
>>
>> Work around this in the vgic, avoiding the cpu issue.
>
> Thanks for getting to the bottom of this.
>
>> 
>> Signed-off-by: D Scott Phillips <scott@os.amperecomputing.com>
>> ---
>> 
>> Hi Marc, we've tracked down the nested virt hang reported
>> previously[1] to a cpu erratum in AmpereOne[2].
>> 
>> Here I'm just sort of parroting the change you had posted for
>> debugging. I'm not familiar with the vgic logic well enough to know if
>> this change is sufficient and doesn't have some unintended
>> consequences. I guess read this more are as "I've tested this and I'm
>> not seeing it hang now."
>>
>> Sorry for sending this half-baked, I didn't want to further delay the
>> errata details on my ongoing vgic education. I'm happy to take any
>> advice you can give, otherwise I'll continue familiarizing myself and
>> will hopefully later have a patch which I can actually claim I think is
>> correct.
>
> No worries.
>
> To be perfectly clear, what I posted at [1] *is* a bug-fix. A very
> minor one. Nothing wrong should come as a result, except when it does,
> such as in your case.
>
> The core reason why this happens is that the L2 will have EOI'd its
> timer, and that the HW bit set in the LR will have propagated the
> deactivation all the way to the HW redistributor. Then L1 takes over,
> and needs to reconcile the LR state with its own, namely its view of
> the active state.
>
> The issue here is that when dealing with a nested vgic (the state
> contained in the LRs is for L2, not L1), the deactivation process
> doesn't need to involve the HW again -- this has already be dealt
> with, and results in the double deactivation I mentioned in my email.

Ah ok, thanks, I think I get it.

>> 
>> [1]: https://lore.kernel.org/linux-arm-kernel/87ecjybz30.wl-maz@kernel.org/
>> 
>> [2]: https://amperecomputing.com/products/developer-errata
>> 
>> The updates with AC03_CPU_57 and AC04_CPU_29 have not yet been
>> published at the time I'm writing this. They should be coming
>> soon. I've reproduced the full entries from those two coming documents
>> collapsed together below:
>> 
>> | {AC03_CPU_57, AC04_CPU_29}: Deactivation of the non-active, highest
>> | priority pending interrupt prevents further interrupt delivery.
>> |
>> | Functional Unit: CPU
>> |
>> | Category: 4
>> |
>> | Affected Version(s): AmpereOne AC03 A0, AmpereOne AC03 B0
>> | Affected Version(s): AmpereOne AC04 A0, AmpereOne AC04_1 A0
>> |
>> | Fixed Version(s): Open
>> |
>> | Overview:
>> |
>> | If software directly deactivates a physical interrupt which is not
>> | in the active state, and the interrupt is also currently the highest
>> | priority pending interrupt, then interrupt delivery will cease on
>> | that PE. Deactivation can happen either through ICC_EOIRx_EL1 if
>> | ICC_CTLR_EL1.EOIMode==0, or through ICC_DIR_EL1 if
>> | ICC_CTLR_EL1.EOIMode==1. Deactivation of virtual interrupts that are
>> | redirected through ICV_ registers will not cause this issue, even
>> | when the virtual interrupt deactivation triggers a physical
>> | interrupt deactivation through ICH_LR<n>_EL2.HW=1.
>
> OK, that's pretty good news. Can I safely assume that your HW doesn't
> support VLPIs/VSGIs in any form (no GICv4+)?

AmpereOne AC04 does have gicv4.1 with vlpis/vsgis. Those take a
different enough path in the core that a deactivation of a pending
vlpi/vsgi won't cause the issue. The wording about "physical interrupt"
is meant to exclude vlpis/vsgis, sorry that it didn't come out clearly.

>> |
>> | This has been observed with Nested Virtualization starting with
>> | Linux-KVM v6.19.
>> |
>> | Impact:
>> |
>> | Physical interrupts will not be delivered after the deactivation of
>> | the non-active, highest priority pending interrupt. A core may
>> | appear to be hung.
>> |
>> | Workaround:
>> |
>> | Software must only deactivate interrupts which are currently active
>> 
>> 
>>  Documentation/arch/arm64/silicon-errata.rst |  4 ++++
>>  arch/arm64/Kconfig                          | 17 +++++++++++++++++
>>  arch/arm64/kernel/cpu_errata.c              | 15 +++++++++++++++
>>  arch/arm64/kvm/vgic/vgic-v3.c               |  4 +++-
>>  arch/arm64/tools/cpucaps                    |  1 +
>>  5 files changed, 40 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
>> index 014aa1c215a16..89130404ce572 100644
>> --- a/Documentation/arch/arm64/silicon-errata.rst
>> +++ b/Documentation/arch/arm64/silicon-errata.rst
>> @@ -55,10 +55,14 @@ stable kernels.
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | Ampere         | AmpereOne       | AC03_CPU_38     | AMPERE_ERRATUM_AC03_CPU_38  |
>>  +----------------+-----------------+-----------------+-----------------------------+
>> +| Ampere         | AmpereOne       | AC03_CPU_57     | AMPERE_ERRATUM_AC03_CPU_57  |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>  | Ampere         | AmpereOne AC04  | AC04_CPU_10     | AMPERE_ERRATUM_AC03_CPU_38  |
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | Ampere         | AmpereOne AC04  | AC04_CPU_23     | AMPERE_ERRATUM_AC04_CPU_23  |
>>  +----------------+-----------------+-----------------+-----------------------------+
>> +| Ampere         | AmpereOne AC04  | AC04_CPU_29     | AMPERE_ERRATUM_AC03_CPU_57  |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Cortex-A510     | #2457168        | ARM64_ERRATUM_2457168       |
>>  +----------------+-----------------+-----------------+-----------------------------+
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index b3afe0688919b..ee5421283d8df 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -436,6 +436,23 @@ config AMPERE_ERRATUM_AC03_CPU_38
>>  
>>  	  If unsure, say Y.
>>  
>> +config AMPERE_ERRATUM_AC03_CPU_57
>> +	bool "AmpereOne: AC03_CPU_57: Deactivation of the non-active, highest priority pending interrupt prevents further interrupt delivery."
>> +	default y
>> +	help
>> +	  This option adds an alternative code sequence to work around Ampere
>> +	  errata AC03_CPU_57 and AC04_CPU_29 on AmpereOne.
>> +
>> +	  Deactivating a physical interrupt through ICC_DIR_EL1 or
>> +	  ICC_EOIR1_EL1 (depending on EOImode) which is not active, but is the
>> +	  highest priority pending interrupt causes the cpu to lose the
>> +	  interrupt pending state and also prevents the delivery of future
>> +	  interrupts.
>> +
>> +	  The workaround is for KVM to not deactivate interrupts for nested vgics.
>> +
>> +	  If unsure, say Y.
>> +
>>  config AMPERE_ERRATUM_AC04_CPU_23
>>          bool "AmpereOne: AC04_CPU_23:  Failure to synchronize writes to HCR_EL2 may corrupt address translations."
>>  	default y
>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index 1995e1198648e..9b03dccd55e09 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -631,6 +631,14 @@ static const struct midr_range erratum_ac03_cpu_38_list[] = {
>>  };
>>  #endif
>>  
>> +#ifdef CONFIG_AMPERE_ERRATUM_AC03_CPU_57
>> +static const struct midr_range erratum_ac03_cpu_57_list[] = {
>> +	MIDR_ALL_VERSIONS(MIDR_AMPERE1),
>> +	MIDR_ALL_VERSIONS(MIDR_AMPERE1A),
>> +	{},
>> +};
>> +#endif
>> +
>>  #ifdef CONFIG_AMPERE_ERRATUM_AC04_CPU_23
>>  static const struct midr_range erratum_ac04_cpu_23_list[] = {
>>  	MIDR_ALL_VERSIONS(MIDR_AMPERE1A),
>> @@ -987,6 +995,13 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>  		ERRATA_MIDR_RANGE_LIST(erratum_ac03_cpu_38_list),
>>  	},
>>  #endif
>> +#ifdef CONFIG_AMPERE_ERRATUM_AC03_CPU_57
>> +	{
>> +		.desc = "AmpereOne erratum AC03_CPU_57",
>> +		.capability = ARM64_WORKAROUND_AMPERE_AC03_CPU_57,
>> +		ERRATA_MIDR_RANGE_LIST(erratum_ac03_cpu_57_list),
>> +	},
>> +#endif
>>  #ifdef CONFIG_AMPERE_ERRATUM_AC04_CPU_23
>>  	{
>>  		.desc = "AmpereOne erratum AC04_CPU_23",
>> diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
>> index 9e841e7afd4a7..8f1d10872360c 100644
>> --- a/arch/arm64/kvm/vgic/vgic-v3.c
>> +++ b/arch/arm64/kvm/vgic/vgic-v3.c
>> @@ -275,7 +275,9 @@ void vgic_v3_deactivate(struct kvm_vcpu *vcpu, u64 val)
>>  		lr = vgic_v3_compute_lr(vcpu, irq) & ~ICH_LR_ACTIVE_BIT;
>>  	}
>>  
>> -	if (lr & ICH_LR_HW)
>> +	if ((lr & ICH_LR_HW) &&
>> +	    !(cpus_have_final_cap(ARM64_WORKAROUND_AMPERE_AC03_CPU_57) &&
>> +	      vgic_state_is_nested(vcpu)))
>>  		vgic_v3_deactivate_phys(FIELD_GET(ICH_LR_PHYS_ID_MASK, lr));
>
> I think this is slightly overkill. The hack I posted should be enough,
> and we can replace all the capability business with a simple comment
> referencing the errata numbers and the entries in silicon-errata.rst.
>
> Use the information provided above to beef up the commit message and
> stick:
>
> Cc: stable@vger.kernel.org
> Fixes: 6dd333c8942b2 ("KVM: arm64: GICv3: nv: Plug L1 LR sync into deactivation primitive")
>
> so that we know how far this needs to be backported.

Thanks, will do