[PATCH v4 0/3] KVM: Introduce dedicated vCPUs hint KVM_HINTS_DEDICATED

[PATCH v4 0/3] KVM: Introduce dedicated vCPUs hint KVM_HINTS_DEDICATED

[Wanpeng Li]

This patchset introduces dedicated vCPUs(vCPU pinning, and there is no 
vCPU over-commitment) hint KVM_HINTS_DEDICATED, it has two users now:

1) Waiman Long mentioned that:

    Generally speaking, unfair lock performs well for VMs with a small
    number of vCPUs. Native qspinlock may perform better than pvqspinlock
    if there is vCPU pinning and there is no vCPU over-commitment.

2) vCPUs are very unlikely to get preempted when they are the only task 
   running on a CPU. PV TLB flush is slower that the native flush in that 
   case.

v3 -> v4:
 * update feature bit document
v2 -> v3:
 * a separate table for CPUID[0x40000001].EDX bits
 * a new kvm_hint_has_feature macro
v1 -> v2:
 * update to KVM_HINTS_DEDICATED

Wanpeng Li (3):
  KVM: Introduce dedicated vCPUs hint KVM_HINTS_DEDICATED
  KVM: X86: Choose qspinlock when dedicated vCPUs available
  KVM: X86: Don't use PV TLB flush with dedicated vCPUs and steal time disabled

 Documentation/virtual/kvm/cpuid.txt  | 15 +++++++++++++--
 arch/mips/include/asm/kvm_para.h     |  5 +++++
 arch/powerpc/include/asm/kvm_para.h  |  5 +++++
 arch/s390/include/asm/kvm_para.h     |  5 +++++
 arch/x86/include/asm/kvm_para.h      |  6 ++++++
 arch/x86/include/uapi/asm/kvm_para.h |  8 ++++++--
 arch/x86/kernel/kvm.c                | 18 ++++++++++++++++--
 include/asm-generic/kvm_para.h       |  5 +++++
 include/linux/kvm_para.h             |  5 +++++
 9 files changed, 66 insertions(+), 6 deletions(-)

-- 
2.7.4

[PATCH v4 1/3] KVM: Introduce dedicated vCPUs hint KVM_HINTS_DEDICATED

[Wanpeng Li]

From: Wanpeng Li <wanpengli@tencent.com>

This patch introduces dedicated vCPUs(vCPU pinning, and there is 
no vCPU over-commitment) hint KVM_HINTS_DEDICATED, guest checks 
this feature bit to determine if they run on dedicated vCPUs, 
allowing optimizations.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
---
 Documentation/virtual/kvm/cpuid.txt  | 15 +++++++++++++--
 arch/mips/include/asm/kvm_para.h     |  5 +++++
 arch/powerpc/include/asm/kvm_para.h  |  5 +++++
 arch/s390/include/asm/kvm_para.h     |  5 +++++
 arch/x86/include/asm/kvm_para.h      |  6 ++++++
 arch/x86/include/uapi/asm/kvm_para.h |  8 ++++++--
 arch/x86/kernel/kvm.c                |  5 +++++
 include/asm-generic/kvm_para.h       |  5 +++++
 include/linux/kvm_para.h             |  5 +++++
 9 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/Documentation/virtual/kvm/cpuid.txt b/Documentation/virtual/kvm/cpuid.txt
index dcab6dc..1600768 100644
--- a/Documentation/virtual/kvm/cpuid.txt
+++ b/Documentation/virtual/kvm/cpuid.txt
@@ -23,8 +23,8 @@ This function queries the presence of KVM cpuid leafs.
 
 
 function: define KVM_CPUID_FEATURES (0x40000001)
-returns : ebx, ecx, edx = 0
-          eax = and OR'ed group of (1 << flag), where each flags is:
+returns : ebx, ecx
+          eax = an OR'ed group of (1 << flag), where each flags is:
 
 
 flag                               || value || meaning
