Re: [PATCH v17 05/20] KVM: arm64: Add vcpu load/put call backs for flavors
From: Marc Zyngier <maz@kernel.org>
Date: 2026-09-13 10:26:55
Also in:
kvm, kvmarm, linux-arm-kernel, lkml
On Tue, 08 Sep 2026 17:22:08 +0100, Suzuki K Poulose [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Add VM flavor specific handlers for VCPU load/put, in an effort to make it easier to follow the code. Based on a patch by Marc Zyngier Suggested-by: Marc Zyngier <maz@kernel.org> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> --- arch/arm64/include/asm/kvm_host.h | 6 ++ arch/arm64/kvm/arm.c | 156 ++++++++++++++++++++++-------- 2 files changed, 123 insertions(+), 39 deletions(-)diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index d0dccc9ad6aa8..b2e99c5cb1cd3 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h@@ -150,6 +150,11 @@ struct kvm_vmid { atomic64_t id; }; +struct kvm_vcpu_ops { + void (*vcpu_load)(struct kvm_vcpu *vcpu, int cpu); + void (*vcpu_put)(struct kvm_vcpu *vcpu); +}; + struct kvm_s2_mmu { struct kvm_vmid vmid;@@ -854,6 +859,7 @@ struct vncr_tlb; struct kvm_vcpu_arch { struct kvm_cpu_context ctxt; + const struct kvm_vcpu_ops *vcpu_ops; /* * Guest floating point statediff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 51fc651267157..9af3bbb2f8c24 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c@@ -74,6 +74,8 @@ struct kvm_ioctl_cap_map { long ext; }; +static const struct kvm_vcpu_ops *arm64_vcpu_ops[VM_FLAVOR_MAX]; + /* Make KVM_CAP_NR_VCPUS the reference for features we always supported */ #define KVM_CAP_ARM_BASIC KVM_CAP_NR_VCPUS@@ -569,6 +571,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) mutex_unlock(&vcpu->mutex); #endif + vcpu->arch.vcpu_ops = arm64_vcpu_ops[vcpu->kvm->arch.vm_flavor]; + /* Force users to call KVM_ARM_VCPU_INIT */ vcpu_clear_flag(vcpu, VCPU_INITIALIZED);@@ -738,36 +742,72 @@ static void vcpu_load_pvtime(struct kvm_vcpu *vcpu) kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu); } +static void vhe_vcpu_load(struct kvm_vcpu *vcpu, int cpu) +{ + vcpu_prepare_mmu(vcpu); + /* + * The timer must be loaded before the vgic to correctly set up physical + * interrupt deactivation in nested state (e.g. timer interrupt). + */ + kvm_timer_vcpu_load(vcpu); + kvm_vgic_load(vcpu); + kvm_vcpu_load_debug(vcpu); + kvm_vcpu_load_fgt(vcpu); + kvm_vcpu_load_vhe(vcpu); + kvm_arch_vcpu_load_fp(vcpu); + kvm_vcpu_pmu_restore_guest(vcpu); + + vcpu_load_pvtime(vcpu); + vcpu_set_wfx_traps(vcpu); + vcpu_set_pauth_traps(vcpu); +} + +static void nvhe_vcpu_load(struct kvm_vcpu *vcpu, int cpu) +{ + vcpu_prepare_mmu(vcpu); + /* + * The timer must be loaded before the vgic to correctly set up physical + * interrupt deactivation in nested state (e.g. timer interrupt). + */
This comment makes no sense here -- it is strictly for VHE, which is the only mode to implement NV. Same thing for the pKVM vcpu_load().
+ kvm_timer_vcpu_load(vcpu);
+ kvm_vgic_load(vcpu);
+ kvm_vcpu_load_debug(vcpu);
+ kvm_vcpu_load_fgt(vcpu);
+ kvm_arch_vcpu_load_fp(vcpu);
+ kvm_vcpu_pmu_restore_guest(vcpu);
+
+ vcpu_load_pvtime(vcpu);
+ vcpu_set_wfx_traps(vcpu);
+ vcpu_set_pauth_traps(vcpu);
+}
+
+static void pkvm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
+{
+ /*
+ * The timer must be loaded before the vgic to correctly set up physical
+ * interrupt deactivation in nested state (e.g. timer interrupt).
+ */
+ kvm_timer_vcpu_load(vcpu);
+ kvm_vgic_load(vcpu);
+ kvm_vcpu_load_debug(vcpu);
+ kvm_vcpu_load_fgt(vcpu);
+ kvm_arch_vcpu_load_fp(vcpu);
+ kvm_vcpu_pmu_restore_guest(vcpu);
+
+ vcpu_load_pvtime(vcpu);
+ vcpu_set_wfx_traps(vcpu);
+
+ kvm_call_hyp_nvhe(__pkvm_vcpu_load,
+ vcpu->kvm->arch.pkvm.handle,
+ vcpu->vcpu_idx, vcpu->arch.hcr_el2);
+ kvm_call_hyp(__vgic_v3_restore_vmcr_aprs,
+ &vcpu->arch.vgic_cpu.vgic_v3);This can also be turned into a kvm_call_hyp_nvhe().
quoted hunk ↗ jump to hunk
+} + void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) { - if (!is_protected_kvm_enabled()) - vcpu_prepare_mmu(vcpu); - vcpu->cpu = cpu; - /* - * The timer must be loaded before the vgic to correctly set up physical - * interrupt deactivation in nested state (e.g. timer interrupt). - */ - kvm_timer_vcpu_load(vcpu); - kvm_vgic_load(vcpu); - kvm_vcpu_load_debug(vcpu); - kvm_vcpu_load_fgt(vcpu); - if (has_vhe()) - kvm_vcpu_load_vhe(vcpu); - kvm_arch_vcpu_load_fp(vcpu); - kvm_vcpu_pmu_restore_guest(vcpu); - - vcpu_load_pvtime(vcpu); - vcpu_set_wfx_traps(vcpu); - vcpu_set_pauth_traps(vcpu); - - if (is_protected_kvm_enabled()) { - kvm_call_hyp_nvhe(__pkvm_vcpu_load, - vcpu->kvm->arch.pkvm.handle, - vcpu->vcpu_idx, vcpu->arch.hcr_el2); - kvm_call_hyp(__vgic_v3_restore_vmcr_aprs, - &vcpu->arch.vgic_cpu.vgic_v3); - } + vcpu->arch.vcpu_ops->vcpu_load(vcpu, cpu); if (!cpumask_test_cpu(cpu, vcpu->kvm->arch.supported_cpus)) vcpu_set_on_unsupported_cpu(vcpu);@@ -775,28 +815,44 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) vcpu->arch.pid = pid_nr(vcpu->pid); } -void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) +static void vhe_vcpu_put(struct kvm_vcpu *vcpu) { - if (is_protected_kvm_enabled()) { - kvm_call_hyp(__vgic_v3_save_aprs, &vcpu->arch.vgic_cpu.vgic_v3); - kvm_call_hyp_nvhe(__pkvm_vcpu_put); - - /* __pkvm_vcpu_put implies a sync of the state */ - if (!kvm_vm_is_protected(vcpu->kvm)) - vcpu_set_flag(vcpu, PKVM_HOST_STATE_DIRTY); - } - kvm_vcpu_put_debug(vcpu); kvm_arch_vcpu_put_fp(vcpu); - if (has_vhe()) - kvm_vcpu_put_vhe(vcpu); + kvm_vcpu_put_vhe(vcpu); kvm_timer_vcpu_put(vcpu); kvm_vgic_put(vcpu); kvm_vcpu_pmu_restore_host(vcpu); if (vcpu_has_nv(vcpu)) kvm_vcpu_put_hw_mmu(vcpu); kvm_arm_vmid_clear_active(); +} +static void nvhe_vcpu_put(struct kvm_vcpu *vcpu) +{ + kvm_vcpu_put_debug(vcpu); + kvm_arch_vcpu_put_fp(vcpu); + kvm_timer_vcpu_put(vcpu); + kvm_vgic_put(vcpu); + kvm_vcpu_pmu_restore_host(vcpu); + kvm_arm_vmid_clear_active(); +} + +static void pkvm_vcpu_put(struct kvm_vcpu *vcpu) +{ + kvm_call_hyp(__vgic_v3_save_aprs, &vcpu->arch.vgic_cpu.vgic_v3);
Same thing here about kvm_call_hyp_nvhe().
+ kvm_call_hyp_nvhe(__pkvm_vcpu_put); + + /* __pkvm_vcpu_put implies a sync of the state */ + if (!kvm_vm_is_protected(vcpu->kvm)) + vcpu_set_flag(vcpu, PKVM_HOST_STATE_DIRTY); + + nvhe_vcpu_put(vcpu);
I'm not overly fond of this. Yes, that was in my original patch. But for example, we end-up calling kvm_arm_vmid_clear_active() for pKVM. This is harmless, but conceptually wrong. I'd rather you expand the whole thing.
quoted hunk ↗ jump to hunk
+} + +void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) +{ + vcpu->arch.vcpu_ops->vcpu_put(vcpu); vcpu_clear_on_unsupported_cpu(vcpu); vcpu->cpu = -1; }@@ -2136,6 +2192,28 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) } } +static const struct kvm_vcpu_ops vhe_vcpu_ops = { + .vcpu_load = vhe_vcpu_load, + .vcpu_put = vhe_vcpu_put, +}; + +static const struct kvm_vcpu_ops nvhe_vcpu_ops = { + .vcpu_load = nvhe_vcpu_load, + .vcpu_put = nvhe_vcpu_put, +}; + +static const struct kvm_vcpu_ops pkvm_vcpu_ops = { + .vcpu_load = pkvm_vcpu_load, + .vcpu_put = pkvm_vcpu_put, +}; + +static const struct kvm_vcpu_ops *arm64_vcpu_ops[] = { + [VM_VHE] = &vhe_vcpu_ops, + [VM_NVHE] = &nvhe_vcpu_ops, + [VM_PKVM] = &pkvm_vcpu_ops, + [VM_PROTECTED_PKVM] = &pkvm_vcpu_ops,
nit: my OCD-self wants to align all the '=' signs vertically... Thanks, M. -- Without deviation from the norm, progress is not possible.