| Message ID | 20190416174857.506-5-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: lapic: Fix a variety of timer adv issues | expand |
> On 16 Apr 2019, at 20:48, Sean Christopherson <sean.j.christopherson@intel.com> wrote: > > Add a new KVM param, lapic_timer_advance_autotune, to allow userspace > to disable auto-tuning the timer advancement, e.g. to manually tune the > delay. > > Note, this does not completely restore the original behavior of > lapic_timer_advance_ns. Prior to tracking the advancement per vCPU, > which is necessary to support autotuning, userspace could adjust > lapic_timer_advance_ns for *running* vCPU. With per-vCPU tracking, the > module params are snapshotted at vCPU creation, i.e. applying a new > advancement effectively requires restarting a VM. > > Dynamically updating a running vCPU is possible, e.g. a helper could be > added to retrieve the desired delay, choosing between the global module > param and the per-VCPU value depending on whether or not auto-tuning is > (globally) enabled, but introduces a great deal of complexity. The > wrapper itself is not complex, but understanding and documenting the > effects of dynamically toggling auto-tuning and/or adjusting the timer > advancement is nigh impossible since the behavior would be dependent on > KVM's implementation as well as compiler optimizations. In other words, > providing stable behavior would require extremely careful consideration > now and in the future. > > Given that the expected use of a manually-tuned timer advancement is to > "tune once, run many", use the vastly simpler approach of recognizing > changes to the module params only when creating a new vCPU. > > Fixes: 3b8a5df6c4dc6 ("KVM: LAPIC: Tune lapic_timer_advance_ns automatically") > Cc: Liran Alon <liran.alon@oracle.com> > Cc: Wanpeng Li <wanpengli@tencent.com> > Cc: stable@vger.kernel.org > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> Reviewed-by: Liran Alon <liran.alon@oracle.com> > --- > arch/x86/kvm/lapic.c | 4 +++- > arch/x86/kvm/lapic.h | 3 ++- > arch/x86/kvm/x86.c | 6 +++++- > 3 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 3cd62edf8084..37d3489f68c9 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -2263,7 +2263,8 @@ static enum hrtimer_restart apic_timer_fn(struct hrtimer *data) > return HRTIMER_NORESTART; > } > > -int kvm_create_lapic(struct kvm_vcpu *vcpu, u32 timer_advance_ns) > +int kvm_create_lapic(struct kvm_vcpu *vcpu, u32 timer_advance_ns, > + bool timer_advance_autotune) > { > struct kvm_lapic *apic; > > @@ -2288,6 +2289,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, u32 timer_advance_ns) > HRTIMER_MODE_ABS_PINNED); > apic->lapic_timer.timer.function = apic_timer_fn; > apic->lapic_timer.timer_advance_ns = timer_advance_ns; > + apic->lapic_timer.timer_advance_adjust_done = !timer_advance_autotune; > > /* > * APIC is created enabled. This will prevent kvm_lapic_set_base from > diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h > index 3e97f8a68967..7fa1aed02c14 100644 > --- a/arch/x86/kvm/lapic.h > +++ b/arch/x86/kvm/lapic.h > @@ -64,7 +64,8 @@ struct kvm_lapic { > > struct dest_map; > > -int kvm_create_lapic(struct kvm_vcpu *vcpu, u32 timer_advance_ns); > +int kvm_create_lapic(struct kvm_vcpu *vcpu, u32 timer_advance_ns, > + bool timer_advance_autotune); > void kvm_free_lapic(struct kvm_vcpu *vcpu); > > int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu); > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index b303a21a2bc2..60d80e354f62 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -140,6 +140,9 @@ module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR); > static u32 __read_mostly lapic_timer_advance_ns = 1000; > module_param(lapic_timer_advance_ns, uint, S_IRUGO | S_IWUSR); > > +static bool __read_mostly lapic_timer_advance_autotune = true; > +module_param(lapic_timer_advance_autotune, bool, S_IRUGO | S_IWUSR); > + > static bool __read_mostly vector_hashing = true; > module_param(vector_hashing, bool, S_IRUGO); > > @@ -9078,7 +9081,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) > > if (irqchip_in_kernel(vcpu->kvm)) { > vcpu->arch.apicv_active = kvm_x86_ops->get_enable_apicv(vcpu); > - r = kvm_create_lapic(vcpu, lapic_timer_advance_ns); > + r = kvm_create_lapic(vcpu, lapic_timer_advance_ns, > + lapic_timer_advance_autotune); > if (r < 0) > goto fail_mmu_destroy; > } else > -- > 2.21.0 >
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 3cd62edf8084..37d3489f68c9 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -2263,7 +2263,8 @@ static enum hrtimer_restart apic_timer_fn(struct hrtimer *data) return HRTIMER_NORESTART; } -int kvm_create_lapic(struct kvm_vcpu *vcpu, u32 timer_advance_ns) +int kvm_create_lapic(struct kvm_vcpu *vcpu, u32 timer_advance_ns, + bool timer_advance_autotune) { struct kvm_lapic *apic; @@ -2288,6 +2289,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, u32 timer_advance_ns) HRTIMER_MODE_ABS_PINNED); apic->lapic_timer.timer.function = apic_timer_fn; apic->lapic_timer.timer_advance_ns = timer_advance_ns; + apic->lapic_timer.timer_advance_adjust_done = !timer_advance_autotune; /* * APIC is created enabled. This will prevent kvm_lapic_set_base from diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h index 3e97f8a68967..7fa1aed02c14 100644 --- a/arch/x86/kvm/lapic.h +++ b/arch/x86/kvm/lapic.h @@ -64,7 +64,8 @@ struct kvm_lapic { struct dest_map; -int kvm_create_lapic(struct kvm_vcpu *vcpu, u32 timer_advance_ns); +int kvm_create_lapic(struct kvm_vcpu *vcpu, u32 timer_advance_ns, + bool timer_advance_autotune); void kvm_free_lapic(struct kvm_vcpu *vcpu); int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index b303a21a2bc2..60d80e354f62 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -140,6 +140,9 @@ module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR); static u32 __read_mostly lapic_timer_advance_ns = 1000; module_param(lapic_timer_advance_ns, uint, S_IRUGO | S_IWUSR); +static bool __read_mostly lapic_timer_advance_autotune = true; +module_param(lapic_timer_advance_autotune, bool, S_IRUGO | S_IWUSR); + static bool __read_mostly vector_hashing = true; module_param(vector_hashing, bool, S_IRUGO); @@ -9078,7 +9081,8 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu) if (irqchip_in_kernel(vcpu->kvm)) { vcpu->arch.apicv_active = kvm_x86_ops->get_enable_apicv(vcpu); - r = kvm_create_lapic(vcpu, lapic_timer_advance_ns); + r = kvm_create_lapic(vcpu, lapic_timer_advance_ns, + lapic_timer_advance_autotune); if (r < 0) goto fail_mmu_destroy; } else
Add a new KVM param, lapic_timer_advance_autotune, to allow userspace to disable auto-tuning the timer advancement, e.g. to manually tune the delay. Note, this does not completely restore the original behavior of lapic_timer_advance_ns. Prior to tracking the advancement per vCPU, which is necessary to support autotuning, userspace could adjust lapic_timer_advance_ns for *running* vCPU. With per-vCPU tracking, the module params are snapshotted at vCPU creation, i.e. applying a new advancement effectively requires restarting a VM. Dynamically updating a running vCPU is possible, e.g. a helper could be added to retrieve the desired delay, choosing between the global module param and the per-VCPU value depending on whether or not auto-tuning is (globally) enabled, but introduces a great deal of complexity. The wrapper itself is not complex, but understanding and documenting the effects of dynamically toggling auto-tuning and/or adjusting the timer advancement is nigh impossible since the behavior would be dependent on KVM's implementation as well as compiler optimizations. In other words, providing stable behavior would require extremely careful consideration now and in the future. Given that the expected use of a manually-tuned timer advancement is to "tune once, run many", use the vastly simpler approach of recognizing changes to the module params only when creating a new vCPU. Fixes: 3b8a5df6c4dc6 ("KVM: LAPIC: Tune lapic_timer_advance_ns automatically") Cc: Liran Alon <liran.alon@oracle.com> Cc: Wanpeng Li <wanpengli@tencent.com> Cc: stable@vger.kernel.org Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/kvm/lapic.c | 4 +++- arch/x86/kvm/lapic.h | 3 ++- arch/x86/kvm/x86.c | 6 +++++- 3 files changed, 10 insertions(+), 3 deletions(-)