@@ -62,3 +62,14 @@ KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ||    24 || host will warn if no guest-side
                                    ||       || per-cpu warps are expected in
                                    ||       || kvmclock.
 ------------------------------------------------------------------------------
+
+          edx = an OR'ed group of (1 << flag), where each flags is:
+
+
+flag                               || value || meaning
+==================================================================================
+KVM_HINTS_DEDICATED                ||     0 || guest checks this feature bit to
+                                   ||       || determine if there is vCPU pinning
+                                   ||       || and there is no vCPU over-commitment,
+                                   ||       || allowing optimizations
+----------------------------------------------------------------------------------
diff --git a/arch/mips/include/asm/kvm_para.h b/arch/mips/include/asm/kvm_para.h
index 60b1aa0..bd1f4ee 100644
--- a/arch/mips/include/asm/kvm_para.h
+++ b/arch/mips/include/asm/kvm_para.h
@@ -94,6 +94,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return 0;
 }
 
+static inline unsigned int kvm_arch_hint_features(void)
+{
+	return 0;
+}
+
 #ifdef CONFIG_MIPS_PARAVIRT
 static inline bool kvm_para_available(void)
 {
diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h
index 336a91a..8e58c00 100644
--- a/arch/powerpc/include/asm/kvm_para.h
+++ b/arch/powerpc/include/asm/kvm_para.h
@@ -61,6 +61,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return r;
 }
 
+static inline unsigned int kvm_arch_hint_features(void)
+{
+	return 0;
+}
+
 static inline bool kvm_check_and_clear_guest_paused(void)
 {
 	return false;
diff --git a/arch/s390/include/asm/kvm_para.h b/arch/s390/include/asm/kvm_para.h
index 74eeec9..b2c935c 100644
--- a/arch/s390/include/asm/kvm_para.h
+++ b/arch/s390/include/asm/kvm_para.h
@@ -193,6 +193,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return 0;
 }
 
