2018-02-09 14:49:54

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v2 1/2] KVM: x86: Add dedicated vCPU hint KVM_HINTS_DEDICATED

From: Wanpeng Li <[email protected]>

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 adds a performance hint to allow hypervisor admin to choose
the qspinlock to be used when a dedicated pCPU 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 <[email protected]>
Cc: Radim Krčmář <[email protected]>
Cc: Eduardo Habkost <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
v1 -> v2:
* update to KVM_HINTS_DEDICATED

Documentation/virtual/kvm/cpuid.txt | 6 ++++++
arch/x86/include/uapi/asm/kvm_para.h | 2 ++
arch/x86/kernel/kvm.c | 6 ++++++
3 files changed, 14 insertions(+)

diff --git a/Documentation/virtual/kvm/cpuid.txt b/Documentation/virtual/kvm/cpuid.txt
index 87a7506..d54cc79 100644
--- a/Documentation/virtual/kvm/cpuid.txt
+++ b/Documentation/virtual/kvm/cpuid.txt
@@ -66,3 +66,9 @@ KVM_FEATURE_CLOCKSOURCE_STABLE_BIT || 24 || host will warn if no guest-side
|| || per-cpu warps are expected in
|| || kvmclock.
------------------------------------------------------------------------------
+KVM_HINTS_DEDICATED || 0 || guest checks this feature bit
+ || || to determine if they run on
+ || || dedicated vCPUs, allowing opti-
+ || || mizations such as usage of
+ || || qspinlocks.
+------------------------------------------------------------------------------
diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
index 6cfa9c8..d65b16b 100644
--- a/arch/x86/include/uapi/asm/kvm_para.h
+++ b/arch/x86/include/uapi/asm/kvm_para.h
@@ -28,6 +28,8 @@
#define KVM_FEATURE_PV_TLB_FLUSH 9
#define KVM_FEATURE_ASYNC_PF_VMEXIT 10

+#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 971babe..7f4c92d 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -724,6 +724,12 @@ void __init kvm_spinlock_init(void)
{
if (!kvm_para_available())
return;
+
+ if (kvm_para_has_feature(KVM_HINTS_DEDICATED)) {
+ static_branch_disable(&virt_spin_lock_key);
+ return;
+ }
+
/* Does host kernel support KVM_FEATURE_PV_UNHALT? */
if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
return;
--
2.7.4



2018-02-09 14:50:23

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v2 2/2] KVM: X86: Don't use PV TLB flush with dedicated vCPUs and steal time disabled

From: Wanpeng Li <[email protected]>

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 <[email protected]>
Cc: Radim Krčmář <[email protected]>
Cc: Eduardo Habkost <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
v1 -> v2:
* update to KVM_HINTS_DEDICATED

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 7f4c92d..b92311a 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))
@@ -633,7 +635,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


2018-02-09 16:35:14

by Eduardo Habkost

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] KVM: x86: Add dedicated vCPU hint KVM_HINTS_DEDICATED

On Fri, Feb 09, 2018 at 06:47:52AM -0800, Wanpeng Li wrote:
> From: Wanpeng Li <[email protected]>
>
> 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 adds a performance hint to allow hypervisor admin to choose
> the qspinlock to be used when a dedicated pCPU 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 <[email protected]>
> Cc: Radim Krčmář <[email protected]>
> Cc: Eduardo Habkost <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> v1 -> v2:
> * update to KVM_HINTS_DEDICATED
>
> Documentation/virtual/kvm/cpuid.txt | 6 ++++++
> arch/x86/include/uapi/asm/kvm_para.h | 2 ++
> arch/x86/kernel/kvm.c | 6 ++++++
> 3 files changed, 14 insertions(+)
>
> diff --git a/Documentation/virtual/kvm/cpuid.txt b/Documentation/virtual/kvm/cpuid.txt
> index 87a7506..d54cc79 100644
> --- a/Documentation/virtual/kvm/cpuid.txt
> +++ b/Documentation/virtual/kvm/cpuid.txt
> @@ -66,3 +66,9 @@ KVM_FEATURE_CLOCKSOURCE_STABLE_BIT || 24 || host will warn if no guest-side
> || || per-cpu warps are expected in
> || || kvmclock.
> ------------------------------------------------------------------------------
> +KVM_HINTS_DEDICATED || 0 || guest checks this feature bit
> + || || to determine if they run on
> + || || dedicated vCPUs, allowing opti-
> + || || mizations such as usage of
> + || || qspinlocks.

I assume you don't mean CPUID[0x40000001].EAX[bit 0] here,
because it is already used for KVM_FEATURE_CLOCKSOURCE.

I guess you will want a separate table for CPUID[0x40000001].EDX
bits?


> +------------------------------------------------------------------------------
> diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
> index 6cfa9c8..d65b16b 100644
> --- a/arch/x86/include/uapi/asm/kvm_para.h
> +++ b/arch/x86/include/uapi/asm/kvm_para.h
> @@ -28,6 +28,8 @@
> #define KVM_FEATURE_PV_TLB_FLUSH 9
> #define KVM_FEATURE_ASYNC_PF_VMEXIT 10
>
> +#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 971babe..7f4c92d 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -724,6 +724,12 @@ void __init kvm_spinlock_init(void)
> {
> if (!kvm_para_available())
> return;
> +
> + if (kvm_para_has_feature(KVM_HINTS_DEDICATED)) {

This is checking CPUID{0x40000001].EAX[bit 0]
(KVM_FEATURE_CLOCKSOURCE), isn't it?

> + static_branch_disable(&virt_spin_lock_key);
> + return;
> + }
> +
> /* Does host kernel support KVM_FEATURE_PV_UNHALT? */
> if (!kvm_para_has_feature(KVM_FEATURE_PV_UNHALT))
> return;
> --
> 2.7.4
>

--
Eduardo