Re: [PATCH v17 05/20] KVM: arm64: Add vcpu load/put call backs for flavors
From: Suzuki K Poulose
Date: Thu Sep 10 2026 - 04:47:00 EST
Hi Gavin
On 10/09/2026 06:33, Gavin Shan wrote:
Hi Suzuki,
On 9/9/26 2:22 AM, Suzuki K Poulose wrote:
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@xxxxxxxxxx>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
---
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_vcpu::cpu has been set to 'cpu' before vcpu_load() is called in
kvm_arch_vcpu_load(), so we needn't explicitly pass @cpu to vcpu_load()?
Ack
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;
I think this would be a field of 'struct kvm_arch' if all vCPUs inside a VM
have same vCPU operations.
This was done to avoid the long list of pointer chasing :
vcpu->kvm->arch.vcpu_ops->
Given this is vcpu specific ops and a single pointer per vcpu, we can make that easier to read.
/*
* Guest floating point state
diff --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).
+ */
+ 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);
It seems vcpu_set_pauth_traps(vcpu) has been missed here?
No, if you see the vcpu_set_pauth_traps(), we do nothing for is_protected_kvm_enabled(). So skipped that explicitly here.
I didn't remove that condition from vcpu_set_pauth_traps()
just to keep it safer.
+
+ 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);
+}
+
...
+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,
+};
+
We may avoid the variables {vhe, nvhe, pkvm}_vcpu_ops which are used for once:
static const struct kvm_vcpu_ops kvm_vcpu_ops[] = {
[VM_VHE] = { vhe_vcpu_load, vhe_vcpu_put },
[VM_NVHE] = { nvhe_vcpu_load, nvhe_vcpu_put },
[VM_PKVM] = { pkvm_vcpu_load, pkvm_vcpu_put },
[VM_PROTECTED_PKVM] = { pkvm_vcpu_load, pkvm_vcpu_put },
};
The structure might grow in the future (I have a vcpu_run callback in testing). So it does make sense to use the variables to keep it tidy.
I will add checks to make sure that the fields are initialised properly.
Cheers
Suzuki
static unsigned long nvhe_percpu_size(void)
{
return (unsigned long)CHOOSE_NVHE_SYM(__per_cpu_end) -
Thanks,
Gavin