From: Marc Zyngier <maz@kernel.org>
To: Fuad Tabba <fuad.tabba@linux.dev>
Cc: Oliver Upton <oupton@kernel.org>,
kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>, Joey Gouly <joey.gouly@arm.com>,
Steffen Eiden <seiden@linux.ibm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Zenghui Yu <yuzenghui@huawei.com>,
Vincent Donnefort <vdonnefort@google.com>,
Quentin Perret <qperret@google.com>,
Fuad Tabba <tabba@google.com>
Subject: Re: [PATCH v2 10/17] KVM: arm64: Prevent host PC adjustments for protected vCPUs
Date: Fri, 11 Sep 2026 14:58:10 +0100 [thread overview]
Message-ID: <86ik4b7uvh.wl-maz@kernel.org> (raw)
In-Reply-To: <20260907070002.3333525-11-fuad.tabba@linux.dev>
On Mon, 07 Sep 2026 07:59:55 +0100,
Fuad Tabba <fuad.tabba@linux.dev> wrote:
>
> __kvm_adjust_pc() lets the host advance a vCPU's PC or inject an
> exception, which for a protected vCPU would let the host redirect
> guest execution. Drop the request there: the entry handlers apply the
> host's PC_UPDATE_REQ on re-entry, where EL2 allows it.
>
> For a non-protected vCPU, adjusting the hyp vCPU while
> PKVM_HOST_STATE_DIRTY is set loses the update at the next flush and
> the guest re-executes an emulated MMIO access, so the flag selects
> which copy to adjust. Adjusting the hyp vCPU copies PC_UPDATE_REQ in
> and back out again. Without the copy back, INCREMENT_PC outlives the
> adjustment and the next KVM_SET_VCPU_EVENTS trips
> WARN_ON(INCREMENT_PC) in kvm_pend_exception().
>
> Unloaded, the host copy is host-writable, so pin it and its VM before
> adjusting. Only commit_pending_events(), under KVM_SET_VCPU_EVENTS,
> reaches EL2 with no hyp vCPU loaded. KVM_RUN's call always has one.
> enter_exception64() reads the VM's MTE flag and a host copy's
> vcpu->kvm is host-written, so __kvm_adjust_pc_vm() takes the VM as a
> parameter: the pinned host struct kvm when unloaded, the hyp VM
> otherwise.
>
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---
> arch/arm64/include/asm/kvm_asm.h | 1 +
> arch/arm64/kvm/hyp/exception.c | 27 ++++++++++-------
> arch/arm64/kvm/hyp/nvhe/hyp-main.c | 47 +++++++++++++++++++++++++++++-
> 3 files changed, 63 insertions(+), 12 deletions(-)
I keep coming to this patch because I really dislike how invasive it
is. Suggestions below.
>
> diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
> index e5b92ac09e69e..d149afee7b4bd 100644
> --- a/arch/arm64/include/asm/kvm_asm.h
> +++ b/arch/arm64/include/asm/kvm_asm.h
> @@ -280,6 +280,7 @@ extern int __kvm_at_s12(struct kvm_vcpu *vcpu, u32 op, u64 vaddr);
> extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
>
> extern void __kvm_adjust_pc(struct kvm_vcpu *vcpu);
> +extern void __kvm_adjust_pc_vm(struct kvm_vcpu *vcpu, struct kvm *kvm);
>
> extern bool __vgic_v3_get_gic_config(void);
> extern void __vgic_v3_init_lrs(void);
> diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c
> index 754e2dc1df54a..bf9d8efce9984 100644
> --- a/arch/arm64/kvm/hyp/exception.c
> +++ b/arch/arm64/kvm/hyp/exception.c
> @@ -66,8 +66,8 @@ static void __vcpu_write_spsr_und(struct kvm_vcpu *vcpu, u64 val)
> * Here we manipulate the fields in order of the AArch64 SPSR_ELx layout, from
> * MSB to LSB.
> */
> -static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
> - enum exception_type type)
> +static void enter_exception64(struct kvm_vcpu *vcpu, struct kvm *kvm,
> + unsigned long target_mode, enum exception_type type)
> {
> unsigned long sctlr, vbar, old, new, mode;
> u64 exc_offset;
> @@ -109,7 +109,7 @@ static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
> new |= (old & PSR_C_BIT);
> new |= (old & PSR_V_BIT);
>
> - if (kvm_has_mte(kern_hyp_va(vcpu->kvm)))
> + if (kvm_has_mte(kvm))
> new |= PSR_TCO_BIT;
So this is the sole reason why we need to access the kvm structure,
and the cause for all of this churn. Can't this be written as some
form of:
kvm = vcpu_get_kvm(vcpu);
[do whatever is needed]
vcpu_put_kvm(vcpu, kvm);
where get/put do the pinning/unpinning as required? These helpers
would become NOPs on VHE, for example.
>
> new |= (old & PSR_DIT_BIT);
> @@ -294,7 +294,7 @@ static void enter_exception32(struct kvm_vcpu *vcpu, u32 mode, u32 vect_offset)
> *vcpu_pc(vcpu) = vect_offset;
> }
>
> -static void kvm_inject_exception(struct kvm_vcpu *vcpu)
> +static void kvm_inject_exception(struct kvm_vcpu *vcpu, struct kvm *kvm)
> {
> if (vcpu_el1_is_32bit(vcpu)) {
> switch (vcpu_get_flag(vcpu, EXCEPT_MASK)) {
> @@ -314,23 +314,23 @@ static void kvm_inject_exception(struct kvm_vcpu *vcpu)
> } else {
> switch (vcpu_get_flag(vcpu, EXCEPT_MASK)) {
> case unpack_vcpu_flag(EXCEPT_AA64_EL1_SYNC):
> - enter_exception64(vcpu, PSR_MODE_EL1h, except_type_sync);
> + enter_exception64(vcpu, kvm, PSR_MODE_EL1h, except_type_sync);
> break;
>
> case unpack_vcpu_flag(EXCEPT_AA64_EL1_SERR):
> - enter_exception64(vcpu, PSR_MODE_EL1h, except_type_serror);
> + enter_exception64(vcpu, kvm, PSR_MODE_EL1h, except_type_serror);
> break;
>
> case unpack_vcpu_flag(EXCEPT_AA64_EL2_SYNC):
> - enter_exception64(vcpu, PSR_MODE_EL2h, except_type_sync);
> + enter_exception64(vcpu, kvm, PSR_MODE_EL2h, except_type_sync);
> break;
>
> case unpack_vcpu_flag(EXCEPT_AA64_EL2_IRQ):
> - enter_exception64(vcpu, PSR_MODE_EL2h, except_type_irq);
> + enter_exception64(vcpu, kvm, PSR_MODE_EL2h, except_type_irq);
> break;
>
> case unpack_vcpu_flag(EXCEPT_AA64_EL2_SERR):
> - enter_exception64(vcpu, PSR_MODE_EL2h, except_type_serror);
> + enter_exception64(vcpu, kvm, PSR_MODE_EL2h, except_type_serror);
> break;
>
> default:
> @@ -348,10 +348,10 @@ static void kvm_inject_exception(struct kvm_vcpu *vcpu)
> * Adjust the guest PC (and potentially exception state) depending on
> * flags provided by the emulation code.
> */
> -void __kvm_adjust_pc(struct kvm_vcpu *vcpu)
> +void __kvm_adjust_pc_vm(struct kvm_vcpu *vcpu, struct kvm *kvm)
> {
> if (vcpu_get_flag(vcpu, PENDING_EXCEPTION)) {
> - kvm_inject_exception(vcpu);
> + kvm_inject_exception(vcpu, kvm);
> vcpu_clear_flag(vcpu, PENDING_EXCEPTION);
> vcpu_clear_flag(vcpu, EXCEPT_MASK);
> } else if (vcpu_get_flag(vcpu, INCREMENT_PC)) {
> @@ -359,3 +359,8 @@ void __kvm_adjust_pc(struct kvm_vcpu *vcpu)
> vcpu_clear_flag(vcpu, INCREMENT_PC);
> }
> }
> +
> +void __kvm_adjust_pc(struct kvm_vcpu *vcpu)
> +{
> + __kvm_adjust_pc_vm(vcpu, kern_hyp_va(vcpu->kvm));
> +}
> diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> index 051c2d95e3b7d..1a3f23e90e563 100644
> --- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> +++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
> @@ -640,11 +640,56 @@ static void handle___pkvm_host_mkyoung_guest(struct kvm_cpu_context *host_ctxt)
> cpu_reg(host_ctxt, 1) = ret;
> }
>
> +static void adjust_pc_loaded(struct pkvm_hyp_vcpu *hyp_vcpu,
> + struct kvm_vcpu *host_vcpu)
> +{
> + /*
> + * PKVM_HOST_STATE_DIRTY names the authoritative copy: the host's
> + * when set, the hyp vCPU's otherwise. Adjust that one.
> + */
> + if (vcpu_get_flag(host_vcpu, PKVM_HOST_STATE_DIRTY)) {
> + __kvm_adjust_pc_vm(host_vcpu, hyp_vcpu->vcpu.kvm);
> + return;
> + }
> +
> + /* Reflect the consumed request back, otherwise it stays pending. */
> + vcpu_copy_flag(&hyp_vcpu->vcpu, host_vcpu, PC_UPDATE_REQ);
> + __kvm_adjust_pc(&hyp_vcpu->vcpu);
> + vcpu_copy_flag(host_vcpu, &hyp_vcpu->vcpu, PC_UPDATE_REQ);
And this stuff could simply be a similar set of helpers in
__kvm_adjust_pc(), the original one.
> +}
> +
> +static void adjust_pc_unloaded(struct kvm_vcpu *host_vcpu)
> +{
> + struct kvm *host_kvm;
> +
> + if (!is_protected_kvm_enabled()) {
> + __kvm_adjust_pc(host_vcpu);
> + return;
> + }
> +
> + /* The host copy is authoritative, used only while pinned. */
> + if (hyp_pin_shared_mem(host_vcpu, host_vcpu + 1))
> + return;
Huh. Really?
> +
> + host_kvm = kern_hyp_va(READ_ONCE(host_vcpu->kvm));
> + if (!hyp_pin_shared_mem(host_kvm, host_kvm + 1)) {
> + __kvm_adjust_pc_vm(host_vcpu, host_kvm);
> + hyp_unpin_shared_mem(host_kvm, host_kvm + 1);
> + }
> + hyp_unpin_shared_mem(host_vcpu, host_vcpu + 1);
> +}
> +
> static void handle___kvm_adjust_pc(struct kvm_cpu_context *host_ctxt)
> {
> DECLARE_REG(struct kvm_vcpu *, vcpu, host_ctxt, 1);
> + struct pkvm_hyp_vcpu *hyp_vcpu;
> + struct kvm_vcpu *host_vcpu;
>
> - __kvm_adjust_pc(kern_hyp_va(vcpu));
> + host_vcpu = __get_host_hyp_vcpus(vcpu, &hyp_vcpu);
> + if (!hyp_vcpu)
> + adjust_pc_unloaded(kern_hyp_va(vcpu));
> + else if (!pkvm_hyp_vcpu_is_protected(hyp_vcpu))
> + adjust_pc_loaded(hyp_vcpu, host_vcpu);
> }
>
> static void handle___kvm_flush_vm_context(struct kvm_cpu_context *host_ctxt)
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
next prev parent reply other threads:[~2026-09-11 13:58 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-07 6:59 [PATCH v2 00/16] KVM: arm64: Confine protected VM vCPU state to EL2 Fuad Tabba
2026-09-07 6:59 ` [PATCH v2 01/17] KVM: arm64: Sync HCR_EL2.VSE back to the host vCPU under pKVM Fuad Tabba
2026-09-07 6:59 ` [PATCH v2 02/17] KVM: arm64: Reject the PVTIME vCPU attribute for protected VMs Fuad Tabba
2026-09-07 6:59 ` [PATCH v2 03/17] KVM: arm64: Introduce per-EC entry handlers for pKVM Fuad Tabba
2026-09-07 6:59 ` [PATCH v2 04/17] KVM: arm64: Skip fixed-feature state flush for protected vCPUs Fuad Tabba
2026-09-07 7:23 ` sashiko-bot
2026-09-07 9:11 ` Fuad Tabba
2026-09-07 6:59 ` [PATCH v2 05/17] KVM: arm64: Add {flush,sync}_hyp_timer_state() primitives Fuad Tabba
2026-09-07 6:59 ` [PATCH v2 06/17] KVM: arm64: Add system register reset framework for protected VMs Fuad Tabba
2026-09-07 7:16 ` sashiko-bot
2026-09-07 9:12 ` Fuad Tabba
2026-09-09 13:50 ` Joey Gouly
2026-09-10 10:05 ` Fuad Tabba
2026-09-07 6:59 ` [PATCH v2 07/17] KVM: arm64: Implement HVC handling for protected guests at EL2 Fuad Tabba
2026-09-07 6:59 ` [PATCH v2 08/17] KVM: arm64: Handle PSCI calls for protected VMs " Fuad Tabba
2026-09-07 7:16 ` sashiko-bot
2026-09-07 9:14 ` Fuad Tabba
2026-09-07 6:59 ` [PATCH v2 09/17] KVM: arm64: Restrict KVM_ARM_VCPU_INIT and PSCI version for protected VMs Fuad Tabba
2026-09-07 6:59 ` [PATCH v2 10/17] KVM: arm64: Prevent host PC adjustments for protected vCPUs Fuad Tabba
2026-09-11 13:23 ` Joey Gouly
2026-09-11 13:58 ` Marc Zyngier [this message]
2026-09-07 6:59 ` [PATCH v2 11/17] KVM: arm64: Inject an UNDEF at EL2 for unhandled protected guest exits Fuad Tabba
2026-09-07 6:59 ` [PATCH v2 12/17] KVM: arm64: Add per-EC entry/exit state marshalling for protected guests Fuad Tabba
2026-09-07 7:26 ` sashiko-bot
2026-09-07 9:15 ` Fuad Tabba
2026-09-07 6:59 ` [PATCH v2 13/17] KVM: arm64: Pend a protected guest's SError with HCR_EL2.VSE only Fuad Tabba
2026-09-11 10:29 ` Marc Zyngier
2026-09-11 10:58 ` Fuad Tabba
2026-09-07 6:59 ` [PATCH v2 14/17] KVM: arm64: Reject host access to protected VM private state Fuad Tabba
2026-09-11 12:58 ` Marc Zyngier
2026-09-07 7:00 ` [PATCH v2 15/17] KVM: arm64: Reject host power-on of a vCPU that EL2 holds powered off Fuad Tabba
2026-09-07 7:29 ` sashiko-bot
2026-09-07 9:17 ` Fuad Tabba
2026-09-07 7:00 ` [PATCH v2 16/17] KVM: arm64: Advertise the capabilities that protected VMs support Fuad Tabba
2026-09-07 7:00 ` [PATCH v2 17/17] KVM: arm64: Document the protected VM userspace API 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=86ik4b7uvh.wl-maz@kernel.org \
--to=maz@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=fuad.tabba@linux.dev \
--cc=joey.gouly@arm.com \
--cc=kvmarm@lists.linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=oupton@kernel.org \
--cc=qperret@google.com \
--cc=seiden@linux.ibm.com \
--cc=suzuki.poulose@arm.com \
--cc=tabba@google.com \
--cc=vdonnefort@google.com \
--cc=will@kernel.org \
--cc=yuzenghui@huawei.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.