+static inline unsigned int kvm_arch_hint_features(void)
+{
+	return 0;
+}
+
 static inline bool kvm_check_and_clear_guest_paused(void)
 {
 	return false;
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 7b407dd..2c7d368 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -88,6 +88,7 @@ static inline long kvm_hypercall4(unsigned int nr, unsigned long p1,
 #ifdef CONFIG_KVM_GUEST
 bool kvm_para_available(void);
 unsigned int kvm_arch_para_features(void);
+unsigned int kvm_arch_hint_features(void);
 void kvm_async_pf_task_wait(u32 token, int interrupt_kernel);
 void kvm_async_pf_task_wake(u32 token);
 u32 kvm_read_and_reset_pf_reason(void);
@@ -115,6 +116,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return 0;
 }
 
+static inline unsigned int kvm_arch_hint_features(void)
+{
+	return 0;
+}
+
 static inline u32 kvm_read_and_reset_pf_reason(void)
 {
 	return 0;
diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
index 7a2ade4..e8f5dfb 100644
--- a/arch/x86/include/uapi/asm/kvm_para.h
+++ b/arch/x86/include/uapi/asm/kvm_para.h
@@ -10,8 +10,10 @@
  */
 #define KVM_CPUID_SIGNATURE	0x40000000
 
-/* This CPUID returns a feature bitmap in eax.  Before enabling a particular
- * paravirtualization, the appropriate feature bit should be checked.
+/* This CPUID returns two feature bitmaps in eax, edx. Before enabling
+ * a particular paravirtualization, the appropriate feature bit should
+ * be checked in eax. The performance hint feature bit should be checked
+ * in edx.
  */
 #define KVM_CPUID_FEATURES	0x40000001
 #define KVM_FEATURE_CLOCKSOURCE		0
@@ -27,6 +29,8 @@
 #define KVM_FEATURE_PV_UNHALT		7
 #define KVM_FEATURE_PV_TLB_FLUSH	9
 
+#define KVM_HINTS_DEDICATED      0
+
 /* The last 8 bits are used to indicate how to interpret the flags field
  * in pvclock structure. If no bits are set, all flags are ignored.
  */
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 4e37d1a..77a0723 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -604,6 +604,11 @@ unsigned int kvm_arch_para_features(void)
 	return cpuid_eax(kvm_cpuid_base() | KVM_CPUID_FEATURES);
 }
 
+unsigned int kvm_arch_hint_features(void)
+{
+	return cpuid_edx(kvm_cpuid_base() | KVM_CPUID_FEATURES);
+}
+
 static uint32_t __init kvm_detect(void)
 {
 	return kvm_cpuid_base();
diff --git a/include/asm-generic/kvm_para.h b/include/asm-generic/kvm_para.h
index 18c6abe..93e133d 100644
--- a/include/asm-generic/kvm_para.h
+++ b/include/asm-generic/kvm_para.h
@@ -19,6 +19,11 @@ static inline unsigned int kvm_arch_para_features(void)
 	return 0;
 }
 
+static inline unsigned int kvm_arch_hint_features(void)
+{
+	return 0;
+}
+
 static inline bool kvm_para_available(void)
 {
 	return false;
diff --git a/include/linux/kvm_para.h b/include/linux/kvm_para.h
index 51f6ef2..30503b7 100644
--- a/include/linux/kvm_para.h
+++ b/include/linux/kvm_para.h
@@ -9,4 +9,9 @@ static inline bool kvm_para_has_feature(unsigned int feature)
 {
 	return !!(kvm_arch_para_features() & (1UL << feature));
 }
+
+static inline bool kvm_hint_has_feature(unsigned int feature)
+{
+	return !!(kvm_arch_hint_features() & (1UL << feature));
+}
 #endif /* __LINUX_KVM_PARA_H */
-- 
2.7.4

[PATCH v4 2/3] KVM: X86: Choose qspinlock when dedicated vCPUs available

[Wanpeng Li]

From: Wanpeng Li <wanpengli@tencent.com>

Waiman Long mentioned that:

 Generally speaking, unfair lock performs well for VMs with a small
 number of vCPUs. Native qspinlock may perform better than pvqspinlock
 if there is vCPU pinning and there is no vCPU over-commitment.

This patch uses a KVM_HINTS_DEDICATED performance hint to allow
hypervisor admin to choose the qspinlock to be used when a dedicated
vCPU is available.

PV_DEDICATED = 1, PV_UNHALT = anything: default is qspinlock
PV_DEDICATED = 0, PV_UNHALT = 1: default is Hybrid PV queued/unfair lock
PV_DEDICATED = 0, PV_UNHALT = 0: default is tas

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
---
 arch/x86/kernel/kvm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 77a0723..c5566d9 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -733,6 +733,11 @@ void __init kvm_spinlock_init(void)
 	if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
 		return;
 
+	if (kvm_hint_has_feature(KVM_HINTS_DEDICATED)) {
+		static_branch_disable(&virt_spin_lock_key);
+		return;
+	}
+
 	__pv_init_lock_hash();
 	pv_lock_ops.queued_spin_lock_slowpath = __pv_queued_spin_lock_slowpath;
 	pv_lock_ops.queued_spin_unlock = PV_CALLEE_SAVE(__pv_queued_spin_unlock);
-- 
2.7.4

[PATCH v4 3/3] KVM: X86: Don't use PV TLB flush with dedicated vCPUs and steal time disabled

[Wanpeng Li]

From: Wanpeng Li <wanpengli@tencent.com>

vCPUs are very unlikely to get preempted when they are the only task
running on a CPU.  PV TLB flush is slower that the native flush in that
case. In addition, avoid traversing all the cpus for pv tlb flush when
steal time is disabled since pv tlb flush depends on the field in steal
time for shared data.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
---
 arch/x86/kernel/kvm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index c5566d9..285822f 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -545,7 +545,9 @@ static void __init kvm_guest_init(void)
 		pv_time_ops.steal_clock = kvm_steal_clock;
 	}
 
-	if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH))
+	if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH) &&
+		!kvm_para_has_feature(KVM_HINTS_DEDICATED) &&
+		!kvm_para_has_feature(KVM_FEATURE_STEAL_TIME))
 		pv_mmu_ops.flush_tlb_others = kvm_flush_tlb_others;
 
 	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
