[PATCH 0/5] X86: Hyper-V: APIC enlightenments

[PATCH 0/5] X86: Hyper-V: APIC enlightenments

[kys]

From: "K. Y. Srinivasan" <kys@microsoft.com>

Implement APIC related enlightenments.

K. Y. Srinivasan (5):
  X86: Hyper-V: Enlighten APIC access
  X86: Hyper-V: Enable IPI enlightenments
  X86: Hyper-V: Enhanced IPI enlightenment
  X86: Hyper-V: Consolidate code for converting cpumask to vpset
  X86: Hyper-V: Consolidate the allocation of the hypercall input page

 arch/x86/hyperv/Makefile           |   2 +-
 arch/x86/hyperv/hv_apic.c          | 268 +++++++++++++++++++++++++++++++++++++
 arch/x86/hyperv/hv_init.c          |  22 ++-
 arch/x86/hyperv/mmu.c              |  78 ++---------
 arch/x86/include/asm/hyperv-tlfs.h |  12 +-
 arch/x86/include/asm/mshyperv.h    |  47 ++++++-
 6 files changed, 356 insertions(+), 73 deletions(-)
 create mode 100644 arch/x86/hyperv/hv_apic.c

-- 
2.15.1

[PATCH 1/5] X86: Hyper-V: Enlighten APIC access

[kys]

From: "K. Y. Srinivasan" <kys@microsoft.com>

Hyper-V supports MSR based APIC access; implement
the enlightenment.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 arch/x86/hyperv/Makefile        |  2 +-
 arch/x86/hyperv/hv_apic.c       | 98 +++++++++++++++++++++++++++++++++++++++++
 arch/x86/hyperv/hv_init.c       |  5 ++-
 arch/x86/include/asm/mshyperv.h |  6 ++-
 4 files changed, 107 insertions(+), 4 deletions(-)
 create mode 100644 arch/x86/hyperv/hv_apic.c

diff --git a/arch/x86/hyperv/Makefile b/arch/x86/hyperv/Makefile
index 367a8203cfcf..00ce4df01a09 100644
--- a/arch/x86/hyperv/Makefile
+++ b/arch/x86/hyperv/Makefile
@@ -1 +1 @@
-obj-y		:= hv_init.o mmu.o
+obj-y		:= hv_init.o mmu.o hv_apic.o
diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
new file mode 100644
index 000000000000..e0a5b36208fc
--- /dev/null
+++ b/arch/x86/hyperv/hv_apic.c
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Hyper-V specific APIC code.
+ *
+ * Copyright (C) 2018, Microsoft, Inc.
+ *
+ * Author : K. Y. Srinivasan <kys@microsoft.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ */
+
+#include <linux/types.h>
+#include <asm/hypervisor.h>
+#include <asm/mshyperv.h>
+#include <linux/version.h>
+#include <linux/vmalloc.h>
+#include <linux/mm.h>
+#include <linux/clockchips.h>
+#include <linux/hyperv.h>
+#include <linux/slab.h>
+#include <linux/cpuhotplug.h>
+
+static u64 hv_apic_icr_read(void)
+{
+	u64 reg_val;
+
+	rdmsrl(HV_X64_MSR_ICR, reg_val);
+	return reg_val;
+}
+
+static void hv_apic_icr_write(u32 low, u32 id)
+{
+	u64 reg_val;
+
+	reg_val = SET_APIC_DEST_FIELD(id);
+	reg_val = reg_val << 32;
+	reg_val |= low;
+
+	wrmsrl(HV_X64_MSR_ICR, reg_val);
+}
+
+static u32 hv_apic_read(u32 reg)
+{
+	u32 reg_val, hi;
+
+	switch (reg) {
+	case APIC_EOI:
+		rdmsr(HV_X64_MSR_EOI, reg_val, hi);
+		return reg_val;
+	case APIC_TASKPRI:
+		rdmsr(HV_X64_MSR_TPR, reg_val, hi);
+		return reg_val;
+
+	default:
+		return native_apic_mem_read(reg);
+	}
+}
+
+static void hv_apic_write(u32 reg, u32 val)
+{
+	switch (reg) {
+	case APIC_EOI:
+		wrmsr(HV_X64_MSR_EOI, val, 0);
+		break;
+	case APIC_TASKPRI:
+		wrmsr(HV_X64_MSR_TPR, val, 0);
+		break;
+	default:
+		native_apic_mem_write(reg, val);
+	}
+}
+
+static void hv_apic_eoi_write(u32 reg, u32 val)
+{
+	wrmsr(HV_X64_MSR_EOI, val, 0);
+}
+
+void __init hv_apic_init(void)
+{
+	if (ms_hyperv.hints & HV_X64_APIC_ACCESS_RECOMMENDED) {
+		pr_info("Hyper-V: Using MSR ased APIC access\n");
+		apic_set_eoi_write(hv_apic_eoi_write);
+		apic->read      = hv_apic_read;
+		apic->write     = hv_apic_write;
+		apic->icr_write = hv_apic_icr_write;
+		apic->icr_read  = hv_apic_icr_read;
+	}
+}
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index cfecc2272f2d..71e50fc2b7ef 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -242,8 +242,9 @@ static int hv_cpu_die(unsigned int cpu)
  *
  * 1. Setup the hypercall page.
  * 2. Register Hyper-V specific clocksource.
+ * 3. Setup Hyper-V specific APIC entry points.
  */
-void hyperv_init(void)
+void __init hyperv_init(void)
 {
 	u64 guest_id, required_msrs;
 	union hv_x64_msr_hypercall_contents hypercall_msr;
@@ -298,6 +299,8 @@ void hyperv_init(void)
 
 	hyper_alloc_mmu();
 
+	hv_apic_init();
+
 	/*
 	 * Register Hyper-V specific clocksource.
 	 */
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index b90e79610cf7..bcced50037c1 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -258,7 +258,7 @@ static inline int hv_cpu_number_to_vp_number(int cpu_number)
 	return hv_vp_index[cpu_number];
 }
 
-void hyperv_init(void);
+void __init hyperv_init(void);
 void hyperv_setup_mmu_ops(void);
 void hyper_alloc_mmu(void);
 void hyperv_report_panic(struct pt_regs *regs, long err);
@@ -269,14 +269,16 @@ void hyperv_reenlightenment_intr(struct pt_regs *regs);
 void set_hv_tscchange_cb(void (*cb)(void));
 void clear_hv_tscchange_cb(void);
 void hyperv_stop_tsc_emulation(void);
+void hv_apic_init(void);
 #else /* CONFIG_HYPERV */
-static inline void hyperv_init(void) {}
+static __init  inline void hyperv_init(void) {}
 static inline bool hv_is_hyperv_initialized(void) { return false; }
 static inline void hyperv_cleanup(void) {}
 static inline void hyperv_setup_mmu_ops(void) {}
 static inline void set_hv_tscchange_cb(void (*cb)(void)) {}
 static inline void clear_hv_tscchange_cb(void) {}
 static inline void hyperv_stop_tsc_emulation(void) {};
