lkml.org 
[lkml]   [2017]   [Sep]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall
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 <mtosatti@redhat.com>
>
> ---
> 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 <peterz@infradead.org> (maintainer:LOCKING PRIMITIVES)
Ingo Molnar <mingo@redhat.com> (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 <linux/debugfs.h>
> #include <linux/nmi.h>
> #include <linux/swait.h>
> +#include <linux/static_key.h>
> #include <asm/timer.h>
> #include <asm/cpu.h>
> #include <asm/traps.h>
> @@ -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 <linux/stringify.h>
> #include <linux/bottom_half.h>
> #include <asm/barrier.h>
> -
> +#include <uapi/linux/kvm_para.h>
>
> /*
> * Must define these before including other files, inline functions need them
>
>

\
 
 \ /
  Last update: 2017-09-21 19:33    [W:0.180 / U:0.352 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site