linux-kernel - Re: [tip:x86/hyperv] x86/hyperv: Enable PV qspinlock for Hyper-V

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0005fc50-4415-87a0-1954-fb15a1f8bd0b@suse.com>
Date:   Tue, 2 Oct 2018 13:38:55 +0200
From:   Juergen Gross <jgross@...e.com>
To:     linux-kernel@...r.kernel.org, tglx@...utronix.de, mingo@...nel.org,
        sthemmin@...rosoft.com, mikelley@...rosoft.com, kys@...rosoft.com,
        haiyangz@...rosoft.com, yi.y.sun@...ux.intel.com, hpa@...or.com
Subject: Re: [tip:x86/hyperv] x86/hyperv: Enable PV qspinlock for Hyper-V

Sorry for noticing this only now, but I have been fighting with
Xen PV qspinlocks last weekend:

On 02/10/2018 13:28, tip-bot for Yi Sun wrote:
> Commit-ID:  aaa7fc34c003bd8133a49f7634480cef6288ad55
> Gitweb:     https://git.kernel.org/tip/aaa7fc34c003bd8133a49f7634480cef6288ad55
> Author:     Yi Sun <yi.y.sun@...ux.intel.com>
> AuthorDate: Thu, 27 Sep 2018 14:01:44 +0800
> Committer:  Thomas Gleixner <tglx@...utronix.de>
> CommitDate: Tue, 2 Oct 2018 13:22:06 +0200
> 
> x86/hyperv: Enable PV qspinlock for Hyper-V
> 
> Implement the necessary callbacks for PV spinlocks which allow vCPU idling
> and kicking operations when running as a guest on Hyper-V
> 
> Signed-off-by: Yi Sun <yi.y.sun@...ux.intel.com>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> Reviewed-by: Michael Kelley <mikelley@...rosoft.com>
> Cc: chao.p.peng@...el.com
> Cc: chao.gao@...el.com
> Cc: isaku.yamahata@...el.com
> Cc: tianyu.lan@...rosoft.com
> Cc: "K. Y. Srinivasan" <kys@...rosoft.com>
> Cc: Haiyang Zhang <haiyangz@...rosoft.com>
> Cc: Stephen Hemminger <sthemmin@...rosoft.com>
> Link: https://lkml.kernel.org/r/1538028104-114050-3-git-send-email-yi.y.sun@linux.intel.com
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  5 ++
>  arch/x86/hyperv/Makefile                        |  4 ++
>  arch/x86/hyperv/hv_spinlock.c                   | 75 +++++++++++++++++++++++++
>  arch/x86/include/asm/mshyperv.h                 |  1 +
>  arch/x86/kernel/cpu/mshyperv.c                  | 14 +++++
>  5 files changed, 99 insertions(+)
> 

...

> diff --git a/arch/x86/hyperv/hv_spinlock.c b/arch/x86/hyperv/hv_spinlock.c
> new file mode 100644
> index 000000000000..6d3221322d0d
> --- /dev/null
> +++ b/arch/x86/hyperv/hv_spinlock.c
> @@ -0,0 +1,75 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Hyper-V specific spinlock code.
> + *
> + * Copyright (C) 2018, Intel, Inc.
> + *
> + * Author : Yi Sun <yi.y.sun@...el.com>
> + */
> +
> +#define pr_fmt(fmt) "Hyper-V: " fmt
> +
> +#include <linux/spinlock.h>
> +
> +#include <asm/mshyperv.h>
> +#include <asm/hyperv-tlfs.h>
> +#include <asm/paravirt.h>
> +#include <asm/qspinlock.h>
> +#include <asm/apic.h>
> +
> +static bool __initdata hv_pvspin = true;
> +
> +static void hv_qlock_kick(int cpu)
> +{
> +	apic->send_IPI(cpu, X86_PLATFORM_IPI_VECTOR);
> +}
> +
> +static void hv_qlock_wait(u8 *byte, u8 val)
> +{
> +	unsigned long msr_val;
> +
> +	if (READ_ONCE(*byte) != val)
> +		return;
> +
> +	/*
> +	 * Read HV_X64_MSR_GUEST_IDLE MSR can trigger the guest's
> +	 * transition to the idle power state which can be exited
> +	 * by an IPI even if IF flag is disabled.
> +	 */

What if interrupts are enabled? Won't a kick happening here just
interrupt and then the following rdmsr result in a hang?

I believe the correct way would be to:

- disable interrupts before above READ_ONCE() and restore them
  after the rdmsrl()

- return early if in_nmi()

similar as the kvm specific variant is doing it.


Juergen