+static inline void hv_apic_init(void) {}
 static inline struct hv_vp_assist_page *hv_get_vp_assist_page(unsigned int cpu)
 {
 	return NULL;
-- 
2.15.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

[kys]

From: "K. Y. Srinivasan" <kys@microsoft.com>

Hyper-V supports hypercalls to implement IPI; use them.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 arch/x86/hyperv/hv_apic.c          | 125 +++++++++++++++++++++++++++++++++++++
 arch/x86/hyperv/hv_init.c          |  17 +++++
 arch/x86/include/asm/hyperv-tlfs.h |   9 +++
 arch/x86/include/asm/mshyperv.h    |   1 +
 4 files changed, 152 insertions(+)

diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index e0a5b36208fc..7f3322ecfb01 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -30,6 +30,14 @@
 #include <linux/slab.h>
 #include <linux/cpuhotplug.h>
 
+struct ipi_arg_non_ex {
+	u32 vector;
+	u32 reserved;
+	u64 cpu_mask;
+};
+
+static struct apic orig_apic;
+
 static u64 hv_apic_icr_read(void)
 {
 	u64 reg_val;
@@ -85,8 +93,125 @@ static void hv_apic_eoi_write(u32 reg, u32 val)
 	wrmsr(HV_X64_MSR_EOI, val, 0);
 }
 
+/*
+ * IPI implementation on Hyper-V.
+ */
+
+static int __send_ipi_mask(const struct cpumask *mask, int vector)
+{
+	int cur_cpu, vcpu;
+	struct ipi_arg_non_ex **arg;
+	struct ipi_arg_non_ex *ipi_arg;
+	int ret = 1;
+	unsigned long flags;
+
+	if (cpumask_empty(mask))
+		return 0;
+
+	if (!hv_hypercall_pg)
+		return ret;
+
+	if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
+		return ret;
+
+	local_irq_save(flags);
+	arg = (struct ipi_arg_non_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
+
+	ipi_arg = *arg;
+	if (unlikely(!ipi_arg))
+		goto ipi_mask_done;
+
+
+	ipi_arg->vector = vector;
+	ipi_arg->reserved = 0;
+	ipi_arg->cpu_mask = 0;
+
+	for_each_cpu(cur_cpu, mask) {
+		vcpu = hv_cpu_number_to_vp_number(cur_cpu);
+		if (vcpu >= 64)
+			goto ipi_mask_done;
+
+		__set_bit(vcpu, (unsigned long *)&ipi_arg->cpu_mask);
+	}
+
+	ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL);
+
+ipi_mask_done:
+	local_irq_restore(flags);
+	return ret;
+}
+
+static int __send_ipi_one(int cpu, int vector)
+{
+	struct cpumask mask = CPU_MASK_NONE;
+
+	cpumask_set_cpu(cpu, &mask);
+	return __send_ipi_mask(&mask, vector);
+}
+
+static void hv_send_ipi(int cpu, int vector)
+{
+	if (__send_ipi_one(cpu, vector))
+		orig_apic.send_IPI(cpu, vector);
+}
+
+static void hv_send_ipi_mask(const struct cpumask *mask, int vector)
+{
+	if (__send_ipi_mask(mask, vector))
+		orig_apic.send_IPI_mask(mask, vector);
+}
+
+static void hv_send_ipi_mask_allbutself(const struct cpumask *mask, int vector)
+{
+	unsigned int this_cpu = smp_processor_id();
+	struct cpumask new_mask;
+	const struct cpumask *local_mask;
+
+	cpumask_copy(&new_mask, mask);
+	cpumask_clear_cpu(this_cpu, &new_mask);
+	local_mask = &new_mask;
+	if (__send_ipi_mask(local_mask, vector))
+		orig_apic.send_IPI_mask_allbutself(mask, vector);
+}
+
+static void hv_send_ipi_allbutself(int vector)
+{
+	hv_send_ipi_mask_allbutself(cpu_online_mask, vector);
+}
+
+static void hv_send_ipi_all(int vector)
+{
+	if (__send_ipi_mask(cpu_online_mask, vector))
+		orig_apic.send_IPI_all(vector);
+}
+
+static void hv_send_ipi_self(int vector)
+{
+	if (__send_ipi_one(smp_processor_id(), vector))
+		orig_apic.send_IPI_self(vector);
+}
+
 void __init hv_apic_init(void)
 {
+	if (ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) {
+		if (hyperv_pcpu_input_arg == NULL)
+			goto msr_based_access;
+
+		pr_info("Hyper-V: Using IPI hypercalls\n");
+		/*
+		 * Set the IPI entry points.
+		 */
+		orig_apic = *apic;
+
+		apic->send_IPI = hv_send_ipi;
+		apic->send_IPI_mask = hv_send_ipi_mask;
+		apic->send_IPI_mask_allbutself = hv_send_ipi_mask_allbutself;
+		apic->send_IPI_allbutself = hv_send_ipi_allbutself;
+		apic->send_IPI_all = hv_send_ipi_all;
+		apic->send_IPI_self = hv_send_ipi_self;
+	}
+
+msr_based_access:
 	if (ms_hyperv.hints & HV_X64_APIC_ACCESS_RECOMMENDED) {
 		pr_info("Hyper-V: Using MSR ased APIC access\n");
 		apic_set_eoi_write(hv_apic_eoi_write);
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 71e50fc2b7ef..a895662b6b4c 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -91,12 +91,19 @@ EXPORT_SYMBOL_GPL(hv_vp_index);
 struct hv_vp_assist_page **hv_vp_assist_page;
 EXPORT_SYMBOL_GPL(hv_vp_assist_page);
 
+void  __percpu **hyperv_pcpu_input_arg;
+EXPORT_SYMBOL_GPL(hyperv_pcpu_input_arg);
+
 u32 hv_max_vp_index;
 
 static int hv_cpu_init(unsigned int cpu)
 {
 	u64 msr_vp_index;
 	struct hv_vp_assist_page **hvp = &hv_vp_assist_page[smp_processor_id()];
+	void **input_arg;
+
+	input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
+	*input_arg = page_address(alloc_page(GFP_ATOMIC));
 
 	hv_get_vp_index(msr_vp_index);
 
@@ -217,6 +224,10 @@ static int hv_cpu_die(unsigned int cpu)
 {
 	struct hv_reenlightenment_control re_ctrl;
 	unsigned int new_cpu;
+	void **input_arg;
+
+	input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
+	free_page((unsigned long)*input_arg);
 
 	if (hv_vp_assist_page && hv_vp_assist_page[cpu])
 		wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
@@ -260,6 +271,12 @@ void __init hyperv_init(void)
 	if ((ms_hyperv.features & required_msrs) != required_msrs)
 		return;
 
+	/* Allocate the per-CPU state for the hypercall input arg */
+	hyperv_pcpu_input_arg = alloc_percpu(void  *);
+
+	if (hyperv_pcpu_input_arg == NULL)
+		return;
+
 	/* Allocate percpu VP index */
 	hv_vp_index = kmalloc_array(num_possible_cpus(), sizeof(*hv_vp_index),
 				    GFP_KERNEL);
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 416cb0e0c496..646cf2ca2aaa 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -164,6 +164,11 @@
  */
 #define HV_X64_DEPRECATING_AEOI_RECOMMENDED	(1 << 9)
 
+/*
+ * Recommend using cluster IPI hypercalls.
+ */
+#define HV_X64_CLUSTER_IPI_RECOMMENDED         (1 << 10)
+
 /* Recommend using the newer ExProcessorMasks interface */
 #define HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED	(1 << 11)
 
@@ -329,10 +334,14 @@ struct hv_tsc_emulation_status {
 #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK	\
 		(~((1ull << HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT) - 1))
 
+#define HV_IPI_LOW_VECTOR	0x10
+#define HV_IPI_HIGH_VECTOR	0xff
+
 /* Declare the various hypercall operations. */
 #define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE	0x0002
 #define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST	0x0003
 #define HVCALL_NOTIFY_LONG_SPIN_WAIT		0x0008
+#define HVCALL_SEND_IPI				0x000b
 #define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX  0x0013
 #define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX   0x0014
 #define HVCALL_POST_MESSAGE			0x005c
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index bcced50037c1..f6045f3611de 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -122,6 +122,7 @@ static inline void hv_disable_stimer0_percpu_irq(int irq) {}
 #if IS_ENABLED(CONFIG_HYPERV)
 extern struct clocksource *hyperv_cs;
 extern void *hv_hypercall_pg;
+extern void  __percpu  **hyperv_pcpu_input_arg;
 
 static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
 {
-- 
2.15.1

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

[Dan Carpenter]

On Wed, Apr 25, 2018 at 11:12:47AM -0700, kys@linuxonhyperv.com wrote:
> +/*
> + * IPI implementation on Hyper-V.
> + */
> +
> +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> +	int cur_cpu, vcpu;
> +	struct ipi_arg_non_ex **arg;
> +	struct ipi_arg_non_ex *ipi_arg;
> +	int ret = 1;

Not specifically related to this patch, but hv code sometimes returns 1
on error or U64_MAX.  It's slightly magical.  Maybe
HV_STATUS_INVALID_HYPERCALL_INPUT (3) would be more appropriate?  Or we
could make a new more generic error code:

#define HV_STATUS_INVALID        1

regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

[KY Srinivasan]

> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@oracle.com>
> Sent: Thursday, April 26, 2018 2:32 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: x86@kernel.org; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; tglx@linutronix.de;
> hpa@zytor.com; Stephen Hemminger <sthemmin@microsoft.com>; Michael
> Kelley (EOSG) <Michael.H.Kelley@microsoft.com>; vkuznets@redhat.com
> Subject: Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> 
> On Wed, Apr 25, 2018 at 11:12:47AM -0700, kys@linuxonhyperv.com wrote:
> > +/*
> > + * IPI implementation on Hyper-V.
> > + */
> > +
> > +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> > +{
> > +	int cur_cpu, vcpu;
> > +	struct ipi_arg_non_ex **arg;
> > +	struct ipi_arg_non_ex *ipi_arg;
> > +	int ret = 1;
> 
> Not specifically related to this patch, but hv code sometimes returns 1
> on error or U64_MAX.  It's slightly magical.  Maybe
> HV_STATUS_INVALID_HYPERCALL_INPUT (3) would be more appropriate?
> Or we
> could make a new more generic error code:
> 
> #define HV_STATUS_INVALID        1

Good point. We will look at cleaning this up.

Regards,

K. Y
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

[Thomas Gleixner]

On Wed, 25 Apr 2018, kys@linuxonhyperv.com wrote:
> +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> +	int cur_cpu, vcpu;
> +	struct ipi_arg_non_ex **arg;
> +	struct ipi_arg_non_ex *ipi_arg;
> +	int ret = 1;

So this indicates whether __send_ipi_mask() can send to @mask or not. So
please make it a bool and let it return false when it does not work, true
otherwise. If you had used -Exxxx then it would have been more obvious, but
this is really a boolean decision.

> +	unsigned long flags;
> +
> +	if (cpumask_empty(mask))
> +		return 0;
> +
> +	if (!hv_hypercall_pg)
> +		return ret;
> +
> +	if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
> +		return ret;
> +
> +	local_irq_save(flags);
> +	arg = (struct ipi_arg_non_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
> +
> +	ipi_arg = *arg;
> +	if (unlikely(!ipi_arg))
> +		goto ipi_mask_done;
> +
> +

Stray newline

> +	ipi_arg->vector = vector;
> +	ipi_arg->reserved = 0;
> +	ipi_arg->cpu_mask = 0;
> +
> +	for_each_cpu(cur_cpu, mask) {
> +		vcpu = hv_cpu_number_to_vp_number(cur_cpu);
> +		if (vcpu >= 64)
> +			goto ipi_mask_done;

This is completely magic and deserves a comment.

> +
> +		__set_bit(vcpu, (unsigned long *)&ipi_arg->cpu_mask);
> +	}
> +
> +	ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL);
> +
> +ipi_mask_done:
> +	local_irq_restore(flags);
> +	return ret;
> +}

....

>  static int hv_cpu_init(unsigned int cpu)
>  {
>  	u64 msr_vp_index;
>  	struct hv_vp_assist_page **hvp = &hv_vp_assist_page[smp_processor_id()];
> +	void **input_arg;
> +
> +	input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> +	*input_arg = page_address(alloc_page(GFP_ATOMIC));

This is called from the cpu hotplug thread and there is no need for an
atomic allocation. Please use GFP_KERNEL.

>  	hv_get_vp_index(msr_vp_index);
>  
> @@ -217,6 +224,10 @@ static int hv_cpu_die(unsigned int cpu)
>  {
>  	struct hv_reenlightenment_control re_ctrl;
>  	unsigned int new_cpu;
> +	void **input_arg;
> +
> +	input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> +	free_page((unsigned long)*input_arg);

Hrm. Again this is called from the CPU hotplug thread when the cou is about
to go down. But you can be scheduled out after free() and before disabling
the assist thing below and the pointer persist. There is no guarantee that
nothing sends an IPI anymore after this point.

So you have two options here:

  1) Disable interrupts, get the pointer, set the per cpu pointer to NULL,
     reenable interruots and free the page

  2) Keep the page around and check for it in the CPU UP path and avoid the
     allocation when the CPU comes online again.

>  	if (hv_vp_assist_page && hv_vp_assist_page[cpu])
>  		wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
> @@ -260,6 +271,12 @@ void __init hyperv_init(void)
>  	if ((ms_hyperv.features & required_msrs) != required_msrs)
>  		return;
>  
> +	/* Allocate the per-CPU state for the hypercall input arg */
> +	hyperv_pcpu_input_arg = alloc_percpu(void  *);
> +
> +	if (hyperv_pcpu_input_arg == NULL)
> +		return;

Huch. When that allocation fails, you return and ignore the rest of the
function which has been there before. Weird decision.

Thanks,

	tglx
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

[KY Srinivasan]

> -----Original Message-----
> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Thursday, April 26, 2018 3:09 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: x86@kernel.org; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; hpa@zytor.com; Stephen
> Hemminger <sthemmin@microsoft.com>; Michael Kelley (EOSG)
> <Michael.H.Kelley@microsoft.com>; vkuznets@redhat.com
> Subject: Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> 
> On Wed, 25 Apr 2018, kys@linuxonhyperv.com wrote:
> > +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> > +{
> > +	int cur_cpu, vcpu;
> > +	struct ipi_arg_non_ex **arg;
> > +	struct ipi_arg_non_ex *ipi_arg;
> > +	int ret = 1;
> 
> So this indicates whether __send_ipi_mask() can send to @mask or not. So
> please make it a bool and let it return false when it does not work, true
> otherwise. If you had used -Exxxx then it would have been more obvious,
> but
> this is really a boolean decision.

Agreed.
> 
> > +	unsigned long flags;
> > +
> > +	if (cpumask_empty(mask))
> > +		return 0;
> > +
> > +	if (!hv_hypercall_pg)
> > +		return ret;
> > +
> > +	if ((vector < HV_IPI_LOW_VECTOR) || (vector >
> HV_IPI_HIGH_VECTOR))
> > +		return ret;
> > +
> > +	local_irq_save(flags);
> > +	arg = (struct ipi_arg_non_ex
> **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > +
> > +	ipi_arg = *arg;
> > +	if (unlikely(!ipi_arg))
> > +		goto ipi_mask_done;
> > +
> > +
> 
> Stray newline
> 
> > +	ipi_arg->vector = vector;
> > +	ipi_arg->reserved = 0;
> > +	ipi_arg->cpu_mask = 0;
> > +
> > +	for_each_cpu(cur_cpu, mask) {
> > +		vcpu = hv_cpu_number_to_vp_number(cur_cpu);
> > +		if (vcpu >= 64)
> > +			goto ipi_mask_done;
> 
> This is completely magic and deserves a comment.
> 
> > +
> > +		__set_bit(vcpu, (unsigned long *)&ipi_arg->cpu_mask);
> > +	}
> > +
> > +	ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL);
> > +
> > +ipi_mask_done:
> > +	local_irq_restore(flags);
> > +	return ret;
> > +}
> 
> ....
> 
> >  static int hv_cpu_init(unsigned int cpu)
> >  {
> >  	u64 msr_vp_index;
> >  	struct hv_vp_assist_page **hvp =
> &hv_vp_assist_page[smp_processor_id()];
> > +	void **input_arg;
> > +
> > +	input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > +	*input_arg = page_address(alloc_page(GFP_ATOMIC));
> 
> This is called from the cpu hotplug thread and there is no need for an
> atomic allocation. Please use GFP_KERNEL.
> 
> >  	hv_get_vp_index(msr_vp_index);
> >
> > @@ -217,6 +224,10 @@ static int hv_cpu_die(unsigned int cpu)
> >  {
> >  	struct hv_reenlightenment_control re_ctrl;
> >  	unsigned int new_cpu;
> > +	void **input_arg;
> > +
> > +	input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > +	free_page((unsigned long)*input_arg);
> 
> Hrm. Again this is called from the CPU hotplug thread when the cou is about
> to go down. But you can be scheduled out after free() and before disabling
> the assist thing below and the pointer persist. There is no guarantee that
> nothing sends an IPI anymore after this point.
> 
> So you have two options here:
> 
>   1) Disable interrupts, get the pointer, set the per cpu pointer to NULL,
>      reenable interruots and free the page
I will implement this approach.
> 
>   2) Keep the page around and check for it in the CPU UP path and avoid the
>      allocation when the CPU comes online again.
> 
> >  	if (hv_vp_assist_page && hv_vp_assist_page[cpu])
> >  		wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
> > @@ -260,6 +271,12 @@ void __init hyperv_init(void)
> >  	if ((ms_hyperv.features & required_msrs) != required_msrs)
> >  		return;
> >
> > +	/* Allocate the per-CPU state for the hypercall input arg */
> > +	hyperv_pcpu_input_arg = alloc_percpu(void  *);
> > +
> > +	if (hyperv_pcpu_input_arg == NULL)
> > +		return;
> 
> Huch. When that allocation fails, you return and ignore the rest of the
> function which has been there before. Weird decision.
I should have explained this. Failure of this allocation means that we would not have the
per-cpu hypercall input page which in turn would mean that we would not be able to invoke
any hypercalls. 

Regards,

K. Y

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

[Michael Kelley (EOSG)]

> -----Original Message-----
> From: kys@linuxonhyperv.com <kys@linuxonhyperv.com>
> Sent: Wednesday, April 25, 2018 11:13 AM
> To: x86@kernel.org; gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; jasowang@redhat.com;
> tglx@linutronix.de; hpa@zytor.com; Stephen Hemminger <sthemmin@microsoft.com>;
> Michael Kelley (EOSG) <Michael.H.Kelley@microsoft.com>; vkuznets@redhat.com
> Cc: KY Srinivasan <kys@microsoft.com>
> Subject: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> 
> From: "K. Y. Srinivasan" <kys@microsoft.com>
> 
> Hyper-V supports hypercalls to implement IPI; use them.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  arch/x86/hyperv/hv_apic.c          | 125 +++++++++++++++++++++++++++++++++++++
>  arch/x86/hyperv/hv_init.c          |  17 +++++
>  arch/x86/include/asm/hyperv-tlfs.h |   9 +++
>  arch/x86/include/asm/mshyperv.h    |   1 +
>  4 files changed, 152 insertions(+)
> 
> diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> index e0a5b36208fc..7f3322ecfb01 100644
> --- a/arch/x86/hyperv/hv_apic.c
> +++ b/arch/x86/hyperv/hv_apic.c
> @@ -30,6 +30,14 @@
>  #include <linux/slab.h>
>  #include <linux/cpuhotplug.h>
> 
> +struct ipi_arg_non_ex {
> +	u32 vector;
> +	u32 reserved;
> +	u64 cpu_mask;
> +};

I think we'd like to put structures like this, which are defined in the
Hyper-V Top Level Functional Spec, in hyperv-tlfs.h.  Also, the 5.0b
version of the TLFS, which is latest, shows this structure on page 100:

	u32 vector;
	u8  targetvtl;
	u8  reserved[3];
	u64 cpu_mask;

> +
> +static struct apic orig_apic;
> +
>  static u64 hv_apic_icr_read(void)
>  {
>  	u64 reg_val;
> @@ -85,8 +93,125 @@ static void hv_apic_eoi_write(u32 reg, u32 val)
>  	wrmsr(HV_X64_MSR_EOI, val, 0);
>  }
> 
> +/*
> + * IPI implementation on Hyper-V.
> + */
> +
> +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> +	int cur_cpu, vcpu;
> +	struct ipi_arg_non_ex **arg;
> +	struct ipi_arg_non_ex *ipi_arg;
> +	int ret = 1;
> +	unsigned long flags;
> +
> +	if (cpumask_empty(mask))
> +		return 0;
> +
> +	if (!hv_hypercall_pg)
> +		return ret;
> +
> +	if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
> +		return ret;
> +
> +	local_irq_save(flags);
> +	arg = (struct ipi_arg_non_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
> +
> +	ipi_arg = *arg;
> +	if (unlikely(!ipi_arg))
> +		goto ipi_mask_done;
> +
> +
> +	ipi_arg->vector = vector;
> +	ipi_arg->reserved = 0;
> +	ipi_arg->cpu_mask = 0;
> +
> +	for_each_cpu(cur_cpu, mask) {
> +		vcpu = hv_cpu_number_to_vp_number(cur_cpu);
> +		if (vcpu >= 64)
> +			goto ipi_mask_done;
> +
> +		__set_bit(vcpu, (unsigned long *)&ipi_arg->cpu_mask);
> +	}
> +
> +	ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL);
> +
> +ipi_mask_done:
> +	local_irq_restore(flags);
> +	return ret;
> +}
> +
> +static int __send_ipi_one(int cpu, int vector)
> +{
> +	struct cpumask mask = CPU_MASK_NONE;
> +
> +	cpumask_set_cpu(cpu, &mask);
> +	return __send_ipi_mask(&mask, vector);
> +}
> +
> +static void hv_send_ipi(int cpu, int vector)
> +{
> +	if (__send_ipi_one(cpu, vector))
> +		orig_apic.send_IPI(cpu, vector);
> +}
> +
> +static void hv_send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> +	if (__send_ipi_mask(mask, vector))
> +		orig_apic.send_IPI_mask(mask, vector);
> +}
> +
> +static void hv_send_ipi_mask_allbutself(const struct cpumask *mask, int vector)
> +{
> +	unsigned int this_cpu = smp_processor_id();
> +	struct cpumask new_mask;
> +	const struct cpumask *local_mask;
> +
> +	cpumask_copy(&new_mask, mask);
> +	cpumask_clear_cpu(this_cpu, &new_mask);
> +	local_mask = &new_mask;
> +	if (__send_ipi_mask(local_mask, vector))
> +		orig_apic.send_IPI_mask_allbutself(mask, vector);
> +}
> +
> +static void hv_send_ipi_allbutself(int vector)
> +{
> +	hv_send_ipi_mask_allbutself(cpu_online_mask, vector);
> +}
> +
> +static void hv_send_ipi_all(int vector)
> +{
> +	if (__send_ipi_mask(cpu_online_mask, vector))
> +		orig_apic.send_IPI_all(vector);
> +}
> +
> +static void hv_send_ipi_self(int vector)
> +{
> +	if (__send_ipi_one(smp_processor_id(), vector))
> +		orig_apic.send_IPI_self(vector);
> +}
> +
>  void __init hv_apic_init(void)
>  {
> +	if (ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) {
> +		if (hyperv_pcpu_input_arg == NULL)
> +			goto msr_based_access;
> +
> +		pr_info("Hyper-V: Using IPI hypercalls\n");
> +		/*
> +		 * Set the IPI entry points.
> +		 */
> +		orig_apic = *apic;
> +
> +		apic->send_IPI = hv_send_ipi;
> +		apic->send_IPI_mask = hv_send_ipi_mask;
> +		apic->send_IPI_mask_allbutself = hv_send_ipi_mask_allbutself;
> +		apic->send_IPI_allbutself = hv_send_ipi_allbutself;
> +		apic->send_IPI_all = hv_send_ipi_all;
> +		apic->send_IPI_self = hv_send_ipi_self;
> +	}
> +
> +msr_based_access:
>  	if (ms_hyperv.hints & HV_X64_APIC_ACCESS_RECOMMENDED) {
>  		pr_info("Hyper-V: Using MSR ased APIC access\n");
>  		apic_set_eoi_write(hv_apic_eoi_write);
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 71e50fc2b7ef..a895662b6b4c 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -91,12 +91,19 @@ EXPORT_SYMBOL_GPL(hv_vp_index);
>  struct hv_vp_assist_page **hv_vp_assist_page;
>  EXPORT_SYMBOL_GPL(hv_vp_assist_page);
> 
> +void  __percpu **hyperv_pcpu_input_arg;
> +EXPORT_SYMBOL_GPL(hyperv_pcpu_input_arg);
> +
>  u32 hv_max_vp_index;
> 
>  static int hv_cpu_init(unsigned int cpu)
>  {
>  	u64 msr_vp_index;
>  	struct hv_vp_assist_page **hvp = &hv_vp_assist_page[smp_processor_id()];
> +	void **input_arg;
> +
> +	input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> +	*input_arg = page_address(alloc_page(GFP_ATOMIC));
> 
>  	hv_get_vp_index(msr_vp_index);
> 
> @@ -217,6 +224,10 @@ static int hv_cpu_die(unsigned int cpu)
>  {
>  	struct hv_reenlightenment_control re_ctrl;
>  	unsigned int new_cpu;
> +	void **input_arg;
> +
> +	input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> +	free_page((unsigned long)*input_arg);
> 
>  	if (hv_vp_assist_page && hv_vp_assist_page[cpu])
>  		wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
> @@ -260,6 +271,12 @@ void __init hyperv_init(void)
>  	if ((ms_hyperv.features & required_msrs) != required_msrs)
>  		return;
> 
> +	/* Allocate the per-CPU state for the hypercall input arg */
> +	hyperv_pcpu_input_arg = alloc_percpu(void  *);
> +
> +	if (hyperv_pcpu_input_arg == NULL)
> +		return;
> +
>  	/* Allocate percpu VP index */
>  	hv_vp_index = kmalloc_array(num_possible_cpus(), sizeof(*hv_vp_index),
>  				    GFP_KERNEL);
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index 416cb0e0c496..646cf2ca2aaa 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -164,6 +164,11 @@
>   */
>  #define HV_X64_DEPRECATING_AEOI_RECOMMENDED	(1 << 9)
> 
> +/*
> + * Recommend using cluster IPI hypercalls.
> + */
> +#define HV_X64_CLUSTER_IPI_RECOMMENDED         (1 << 10)
> +
>  /* Recommend using the newer ExProcessorMasks interface */
>  #define HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED	(1 << 11)
> 
> @@ -329,10 +334,14 @@ struct hv_tsc_emulation_status {
>  #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK	\
>  		(~((1ull << HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT) - 1))
> 
> +#define HV_IPI_LOW_VECTOR	0x10
> +#define HV_IPI_HIGH_VECTOR	0xff
> +
>  /* Declare the various hypercall operations. */
>  #define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE	0x0002
>  #define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST	0x0003
>  #define HVCALL_NOTIFY_LONG_SPIN_WAIT		0x0008
> +#define HVCALL_SEND_IPI				0x000b
>  #define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX  0x0013
>  #define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX   0x0014
>  #define HVCALL_POST_MESSAGE			0x005c
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index bcced50037c1..f6045f3611de 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -122,6 +122,7 @@ static inline void hv_disable_stimer0_percpu_irq(int irq) {}
>  #if IS_ENABLED(CONFIG_HYPERV)
>  extern struct clocksource *hyperv_cs;
>  extern void *hv_hypercall_pg;
> +extern void  __percpu  **hyperv_pcpu_input_arg;
> 
>  static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
>  {
> --
> 2.15.1

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

[KY Srinivasan]

> -----Original Message-----
> From: Michael Kelley (EOSG)
> Sent: Thursday, April 26, 2018 3:55 PM
> To: KY Srinivasan <kys@microsoft.com>; x86@kernel.org;
> gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com; tglx@linutronix.de; hpa@zytor.com; Stephen
> Hemminger <sthemmin@microsoft.com>; vkuznets@redhat.com
> Subject: RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> 
> > -----Original Message-----
> > From: kys@linuxonhyperv.com <kys@linuxonhyperv.com>
> > Sent: Wednesday, April 25, 2018 11:13 AM
> > To: x86@kernel.org; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com;
> > tglx@linutronix.de; hpa@zytor.com; Stephen Hemminger
> <sthemmin@microsoft.com>;
> > Michael Kelley (EOSG) <Michael.H.Kelley@microsoft.com>;
> vkuznets@redhat.com
> > Cc: KY Srinivasan <kys@microsoft.com>
> > Subject: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> >
> > From: "K. Y. Srinivasan" <kys@microsoft.com>
> >
> > Hyper-V supports hypercalls to implement IPI; use them.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> >  arch/x86/hyperv/hv_apic.c          | 125
> +++++++++++++++++++++++++++++++++++++
> >  arch/x86/hyperv/hv_init.c          |  17 +++++
> >  arch/x86/include/asm/hyperv-tlfs.h |   9 +++
> >  arch/x86/include/asm/mshyperv.h    |   1 +
> >  4 files changed, 152 insertions(+)
> >
> > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> > index e0a5b36208fc..7f3322ecfb01 100644
> > --- a/arch/x86/hyperv/hv_apic.c
> > +++ b/arch/x86/hyperv/hv_apic.c
> > @@ -30,6 +30,14 @@
> >  #include <linux/slab.h>
> >  #include <linux/cpuhotplug.h>
> >
> > +struct ipi_arg_non_ex {
> > +	u32 vector;
> > +	u32 reserved;
> > +	u64 cpu_mask;
> > +};
> 
> I think we'd like to put structures like this, which are defined in the
> Hyper-V Top Level Functional Spec, in hyperv-tlfs.h.  Also, the 5.0b
> version of the TLFS, which is latest, shows this structure on page 100:
> 
> 	u32 vector;
> 	u8  targetvtl;
> 	u8  reserved[3];
> 	u64 cpu_mask;
> 

Good point. I will make the necessary adjustments.

Regards,

K. Y
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 6987 bytes --]

Hi Srinivasan,

I love your patch! Yet something to improve:

[auto build test ERROR on v4.17-rc2]
[also build test ERROR on next-20180426]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/kys-linuxonhyperv-com/X86-Hyper-V-APIC-enlightenments/20180427-114416
config: i386-randconfig-b0-04270034 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86//hyperv/hv_apic.c: In function 'hv_apic_read':
   arch/x86//hyperv/hv_apic.c:73:3: error: implicit declaration of function 'native_apic_mem_read' [-Werror=implicit-function-declaration]
      return native_apic_mem_read(reg);
      ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_apic_write':
   arch/x86//hyperv/hv_apic.c:87:3: error: implicit declaration of function 'native_apic_mem_write' [-Werror=implicit-function-declaration]
      native_apic_mem_write(reg, val);
      ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi':
>> arch/x86//hyperv/hv_apic.c:155:3: error: invalid use of undefined type 'struct apic'
      orig_apic.send_IPI(cpu, vector);
      ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi_mask':
   arch/x86//hyperv/hv_apic.c:161:3: error: invalid use of undefined type 'struct apic'
      orig_apic.send_IPI_mask(mask, vector);
      ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi_mask_allbutself':
   arch/x86//hyperv/hv_apic.c:174:3: error: invalid use of undefined type 'struct apic'
      orig_apic.send_IPI_mask_allbutself(mask, vector);
      ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi_all':
   arch/x86//hyperv/hv_apic.c:185:3: error: invalid use of undefined type 'struct apic'
      orig_apic.send_IPI_all(vector);
      ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi_self':
   arch/x86//hyperv/hv_apic.c:191:3: error: invalid use of undefined type 'struct apic'
      orig_apic.send_IPI_self(vector);
      ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_apic_init':
   arch/x86//hyperv/hv_apic.c:204:16: error: 'apic' undeclared (first use in this function)
      orig_apic = *apic;
                   ^
   arch/x86//hyperv/hv_apic.c:204:16: note: each undeclared identifier is reported only once for each function it appears in
>> arch/x86//hyperv/hv_apic.c:204:3: error: 'orig_apic' has an incomplete type
      orig_apic = *apic;
      ^
   arch/x86//hyperv/hv_apic.c:217:3: error: implicit declaration of function 'apic_set_eoi_write' [-Werror=implicit-function-declaration]
      apic_set_eoi_write(hv_apic_eoi_write);
      ^
   cc1: some warnings being treated as errors

vim +/orig_apic +204 arch/x86//hyperv/hv_apic.c

    59	
    60	static u32 hv_apic_read(u32 reg)
    61	{
    62		u32 reg_val, hi;
    63	
    64		switch (reg) {
    65		case APIC_EOI:
    66			rdmsr(HV_X64_MSR_EOI, reg_val, hi);
    67			return reg_val;
    68		case APIC_TASKPRI:
    69			rdmsr(HV_X64_MSR_TPR, reg_val, hi);
    70			return reg_val;
    71	
    72		default:
  > 73			return native_apic_mem_read(reg);
    74		}
    75	}
    76	
    77	static void hv_apic_write(u32 reg, u32 val)
    78	{
    79		switch (reg) {
    80		case APIC_EOI:
    81			wrmsr(HV_X64_MSR_EOI, val, 0);
    82			break;
    83		case APIC_TASKPRI:
    84			wrmsr(HV_X64_MSR_TPR, val, 0);
    85			break;
    86		default:
    87			native_apic_mem_write(reg, val);
    88		}
    89	}
    90	
    91	static void hv_apic_eoi_write(u32 reg, u32 val)
    92	{
    93		wrmsr(HV_X64_MSR_EOI, val, 0);
    94	}
    95	
    96	/*
    97	 * IPI implementation on Hyper-V.
    98	 */
    99	
   100	static int __send_ipi_mask(const struct cpumask *mask, int vector)
   101	{
   102		int cur_cpu, vcpu;
   103		struct ipi_arg_non_ex **arg;
   104		struct ipi_arg_non_ex *ipi_arg;
   105		int ret = 1;
   106		unsigned long flags;
   107	
   108		if (cpumask_empty(mask))
   109			return 0;
   110	
   111		if (!hv_hypercall_pg)
   112			return ret;
   113	
   114		if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
   115			return ret;
   116	
   117		local_irq_save(flags);
   118		arg = (struct ipi_arg_non_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
   119	
   120		ipi_arg = *arg;
   121		if (unlikely(!ipi_arg))
   122			goto ipi_mask_done;
   123	
   124	
   125		ipi_arg->vector = vector;
   126		ipi_arg->reserved = 0;
   127		ipi_arg->cpu_mask = 0;
   128	
   129		for_each_cpu(cur_cpu, mask) {
   130			vcpu = hv_cpu_number_to_vp_number(cur_cpu);
   131			if (vcpu >= 64)
   132				goto ipi_mask_done;
   133	
   134			__set_bit(vcpu, (unsigned long *)&ipi_arg->cpu_mask);
   135		}
   136	
   137		ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL);
   138	
   139	ipi_mask_done:
   140		local_irq_restore(flags);
   141		return ret;
   142	}
   143	
   144	static int __send_ipi_one(int cpu, int vector)
   145	{
   146		struct cpumask mask = CPU_MASK_NONE;
   147	
   148		cpumask_set_cpu(cpu, &mask);
   149		return __send_ipi_mask(&mask, vector);
   150	}
   151	
   152	static void hv_send_ipi(int cpu, int vector)
   153	{
   154		if (__send_ipi_one(cpu, vector))
 > 155			orig_apic.send_IPI(cpu, vector);
   156	}
   157	
   158	static void hv_send_ipi_mask(const struct cpumask *mask, int vector)
   159	{
   160		if (__send_ipi_mask(mask, vector))
   161			orig_apic.send_IPI_mask(mask, vector);
   162	}
   163	
   164	static void hv_send_ipi_mask_allbutself(const struct cpumask *mask, int vector)
   165	{
   166		unsigned int this_cpu = smp_processor_id();
   167		struct cpumask new_mask;
   168		const struct cpumask *local_mask;
   169	
   170		cpumask_copy(&new_mask, mask);
   171		cpumask_clear_cpu(this_cpu, &new_mask);
   172		local_mask = &new_mask;
   173		if (__send_ipi_mask(local_mask, vector))
   174			orig_apic.send_IPI_mask_allbutself(mask, vector);
   175	}
   176	
   177	static void hv_send_ipi_allbutself(int vector)
   178	{
   179		hv_send_ipi_mask_allbutself(cpu_online_mask, vector);
   180	}
   181	
   182	static void hv_send_ipi_all(int vector)
   183	{
   184		if (__send_ipi_mask(cpu_online_mask, vector))
   185			orig_apic.send_IPI_all(vector);
   186	}
   187	
   188	static void hv_send_ipi_self(int vector)
   189	{
   190		if (__send_ipi_one(smp_processor_id(), vector))
 > 191			orig_apic.send_IPI_self(vector);
   192	}
   193	
   194	void __init hv_apic_init(void)
   195	{
   196		if (ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) {
   197			if (hyperv_pcpu_input_arg == NULL)
   198				goto msr_based_access;
   199	
   200			pr_info("Hyper-V: Using IPI hypercalls\n");
   201			/*
   202			 * Set the IPI entry points.
   203			 */
 > 204			orig_apic = *apic;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25200 bytes --]

[-- Attachment #3: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

[kbuild test robot]

Hi Srinivasan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on v4.17-rc2]
[also build test WARNING on next-20180426]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/kys-linuxonhyperv-com/X86-Hyper-V-APIC-enlightenments/20180427-114416
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> arch/x86/hyperv/hv_init.c:105:30: sparse: incorrect type in initializer (different address spaces) @@    expected void const [noderef] <asn:3>*__vpp_verify @@    got const [noderef] <asn:3>*__vpp_verify @@
   arch/x86/hyperv/hv_init.c:105:30:    expected void const [noderef] <asn:3>*__vpp_verify
   arch/x86/hyperv/hv_init.c:105:30:    got void [noderef] <asn:3>**<noident>
   arch/x86/hyperv/hv_init.c:229:30: sparse: incorrect type in initializer (different address spaces) @@    expected void const [noderef] <asn:3>*__vpp_verify @@    got const [noderef] <asn:3>*__vpp_verify @@
   arch/x86/hyperv/hv_init.c:229:30:    expected void const [noderef] <asn:3>*__vpp_verify
   arch/x86/hyperv/hv_init.c:229:30:    got void [noderef] <asn:3>**<noident>
>> arch/x86/hyperv/hv_init.c:275:31: sparse: incorrect type in assignment (different address spaces) @@    expected void [noderef] <asn:3>**extern [addressable] [toplevel] hyperv_pcpu_input_arg @@    got addressable] [toplevel] hyperv_pcpu_input_arg @@
   arch/x86/hyperv/hv_init.c:275:31:    expected void [noderef] <asn:3>**extern [addressable] [toplevel] hyperv_pcpu_input_arg
   arch/x86/hyperv/hv_init.c:275:31:    got void *[noderef] <asn:3>*<noident>
   arch/x86/include/asm/paravirt.h:150:9: sparse: cast truncates bits from constant value (8000000000000000 becomes 0)
--
>> arch/x86/hyperv/hv_apic.c:118:41: sparse: incorrect type in initializer (different address spaces) @@    expected void const [noderef] <asn:3>*__vpp_verify @@    got const [noderef] <asn:3>*__vpp_verify @@
   arch/x86/hyperv/hv_apic.c:118:41:    expected void const [noderef] <asn:3>*__vpp_verify
   arch/x86/hyperv/hv_apic.c:118:41:    got void [noderef] <asn:3>**<noident>

vim +105 arch/x86/hyperv/hv_init.c

    98	
    99	static int hv_cpu_init(unsigned int cpu)
   100	{
   101		u64 msr_vp_index;
   102		struct hv_vp_assist_page **hvp = &hv_vp_assist_page[smp_processor_id()];
   103		void **input_arg;
   104	
 > 105		input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
   106		*input_arg = page_address(alloc_page(GFP_ATOMIC));
   107	
   108		hv_get_vp_index(msr_vp_index);
   109	
   110		hv_vp_index[smp_processor_id()] = msr_vp_index;
   111	
   112		if (msr_vp_index > hv_max_vp_index)
   113			hv_max_vp_index = msr_vp_index;
   114	
   115		if (!hv_vp_assist_page)
   116			return 0;
   117	
   118		if (!*hvp)
   119			*hvp = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL);
   120	
   121		if (*hvp) {
   122			u64 val;
   123	
   124			val = vmalloc_to_pfn(*hvp);
   125			val = (val << HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT) |
   126				HV_X64_MSR_VP_ASSIST_PAGE_ENABLE;
   127	
   128			wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, val);
   129		}
   130	
   131		return 0;
   132	}
   133	
   134	static void (*hv_reenlightenment_cb)(void);
   135	
   136	static void hv_reenlightenment_notify(struct work_struct *dummy)
   137	{
   138		struct hv_tsc_emulation_status emu_status;
   139	
   140		rdmsrl(HV_X64_MSR_TSC_EMULATION_STATUS, *(u64 *)&emu_status);
   141	
   142		/* Don't issue the callback if TSC accesses are not emulated */
   143		if (hv_reenlightenment_cb && emu_status.inprogress)
   144			hv_reenlightenment_cb();
   145	}
   146	static DECLARE_DELAYED_WORK(hv_reenlightenment_work, hv_reenlightenment_notify);
   147	
   148	void hyperv_stop_tsc_emulation(void)
   149	{
   150		u64 freq;
   151		struct hv_tsc_emulation_status emu_status;
   152	
   153		rdmsrl(HV_X64_MSR_TSC_EMULATION_STATUS, *(u64 *)&emu_status);
   154		emu_status.inprogress = 0;
   155		wrmsrl(HV_X64_MSR_TSC_EMULATION_STATUS, *(u64 *)&emu_status);
   156	
   157		rdmsrl(HV_X64_MSR_TSC_FREQUENCY, freq);
   158		tsc_khz = div64_u64(freq, 1000);
   159	}
   160	EXPORT_SYMBOL_GPL(hyperv_stop_tsc_emulation);
   161	
   162	static inline bool hv_reenlightenment_available(void)
   163	{
   164		/*
   165		 * Check for required features and priviliges to make TSC frequency
   166		 * change notifications work.
   167		 */
   168		return ms_hyperv.features & HV_X64_ACCESS_FREQUENCY_MSRS &&
   169			ms_hyperv.misc_features & HV_FEATURE_FREQUENCY_MSRS_AVAILABLE &&
   170			ms_hyperv.features & HV_X64_ACCESS_REENLIGHTENMENT;
   171	}
   172	
   173	__visible void __irq_entry hyperv_reenlightenment_intr(struct pt_regs *regs)
   174	{
   175		entering_ack_irq();
   176	
   177		inc_irq_stat(irq_hv_reenlightenment_count);
   178	
   179		schedule_delayed_work(&hv_reenlightenment_work, HZ/10);
   180	
   181		exiting_irq();
   182	}
   183	
   184	void set_hv_tscchange_cb(void (*cb)(void))
   185	{
   186		struct hv_reenlightenment_control re_ctrl = {
   187			.vector = HYPERV_REENLIGHTENMENT_VECTOR,
   188			.enabled = 1,
   189			.target_vp = hv_vp_index[smp_processor_id()]
   190		};
   191		struct hv_tsc_emulation_control emu_ctrl = {.enabled = 1};
   192	
   193		if (!hv_reenlightenment_available()) {
   194			pr_warn("Hyper-V: reenlightenment support is unavailable\n");
   195			return;
   196		}
   197	
   198		hv_reenlightenment_cb = cb;
   199	
   200		/* Make sure callback is registered before we write to MSRs */
   201		wmb();
   202	
   203		wrmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *((u64 *)&re_ctrl));
   204		wrmsrl(HV_X64_MSR_TSC_EMULATION_CONTROL, *((u64 *)&emu_ctrl));
   205	}
   206	EXPORT_SYMBOL_GPL(set_hv_tscchange_cb);
   207	
   208	void clear_hv_tscchange_cb(void)
   209	{
   210		struct hv_reenlightenment_control re_ctrl;
   211	
   212		if (!hv_reenlightenment_available())
   213			return;
   214	
   215		rdmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *(u64 *)&re_ctrl);
   216		re_ctrl.enabled = 0;
   217		wrmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *(u64 *)&re_ctrl);
   218	
   219		hv_reenlightenment_cb = NULL;
   220	}
   221	EXPORT_SYMBOL_GPL(clear_hv_tscchange_cb);
   222	
   223	static int hv_cpu_die(unsigned int cpu)
   224	{
   225		struct hv_reenlightenment_control re_ctrl;
   226		unsigned int new_cpu;
   227		void **input_arg;
   228	
   229		input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
   230		free_page((unsigned long)*input_arg);
   231	
   232		if (hv_vp_assist_page && hv_vp_assist_page[cpu])
   233			wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
   234	
   235		if (hv_reenlightenment_cb == NULL)
   236			return 0;
   237	
   238		rdmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *((u64 *)&re_ctrl));
   239		if (re_ctrl.target_vp == hv_vp_index[cpu]) {
   240			/* Reassign to some other online CPU */
   241			new_cpu = cpumask_any_but(cpu_online_mask, cpu);
   242	
   243			re_ctrl.target_vp = hv_vp_index[new_cpu];
   244			wrmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *((u64 *)&re_ctrl));
   245		}
   246	
   247		return 0;
   248	}
   249	
   250	/*
   251	 * This function is to be invoked early in the boot sequence after the
   252	 * hypervisor has been detected.
   253	 *
   254	 * 1. Setup the hypercall page.
   255	 * 2. Register Hyper-V specific clocksource.
   256	 * 3. Setup Hyper-V specific APIC entry points.
   257	 */
   258	void __init hyperv_init(void)
   259	{
   260		u64 guest_id, required_msrs;
   261		union hv_x64_msr_hypercall_contents hypercall_msr;
   262		int cpuhp;
   263	
   264		if (x86_hyper_type != X86_HYPER_MS_HYPERV)
   265			return;
   266	
   267		/* Absolutely required MSRs */
   268		required_msrs = HV_X64_MSR_HYPERCALL_AVAILABLE |
   269			HV_X64_MSR_VP_INDEX_AVAILABLE;
   270	
   271		if ((ms_hyperv.features & required_msrs) != required_msrs)
   272			return;
   273	
   274		/* Allocate the per-CPU state for the hypercall input arg */
 > 275		hyperv_pcpu_input_arg = alloc_percpu(void  *);
   276	
   277		if (hyperv_pcpu_input_arg == NULL)
   278			return;
   279	
   280		/* Allocate percpu VP index */
   281		hv_vp_index = kmalloc_array(num_possible_cpus(), sizeof(*hv_vp_index),
   282					    GFP_KERNEL);
   283		if (!hv_vp_index)
   284			return;
   285	
   286		hv_vp_assist_page = kcalloc(num_possible_cpus(),
   287					    sizeof(*hv_vp_assist_page), GFP_KERNEL);
   288		if (!hv_vp_assist_page) {
   289			ms_hyperv.hints &= ~HV_X64_ENLIGHTENED_VMCS_RECOMMENDED;
   290			goto free_vp_index;
   291		}
   292	
   293		cpuhp = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/hyperv_init:online",
   294					  hv_cpu_init, hv_cpu_die);
   295		if (cpuhp < 0)
   296			goto free_vp_assist_page;
   297	
   298		/*
   299		 * Setup the hypercall page and enable hypercalls.
   300		 * 1. Register the guest ID
   301		 * 2. Enable the hypercall and register the hypercall page
   302		 */
   303		guest_id = generate_guest_id(0, LINUX_VERSION_CODE, 0);
   304		wrmsrl(HV_X64_MSR_GUEST_OS_ID, guest_id);
   305	
   306		hv_hypercall_pg  = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL_RX);
   307		if (hv_hypercall_pg == NULL) {
   308			wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
   309			goto remove_cpuhp_state;
   310		}
   311	
   312		rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
   313		hypercall_msr.enable = 1;
   314		hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
   315		wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
   316	
   317		hyper_alloc_mmu();
   318	
   319		hv_apic_init();
   320	
   321		/*
   322		 * Register Hyper-V specific clocksource.
   323		 */
   324	#ifdef CONFIG_HYPERV_TSCPAGE
   325		if (ms_hyperv.features & HV_X64_MSR_REFERENCE_TSC_AVAILABLE) {
   326			union hv_x64_msr_hypercall_contents tsc_msr;
   327	
   328			tsc_pg = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL);
   329			if (!tsc_pg)
   330				goto register_msr_cs;
   331	
   332			hyperv_cs = &hyperv_cs_tsc;
   333	
   334			rdmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
   335	
   336			tsc_msr.enable = 1;
   337			tsc_msr.guest_physical_address = vmalloc_to_pfn(tsc_pg);
   338	
   339			wrmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64);
   340	
   341			hyperv_cs_tsc.archdata.vclock_mode = VCLOCK_HVCLOCK;
   342	
   343			clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100);
   344			return;
   345		}
   346	register_msr_cs:
   347	#endif
   348		/*
   349		 * For 32 bit guests just use the MSR based mechanism for reading
   350		 * the partition counter.
   351		 */
   352	
   353		hyperv_cs = &hyperv_cs_msr;
   354		if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)
   355			clocksource_register_hz(&hyperv_cs_msr, NSEC_PER_SEC/100);
   356	
   357		return;
   358	
   359	remove_cpuhp_state:
   360		cpuhp_remove_state(cpuhp);
   361	free_vp_assist_page:
   362		kfree(hv_vp_assist_page);
   363		hv_vp_assist_page = NULL;
   364	free_vp_index:
   365		kfree(hv_vp_index);
   366		hv_vp_index = NULL;
   367	}
   368	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 7749 bytes --]