@@ -638,7 +640,9 @@ static __init int kvm_setup_pv_tlb_flush(void)
 {
 	int cpu;
 
-	if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH)) {
+	if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH) &&
+		!kvm_para_has_feature(KVM_HINTS_DEDICATED) &&
+		!kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
 		for_each_possible_cpu(cpu) {
 			zalloc_cpumask_var_node(per_cpu_ptr(&__pv_tlb_mask, cpu),
 				GFP_KERNEL, cpu_to_node(cpu));
-- 
2.7.4

Re: [PATCH v4 3/3] KVM: X86: Don't use PV TLB flush with dedicated vCPUs and steal time disabled

[Paolo Bonzini]

On 13/02/2018 02:05, Wanpeng Li wrote:
> From: Wanpeng Li <wanpengli@tencent.com>
> 
> vCPUs are very unlikely to get preempted when they are the only task
> running on a CPU.  PV TLB flush is slower that the native flush in that
> case. In addition, avoid traversing all the cpus for pv tlb flush when
> steal time is disabled since pv tlb flush depends on the field in steal
> time for shared data.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
>  arch/x86/kernel/kvm.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index c5566d9..285822f 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -545,7 +545,9 @@ static void __init kvm_guest_init(void)
>  		pv_time_ops.steal_clock = kvm_steal_clock;
>  	}
>  
> -	if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH))
> +	if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH) &&
> +		!kvm_para_has_feature(KVM_HINTS_DEDICATED) &&
> +		!kvm_para_has_feature(KVM_FEATURE_STEAL_TIME))
>  		pv_mmu_ops.flush_tlb_others = kvm_flush_tlb_others;
>  
>  	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
> @@ -638,7 +640,9 @@ static __init int kvm_setup_pv_tlb_flush(void)
>  {
>  	int cpu;
>  
> -	if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH)) {
> +	if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH) &&
> +		!kvm_para_has_feature(KVM_HINTS_DEDICATED) &&

This should have checked the hints word.

In general, I'm going to change in the whole series
kvm_hint_has_feature with kvm_para_has_hint, and kvm_arch_hint_features
with kvm_arch_para_hints.  But apart from this small naming issue, the
series looks good, and I'm applying it to kvm/queue.

Paolo

