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

[Juergen Gross]

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@xxxxxxxxxxxxxxx>
> AuthorDate: Thu, 27 Sep 2018 14:01:44 +0800
> Committer:  Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxxx>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Reviewed-by: Michael Kelley <mikelley@xxxxxxxxxxxxx>
> Cc: chao.p.peng@xxxxxxxxx
> Cc: chao.gao@xxxxxxxxx
> Cc: isaku.yamahata@xxxxxxxxx
> Cc: tianyu.lan@xxxxxxxxxxxxx
> Cc: "K. Y. Srinivasan" <kys@xxxxxxxxxxxxx>
> Cc: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> Cc: Stephen Hemminger <sthemmin@xxxxxxxxxxxxx>
> Link: https://lkml.kernel.org/r/1538028104-114050-3-git-send-email-yi.y.sun@xxxxxxxxxxxxxxx
> ---
>  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@xxxxxxxxx>
> + */
> +
> +#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