| Message ID | 1464128852-14138-3-git-send-email-yunhong.jiang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 24, 2016 at 03:27:30PM -0700, Yunhong Jiang wrote: > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -110,6 +110,10 @@ module_param_named(pml, enable_pml, bool, S_IRUGO); > > #define KVM_VMX_TSC_MULTIPLIER_MAX 0xffffffffffffffffULL > > +static int cpu_preemption_timer_multi; > +static bool __read_mostly enable_hv_timer; > +module_param_named(enable_hv_timer, enable_hv_timer, bool, S_IRUGO); > + Non-techincal issue: I think the naming for the parameter is unfortunate, as it looks similar to a number of CPU properties in QEMU that start with "hv-", which are all related to MS Hyper-V compatibility features. Roman. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14/06/2016 15:23, Roman Kagan wrote: >> > >> > +static int cpu_preemption_timer_multi; >> > +static bool __read_mostly enable_hv_timer; >> > +module_param_named(enable_hv_timer, enable_hv_timer, bool, S_IRUGO); >> > + > Non-techincal issue: I think the naming for the parameter is > unfortunate, as it looks similar to a number of CPU properties in QEMU > that start with "hv-", which are all related to MS Hyper-V compatibility > features. Non-technical but important. Let's change it to preemption_timer (and the variable to enable_preemption_timer). The code can keep the "hv_timer" moniker though. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 14 Jun 2016 15:41:07 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 14/06/2016 15:23, Roman Kagan wrote: > >> > > >> > +static int cpu_preemption_timer_multi; > >> > +static bool __read_mostly enable_hv_timer; > >> > +module_param_named(enable_hv_timer, enable_hv_timer, bool, > >> > S_IRUGO); + > > Non-techincal issue: I think the naming for the parameter is > > unfortunate, as it looks similar to a number of CPU properties in > > QEMU that start with "hv-", which are all related to MS Hyper-V > > compatibility features. > > Non-technical but important. Let's change it to preemption_timer (and > the variable to enable_preemption_timer). > > The code can keep the "hv_timer" moniker though. Hi, Paolo, should I resend the patch for this change? Thanks --jyh > > Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 14 Jun 2016 16:23:19 +0300 Roman Kagan <rkagan@virtuozzo.com> wrote: > On Tue, May 24, 2016 at 03:27:30PM -0700, Yunhong Jiang wrote: > > --- a/arch/x86/kvm/vmx.c > > +++ b/arch/x86/kvm/vmx.c > > @@ -110,6 +110,10 @@ module_param_named(pml, enable_pml, bool, > > S_IRUGO); > > #define KVM_VMX_TSC_MULTIPLIER_MAX 0xffffffffffffffffULL > > > > +static int cpu_preemption_timer_multi; > > +static bool __read_mostly enable_hv_timer; > > +module_param_named(enable_hv_timer, enable_hv_timer, bool, > > S_IRUGO); + > > Non-techincal issue: I think the naming for the parameter is > unfortunate, as it looks similar to a number of CPU properties in QEMU > that start with "hv-", which are all related to MS Hyper-V > compatibility features. Yes, this is a good point and thanks for it. --jyh > > Roman. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 14/06/2016 18:46, yunhong jiang wrote: > On Tue, 14 Jun 2016 15:41:07 +0200 > Paolo Bonzini <pbonzini@redhat.com> wrote: > >> >> >> On 14/06/2016 15:23, Roman Kagan wrote: >>>>> >>>>> +static int cpu_preemption_timer_multi; >>>>> +static bool __read_mostly enable_hv_timer; >>>>> +module_param_named(enable_hv_timer, enable_hv_timer, bool, >>>>> S_IRUGO); + >>> Non-techincal issue: I think the naming for the parameter is >>> unfortunate, as it looks similar to a number of CPU properties in >>> QEMU that start with "hv-", which are all related to MS Hyper-V >>> compatibility features. >> >> Non-technical but important. Let's change it to preemption_timer (and >> the variable to enable_preemption_timer). >> >> The code can keep the "hv_timer" moniker though. > > Hi, Paolo, should I resend the patch for this change? No, no need to. I've already pushed the patches to kvm/queue. I've also separated all vmx bits to one patch and x86 to another. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 14 Jun 2016 23:56:27 +0200 Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 14/06/2016 18:46, yunhong jiang wrote: > > On Tue, 14 Jun 2016 15:41:07 +0200 > > Paolo Bonzini <pbonzini@redhat.com> wrote: > > > >> > >> > >> On 14/06/2016 15:23, Roman Kagan wrote: > >>>>> > >>>>> +static int cpu_preemption_timer_multi; > >>>>> +static bool __read_mostly enable_hv_timer; > >>>>> +module_param_named(enable_hv_timer, enable_hv_timer, bool, > >>>>> S_IRUGO); + > >>> Non-techincal issue: I think the naming for the parameter is > >>> unfortunate, as it looks similar to a number of CPU properties in > >>> QEMU that start with "hv-", which are all related to MS Hyper-V > >>> compatibility features. > >> > >> Non-technical but important. Let's change it to preemption_timer > >> (and the variable to enable_preemption_timer). > >> > >> The code can keep the "hv_timer" moniker though. > > > > Hi, Paolo, should I resend the patch for this change? > > No, no need to. I've already pushed the patches to kvm/queue. I've > also separated all vmx bits to one patch and x86 to another. Aha, I see that. Thanks a lot for the changes. --jyh > > Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 5e6b3ce7748f..c0bae55a9a81 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1006,6 +1006,9 @@ struct kvm_x86_ops { int (*update_pi_irte)(struct kvm *kvm, unsigned int host_irq, uint32_t guest_irq, bool set); void (*apicv_post_state_restore)(struct kvm_vcpu *vcpu); + + void (*set_hv_timer)(struct kvm_vcpu *vcpu, u64 tsc); + void (*cancel_hv_timer)(struct kvm_vcpu *vcpu); }; struct kvm_arch_async_pf { diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index e605d1ed334f..2b29afa61715 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -110,6 +110,10 @@ module_param_named(pml, enable_pml, bool, S_IRUGO); #define KVM_VMX_TSC_MULTIPLIER_MAX 0xffffffffffffffffULL +static int cpu_preemption_timer_multi; +static bool __read_mostly enable_hv_timer; +module_param_named(enable_hv_timer, enable_hv_timer, bool, S_IRUGO); + #define KVM_GUEST_CR0_MASK (X86_CR0_NW | X86_CR0_CD) #define KVM_VM_CR0_ALWAYS_ON_UNRESTRICTED_GUEST (X86_CR0_WP | X86_CR0_NE) #define KVM_VM_CR0_ALWAYS_ON \ @@ -1056,6 +1060,12 @@ static inline bool cpu_has_vmx_virtual_intr_delivery(void) SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY; } +static inline bool cpu_has_vmx_preemption_timer(void) +{ + return vmcs_config.pin_based_exec_ctrl & + PIN_BASED_VMX_PREEMPTION_TIMER; +} + static inline bool cpu_has_vmx_posted_intr(void) { return IS_ENABLED(CONFIG_X86_LOCAL_APIC) && @@ -3306,7 +3316,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf) return -EIO; min = PIN_BASED_EXT_INTR_MASK | PIN_BASED_NMI_EXITING; - opt = PIN_BASED_VIRTUAL_NMIS | PIN_BASED_POSTED_INTR; + opt = PIN_BASED_VIRTUAL_NMIS | PIN_BASED_POSTED_INTR | + PIN_BASED_VMX_PREEMPTION_TIMER; if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_PINBASED_CTLS, &_pin_based_exec_control) < 0) return -EIO; @@ -4779,6 +4790,8 @@ static u32 vmx_pin_based_exec_ctrl(struct vcpu_vmx *vmx) if (!kvm_vcpu_apicv_active(&vmx->vcpu)) pin_based_exec_ctrl &= ~PIN_BASED_POSTED_INTR; + /* Enable the preemption timer dynamically */ + pin_based_exec_ctrl &= ~PIN_BASED_VMX_PREEMPTION_TIMER; return pin_based_exec_ctrl; } @@ -6377,6 +6390,17 @@ static __init int hardware_setup(void) kvm_x86_ops->enable_log_dirty_pt_masked = NULL; } + if (cpu_has_vmx_preemption_timer() && enable_hv_timer) { + u64 vmx_msr; + + rdmsrl(MSR_IA32_VMX_MISC, vmx_msr); + cpu_preemption_timer_multi = + vmx_msr & VMX_MISC_PREEMPTION_TIMER_RATE_MASK; + } else { + kvm_x86_ops->set_hv_timer = NULL; + kvm_x86_ops->cancel_hv_timer = NULL; + } + kvm_set_posted_intr_wakeup_handler(wakeup_handler); return alloc_kvm_area(); @@ -10650,6 +10674,20 @@ static int vmx_check_intercept(struct kvm_vcpu *vcpu, return X86EMUL_CONTINUE; } +static void vmx_set_hv_timer(struct kvm_vcpu *vcpu, u64 delta_tsc) +{ + vmcs_write32(VMX_PREEMPTION_TIMER_VALUE, + delta_tsc >> cpu_preemption_timer_multi); + vmcs_set_bits(PIN_BASED_VM_EXEC_CONTROL, + PIN_BASED_VMX_PREEMPTION_TIMER); +} + +static void vmx_cancel_hv_timer(struct kvm_vcpu *vcpu) +{ + vmcs_clear_bits(PIN_BASED_VM_EXEC_CONTROL, + PIN_BASED_VMX_PREEMPTION_TIMER); +} + static void vmx_sched_in(struct kvm_vcpu *vcpu, int cpu) { if (ple_gap) @@ -11013,6 +11051,9 @@ static struct kvm_x86_ops vmx_x86_ops = { .pmu_ops = &intel_pmu_ops, .update_pi_irte = vmx_update_pi_irte, + + .set_hv_timer = vmx_set_hv_timer, + .cancel_hv_timer = vmx_cancel_hv_timer, }; static int __init vmx_init(void)