> +		!kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
>  		for_each_possible_cpu(cpu) {
>  			zalloc_cpumask_var_node(per_cpu_ptr(&__pv_tlb_mask, cpu),
>  				GFP_KERNEL, cpu_to_node(cpu));
> 

Re: [PATCH v4 3/3] KVM: X86: Don't use PV TLB flush with dedicated vCPUs and steal time disabled

[Wanpeng Li]

2018-02-14 0:12 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
> On 13/02/2018 02:05, Wanpeng Li wrote:
>> From: Wanpeng Li <wanpengli@tencent.com>
>>
>> vCPUs are very unlikely to get preempted when they are the only task
>> running on a CPU.  PV TLB flush is slower that the native flush in that
>> case. In addition, avoid traversing all the cpus for pv tlb flush when
>> steal time is disabled since pv tlb flush depends on the field in steal
>> time for shared data.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Cc: Eduardo Habkost <ehabkost@redhat.com>
>> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
>> ---
>>  arch/x86/kernel/kvm.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
>> index c5566d9..285822f 100644
>> --- a/arch/x86/kernel/kvm.c
>> +++ b/arch/x86/kernel/kvm.c
>> @@ -545,7 +545,9 @@ static void __init kvm_guest_init(void)
>>               pv_time_ops.steal_clock = kvm_steal_clock;
>>       }
>>
>> -     if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH))
>> +     if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH) &&
>> +             !kvm_para_has_feature(KVM_HINTS_DEDICATED) &&
>> +             !kvm_para_has_feature(KVM_FEATURE_STEAL_TIME))
>>               pv_mmu_ops.flush_tlb_others = kvm_flush_tlb_others;
>>
>>       if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
>> @@ -638,7 +640,9 @@ static __init int kvm_setup_pv_tlb_flush(void)
>>  {
>>       int cpu;
>>
>> -     if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH)) {
>> +     if (kvm_para_has_feature(KVM_FEATURE_PV_TLB_FLUSH) &&
>> +             !kvm_para_has_feature(KVM_HINTS_DEDICATED) &&
>
> This should have checked the hints word.
>
> In general, I'm going to change in the whole series
> kvm_hint_has_feature with kvm_para_has_hint, and kvm_arch_hint_features
> with kvm_arch_para_hints.  But apart from this small naming issue, the
> series looks good, and I'm applying it to kvm/queue.

Thanks Paolo.

Regards,
Wanpeng Li

Re: [PATCH v4 0/3] KVM: Introduce dedicated vCPUs hint KVM_HINTS_DEDICATED

[Eduardo Valentin]

Folks,

On Tue, Feb 13, 2018 at 09:05:39AM +0800, Wanpeng Li wrote:
> This patchset introduces dedicated vCPUs(vCPU pinning, and there is no 
> vCPU over-commitment) hint KVM_HINTS_DEDICATED, it has two users now:
> 
> 1) Waiman Long mentioned that:
> 
>     Generally speaking, unfair lock performs well for VMs with a small
>     number of vCPUs. Native qspinlock may perform better than pvqspinlock
>     if there is vCPU pinning and there is no vCPU over-commitment.
> 
> 2) vCPUs are very unlikely to get preempted when they are the only task 
>    running on a CPU. PV TLB flush is slower that the native flush in that 
>    case.
> 
> v3 -> v4:
>  * update feature bit document
> v2 -> v3:
>  * a separate table for CPUID[0x40000001].EDX bits
>  * a new kvm_hint_has_feature macro
> v1 -> v2:
>  * update to KVM_HINTS_DEDICATED

Great to see this has finally moved forward! Thanks for picking it up Wanpeng.

I would just request to next time at least reference the original thread and
copy the original authors of the patch/idea.

Thanks a lot.

-- 
All the best,
Eduardo Valentin

Re: [PATCH v4 0/3] KVM: Introduce dedicated vCPUs hint KVM_HINTS_DEDICATED

[Wanpeng Li]

2018-03-06 22:39 GMT+08:00 Eduardo Valentin <eduval@amazon.com>:
> Folks,
>
> On Tue, Feb 13, 2018 at 09:05:39AM +0800, Wanpeng Li wrote:
>> This patchset introduces dedicated vCPUs(vCPU pinning, and there is no
>> vCPU over-commitment) hint KVM_HINTS_DEDICATED, it has two users now:
>>
>> 1) Waiman Long mentioned that:
>>
>>     Generally speaking, unfair lock performs well for VMs with a small
>>     number of vCPUs. Native qspinlock may perform better than pvqspinlock
>>     if there is vCPU pinning and there is no vCPU over-commitment.
>>
>> 2) vCPUs are very unlikely to get preempted when they are the only task
>>    running on a CPU. PV TLB flush is slower that the native flush in that
>>    case.
>>
>> v3 -> v4:
>>  * update feature bit document
>> v2 -> v3:
>>  * a separate table for CPUID[0x40000001].EDX bits
>>  * a new kvm_hint_has_feature macro
>> v1 -> v2:
>>  * update to KVM_HINTS_DEDICATED
>
> Great to see this has finally moved forward! Thanks for picking it up Wanpeng.
>
> I would just request to next time at least reference the original thread and
> copy the original authors of the patch/idea.

Thanks for the original idea, Eduardo!

Regards,
Wanpeng Li