RE: [PATCH 6/7] x86/hyper-v: use hypercall for remove TLB flush

[Jork Loeser]

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx]
> Sent: Friday, April 7, 2017 04:27
> To: devel@xxxxxxxxxxxxxxxxxxxxxx; x86@xxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx; KY Srinivasan <kys@xxxxxxxxxxxxx>;
> Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>; Stephen Hemminger
> <sthemmin@xxxxxxxxxxxxx>; Thomas Gleixner <tglx@xxxxxxxxxxxxx>; Ingo
> Molnar <mingo@xxxxxxxxxx>; H. Peter Anvin <hpa@xxxxxxxxx>; Steven
> Rostedt <rostedt@xxxxxxxxxxx>; Jork Loeser <Jork.Loeser@xxxxxxxxxxxxx>
> Subject: [PATCH 6/7] x86/hyper-v: use hypercall for remove TLB flush

> diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c new file
> mode 100644 index 0000000..fb487cb
> --- /dev/null
> +++ b/arch/x86/hyperv/mmu.c
> @@ -0,0 +1,128 @@
> +#include <linux/types.h>
> +#include <linux/hyperv.h>
> +#include <linux/slab.h>
> +#include <asm/mshyperv.h>
> +#include <asm/tlbflush.h>
> +#include <asm/msr.h>
> +#include <asm/fpu/api.h>
> +
> +/*
> + * Arbitrary number; we need to pre-allocate per-cpu struct for doing
> +TLB
> + * flush hypercalls and we need to pick a size. '16' means we'll be
> +able
> + * to flush 16 * 4096 pages (256MB) with one hypercall.
> + */
> +#define HV_MMU_MAX_GVAS 16
> +
> +/* HvFlushVirtualAddressSpace*, HvFlushVirtualAddressList hypercalls */
> +struct hv_flush_pcpu {
> +	struct {
> +		__u64 address_space;
> +		__u64 flags;
> +		__u64 processor_mask;
> +		__u64 gva_list[HV_MMU_MAX_GVAS];
> +	} flush;
> +
> +	spinlock_t lock;
> +};
Does this need an alignment declaration, so that the flush portion never crosses a page boundary when allocated with alloc_percpu()?

> +
> +static struct hv_flush_pcpu __percpu *pcpu_flush;
> +
> +static void hyperv_flush_tlb_others(const struct cpumask *cpus,
> +				    struct mm_struct *mm, unsigned long
> start,
> +				    unsigned long end)
> +{
> +	struct hv_flush_pcpu *flush;
> +	unsigned long cur, flags;
> +	u64 status = -1ULL;
> +	int cpu, vcpu, gva_n;
> +
> +	if (!pcpu_flush || !hv_hypercall_pg)
> +		goto do_native;
> +
> +	if (cpumask_empty(cpus))
> +		return;
> +
> +	flush = this_cpu_ptr(pcpu_flush);
> +	spin_lock_irqsave(&flush->lock, flags);

What purpose does the spinlock on the CPU-local struct serve? Would a local_irq_save() do?

Could this be called from NMI context, such as from the debugger?

Could this be a long-running loop, e.g. due to a large start/end range? If so, consider disabling interrupts only in the inner loop / flush the entire space?

Regards,
Jork