Re: [PATCHv2 1/1] locking/qspinlock/x86: Avoid test-and-set when PV_DEDICATED is set

[Paolo Bonzini]

On 02/11/2017 18:45, Eduardo Valentin wrote:
> Currently, the existing qspinlock implementation will fallback to
> test-and-set if the hypervisor has not set the PV_UNHALT flag.
> 
> This patch gives the opportunity to guest kernels to select
> between test-and-set and the regular queueu fair lock implementation
> based on the PV_DEDICATED KVM feature flag. When the PV_DEDICATED
> flag is not set, the code will still fall back to test-and-set,
> but when the PV_DEDICATED flag is set, the code will use
> the regular queue spinlock implementation.

Have you seen Waiman's series that lets you specify this on the guest
command line instead?  Would this be acceptable for your use case?

(In other words, is there a difference for you between making the host
vs. guest administrator toggle the feature?  "@amazon.com" means you are
the host admin, how would you use it?)

Thanks,

Paolo

> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: "Radim KrÄmÃÅ" <rkrcmar@xxxxxxxxxx>
> Cc: Jonathan Corbet <corbet@xxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> Cc: x86@xxxxxxxxxx
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Waiman Long <longman@xxxxxxxxxx>
> Cc: kvm@xxxxxxxxxxxxxxx
> Cc: linux-doc@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Cc: Jan H. Schoenherr <jschoenh@xxxxxxxxx>
> Cc: Anthony Liguori <aliguori@xxxxxxxxxx>
> Suggested-by: Matt Wilson <msw@xxxxxxxxxx>
> Signed-off-by: Eduardo Valentin <eduval@xxxxxxxxxx>
> ---
> V2:
>  - rebase on top of tip/master
> 
>  Documentation/virtual/kvm/cpuid.txt  | 6 ++++++
>  arch/x86/include/asm/qspinlock.h     | 4 ++++
>  arch/x86/include/uapi/asm/kvm_para.h | 1 +
>  3 files changed, 11 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/cpuid.txt b/Documentation/virtual/kvm/cpuid.txt
> index 3c65feb..117066a 100644
> --- a/Documentation/virtual/kvm/cpuid.txt
> +++ b/Documentation/virtual/kvm/cpuid.txt
> @@ -54,6 +54,12 @@ KVM_FEATURE_PV_UNHALT              ||     7 || guest checks this feature bit
>                                     ||       || before enabling paravirtualized
>                                     ||       || spinlock support.
>  ------------------------------------------------------------------------------
> +KVM_FEATURE_PV_DEDICATED           ||     8 || guest checks this feature bit
> +                                   ||       || to determine if they run on
> +                                   ||       || dedicated vCPUs, allowing opti-
> +                                   ||       || mizations such as usage of
> +                                   ||       || qspinlocks.
> +------------------------------------------------------------------------------
>  KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ||    24 || host will warn if no guest-side
>                                     ||       || per-cpu warps are expected in
>                                     ||       || kvmclock.
> diff --git a/arch/x86/include/asm/qspinlock.h b/arch/x86/include/asm/qspinlock.h
> index 308dfd0..3751898 100644
> --- a/arch/x86/include/asm/qspinlock.h
> +++ b/arch/x86/include/asm/qspinlock.h
> @@ -2,6 +2,8 @@
>  #define _ASM_X86_QSPINLOCK_H
>  
>  #include <linux/jump_label.h>
> +#include <linux/kvm_para.h>
> +
>  #include <asm/cpufeature.h>
>  #include <asm-generic/qspinlock_types.h>
>  #include <asm/paravirt.h>
> @@ -57,6 +59,8 @@ static inline bool virt_spin_lock(struct qspinlock *lock)
>  	if (!static_branch_likely(&virt_spin_lock_key))
>  		return false;
>  
> +	if (kvm_para_has_feature(KVM_FEATURE_PV_DEDICATED))
> +		return false;
>  	/*
>  	 * On hypervisors without PARAVIRT_SPINLOCKS support we fall
>  	 * back to a Test-and-Set spinlock, because fair locks have
> diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
> index a965e5b0..d151300 100644
> --- a/arch/x86/include/uapi/asm/kvm_para.h
> +++ b/arch/x86/include/uapi/asm/kvm_para.h
> @@ -24,6 +24,7 @@
>  #define KVM_FEATURE_STEAL_TIME		5
>  #define KVM_FEATURE_PV_EOI		6
>  #define KVM_FEATURE_PV_UNHALT		7
> +#define KVM_FEATURE_PV_DEDICATED	8
>  
>  /* 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.
>