All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: "D Scott Phillips" <scott@os.amperecomputing.com>,
	"Oliver Upton" <oupton@kernel.org>,
	kvmarm@lists.linux.dev
Subject: Re: [PATCH] arm64: errata: Mitigate AmpereOne erratum AC03_CPU_57 and AC04_CPU_29
Date: Sat, 11 Jul 2026 08:53:31 +0100	[thread overview]
Message-ID: <87ldbi55dw.wl-maz@kernel.org> (raw)
In-Reply-To: <20260710223718.2C85B1F000E9@smtp.kernel.org>

On Fri, 10 Jul 2026 23:37:17 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] The AmpereOne erratum workaround unconditionally skips deactivating physical interrupts for all nested VMs, causing a physical interrupt state leak.
> 
> Pre-existing issues:
> - [Critical] The AmpereOne erratum workaround fails to mitigate a Guest-to-Host Denial of Service vector for non-nested (L1) guests.
> - [High] Dropping `irq->irq_lock` between computing the LR snapshot and folding it creates a race condition that loses concurrent virtual interrupts.
> --
> 
> --- Patch [1]: [PATCH] arm64: errata: Mitigate AmpereOne erratum AC03_CPU_57 and AC04_CPU_29 ---
> commit d1d195cfc998f3f14be1b81eaa8484fcb7eb08f8
> Author: D Scott Phillips <scott@os.amperecomputing.com>
> 
> arm64: errata: Mitigate AmpereOne erratum AC03_CPU_57 and AC04_CPU_29
> 
> This commit introduces a workaround for an AmpereOne CPU erratum where
> deactivating a non-active physical interrupt that is the highest priority
> pending interrupt hangs the CPU. The mitigation avoids physical deactivations
> when KVM handles a nested VM context.
> 
> Link: https://lore.kernel.org/linux-arm-kernel/87ecjybz30.wl-maz@kernel.org/
> Link: https://amperecomputing.com/products/developer-errata
> 
> [ ... ]
> 
> > 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)))
> 
> [Severity: Critical]
> This is a pre-existing issue, but does the system remain vulnerable to the
> erratum for L1 guests?
> 
> Because vgic_state_is_nested(vcpu) evaluates to false for an L1 guest, KVM
> bypasses the workaround and still calls vgic_v3_deactivate_phys(). If an
> unprivileged guest operating in GICv2 mode on a GICv3 host writes the INTID of

There is no such thing with NV, as we only support GICv3 in guests.

> a hardware-backed interrupt to the GICC_DIR MMIO register while it is inactive,
> can this directly trigger the host CPU hang?
>
> [Severity: High]
> Does unconditionally bypassing the physical deactivation for nested VMs cause
> a physical interrupt state leak?
> 
> When an L2 guest legitimately deactivates an active hardware-backed
> interrupt and it traps to KVM, vgic_v3_deactivate() is called. Since
> vgic_state_is_nested(vcpu) evaluates to true, it successfully updates the
> virtual state via vgic_v3_fold_lr() but unconditionally skips calling
> vgic_v3_deactivate_phys().
> 
> Could this leave hardware-backed interrupts, such as the physical timer,
> permanently active in the physical GIC distributor and block future timer
> interrupts?

Go read the spec and the effects of the HW bit. This is actively
advocating for the reintroduction of the bug... :-/

> 
> >  		vgic_v3_deactivate_phys(FIELD_GET(ICH_LR_PHYS_ID_MASK, lr));
> >  
> >  	vgic_v3_fold_lr(vcpu, lr);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does dropping irq->irq_lock
> between computing the LR snapshot and folding it create a race condition that
> loses concurrent virtual interrupts?
> 
> vgic_v3_deactivate() takes the lock, computes the snapshot, and then the
> scoped_guard ends:
> 
> scoped_guard(raw_spinlock, &irq->irq_lock) {
>     ...
>     lr = vgic_v3_compute_lr(vcpu, irq) & ~ICH_LR_ACTIVE_BIT;
> }
> 
> If a concurrent device injects an interrupt here via vgic_inject_irq() and
> sets irq->pending_latch = true, won't the subsequent call to
> vgic_v3_fold_lr(vcpu, lr) re-acquire the lock and evaluate the stale snapshot?
> 
> vgic_v3_fold_lr() would see !(val & ICH_LR_STATE) is true and execute:
> 
> if (irq->config == VGIC_CONFIG_LEVEL && !(val & ICH_LR_STATE))
>     irq->pending_latch = false;
> 
> Could this wipe out the newly arrived interrupt and cause guest devices to
> stall?

This one is a potential problem. I'll have a think...

	M.

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

  reply	other threads:[~2026-07-11  7:51 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10 22:21 [PATCH] arm64: errata: Mitigate AmpereOne erratum AC03_CPU_57 and AC04_CPU_29 D Scott Phillips
2026-07-10 22:37 ` sashiko-bot
2026-07-11  7:53   ` Marc Zyngier [this message]
2026-07-11 22:27   ` Marc Zyngier
2026-07-11  7:37 ` Marc Zyngier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ldbi55dw.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=oupton@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=scott@os.amperecomputing.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.