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

[Marc Zyngier <maz@kernel.org>]

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.

> 
> [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+)?

> |
> | 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,

	M.

-- 
Jazz isn't dead. It just smells funny.