2018-02-11 03:32:38

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v3 0/3] KVM: Introduce dedicated vCPUs hint KVM_HINTS_DEDICATED

This patchset introduces dedicated vCPUs 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.

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 | 12 +++++++++++-
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, 64 insertions(+), 5 deletions(-)

--
2.7.4



2018-02-11 03:30:53

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v3 1/3] KVM: Introduce dedicated vCPUs hint KVM_HINTS_DEDICATED

From: Wanpeng Li <[email protected]>

This patch introduces dedicated vCPUs hint KVM_HINTS_DEDICATED,
guest checks this feature bit to determine if they run on dedicated
vCPUs, allowing optimizations.

Cc: Paolo Bonzini <[email protected]>
Cc: Radim Krčmář <[email protected]>
Cc: Eduardo Habkost <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
Documentation/virtual/kvm/cpuid.txt | 12 +++++++++++-
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, 53 insertions(+), 3 deletions(-)

diff --git a/Documentation/virtual/kvm/cpuid.txt b/Documentation/virtual/kvm/cpuid.txt
index dcab6dc..e283b88 100644
--- a/Documentation/virtual/kvm/cpuid.txt
+++ b/Documentation/virtual/kvm/cpuid.txt
@@ -23,7 +23,7 @@ This function queries the presence of KVM cpuid leafs.


function: define KVM_CPUID_FEATURES (0x40000001)
-returns : ebx, ecx, edx = 0
+returns : ebx, ecx
eax = and OR'ed group of (1 << flag), where each flags is:


@@ -62,3 +62,13 @@ KVM_FEATURE_CLOCKSOURCE_STABLE_BIT || 24 || host will warn if no guest-side
|| || per-cpu warps are expected in
|| || kvmclock.
------------------------------------------------------------------------------
+
+ edx = and 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 they run on dedicated
+ || || vCPUs, 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


2018-02-11 03:31:02

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v3 3/3] 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]>
---
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


2018-02-11 03:32:18

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v3 2/3] KVM: X86: Choose qspinlock when dedicated vCPUs available

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


2018-02-12 11:59:20

by Andrew Jones

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] KVM: Introduce dedicated vCPUs hint KVM_HINTS_DEDICATED

On Sun, Feb 11, 2018 at 11:29:44AM +0800, Wanpeng Li wrote:
> From: Wanpeng Li <[email protected]>
>
> This patch introduces dedicated vCPUs hint KVM_HINTS_DEDICATED,
> guest checks this feature bit to determine if they run on dedicated
> vCPUs, allowing optimizations.
>
> Cc: Paolo Bonzini <[email protected]>
> Cc: Radim Krčmář <[email protected]>
> Cc: Eduardo Habkost <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> Documentation/virtual/kvm/cpuid.txt | 12 +++++++++++-
> 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, 53 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/cpuid.txt b/Documentation/virtual/kvm/cpuid.txt
> index dcab6dc..e283b88 100644
> --- a/Documentation/virtual/kvm/cpuid.txt
> +++ b/Documentation/virtual/kvm/cpuid.txt
> @@ -23,7 +23,7 @@ This function queries the presence of KVM cpuid leafs.
>
>
> function: define KVM_CPUID_FEATURES (0x40000001)
> -returns : ebx, ecx, edx = 0
> +returns : ebx, ecx
> eax = and OR'ed group of (1 << flag), where each flags is:

Now that the above line is getting repeated, we should fix it: s/and/an/

>
>
> @@ -62,3 +62,13 @@ KVM_FEATURE_CLOCKSOURCE_STABLE_BIT || 24 || host will warn if no guest-side
> || || per-cpu warps are expected in
> || || kvmclock.
> ------------------------------------------------------------------------------
> +
> + edx = and OR'ed group of (1 << flag), where each flags is:

And then copy the corrected version here.

Thanks,
drew

> +
> +
> +flag || value || meaning
> +================================================================================
> +KVM_HINTS_DEDICATED || 0 || guest checks this feature bit
> + || || to determine if they run on dedicated
> + || || vCPUs, 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
>

2018-02-12 16:54:15

by Eduardo Habkost

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] KVM: Introduce dedicated vCPUs hint KVM_HINTS_DEDICATED

On Sun, Feb 11, 2018 at 11:29:44AM +0800, Wanpeng Li wrote:
[...]
> +KVM_HINTS_DEDICATED || 0 || guest checks this feature bit
> + || || to determine if they run on dedicated
> + || || vCPUs, allowing optimizations

What "dedicated vCPUs" mean, exactly? Can we document this more
explicitly?

--
Eduardo

2018-02-13 01:10:23

by Wanpeng Li

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] KVM: Introduce dedicated vCPUs hint KVM_HINTS_DEDICATED

2018-02-12 22:12 GMT+08:00 Eduardo Habkost <[email protected]>:
> On Sun, Feb 11, 2018 at 11:29:44AM +0800, Wanpeng Li wrote:
> [...]
>> +KVM_HINTS_DEDICATED || 0 || guest checks this feature bit
>> + || || to determine if they run on dedicated
>> + || || vCPUs, allowing optimizations
>
> What "dedicated vCPUs" mean, exactly? Can we document this more
> explicitly?

Follow this and Andrew's comments in v4.

Regards,
Wanpeng Li