Hi Srinivasan,

I love your patch! Yet something to improve:

[auto build test ERROR on v4.17-rc2]
[also build test ERROR on next-20180426]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/kys-linuxonhyperv-com/X86-Hyper-V-APIC-enlightenments/20180427-114416
config: i386-randconfig-s1-04271426 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86/hyperv/hv_apic.c: In function 'hv_apic_read':
   arch/x86/hyperv/hv_apic.c:73:10: error: implicit declaration of function 'native_apic_mem_read' [-Werror=implicit-function-declaration]
      return native_apic_mem_read(reg);
             ^~~~~~~~~~~~~~~~~~~~
   arch/x86/hyperv/hv_apic.c: In function 'hv_apic_write':
   arch/x86/hyperv/hv_apic.c:87:3: error: implicit declaration of function 'native_apic_mem_write' [-Werror=implicit-function-declaration]
      native_apic_mem_write(reg, val);
      ^~~~~~~~~~~~~~~~~~~~~
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi':
   arch/x86/hyperv/hv_apic.c:155:3: error: invalid use of undefined type 'struct apic'
      orig_apic.send_IPI(cpu, vector);
      ^~~~~~~~~
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_mask':
   arch/x86/hyperv/hv_apic.c:161:3: error: invalid use of undefined type 'struct apic'
      orig_apic.send_IPI_mask(mask, vector);
      ^~~~~~~~~
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_mask_allbutself':
   arch/x86/hyperv/hv_apic.c:174:3: error: invalid use of undefined type 'struct apic'
      orig_apic.send_IPI_mask_allbutself(mask, vector);
      ^~~~~~~~~
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_all':
   arch/x86/hyperv/hv_apic.c:185:3: error: invalid use of undefined type 'struct apic'
      orig_apic.send_IPI_all(vector);
      ^~~~~~~~~
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_self':
   arch/x86/hyperv/hv_apic.c:191:3: error: invalid use of undefined type 'struct apic'
      orig_apic.send_IPI_self(vector);
      ^~~~~~~~~
   arch/x86/hyperv/hv_apic.c: In function 'hv_apic_init':
   arch/x86/hyperv/hv_apic.c:204:16: error: 'apic' undeclared (first use in this function)
      orig_apic = *apic;
                   ^~~~
   arch/x86/hyperv/hv_apic.c:204:16: note: each undeclared identifier is reported only once for each function it appears in
