All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Fuad Tabba" <tabba@google.com>
Cc: kvmarm@lists.linux.dev, "Oliver Upton" <oupton@kernel.org>,
	"Marc Zyngier" <maz@kernel.org>
Subject: Re: [PATCH 8/8] KVM: arm64: Implement lazy vCPU state sync for non-protected guests
Date: Fri, 19 Jun 2026 07:24:32 +0000	[thread overview]
Message-ID: <20260619072432.80E331F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260619070508.802802-9-tabba@google.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Removal of `hcr_el2` sync-back causes infinite virtual SError injections.
- [High] Injected exceptions for Illegal Execution State (ARM_EXCEPTION_IL) are silently dropped on re-entry.
- [High] Asynchronous guest context modifications (e.g., KVM_REQ_VCPU_RESET via PSCI CPU_ON) are dropped, ignoring resets.
--

commit 479bc386009beb82e67cb9e9a04495d0872f077c
Author: Fuad Tabba <tabba@google.com>

KVM: arm64: Implement lazy vCPU state sync for non-protected guests

This commit defers copying a non-protected guest's register context
between the host and hypervisor until the host explicitly needs it or
modifies it. It introduces the PKVM_HOST_STATE_DIRTY flag to track when
the state must be flushed to the hypervisor on guest entry.

> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c

[ ... ]

> @@ -489,6 +503,22 @@ int handle_exit(struct kvm_vcpu *vcpu, int exception_index)
>  /* For exit types that need handling before we can be preempted */
>  void handle_exit_early(struct kvm_vcpu *vcpu, int exception_index)
>  {
> +	bool inject_serror = ARM_SERROR_PENDING(exception_index) ||
> +		ARM_EXCEPTION_CODE(exception_index) == ARM_EXCEPTION_EL1_SERROR;
> +
> +	/*
> +	 * An SError injected below writes the host ctxt; for a non-protected
> +	 * guest, sync from the hyp vCPU and keep it dirty so it isn't dropped.
> +	 */
> +	if (is_protected_kvm_enabled()) {
> +		vcpu_clear_flag(vcpu, PKVM_HOST_STATE_DIRTY);

[Severity: High]
Does unconditionally clearing the dirty flag here cause other injected
exceptions or asynchronous state changes to be silently dropped on the next
guest entry?

For instance, when a non-protected guest exits with an illegal execution
state (ARM_EXCEPTION_IL), handle_exit_early() clears PKVM_HOST_STATE_DIRTY.
Later, handle_exit() processes the exception by calling
kvm_inject_undefined(), which modifies the host's copy of the guest register
context (vcpu->arch.ctxt) to deliver an UNDEF exception. Because this happens
outside of handle_trap_exceptions(), the dirty flag is never set back to 1.
Consequently, on the next entry, flush_hyp_vcpu() skips syncing the modified
context, and the guest resumes execution with its previous state instead of
taking the UNDEF exception, leading to an infinite trap loop.

Similarly, if a PSCI CPU_ON call triggers an IPI, the target vCPU exits with
ARM_EXCEPTION_IRQ, and the dirty flag is cleared here. The run loop then
processes pending requests via check_vcpu_requests(), handling
KVM_REQ_VCPU_RESET by calling kvm_reset_vcpu(). The host's context is reset,
but since the dirty flag remains 0, the next entry skips syncing the reset
context to the hypervisor, causing the PSCI wake/reset request to be ignored.

> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c

[ ... ]

> @@ -203,9 +255,13 @@ static void sync_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu)
>  	fpsimd_sve_sync(&hyp_vcpu->vcpu);
>  	sync_debug_state(hyp_vcpu);
>  
> -	host_vcpu->arch.ctxt		= hyp_vcpu->vcpu.arch.ctxt;
> -
> -	host_vcpu->arch.hcr_el2		= hyp_vcpu->vcpu.arch.hcr_el2;

[Severity: Critical]
Does removing the synchronization of arch.hcr_el2 back to the host cause an
infinite loop of virtual SError injections?

The host uses HCR_VSE to inject a virtual SError. When the guest takes the
exception, the hardware clears the physical HCR_EL2.VSE bit, which is saved in
the hyp vCPU's context on exit. Because this sync-back assignment is removed,
host_vcpu->arch.hcr_el2 indefinitely retains the stale HCR_VSE bit set to 1.

On every subsequent guest entry, flush_hyp_vcpu() will re-apply this stale
bit from the host into the hyp context, causing infinite, spurious virtual
SError injections that will hang or crash the guest.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619070508.802802-1-tabba@google.com?part=8

  reply	other threads:[~2026-06-19  7:24 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-19  7:05 [PATCH 0/8] KVM: arm64: Rework pKVM vCPU state synchronisation Fuad Tabba
2026-06-19  7:05 ` [PATCH 1/8] KVM: arm64: Extract MPIDR computation into a shared header Fuad Tabba
2026-06-19  7:05 ` [PATCH 2/8] KVM: arm64: Make vcpu_{read,write}_sys_reg available to HYP code Fuad Tabba
2026-06-19  7:05 ` [PATCH 3/8] KVM: arm64: Factor out reusable vCPU reset helpers Fuad Tabba
2026-06-19  7:05 ` [PATCH 4/8] KVM: arm64: Move PSCI helper functions to a shared header Fuad Tabba
2026-06-19  7:16   ` sashiko-bot
2026-06-19  7:24     ` Fuad Tabba
2026-06-19  7:05 ` [PATCH 5/8] KVM: arm64: Add host and hypervisor vCPU lookup primitives Fuad Tabba
2026-06-19  7:05 ` [PATCH 6/8] KVM: arm64: Minimise EL2's exposure of host VGIC state during world switch Fuad Tabba
2026-06-19  7:22   ` sashiko-bot
2026-06-19  7:29     ` Fuad Tabba
2026-06-19  7:05 ` [PATCH 7/8] KVM: arm64: Add primitives to flush/sync the VGIC state at EL2 Fuad Tabba
2026-06-19  7:05 ` [PATCH 8/8] KVM: arm64: Implement lazy vCPU state sync for non-protected guests Fuad Tabba
2026-06-19  7:24   ` sashiko-bot [this message]
2026-06-19  7:55     ` Fuad Tabba
2026-06-19  7:06 ` [PATCH 0/8] KVM: arm64: Rework pKVM vCPU state synchronisation Fuad Tabba

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=20260619072432.80E331F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=maz@kernel.org \
    --cc=oupton@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=tabba@google.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.