From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751669AbdIUNh1 (ORCPT ); Thu, 21 Sep 2017 09:37:27 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:31417 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751593AbdIUNh0 (ORCPT ); Thu, 21 Sep 2017 09:37:26 -0400 Date: Thu, 21 Sep 2017 09:36:53 -0400 From: Konrad Rzeszutek Wilk To: Marcelo Tosatti , peterz@infradead.org, mingo@redhat.com Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall Message-ID: <20170921133653.GO26248@char.us.oracle.com> References: <20170921113835.031375194@redhat.com> <20170921114039.466130276@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170921114039.466130276@redhat.com> User-Agent: Mutt/1.8.3 (2017-05-23) X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Sep 21, 2017 at 08:38:38AM -0300, Marcelo Tosatti wrote: > Add hypercalls to spinlock/unlock to set/unset FIFO priority > for the vcpu, protected by a static branch to avoid performance > increase in the normal kernels. > > Enable option by "kvmfifohc" kernel command line parameter (disabled > by default). Wouldn't be better if there was a global 'kvm=' which could have the various overrides? > > Signed-off-by: Marcelo Tosatti > > --- > arch/x86/kernel/kvm.c | 31 +++++++++++++++++++++++++++++++ > include/linux/spinlock.h | 2 +- > include/linux/spinlock_api_smp.h | 17 +++++++++++++++++ Hm. It looks like you forgot to CC the maintainers: $ scripts/get_maintainer.pl -f include/linux/spinlock.h Peter Zijlstra (maintainer:LOCKING PRIMITIVES) Ingo Molnar (maintainer:LOCKING PRIMITIVES) linux-kernel@vger.kernel.org (open list:LOCKING PRIMITIVES) Doing that for you. > 3 files changed, 49 insertions(+), 1 deletion(-) > > Index: kvm.fifopriohc-submit/arch/x86/kernel/kvm.c > =================================================================== > --- kvm.fifopriohc-submit.orig/arch/x86/kernel/kvm.c > +++ kvm.fifopriohc-submit/arch/x86/kernel/kvm.c > @@ -37,6 +37,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -321,6 +322,36 @@ static notrace void kvm_guest_apic_eoi_w > apic->native_eoi_write(APIC_EOI, APIC_EOI_ACK); > } > > +static int kvmfifohc; > + > +static int parse_kvmfifohc(char *arg) > +{ > + kvmfifohc = 1; > + return 0; > +} > + > +early_param("kvmfifohc", parse_kvmfifohc); > + > +DEFINE_STATIC_KEY_FALSE(kvm_fifo_hc_key); > + > +static void kvm_init_fifo_hc(void) > +{ > + long ret; > + > + ret = kvm_hypercall1(KVM_HC_RT_PRIO, 0); > + > + if (ret == 0 && kvmfifohc == 1) > + static_branch_enable(&kvm_fifo_hc_key); > +} > + > +static __init int kvmguest_late_init(void) > +{ > + kvm_init_fifo_hc(); > + return 0; > +} > + > +late_initcall(kvmguest_late_init); > + > static void kvm_guest_cpu_init(void) > { > if (!kvm_para_available()) > Index: kvm.fifopriohc-submit/include/linux/spinlock_api_smp.h > =================================================================== > --- kvm.fifopriohc-submit.orig/include/linux/spinlock_api_smp.h > +++ kvm.fifopriohc-submit/include/linux/spinlock_api_smp.h > @@ -136,11 +136,28 @@ static inline void __raw_spin_lock_bh(ra > LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock); > } > > +#ifdef CONFIG_KVM_GUEST > +DECLARE_STATIC_KEY_FALSE(kvm_fifo_hc_key); > +#endif > + > static inline void __raw_spin_lock(raw_spinlock_t *lock) > { > preempt_disable(); > + > +#if defined(CONFIG_KVM_GUEST) && defined(CONFIG_SMP) > + /* enable FIFO priority */ > + if (static_branch_unlikely(&kvm_fifo_hc_key)) > + kvm_hypercall1(KVM_HC_RT_PRIO, 0x1); > +#endif I am assuming the reason you choose not to wrap this in a pvops or any other structure that is more of hypervisor agnostic is that only KVM exposes this. But what if other hypervisors expose something similar? Or some other mechanism similar to this? > + > spin_acquire(&lock->dep_map, 0, 0, _RET_IP_); > LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock); > + > +#if defined(CONFIG_KVM_GUEST) && defined(CONFIG_SMP) > + /* disable FIFO priority */ > + if (static_branch_unlikely(&kvm_fifo_hc_key)) > + kvm_hypercall1(KVM_HC_RT_PRIO, 0); > +#endif > } > > #endif /* !CONFIG_GENERIC_LOCKBREAK || CONFIG_DEBUG_LOCK_ALLOC */ > Index: kvm.fifopriohc-submit/include/linux/spinlock.h > =================================================================== > --- kvm.fifopriohc-submit.orig/include/linux/spinlock.h > +++ kvm.fifopriohc-submit/include/linux/spinlock.h > @@ -56,7 +56,7 @@ > #include > #include > #include > - > +#include > > /* > * Must define these before including other files, inline functions need them > >