KVM: arm64: Reorganise vcpu first run
Marc Zyngier
maintainerv2 cover KVM: arm64: Reorganise vcpu first run
This is v2 of this series reworking the vcpu first run.
KVM/arm64 relies heavily on a bunch of things to be done on the first run of the vcpu. We also do a bunch of things on PID change. It turns out that these two things are pretty similar (the first PID change is also the first run).
This small series aims at simplifying all that, and to get rid of the vcpu->arch.has_run_once state. On top of being a decent cleanup, it also results in a minor performance improvement (my Debian installer on M1 consistently went from 2:23 down to 2:22, a ~0.6% reduction in execution time).
* From v1 [1]:
- Moved the kvm_vcpu_initialized() check into kvm_vcpu_first_run_init()
- Reorganise the series so that the userspace irqchip static key change appears earlier
- Collected Andrew's RBs
[1] https://lore.kernel/org/r/[email protected]
Marc Zyngier (5):
KVM: arm64: Move SVE state mapping at HYP to finalize-time
KVM: arm64: Move kvm_arch_vcpu_run_pid_change() out of line
KVM: arm64: Restructure the point where has_run_once is advertised
KVM: arm64: Merge kvm_arch_vcpu_run_pid_change() and
kvm_vcpu_first_run_init()
KVM: arm64: Drop vcpu->arch.has_run_once for vcpu->pid
arch/arm64/include/asm/kvm_host.h | 12 ++----- arch/arm64/kvm/arm.c | 57 +++++++++++++++++-------------- arch/arm64/kvm/fpsimd.c | 11 ------ arch/arm64/kvm/reset.c | 11 +++++- arch/arm64/kvm/vgic/vgic-init.c | 2 +- 5 files changed, 46 insertions(+), 47 deletions(-)
Signature · 1 line
2.30.2
Marc Zyngier
maintainerPATCH v2 1/5 KVM: arm64: Move SVE state mapping at HYP to finalize-time
We currently map the SVE state to HYP on detection of a PID change. Although this matches what we do for FPSIMD, this is pretty pointless for SVE, as the buffer is per-vcpu and has nothing to do with the thread that is being run.
Move the mapping of the SVE state to finalize-time, which is where we allocate the state memory, and thus the most logical place to do this.
--- arch/arm64/kvm/fpsimd.c | 11 ----------- arch/arm64/kvm/reset.c | 11 ++++++++++- 2 files changed, 10 insertions(+), 12 deletions(-)
arch/arm64/kvm/fpsimd.c +0 -11
@@ -43,17 +43,6 @@ int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu)if (ret) goto error; -if (vcpu->arch.sve_state) { -void *sve_end; - - sve_end = vcpu->arch.sve_state +vcpu_sve_state_size (vcpu); - - ret =create_hyp_mappings (vcpu->arch.sve_state, sve_end, -PAGE_HYP ); -if (ret) - goto error; - } - vcpu->arch.host_thread_info =kern_hyp_va (ti); vcpu->arch.host_fpsimd_state =kern_hyp_va (fpsimd); error:
arch/arm64/kvm/reset.c +10 -1
@@ -94,6 +94,8 @@ static int kvm_vcpu_finalize_sve(struct kvm_vcpu *vcpu) {void *buf;unsigned int vl; +size_t reg_sz; +int ret; vl = vcpu->arch.sve_max_vl;
@@ -106,10 +108,17 @@ static int kvm_vcpu_finalize_sve(struct kvm_vcpu *vcpu) vl > SVE_VL_ARCH_MAX))return -EIO ; - buf =kzalloc (SVE_SIG_REGS_SIZE (sve_vq_from_vl (vl)),GFP_KERNEL ); + reg_sz =vcpu_sve_state_size (vcpu); + buf =kzalloc (reg_sz,GFP_KERNEL );if (!buf)return -ENOMEM ; + ret =create_hyp_mappings (buf, buf + reg_sz,PAGE_HYP ); +if (ret) { +kfree (buf); +return ret; + } + vcpu->arch.sve_state = buf; vcpu->arch.flags |=KVM_ARM64_VCPU_SVE_FINALIZED ;return 0 ;
Signature · 1 line
2.30.2
Marc Zyngier
maintainerPATCH v2 2/5 KVM: arm64: Move kvm_arch_vcpu_run_pid_change() out of line
In reply to Marc ZyngierHaving kvm_arch_vcpu_run_pid_change() inline doesn't bring anything to the table. Move it next to kvm_vcpu_first_run_init(), which will be convenient for what is next to come.
--- arch/arm64/include/asm/kvm_host.h | 7 +------ arch/arm64/kvm/arm.c | 5 +++++ 2 files changed, 6 insertions(+), 6 deletions(-)
arch/arm64/include/asm/kvm_host.h +1 -6
@@ -748,12 +748,7 @@ static inline bool kvm_pmu_counter_deferred(struct perf_event_attr *attr)void kvm_arch_vcpu_load_debug_state_flags (struct kvm_vcpu *vcpu);void kvm_arch_vcpu_put_debug_state_flags (struct kvm_vcpu *vcpu); -#ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */ -static inline int kvm_arch_vcpu_run_pid_change (struct kvm_vcpu *vcpu) -{ -return kvm_arch_vcpu_run_map_fp (vcpu); -} - +#ifdef CONFIG_KVM void kvm_set_pmu_events (u32 set,struct perf_event_attr *attr);void kvm_clr_pmu_events (u32 clr);
arch/arm64/kvm/arm.c +5 -0
@@ -583,6 +583,11 @@ static void update_vmid(struct kvm_vmid *vmid)spin_unlock (&kvm_vmid_lock); } +int kvm_arch_vcpu_run_pid_change (struct kvm_vcpu *vcpu) +{ +return kvm_arch_vcpu_run_map_fp (vcpu); +} +static int kvm_vcpu_first_run_init (struct kvm_vcpu *vcpu) {struct kvm *kvm = vcpu->kvm;
Signature · 1 line
2.30.2
Marc Zyngier
maintainerPATCH v2 3/5 KVM: arm64: Restructure the point where has_run_once is advertised
In reply to Marc ZyngierRestructure kvm_vcpu_first_run_init() to set the has_run_once flag after having completed all the "run once" activities.
This includes moving the flip of the userspace irqchip static key to a point where nothing can fail.
--- arch/arm64/kvm/arm.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
arch/arm64/kvm/arm.c +12 -8
@@ -599,8 +599,6 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)if (!kvm_arm_vcpu_is_finalized (vcpu))return -EPERM ; - vcpu->arch.has_run_once = true; -kvm_arm_vcpu_init_debug (vcpu);if (likely (irqchip_in_kernel (kvm))) {
@@ -611,12 +609,6 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu) ret =kvm_vgic_map_resources (kvm);if (ret)return ret; - }else { -/* -* Tell the rest of the code that there are userspace irqchip -* VMs in the wild. -*/ -static_branch_inc (&userspace_irqchip_in_use); } ret =kvm_timer_enable (vcpu);
@@ -624,6 +616,18 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)return ret; ret =kvm_arm_pmu_v3_enable (vcpu); +if (ret) +return ret; + +if (!irqchip_in_kernel (kvm)) { +/* +* Tell the rest of the code that there are userspace irqchip +* VMs in the wild. +*/ +static_branch_inc (&userspace_irqchip_in_use); + } + + vcpu->arch.has_run_once = true;return ret; }
Signature · 1 line
2.30.2
Marc Zyngier
maintainerPATCH v2 4/5 KVM: arm64: Merge kvm_arch_vcpu_run_pid_change() and kvm_vcpu_first_run_init()
In reply to Marc ZyngierThe kvm_arch_vcpu_run_pid_change() helper gets called on each PID change. The kvm_vcpu_first_run_init() helper gets run on the... first run(!) of a vcpu.
As it turns out, the first run of a vcpu also triggers a PID change event (vcpu->pid is initially NULL).
Use this property to merge these two helpers and get rid of another arm64-specific oddity.
--- arch/arm64/kvm/arm.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-)
arch/arm64/kvm/arm.c +18 -18
@@ -583,22 +583,34 @@ static void update_vmid(struct kvm_vmid *vmid)spin_unlock (&kvm_vmid_lock); } -int kvm_arch_vcpu_run_pid_change (struct kvm_vcpu *vcpu) +static int kvm_vcpu_initialized (struct kvm_vcpu *vcpu) { -return kvm_arch_vcpu_run_map_fp (vcpu); +return vcpu->arch.target >=0 ; } -static int kvm_vcpu_first_run_init (struct kvm_vcpu *vcpu) +/* +* Handle both the initialisation that is being done when the vcpu is +* run for the first time, as well as the updates that must be +* performed each time we get a new thread dealing with this vcpu. +*/ +int kvm_arch_vcpu_run_pid_change (struct kvm_vcpu *vcpu) {struct kvm *kvm = vcpu->kvm; -int ret =0 ; +int ret; -if (likely (vcpu->arch.has_run_once)) -return 0 ; +if (!kvm_vcpu_initialized (vcpu)) +return -ENOEXEC ;if (!kvm_arm_vcpu_is_finalized (vcpu))return -EPERM ; + ret =kvm_arch_vcpu_run_map_fp (vcpu); +if (ret) +return ret; + +if (likely (vcpu->arch.has_run_once)) +return 0 ; +kvm_arm_vcpu_init_debug (vcpu);if (likely (irqchip_in_kernel (kvm))) {
@@ -679,11 +691,6 @@ static void vcpu_req_sleep(struct kvm_vcpu *vcpu)smp_rmb (); } -static int kvm_vcpu_initialized (struct kvm_vcpu *vcpu) -{ -return vcpu->arch.target >=0 ; -} -static void check_vcpu_requests (struct kvm_vcpu *vcpu) {if (kvm_request_pending (vcpu)) {
@@ -779,13 +786,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)struct kvm_run *run = vcpu->run;int ret; -if (unlikely (!kvm_vcpu_initialized (vcpu))) -return -ENOEXEC ; - - ret =kvm_vcpu_first_run_init (vcpu); -if (ret) -return ret; -if (run->exit_reason ==KVM_EXIT_MMIO ) { ret =kvm_handle_mmio_return (vcpu);if (ret)
Signature · 1 line
2.30.2
Marc Zyngier
maintainerPATCH v2 5/5 KVM: arm64: Drop vcpu->arch.has_run_once for vcpu->pid
In reply to Marc ZyngierWith the transition to kvm_arch_vcpu_run_pid_change() to handle the "run once" activities, it becomes obvious that has_run_once is now an exact shadow of vcpu->pid.
Replace vcpu->arch.has_run_once with a new vcpu_has_run_once() helper that directly checks for vcpu->pid, and get rid of the now unused field.
--- arch/arm64/include/asm/kvm_host.h | 5 ++--- arch/arm64/kvm/arm.c | 8 +++----- arch/arm64/kvm/vgic/vgic-init.c | 2 +- 3 files changed, 6 insertions(+), 9 deletions(-)
arch/arm64/include/asm/kvm_host.h +2 -3
@@ -366,9 +366,6 @@ struct kvm_vcpu_arch {int target;DECLARE_BITMAP (features,KVM_VCPU_MAX_FEATURES ); -/* Detect first run of a vcpu */ -bool has_run_once; -/* Virtual SError ESR to restore when HCR_EL2.VSE is set */ u64 vsesr_el2;
@@ -605,6 +602,8 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,void kvm_arm_halt_guest (struct kvm *kvm);void kvm_arm_resume_guest (struct kvm *kvm); +#define vcpu_has_run_once (vcpu) !!rcu_access_pointer((vcpu)->pid) +#ifndef __KVM_NVHE_HYPERVISOR__#define kvm_call_hyp_nvhe (f, ...) \ ({ \
arch/arm64/kvm/arm.c +3 -5
@@ -350,7 +350,7 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)void kvm_arch_vcpu_destroy (struct kvm_vcpu *vcpu) { -if (vcpu->arch.has_run_once &&unlikely (!irqchip_in_kernel (vcpu->kvm))) +if (vcpu_has_run_once (vcpu) &&unlikely (!irqchip_in_kernel (vcpu->kvm)))static_branch_dec (&userspace_irqchip_in_use);kvm_mmu_free_memory_cache (&vcpu->arch.mmu_page_cache);
@@ -608,7 +608,7 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)if (ret)return ret; -if (likely (vcpu->arch.has_run_once)) +if (likely (vcpu_has_run_once (vcpu)))return 0 ;kvm_arm_vcpu_init_debug (vcpu);
@@ -639,8 +639,6 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)static_branch_inc (&userspace_irqchip_in_use); } - vcpu->arch.has_run_once = true; -return ret; }
@@ -1123,7 +1121,7 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu,* need to invalidate the I-cache though, as FWB does *not* * imply CTR_EL0.DIC. */ -if (vcpu->arch.has_run_once) { +if (vcpu_has_run_once (vcpu)) {if (!cpus_have_final_cap (ARM64_HAS_STAGE2_FWB ))stage2_unmap_vm (vcpu->kvm);else
arch/arm64/kvm/vgic/vgic-init.c +1 -1
@@ -91,7 +91,7 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)return ret;kvm_for_each_vcpu (i, vcpu, kvm) { -if (vcpu->arch.has_run_once) +if (vcpu_has_run_once (vcpu)) goto out_unlock; } ret =0 ;
Signature · 1 line
2.30.2