>> arch/x86/hyperv/hv_apic.c:204:3: error: 'orig_apic' has an incomplete type 'struct apic'
      orig_apic = *apic;
      ^~~~~~~~~
   arch/x86/hyperv/hv_apic.c:217:3: error: implicit declaration of function 'apic_set_eoi_write' [-Werror=implicit-function-declaration]
      apic_set_eoi_write(hv_apic_eoi_write);
      ^~~~~~~~~~~~~~~~~~
   arch/x86/hyperv/hv_apic.c: At top level:
>> arch/x86/hyperv/hv_apic.c:39:20: error: storage size of 'orig_apic' isn't known
    static struct apic orig_apic;
                       ^~~~~~~~~
   cc1: some warnings being treated as errors

vim +204 arch/x86/hyperv/hv_apic.c

    38	
  > 39	static struct apic orig_apic;
    40	
    41	static u64 hv_apic_icr_read(void)
    42	{
    43		u64 reg_val;
    44	
    45		rdmsrl(HV_X64_MSR_ICR, reg_val);
    46		return reg_val;
    47	}
    48	
    49	static void hv_apic_icr_write(u32 low, u32 id)
    50	{
    51		u64 reg_val;
    52	
    53		reg_val = SET_APIC_DEST_FIELD(id);
    54		reg_val = reg_val << 32;
    55		reg_val |= low;
    56	
    57		wrmsrl(HV_X64_MSR_ICR, reg_val);
    58	}
    59	
    60	static u32 hv_apic_read(u32 reg)
    61	{
    62		u32 reg_val, hi;
    63	
    64		switch (reg) {
    65		case APIC_EOI:
    66			rdmsr(HV_X64_MSR_EOI, reg_val, hi);
    67			return reg_val;
    68		case APIC_TASKPRI:
    69			rdmsr(HV_X64_MSR_TPR, reg_val, hi);
    70			return reg_val;
    71	
    72		default:
    73			return native_apic_mem_read(reg);
    74		}
    75	}
    76	
    77	static void hv_apic_write(u32 reg, u32 val)
    78	{
    79		switch (reg) {
    80		case APIC_EOI:
    81			wrmsr(HV_X64_MSR_EOI, val, 0);
    82			break;
    83		case APIC_TASKPRI:
    84			wrmsr(HV_X64_MSR_TPR, val, 0);
    85			break;
    86		default:
    87			native_apic_mem_write(reg, val);
    88		}
    89	}
    90	
    91	static void hv_apic_eoi_write(u32 reg, u32 val)
    92	{
    93		wrmsr(HV_X64_MSR_EOI, val, 0);
    94	}
    95	
    96	/*
    97	 * IPI implementation on Hyper-V.
    98	 */
    99	
   100	static int __send_ipi_mask(const struct cpumask *mask, int vector)
   101	{
   102		int cur_cpu, vcpu;
   103		struct ipi_arg_non_ex **arg;
   104		struct ipi_arg_non_ex *ipi_arg;
   105		int ret = 1;
   106		unsigned long flags;
   107	
   108		if (cpumask_empty(mask))
   109			return 0;
   110	
   111		if (!hv_hypercall_pg)
   112			return ret;
   113	
   114		if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
   115			return ret;
   116	
   117		local_irq_save(flags);
   118		arg = (struct ipi_arg_non_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
   119	
   120		ipi_arg = *arg;
   121		if (unlikely(!ipi_arg))
   122			goto ipi_mask_done;
   123	
   124	
   125		ipi_arg->vector = vector;
   126		ipi_arg->reserved = 0;
   127		ipi_arg->cpu_mask = 0;
   128	
   129		for_each_cpu(cur_cpu, mask) {
   130			vcpu = hv_cpu_number_to_vp_number(cur_cpu);
   131			if (vcpu >= 64)
   132				goto ipi_mask_done;
   133	
   134			__set_bit(vcpu, (unsigned long *)&ipi_arg->cpu_mask);
   135		}
   136	
   137		ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL);
   138	
   139	ipi_mask_done:
   140		local_irq_restore(flags);
   141		return ret;
   142	}
   143	
   144	static int __send_ipi_one(int cpu, int vector)
   145	{
   146		struct cpumask mask = CPU_MASK_NONE;
   147	
   148		cpumask_set_cpu(cpu, &mask);
   149		return __send_ipi_mask(&mask, vector);
   150	}
   151	
   152	static void hv_send_ipi(int cpu, int vector)
   153	{
   154		if (__send_ipi_one(cpu, vector))
   155			orig_apic.send_IPI(cpu, vector);
   156	}
   157	
   158	static void hv_send_ipi_mask(const struct cpumask *mask, int vector)
   159	{
   160		if (__send_ipi_mask(mask, vector))
   161			orig_apic.send_IPI_mask(mask, vector);
   162	}
   163	
   164	static void hv_send_ipi_mask_allbutself(const struct cpumask *mask, int vector)
   165	{
   166		unsigned int this_cpu = smp_processor_id();
   167		struct cpumask new_mask;
   168		const struct cpumask *local_mask;
   169	
   170		cpumask_copy(&new_mask, mask);
   171		cpumask_clear_cpu(this_cpu, &new_mask);
   172		local_mask = &new_mask;
   173		if (__send_ipi_mask(local_mask, vector))
   174			orig_apic.send_IPI_mask_allbutself(mask, vector);
   175	}
   176	
   177	static void hv_send_ipi_allbutself(int vector)
   178	{
   179		hv_send_ipi_mask_allbutself(cpu_online_mask, vector);
   180	}
   181	
   182	static void hv_send_ipi_all(int vector)
   183	{
   184		if (__send_ipi_mask(cpu_online_mask, vector))
   185			orig_apic.send_IPI_all(vector);
   186	}
   187	
   188	static void hv_send_ipi_self(int vector)
   189	{
   190		if (__send_ipi_one(smp_processor_id(), vector))
 > 191			orig_apic.send_IPI_self(vector);
   192	}
   193	
   194	void __init hv_apic_init(void)
   195	{
   196		if (ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) {
   197			if (hyperv_pcpu_input_arg == NULL)
   198				goto msr_based_access;
   199	
   200			pr_info("Hyper-V: Using IPI hypercalls\n");
   201			/*
   202			 * Set the IPI entry points.
   203			 */
 > 204			orig_apic = *apic;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26738 bytes --]

[PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment

[kys]

From: "K. Y. Srinivasan" <kys@microsoft.com>

Support enhanced IPI enlightenments (to target more than 64 CPUs).

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 arch/x86/hyperv/hv_apic.c          | 49 ++++++++++++++++++++++++++++++++++++--
 arch/x86/include/asm/hyperv-tlfs.h |  1 +
 arch/x86/include/asm/mshyperv.h    | 39 ++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index 7f3322ecfb01..fbb408b91e25 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -36,6 +36,12 @@ struct ipi_arg_non_ex {
 	u64 cpu_mask;
 };
 
+struct ipi_arg_ex {
+	u32 vector;
+	u32  reserved;
+	struct hv_vpset vp_set;
+};
+
 static struct apic orig_apic;
 
 static u64 hv_apic_icr_read(void)
@@ -97,6 +103,40 @@ static void hv_apic_eoi_write(u32 reg, u32 val)
  * IPI implementation on Hyper-V.
  */
 
+static int __send_ipi_mask_ex(const struct cpumask *mask, int vector)
+{
+	int nr_bank = 0;
+	struct ipi_arg_ex **arg;
+	struct ipi_arg_ex *ipi_arg;
+	int ret = 1;
+	unsigned long flags;
+
+	local_irq_save(flags);
+	arg = (struct ipi_arg_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
+
+	ipi_arg = *arg;
+	if (unlikely(!ipi_arg))
+		goto ipi_mask_ex_done;
+
+	ipi_arg->vector = vector;
+	ipi_arg->reserved = 0;
+	ipi_arg->vp_set.valid_bank_mask = 0;
+
+	if (!cpumask_equal(mask, cpu_present_mask)) {
+		ipi_arg->vp_set.format = HV_GENERIC_SET_SPARCE_4K;
+		nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask);
+	}
+	if (!nr_bank)
+		ipi_arg->vp_set.format = HV_GENERIC_SET_ALL;
+
+	ret = hv_do_rep_hypercall(HVCALL_SEND_IPI_EX, 0, nr_bank,
+			      ipi_arg, NULL);
+
+ipi_mask_ex_done:
+	local_irq_restore(flags);
+	return ret;
+}
+
 static int __send_ipi_mask(const struct cpumask *mask, int vector)
 {
 	int cur_cpu, vcpu;
@@ -114,6 +154,9 @@ static int __send_ipi_mask(const struct cpumask *mask, int vector)
 	if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
 		return ret;
 
+	if ((ms_hyperv.hints & HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED))
+		return __send_ipi_mask_ex(mask, vector);
+
 	local_irq_save(flags);
 	arg = (struct ipi_arg_non_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
 
@@ -196,8 +239,10 @@ void __init hv_apic_init(void)
 	if (ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) {
 		if (hyperv_pcpu_input_arg == NULL)
 			goto msr_based_access;
-
-		pr_info("Hyper-V: Using IPI hypercalls\n");
+		if ((ms_hyperv.hints & HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED))
+			pr_info("Hyper-V: Using ext hypercalls for IPI\n");
+		else
+			pr_info("Hyper-V: Using IPI hypercalls\n");
 		/*
 		 * Set the IPI entry points.
 		 */
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 646cf2ca2aaa..53ea30d768d9 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -344,6 +344,7 @@ struct hv_tsc_emulation_status {
 #define HVCALL_SEND_IPI				0x000b
 #define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX  0x0013
 #define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX   0x0014
+#define HVCALL_SEND_IPI_EX			0x0015
 #define HVCALL_POST_MESSAGE			0x005c
 #define HVCALL_SIGNAL_EVENT			0x005d
 
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index f6045f3611de..956e8603bed2 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -259,6 +259,45 @@ static inline int hv_cpu_number_to_vp_number(int cpu_number)
 	return hv_vp_index[cpu_number];
 }
 
+struct hv_vpset {
+	u64 format;
+	u64 valid_bank_mask;
+	u64 bank_contents[];
+};
+
+static inline int cpumask_to_vpset(struct hv_vpset *vpset,
+				    const struct cpumask *cpus)
+{
+	int cpu, vcpu, vcpu_bank, vcpu_offset, nr_bank = 1;
+
+	/* valid_bank_mask can represent up to 64 banks */
+	if (hv_max_vp_index / 64 >= 64)
+		return 0;
+
+	/*
+	 * Clear all banks up to the maximum possible bank as hv_flush_pcpu_ex
+	 * structs are not cleared between calls, we risk flushing unneeded
+	 * vCPUs otherwise.
+	 */
+	for (vcpu_bank = 0; vcpu_bank <= hv_max_vp_index / 64; vcpu_bank++)
+		vpset->bank_contents[vcpu_bank] = 0;
+
+	/*
+	 * Some banks may end up being empty but this is acceptable.
+	 */
+	for_each_cpu(cpu, cpus) {
+		vcpu = hv_cpu_number_to_vp_number(cpu);
+		vcpu_bank = vcpu / 64;
+		vcpu_offset = vcpu % 64;
+		__set_bit(vcpu_offset, (unsigned long *)
+			  &vpset->bank_contents[vcpu_bank]);
+		if (vcpu_bank >= nr_bank)
+			nr_bank = vcpu_bank + 1;
+	}
+	vpset->valid_bank_mask = GENMASK_ULL(nr_bank - 1, 0);
+	return nr_bank;
+}
+
 void __init hyperv_init(void);
 void hyperv_setup_mmu_ops(void);
 void hyper_alloc_mmu(void);
-- 
2.15.1

Re: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment

[Thomas Gleixner]

On Wed, 25 Apr 2018, kys@linuxonhyperv.com wrote:
>  
> +struct ipi_arg_ex {
> +	u32 vector;
> +	u32  reserved;
> +	struct hv_vpset vp_set;

Please align that in tabular fashion for easy of reading

	u32		vector;
	u32  		reserved;
	struct hv_vpset	vp_set;

> +};
> +
>  static struct apic orig_apic;
>  
>  static u64 hv_apic_icr_read(void)
> @@ -97,6 +103,40 @@ static void hv_apic_eoi_write(u32 reg, u32 val)
>   * IPI implementation on Hyper-V.
>   */
>  
> +static int __send_ipi_mask_ex(const struct cpumask *mask, int vector)
> +{
> +	int nr_bank = 0;
> +	struct ipi_arg_ex **arg;
> +	struct ipi_arg_ex *ipi_arg;
> +	int ret = 1;
> +	unsigned long flags;

This is really horrible to read.

	struct ipi_arg_ex *ipi_arg;
	struct ipi_arg_ex **arg;
	unsigned long flags;
	bool ret = false;
	int nr_bank = 0;

is really more conveniant for quick reading.

So the other more limited function has a lot more sanity checks vs. vector
number and other things. Why are they not required here? Comment please.

> +	local_irq_save(flags);
> +	arg = (struct ipi_arg_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
> +
> +	ipi_arg = *arg;
> +	if (unlikely(!ipi_arg))
> +		goto ipi_mask_ex_done;
> +
> +	ipi_arg->vector = vector;
> +	ipi_arg->reserved = 0;
> +	ipi_arg->vp_set.valid_bank_mask = 0;
> +
> +	if (!cpumask_equal(mask, cpu_present_mask)) {
> +		ipi_arg->vp_set.format = HV_GENERIC_SET_SPARCE_4K;
> +		nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask);

nr_bank really confused me. bank_nr is what you mean, not number of banks,
right?

> +	}
> +	if (!nr_bank)
> +		ipi_arg->vp_set.format = HV_GENERIC_SET_ALL;
> +
> +	ret = hv_do_rep_hypercall(HVCALL_SEND_IPI_EX, 0, nr_bank,
> +			      ipi_arg, NULL);
> +
> +ipi_mask_ex_done:
> +	local_irq_restore(flags);
> +	return ret;
> +}

Thanks,

	tglx
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment

[KY Srinivasan]

> -----Original Message-----
> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Thursday, April 26, 2018 3:17 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: x86@kernel.org; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; hpa@zytor.com; Stephen
> Hemminger <sthemmin@microsoft.com>; Michael Kelley (EOSG)
> <Michael.H.Kelley@microsoft.com>; vkuznets@redhat.com
> Subject: Re: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment
> 
> On Wed, 25 Apr 2018, kys@linuxonhyperv.com wrote:
> >
> > +struct ipi_arg_ex {
> > +	u32 vector;
> > +	u32  reserved;
> > +	struct hv_vpset vp_set;
> 
> Please align that in tabular fashion for easy of reading
> 
> 	u32		vector;
> 	u32  		reserved;
> 	struct hv_vpset	vp_set;
> 
> > +};
> > +
> >  static struct apic orig_apic;
> >
> >  static u64 hv_apic_icr_read(void)
> > @@ -97,6 +103,40 @@ static void hv_apic_eoi_write(u32 reg, u32 val)
> >   * IPI implementation on Hyper-V.
> >   */
> >
> > +static int __send_ipi_mask_ex(const struct cpumask *mask, int vector)
> > +{
> > +	int nr_bank = 0;
> > +	struct ipi_arg_ex **arg;
> > +	struct ipi_arg_ex *ipi_arg;
> > +	int ret = 1;
> > +	unsigned long flags;
> 
> This is really horrible to read.
> 
> 	struct ipi_arg_ex *ipi_arg;
> 	struct ipi_arg_ex **arg;
> 	unsigned long flags;
> 	bool ret = false;
> 	int nr_bank = 0;
> 
> is really more conveniant for quick reading.
> 
> So the other more limited function has a lot more sanity checks vs. vector
> number and other things. Why are they not required here? Comment
> please.

Yes, I will add the comments. This function is called from the other function
after all the sanity checks have been done and hence are not replicated here.
> 
> > +	local_irq_save(flags);
> > +	arg = (struct ipi_arg_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > +
> > +	ipi_arg = *arg;
> > +	if (unlikely(!ipi_arg))
> > +		goto ipi_mask_ex_done;
> > +
> > +	ipi_arg->vector = vector;
> > +	ipi_arg->reserved = 0;
> > +	ipi_arg->vp_set.valid_bank_mask = 0;
> > +
> > +	if (!cpumask_equal(mask, cpu_present_mask)) {
> > +		ipi_arg->vp_set.format = HV_GENERIC_SET_SPARCE_4K;
> > +		nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask);
> 
> nr_bank really confused me. bank_nr is what you mean, not number of
> banks,
> right?
It is the number of banks. The hypercall used here is a variable length
hypercall.

Regards,

K. Y  
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 3/5] X86: Hyper-V: Enhanced IPI enlightenment

[Michael Kelley (EOSG)]

On Wed, 25 Apr 2018, KY Srinivasan <kys@microsoft.com> wrote:
> 
> +struct ipi_arg_ex {
> +	u32 vector;
> +	u32  reserved;
> +	struct hv_vpset vp_set;
> +};

Again, suggest moving to hyperv-tlfs.h.  And the 5.0b version
of the TLFS has:

	u32		vector;
	u8		targetvtl;
	u8		reserved[3];
	struct hv_vpset	vp_set;

....
 
> 
> +struct hv_vpset {
> +	u64 format;
> +	u64 valid_bank_mask;
> +	u64 bank_contents[];
> +};

And this as well.

Michael
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 4/5] X86: Hyper-V: Consolidate code for converting cpumask to vpset

[kys]

From: "K. Y. Srinivasan" <kys@microsoft.com>

Consolidate code for converting cpumask to vpset.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 arch/x86/hyperv/hv_apic.c          |  2 +-
 arch/x86/hyperv/mmu.c              | 45 +++-----------------------------------
 arch/x86/include/asm/hyperv-tlfs.h |  2 +-
 3 files changed, 5 insertions(+), 44 deletions(-)

diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index fbb408b91e25..508889c8499e 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -123,7 +123,7 @@ static int __send_ipi_mask_ex(const struct cpumask *mask, int vector)
 	ipi_arg->vp_set.valid_bank_mask = 0;
 
 	if (!cpumask_equal(mask, cpu_present_mask)) {
-		ipi_arg->vp_set.format = HV_GENERIC_SET_SPARCE_4K;
+		ipi_arg->vp_set.format = HV_GENERIC_SET_SPARSE_4K;
 		nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask);
 	}
 	if (!nr_bank)
diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c
index 56c9ebac946f..c9cd28f0bae4 100644
--- a/arch/x86/hyperv/mmu.c
+++ b/arch/x86/hyperv/mmu.c
@@ -25,11 +25,7 @@ struct hv_flush_pcpu {
 struct hv_flush_pcpu_ex {
 	u64 address_space;
 	u64 flags;
-	struct {
-		u64 format;
-		u64 valid_bank_mask;
-		u64 bank_contents[];
-	} hv_vp_set;
+	struct hv_vpset hv_vp_set;
 	u64 gva_list[];
 };
 
@@ -70,41 +66,6 @@ static inline int fill_gva_list(u64 gva_list[], int offset,
 	return gva_n - offset;
 }
 
-/* Return the number of banks in the resulting vp_set */
-static inline int cpumask_to_vp_set(struct hv_flush_pcpu_ex *flush,
-				    const struct cpumask *cpus)
-{
-	int cpu, vcpu, vcpu_bank, vcpu_offset, nr_bank = 1;
-
-	/* valid_bank_mask can represent up to 64 banks */
-	if (hv_max_vp_index / 64 >= 64)
-		return 0;
-
-	/*
-	 * Clear all banks up to the maximum possible bank as hv_flush_pcpu_ex
-	 * structs are not cleared between calls, we risk flushing unneeded
-	 * vCPUs otherwise.
-	 */
-	for (vcpu_bank = 0; vcpu_bank <= hv_max_vp_index / 64; vcpu_bank++)
-		flush->hv_vp_set.bank_contents[vcpu_bank] = 0;
-
-	/*
-	 * Some banks may end up being empty but this is acceptable.
-	 */
-	for_each_cpu(cpu, cpus) {
-		vcpu = hv_cpu_number_to_vp_number(cpu);
-		vcpu_bank = vcpu / 64;
-		vcpu_offset = vcpu % 64;
-		__set_bit(vcpu_offset, (unsigned long *)
-			  &flush->hv_vp_set.bank_contents[vcpu_bank]);
-		if (vcpu_bank >= nr_bank)
-			nr_bank = vcpu_bank + 1;
-	}
-	flush->hv_vp_set.valid_bank_mask = GENMASK_ULL(nr_bank - 1, 0);
-
-	return nr_bank;
-}
-
 static void hyperv_flush_tlb_others(const struct cpumask *cpus,
 				    const struct flush_tlb_info *info)
 {
@@ -239,8 +200,8 @@ static void hyperv_flush_tlb_others_ex(const struct cpumask *cpus,
 	flush->hv_vp_set.valid_bank_mask = 0;
 
 	if (!cpumask_equal(cpus, cpu_present_mask)) {
-		flush->hv_vp_set.format = HV_GENERIC_SET_SPARCE_4K;
-		nr_bank = cpumask_to_vp_set(flush, cpus);
+		flush->hv_vp_set.format = HV_GENERIC_SET_SPARSE_4K;
+		nr_bank = cpumask_to_vpset(&(flush->hv_vp_set), cpus);
 	}
 
 	if (!nr_bank) {
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 53ea30d768d9..b32b87945538 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -370,7 +370,7 @@ struct hv_tsc_emulation_status {
 #define HV_FLUSH_USE_EXTENDED_RANGE_FORMAT	BIT(3)
 
 enum HV_GENERIC_SET_FORMAT {
-	HV_GENERIC_SET_SPARCE_4K,
+	HV_GENERIC_SET_SPARSE_4K,
 	HV_GENERIC_SET_ALL,
 };
 
-- 
2.15.1

Re: [PATCH 4/5] X86: Hyper-V: Consolidate code for converting cpumask to vpset

[Thomas Gleixner]

On Wed, 25 Apr 2018, kys@linuxonhyperv.com wrote:
>  
>  	if (!cpumask_equal(mask, cpu_present_mask)) {
> -		ipi_arg->vp_set.format = HV_GENERIC_SET_SPARCE_4K;
> +		ipi_arg->vp_set.format = HV_GENERIC_SET_SPARSE_4K;

Please move this patch before the others, so you can use SPARSE in the new
code right away.

Thanks,

	tglx
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 5/5] X86: Hyper-V: Consolidate the allocation of the hypercall input page

[kys]

From: "K. Y. Srinivasan" <kys@microsoft.com>

Consolidate the allocation of the hypercall input page.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 arch/x86/hyperv/hv_init.c       |  2 --
 arch/x86/hyperv/mmu.c           | 33 +++++++++------------------------
 arch/x86/include/asm/mshyperv.h |  1 -
 3 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index a895662b6b4c..72befd0e2c35 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -314,8 +314,6 @@ void __init hyperv_init(void)
 	hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
 	wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
 
-	hyper_alloc_mmu();
-
 	hv_apic_init();
 
 	/*
diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c
index c9cd28f0bae4..c4466af72e77 100644
--- a/arch/x86/hyperv/mmu.c
+++ b/arch/x86/hyperv/mmu.c
@@ -32,9 +32,6 @@ struct hv_flush_pcpu_ex {
 /* Each gva in gva_list encodes up to 4096 pages to flush */
 #define HV_TLB_FLUSH_UNIT (4096 * PAGE_SIZE)
 
-static struct hv_flush_pcpu __percpu **pcpu_flush;
-
-static struct hv_flush_pcpu_ex __percpu **pcpu_flush_ex;
 
 /*
  * Fills in gva_list starting from offset. Returns the number of items added.
@@ -77,7 +74,7 @@ static void hyperv_flush_tlb_others(const struct cpumask *cpus,
 
 	trace_hyperv_mmu_flush_tlb_others(cpus, info);
 
-	if (!pcpu_flush || !hv_hypercall_pg)
+	if (!hv_hypercall_pg)
 		goto do_native;
 
 	if (cpumask_empty(cpus))
@@ -85,10 +82,8 @@ static void hyperv_flush_tlb_others(const struct cpumask *cpus,
 
 	local_irq_save(flags);
 
-	flush_pcpu = this_cpu_ptr(pcpu_flush);
-
-	if (unlikely(!*flush_pcpu))
-		*flush_pcpu = page_address(alloc_page(GFP_ATOMIC));
+	flush_pcpu = (struct hv_flush_pcpu **)
+		     this_cpu_ptr(hyperv_pcpu_input_arg);
 
 	flush = *flush_pcpu;
 
@@ -164,7 +159,7 @@ static void hyperv_flush_tlb_others_ex(const struct cpumask *cpus,
 
 	trace_hyperv_mmu_flush_tlb_others(cpus, info);
 
-	if (!pcpu_flush_ex || !hv_hypercall_pg)
+	if (!hv_hypercall_pg)
 		goto do_native;
 
 	if (cpumask_empty(cpus))
@@ -172,10 +167,8 @@ static void hyperv_flush_tlb_others_ex(const struct cpumask *cpus,
 
 	local_irq_save(flags);
 
-	flush_pcpu = this_cpu_ptr(pcpu_flush_ex);
-
-	if (unlikely(!*flush_pcpu))
-		*flush_pcpu = page_address(alloc_page(GFP_ATOMIC));
+	flush_pcpu = (struct hv_flush_pcpu_ex **)
+		     this_cpu_ptr(hyperv_pcpu_input_arg);
 
 	flush = *flush_pcpu;
 
@@ -249,6 +242,9 @@ void hyperv_setup_mmu_ops(void)
 	if (!(ms_hyperv.hints & HV_X64_REMOTE_TLB_FLUSH_RECOMMENDED))
 		return;
 
+	if (hyperv_pcpu_input_arg == NULL)
+		return;
+
 	if (!(ms_hyperv.hints & HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED)) {
 		pr_info("Using hypercall for remote TLB flush\n");
 		pv_mmu_ops.flush_tlb_others = hyperv_flush_tlb_others;
@@ -257,14 +253,3 @@ void hyperv_setup_mmu_ops(void)
 		pv_mmu_ops.flush_tlb_others = hyperv_flush_tlb_others_ex;
 	}
 }
-
-void hyper_alloc_mmu(void)
-{
-	if (!(ms_hyperv.hints & HV_X64_REMOTE_TLB_FLUSH_RECOMMENDED))
-		return;
-
-	if (!(ms_hyperv.hints & HV_X64_EX_PROCESSOR_MASKS_RECOMMENDED))
-		pcpu_flush = alloc_percpu(struct hv_flush_pcpu *);
-	else
-		pcpu_flush_ex = alloc_percpu(struct hv_flush_pcpu_ex *);
-}
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 956e8603bed2..9931f9908fe2 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -300,7 +300,6 @@ static inline int cpumask_to_vpset(struct hv_vpset *vpset,
 
 void __init hyperv_init(void);
 void hyperv_setup_mmu_ops(void);
-void hyper_alloc_mmu(void);
 void hyperv_report_panic(struct pt_regs *regs, long err);
 bool hv_is_hyperv_initialized(void);
 void hyperv_cleanup(void);
-- 
2.15.1

Re: [PATCH 5/5] X86: Hyper-V: Consolidate the allocation of the hypercall input page

[Thomas Gleixner]

On Wed, 25 Apr 2018, kys@linuxonhyperv.com wrote:

> From: "K. Y. Srinivasan" <kys@microsoft.com>
> 
> Consolidate the allocation of the hypercall input page.

Again. You can provide the new way of allocation first, then you don't have
to add code first in order to remove it later again.

Thanks,

	tglx
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 5/5] X86: Hyper-V: Consolidate the allocation of the hypercall input page

[KY Srinivasan]

> -----Original Message-----
> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Thursday, April 26, 2018 3:24 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: x86@kernel.org; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; hpa@zytor.com; Stephen
> Hemminger <sthemmin@microsoft.com>; Michael Kelley (EOSG)
> <Michael.H.Kelley@microsoft.com>; vkuznets@redhat.com
> Subject: Re: [PATCH 5/5] X86: Hyper-V: Consolidate the allocation of the
> hypercall input page
> 
> On Wed, 25 Apr 2018, kys@linuxonhyperv.com wrote:
> 
> > From: "K. Y. Srinivasan" <kys@microsoft.com>
> >
> > Consolidate the allocation of the hypercall input page.
> 
> Again. You can provide the new way of allocation first, then you don't have
> to add code first in order to remove it later again. 

I have implemented the new way upfront for the new code - the IPI code
[PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments.
What I am doing here using that infrastructure for the TLB flush enlightenments
and getting rid of unnecessary code.

Regards,

K. Y
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 5/5] X86: Hyper-V: Consolidate the allocation of the hypercall input page

[Thomas Gleixner]

On Fri, 27 Apr 2018, KY Srinivasan wrote:

> 
> 
> > -----Original Message-----
> > From: Thomas Gleixner <tglx@linutronix.de>
> > Sent: Thursday, April 26, 2018 3:24 PM
> > To: KY Srinivasan <kys@microsoft.com>
> > Cc: x86@kernel.org; gregkh@linuxfoundation.org; linux-
> > kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> > apw@canonical.com; jasowang@redhat.com; hpa@zytor.com; Stephen
> > Hemminger <sthemmin@microsoft.com>; Michael Kelley (EOSG)
> > <Michael.H.Kelley@microsoft.com>; vkuznets@redhat.com
> > Subject: Re: [PATCH 5/5] X86: Hyper-V: Consolidate the allocation of the
> > hypercall input page
> > 
> > On Wed, 25 Apr 2018, kys@linuxonhyperv.com wrote:
> > 
> > > From: "K. Y. Srinivasan" <kys@microsoft.com>
> > >
> > > Consolidate the allocation of the hypercall input page.
> > 
> > Again. You can provide the new way of allocation first, then you don't have
> > to add code first in order to remove it later again. 
> 
> I have implemented the new way upfront for the new code - the IPI code
> [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments.
> What I am doing here using that infrastructure for the TLB flush enlightenments
> and getting rid of unnecessary code.

Ok. I maybe misread it, but a bit more elaborate change log might help to
avoid that.

Thanks,

	tglx

Re: [PATCH 5/5] X86: Hyper-V: Consolidate the allocation of the hypercall input page

[kbuild test robot]

Hi Srinivasan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on v4.17-rc2]
[also build test WARNING on next-20180426]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/kys-linuxonhyperv-com/X86-Hyper-V-APIC-enlightenments/20180427-114416
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> arch/x86/hyperv/mmu.c:86:22: sparse: incorrect type in initializer (different address spaces) @@    expected void const [noderef] <asn:3>*__vpp_verify @@    got const [noderef] <asn:3>*__vpp_verify @@
   arch/x86/hyperv/mmu.c:86:22:    expected void const [noderef] <asn:3>*__vpp_verify
   arch/x86/hyperv/mmu.c:86:22:    got void [noderef] <asn:3>**<noident>
   arch/x86/hyperv/mmu.c:171:22: sparse: incorrect type in initializer (different address spaces) @@    expected void const [noderef] <asn:3>*__vpp_verify @@    got const [noderef] <asn:3>*__vpp_verify @@
   arch/x86/hyperv/mmu.c:171:22:    expected void const [noderef] <asn:3>*__vpp_verify
   arch/x86/hyperv/mmu.c:171:22:    got void [noderef] <asn:3>**<noident>

vim +86 arch/x86/hyperv/mmu.c

    65	
    66	static void hyperv_flush_tlb_others(const struct cpumask *cpus,
    67					    const struct flush_tlb_info *info)
    68	{
    69		int cpu, vcpu, gva_n, max_gvas;
    70		struct hv_flush_pcpu **flush_pcpu;
    71		struct hv_flush_pcpu *flush;
    72		u64 status = U64_MAX;
    73		unsigned long flags;
    74	
    75		trace_hyperv_mmu_flush_tlb_others(cpus, info);
    76	
    77		if (!hv_hypercall_pg)
    78			goto do_native;
    79	
    80		if (cpumask_empty(cpus))
    81			return;
    82	
    83		local_irq_save(flags);
    84	
    85		flush_pcpu = (struct hv_flush_pcpu **)
  > 86			     this_cpu_ptr(hyperv_pcpu_input_arg);
    87	
    88		flush = *flush_pcpu;
    89	
    90		if (unlikely(!flush)) {
    91			local_irq_restore(flags);
    92			goto do_native;
    93		}
    94	
    95		if (info->mm) {
    96			/*
    97			 * AddressSpace argument must match the CR3 with PCID bits
    98			 * stripped out.
    99			 */
   100			flush->address_space = virt_to_phys(info->mm->pgd);
   101			flush->address_space &= CR3_ADDR_MASK;
   102			flush->flags = 0;
   103		} else {
   104			flush->address_space = 0;
   105			flush->flags = HV_FLUSH_ALL_VIRTUAL_ADDRESS_SPACES;
   106		}
   107	
   108		flush->processor_mask = 0;
   109		if (cpumask_equal(cpus, cpu_present_mask)) {
   110			flush->flags |= HV_FLUSH_ALL_PROCESSORS;
   111		} else {
   112			for_each_cpu(cpu, cpus) {
   113				vcpu = hv_cpu_number_to_vp_number(cpu);
   114				if (vcpu >= 64)
   115					goto do_native;
   116	
   117				__set_bit(vcpu, (unsigned long *)
   118					  &flush->processor_mask);
   119			}
   120		}
   121	
   122		/*
   123		 * We can flush not more than max_gvas with one hypercall. Flush the
   124		 * whole address space if we were asked to do more.
   125		 */
   126		max_gvas = (PAGE_SIZE - sizeof(*flush)) / sizeof(flush->gva_list[0]);
   127	
   128		if (info->end == TLB_FLUSH_ALL) {
   129			flush->flags |= HV_FLUSH_NON_GLOBAL_MAPPINGS_ONLY;
   130			status = hv_do_hypercall(HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE,
   131						 flush, NULL);
   132		} else if (info->end &&
   133			   ((info->end - info->start)/HV_TLB_FLUSH_UNIT) > max_gvas) {
   134			status = hv_do_hypercall(HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE,
   135						 flush, NULL);
   136		} else {
   137			gva_n = fill_gva_list(flush->gva_list, 0,
   138					      info->start, info->end);
   139			status = hv_do_rep_hypercall(HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST,
   140						     gva_n, 0, flush, NULL);
   141		}
   142	
   143		local_irq_restore(flags);
   144	
   145		if (!(status & HV_HYPERCALL_RESULT_MASK))
   146			return;
   147	do_native:
   148		native_flush_tlb_others(cpus, info);
   149	}
   150	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH 1/5] X86: Hyper-V: Enlighten APIC access

[Thomas Gleixner]

On Wed, 25 Apr 2018, kys@linuxonhyperv.com wrote:
> --- /dev/null
> +++ b/arch/x86/hyperv/hv_apic.c
> @@ -0,0 +1,98 @@
> +// SPDX-License-Identifier: GPL-2.0

Thanks for putting the license identifier in.

> +
> +/*
> + * Hyper-V specific APIC code.
> + *
> + * Copyright (C) 2018, Microsoft, Inc.
> + *
> + * Author : K. Y. Srinivasan <kys@microsoft.com>

But can you please check with your lawyers whether you can avoid the
pointless boilerplate? The SPDX identifier should cover it.

> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT.  See the GNU General Public License for more
> + * details.
> + *
> + */
> +
> +#include <linux/types.h>
> +#include <asm/hypervisor.h>
> +#include <asm/mshyperv.h>
> +#include <linux/version.h>
> +#include <linux/vmalloc.h>
> +#include <linux/mm.h>
> +#include <linux/clockchips.h>
> +#include <linux/hyperv.h>
> +#include <linux/slab.h>
> +#include <linux/cpuhotplug.h>

We usually order the includes

#include <linux/....>
...
#include <linux/....>

#include <asm/....>
#include <asm/....>


> -void hyperv_init(void);
> +void __init hyperv_init(void);
>  void hyperv_setup_mmu_ops(void);
>  void hyper_alloc_mmu(void);
>  void hyperv_report_panic(struct pt_regs *regs, long err);
> @@ -269,14 +269,16 @@ void hyperv_reenlightenment_intr(struct pt_regs *regs);
>  void set_hv_tscchange_cb(void (*cb)(void));
>  void clear_hv_tscchange_cb(void);
>  void hyperv_stop_tsc_emulation(void);
> +void hv_apic_init(void);
>  #else /* CONFIG_HYPERV */
> -static inline void hyperv_init(void) {}
> +static __init  inline void hyperv_init(void) {}

The __init on the empty inline function is pointless.

Other than the few nits. This looks well done!

Thanks,

	tglx
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 1/5] X86: Hyper-V: Enlighten APIC access

[KY Srinivasan]

> -----Original Message-----
> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Thursday, April 26, 2018 2:49 PM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: x86@kernel.org; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; hpa@zytor.com; Stephen
> Hemminger <sthemmin@microsoft.com>; Michael Kelley (EOSG)
> <Michael.H.Kelley@microsoft.com>; vkuznets@redhat.com
> Subject: Re: [PATCH 1/5] X86: Hyper-V: Enlighten APIC access
> 
> On Wed, 25 Apr 2018, kys@linuxonhyperv.com wrote:
> > --- /dev/null
> > +++ b/arch/x86/hyperv/hv_apic.c
> > @@ -0,0 +1,98 @@
> > +// SPDX-License-Identifier: GPL-2.0
> 
> Thanks for putting the license identifier in.
> 
> > +
> > +/*
> > + * Hyper-V specific APIC code.
> > + *
> > + * Copyright (C) 2018, Microsoft, Inc.
> > + *
> > + * Author : K. Y. Srinivasan <kys@microsoft.com>
> 
> But can you please check with your lawyers whether you can avoid the
> pointless boilerplate? The SPDX identifier should cover it.

I will consult with MSFT legal on this.
> 
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License version 2 as
> published
> > + * by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD
> TITLE or
> > + * NON INFRINGEMENT.  See the GNU General Public License for more
> > + * details.
> > + *
> > + */
> > +
> > +#include <linux/types.h>
> > +#include <asm/hypervisor.h>
> > +#include <asm/mshyperv.h>
> > +#include <linux/version.h>
> > +#include <linux/vmalloc.h>
> > +#include <linux/mm.h>
> > +#include <linux/clockchips.h>
> > +#include <linux/hyperv.h>
> > +#include <linux/slab.h>
> > +#include <linux/cpuhotplug.h>
> 
> We usually order the includes
> 
> #include <linux/....>
> ...
> #include <linux/....>
> 
> #include <asm/....>
> #include <asm/....>
> 
> 
> > -void hyperv_init(void);
> > +void __init hyperv_init(void);
> >  void hyperv_setup_mmu_ops(void);
> >  void hyper_alloc_mmu(void);
> >  void hyperv_report_panic(struct pt_regs *regs, long err);
> > @@ -269,14 +269,16 @@ void hyperv_reenlightenment_intr(struct pt_regs
> *regs);
> >  void set_hv_tscchange_cb(void (*cb)(void));
> >  void clear_hv_tscchange_cb(void);
> >  void hyperv_stop_tsc_emulation(void);
> > +void hv_apic_init(void);
> >  #else /* CONFIG_HYPERV */
> > -static inline void hyperv_init(void) {}
> > +static __init  inline void hyperv_init(void) {}
> 
> The __init on the empty inline function is pointless.
> 
> Other than the few nits. This looks well done!

Thanks Thomas. I will address all the issues you have brought up in
the next version.

Regards,

K. Y
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 1/5] X86: Hyper-V: Enlighten APIC access

[Michael Kelley (EOSG)]

> -----Original Message-----
> From: kys@linuxonhyperv.com <kys@linuxonhyperv.com>
> Sent: Wednesday, April 25, 2018 11:13 AM
> To: x86@kernel.org; gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; jasowang@redhat.com;
> tglx@linutronix.de; hpa@zytor.com; Stephen Hemminger <sthemmin@microsoft.com>;
> Michael Kelley (EOSG) <Michael.H.Kelley@microsoft.com>; vkuznets@redhat.com
> Cc: KY Srinivasan <kys@microsoft.com>
> Subject: [PATCH 1/5] X86: Hyper-V: Enlighten APIC access
> 
> From: "K. Y. Srinivasan" <kys@microsoft.com>
> 
> Hyper-V supports MSR based APIC access; implement
> the enlightenment.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  arch/x86/hyperv/Makefile        |  2 +-
>  arch/x86/hyperv/hv_apic.c       | 98 +++++++++++++++++++++++++++++++++++++++++
>  arch/x86/hyperv/hv_init.c       |  5 ++-
>  arch/x86/include/asm/mshyperv.h |  6 ++-
>  4 files changed, 107 insertions(+), 4 deletions(-)
>  create mode 100644 arch/x86/hyperv/hv_apic.c
> 
> diff --git a/arch/x86/hyperv/Makefile b/arch/x86/hyperv/Makefile
> index 367a8203cfcf..00ce4df01a09 100644
> --- a/arch/x86/hyperv/Makefile
> +++ b/arch/x86/hyperv/Makefile
> @@ -1 +1 @@
> -obj-y		:= hv_init.o mmu.o
> +obj-y		:= hv_init.o mmu.o hv_apic.o
> diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> new file mode 100644
> index 000000000000..e0a5b36208fc
> --- /dev/null
> +++ b/arch/x86/hyperv/hv_apic.c
> @@ -0,0 +1,98 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Hyper-V specific APIC code.
> + *
> + * Copyright (C) 2018, Microsoft, Inc.
> + *
> + * Author : K. Y. Srinivasan <kys@microsoft.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT.  See the GNU General Public License for more
> + * details.
> + *
> + */
> +
> +#include <linux/types.h>
> +#include <asm/hypervisor.h>
> +#include <asm/mshyperv.h>
> +#include <linux/version.h>
> +#include <linux/vmalloc.h>
> +#include <linux/mm.h>
> +#include <linux/clockchips.h>
> +#include <linux/hyperv.h>
> +#include <linux/slab.h>
> +#include <linux/cpuhotplug.h>
> +
> +static u64 hv_apic_icr_read(void)
> +{
> +	u64 reg_val;
> +
> +	rdmsrl(HV_X64_MSR_ICR, reg_val);
> +	return reg_val;
> +}
> +
> +static void hv_apic_icr_write(u32 low, u32 id)
> +{
> +	u64 reg_val;
> +
> +	reg_val = SET_APIC_DEST_FIELD(id);
> +	reg_val = reg_val << 32;
> +	reg_val |= low;
> +
> +	wrmsrl(HV_X64_MSR_ICR, reg_val);
> +}
> +
> +static u32 hv_apic_read(u32 reg)
> +{
> +	u32 reg_val, hi;
> +
> +	switch (reg) {
> +	case APIC_EOI:
> +		rdmsr(HV_X64_MSR_EOI, reg_val, hi);
> +		return reg_val;
> +	case APIC_TASKPRI:
> +		rdmsr(HV_X64_MSR_TPR, reg_val, hi);
> +		return reg_val;
> +
> +	default:
> +		return native_apic_mem_read(reg);
> +	}
> +}
> +
> +static void hv_apic_write(u32 reg, u32 val)
> +{
> +	switch (reg) {
> +	case APIC_EOI:
> +		wrmsr(HV_X64_MSR_EOI, val, 0);
> +		break;
> +	case APIC_TASKPRI:
> +		wrmsr(HV_X64_MSR_TPR, val, 0);
> +		break;
> +	default:
> +		native_apic_mem_write(reg, val);
> +	}
> +}
> +
> +static void hv_apic_eoi_write(u32 reg, u32 val)
> +{
> +	wrmsr(HV_X64_MSR_EOI, val, 0);
> +}
> +
> +void __init hv_apic_init(void)
> +{
> +	if (ms_hyperv.hints & HV_X64_APIC_ACCESS_RECOMMENDED) {
> +		pr_info("Hyper-V: Using MSR ased APIC access\n");

Typo here.  "ased" should be "based".

> +		apic_set_eoi_write(hv_apic_eoi_write);
> +		apic->read      = hv_apic_read;
> +		apic->write     = hv_apic_write;
> +		apic->icr_write = hv_apic_icr_write;
> +		apic->icr_read  = hv_apic_icr_read;
> +	}
> +}
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index cfecc2272f2d..71e50fc2b7ef 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -242,8 +242,9 @@ static int hv_cpu_die(unsigned int cpu)
>   *
>   * 1. Setup the hypercall page.
>   * 2. Register Hyper-V specific clocksource.
> + * 3. Setup Hyper-V specific APIC entry points.
>   */
> -void hyperv_init(void)
> +void __init hyperv_init(void)
>  {
>  	u64 guest_id, required_msrs;
>  	union hv_x64_msr_hypercall_contents hypercall_msr;
> @@ -298,6 +299,8 @@ void hyperv_init(void)
> 
>  	hyper_alloc_mmu();
> 
> +	hv_apic_init();
> +
>  	/*
>  	 * Register Hyper-V specific clocksource.
>  	 */
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index b90e79610cf7..bcced50037c1 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -258,7 +258,7 @@ static inline int hv_cpu_number_to_vp_number(int cpu_number)
>  	return hv_vp_index[cpu_number];
>  }
> 
> -void hyperv_init(void);
> +void __init hyperv_init(void);
>  void hyperv_setup_mmu_ops(void);
>  void hyper_alloc_mmu(void);
>  void hyperv_report_panic(struct pt_regs *regs, long err);
> @@ -269,14 +269,16 @@ void hyperv_reenlightenment_intr(struct pt_regs *regs);
>  void set_hv_tscchange_cb(void (*cb)(void));
>  void clear_hv_tscchange_cb(void);
>  void hyperv_stop_tsc_emulation(void);
> +void hv_apic_init(void);
>  #else /* CONFIG_HYPERV */
> -static inline void hyperv_init(void) {}
> +static __init  inline void hyperv_init(void) {}
>  static inline bool hv_is_hyperv_initialized(void) { return false; }
>  static inline void hyperv_cleanup(void) {}
>  static inline void hyperv_setup_mmu_ops(void) {}
>  static inline void set_hv_tscchange_cb(void (*cb)(void)) {}
>  static inline void clear_hv_tscchange_cb(void) {}
>  static inline void hyperv_stop_tsc_emulation(void) {};
> +static inline void hv_apic_init(void) {}
>  static inline struct hv_vp_assist_page *hv_get_vp_assist_page(unsigned int cpu)
>  {
>  	return NULL;
> --
> 2.15.1

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 1/5] X86: Hyper-V: Enlighten APIC access

[Thomas Gleixner]

Michael,

On Thu, 26 Apr 2018, Michael Kelley (EOSG) wrote:

> > -----Original Message-----
> > From: kys@linuxonhyperv.com <kys@linuxonhyperv.com>
> > Sent: Wednesday, April 25, 2018 11:13 AM
> > To: x86@kernel.org; gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; jasowang@redhat.com;
> > tglx@linutronix.de; hpa@zytor.com; Stephen Hemminger <sthemmin@microsoft.com>;
> > Michael Kelley (EOSG) <Michael.H.Kelley@microsoft.com>; vkuznets@redhat.com
> > Cc: KY Srinivasan <kys@microsoft.com>
> > Subject: [PATCH 1/5] X86: Hyper-V: Enlighten APIC access

Please fix your MUA not to pointlessly copy the whole mail header.

> > +void __init hv_apic_init(void)
> > +{
> > +	if (ms_hyperv.hints & HV_X64_APIC_ACCESS_RECOMMENDED) {
> > +		pr_info("Hyper-V: Using MSR ased APIC access\n");
> 
> Typo here.  "ased" should be "based".

And please trim the reply to the relevant point. It's annoying to find that
single line of review comment in the useless pile of quoted patch.

Thanks,

	tglx

Re: [PATCH 1/5] X86: Hyper-V: Enlighten APIC access

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2987 bytes --]

Hi Srinivasan,

I love your patch! Yet something to improve:

[auto build test ERROR on v4.17-rc2]
[also build test ERROR on next-20180426]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/kys-linuxonhyperv-com/X86-Hyper-V-APIC-enlightenments/20180427-114416
config: i386-randconfig-b0-04270034 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86/hyperv/hv_apic.c: In function 'hv_apic_read':
>> arch/x86/hyperv/hv_apic.c:65:3: error: implicit declaration of function 'native_apic_mem_read' [-Werror=implicit-function-declaration]
      return native_apic_mem_read(reg);
      ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_apic_write':
>> arch/x86/hyperv/hv_apic.c:79:3: error: implicit declaration of function 'native_apic_mem_write' [-Werror=implicit-function-declaration]
      native_apic_mem_write(reg, val);
      ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_apic_init':
>> arch/x86/hyperv/hv_apic.c:92:3: error: implicit declaration of function 'apic_set_eoi_write' [-Werror=implicit-function-declaration]
      apic_set_eoi_write(hv_apic_eoi_write);
      ^
>> arch/x86/hyperv/hv_apic.c:93:3: error: 'apic' undeclared (first use in this function)
      apic->read      = hv_apic_read;
      ^
   arch/x86/hyperv/hv_apic.c:93:3: note: each undeclared identifier is reported only once for each function it appears in
   cc1: some warnings being treated as errors

vim +/native_apic_mem_read +65 arch/x86/hyperv/hv_apic.c

    51	
    52	static u32 hv_apic_read(u32 reg)
    53	{
    54		u32 reg_val, hi;
    55	
    56		switch (reg) {
    57		case APIC_EOI:
    58			rdmsr(HV_X64_MSR_EOI, reg_val, hi);
    59			return reg_val;
    60		case APIC_TASKPRI:
    61			rdmsr(HV_X64_MSR_TPR, reg_val, hi);
    62			return reg_val;
    63	
    64		default:
  > 65			return native_apic_mem_read(reg);
    66		}
    67	}
    68	
    69	static void hv_apic_write(u32 reg, u32 val)
    70	{
    71		switch (reg) {
    72		case APIC_EOI:
    73			wrmsr(HV_X64_MSR_EOI, val, 0);
    74			break;
    75		case APIC_TASKPRI:
    76			wrmsr(HV_X64_MSR_TPR, val, 0);
    77			break;
    78		default:
  > 79			native_apic_mem_write(reg, val);
    80		}
    81	}
    82	
    83	static void hv_apic_eoi_write(u32 reg, u32 val)
    84	{
    85		wrmsr(HV_X64_MSR_EOI, val, 0);
    86	}
    87	
    88	void __init hv_apic_init(void)
    89	{
    90		if (ms_hyperv.hints & HV_X64_APIC_ACCESS_RECOMMENDED) {
    91			pr_info("Hyper-V: Using MSR ased APIC access\n");
  > 92			apic_set_eoi_write(hv_apic_eoi_write);
  > 93			apic->read      = hv_apic_read;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25200 bytes --]

[-- Attachment #3: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel