linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC v1 01/18] asm-generic/hyperv: change HV_CPU_POWER_MANAGEMENT to HV_CPU_MANAGEMENT
       [not found] <20200914112802.80611-1-wei.liu@kernel.org>
@ 2020-09-14 11:27 ` Wei Liu
  2020-09-14 11:27 ` [PATCH RFC v1 07/18] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary Wei Liu
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Wei Liu @ 2020-09-14 11:27 UTC (permalink / raw)
  To: Linux on Hyper-V List
  Cc: virtualization, Linux Kernel List, Michael Kelley,
	Vineeth Pillai, Sunil Muthuswamy, Nuno Das Neves, Wei Liu,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Arnd Bergmann, open list:GENERIC INCLUDE/ASM HEADER FILES

This makes the name match Hyper-V TLFS.

Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
 include/asm-generic/hyperv-tlfs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
index e73a11850055..e6903589a82a 100644
--- a/include/asm-generic/hyperv-tlfs.h
+++ b/include/asm-generic/hyperv-tlfs.h
@@ -88,7 +88,7 @@
 #define HV_CONNECT_PORT				BIT(7)
 #define HV_ACCESS_STATS				BIT(8)
 #define HV_DEBUGGING				BIT(11)
-#define HV_CPU_POWER_MANAGEMENT			BIT(12)
+#define HV_CPU_MANAGEMENT			BIT(12)
 
 
 /*
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH RFC v1 07/18] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary
       [not found] <20200914112802.80611-1-wei.liu@kernel.org>
  2020-09-14 11:27 ` [PATCH RFC v1 01/18] asm-generic/hyperv: change HV_CPU_POWER_MANAGEMENT to HV_CPU_MANAGEMENT Wei Liu
@ 2020-09-14 11:27 ` Wei Liu
  2020-09-15 10:27   ` Vitaly Kuznetsov
  2020-09-14 11:59 ` [PATCH RFC v1 09/18] x86/hyperv: provide a bunch of helper functions Wei Liu
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Wei Liu @ 2020-09-14 11:27 UTC (permalink / raw)
  To: Linux on Hyper-V List
  Cc: virtualization, Linux Kernel List, Michael Kelley,
	Vineeth Pillai, Sunil Muthuswamy, Nuno Das Neves, Wei Liu,
	Lillian Grassin-Drake, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, Arnd Bergmann,
	open list:GENERIC INCLUDE/ASM HEADER FILES

We will need the partition ID for executing some hypercalls later.

Signed-off-by: Lillian Grassin-Drake <ligrassi@microsoft.com>
Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
 arch/x86/hyperv/hv_init.c         | 26 ++++++++++++++++++++++++++
 arch/x86/include/asm/mshyperv.h   |  2 ++
 include/asm-generic/hyperv-tlfs.h |  6 ++++++
 3 files changed, 34 insertions(+)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index ebba4be4185d..0eec1ed32023 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -30,6 +30,9 @@
 bool hv_root_partition;
 EXPORT_SYMBOL_GPL(hv_root_partition);
 
+u64 hv_current_partition_id;
+EXPORT_SYMBOL_GPL(hv_current_partition_id);
+
 void *hv_hypercall_pg;
 EXPORT_SYMBOL_GPL(hv_hypercall_pg);
 
@@ -345,6 +348,26 @@ static struct syscore_ops hv_syscore_ops = {
 	.resume		= hv_resume,
 };
 
+void __init hv_get_partition_id(void)
+{
+	struct hv_get_partition_id *output_page;
+	int status;
+	unsigned long flags;
+
+	local_irq_save(flags);
+	output_page = *this_cpu_ptr(hyperv_pcpu_output_arg);
+	status = hv_do_hypercall(HVCALL_GET_PARTITION_ID, NULL, output_page) &
+		HV_HYPERCALL_RESULT_MASK;
+	if (status != HV_STATUS_SUCCESS)
+		pr_err("Failed to get partition ID: %d\n", status);
+	else
+		hv_current_partition_id = output_page->partition_id;
+	local_irq_restore(flags);
+
+	/* No point in proceeding if this failed */
+	BUG_ON(status != HV_STATUS_SUCCESS);
+}
+
 /*
  * This function is to be invoked early in the boot sequence after the
  * hypervisor has been detected.
@@ -440,6 +463,9 @@ void __init hyperv_init(void)
 
 	register_syscore_ops(&hv_syscore_ops);
 
+	if (hv_root_partition)
+		hv_get_partition_id();
+
 	return;
 
 remove_cpuhp_state:
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index f5c62140f28d..4039302e0ae9 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -65,6 +65,8 @@ extern void *hv_hypercall_pg;
 extern void  __percpu  **hyperv_pcpu_input_arg;
 extern void  __percpu  **hyperv_pcpu_output_arg;
 
+extern u64 hv_current_partition_id;
+
 static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
 {
 	u64 input_address = input ? virt_to_phys(input) : 0;
diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
index e6903589a82a..87b1a79b19eb 100644
--- a/include/asm-generic/hyperv-tlfs.h
+++ b/include/asm-generic/hyperv-tlfs.h
@@ -141,6 +141,7 @@ struct ms_hyperv_tsc_page {
 #define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX	0x0013
 #define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX	0x0014
 #define HVCALL_SEND_IPI_EX			0x0015
+#define HVCALL_GET_PARTITION_ID			0x0046
 #define HVCALL_GET_VP_REGISTERS			0x0050
 #define HVCALL_SET_VP_REGISTERS			0x0051
 #define HVCALL_POST_MESSAGE			0x005c
@@ -407,6 +408,11 @@ struct hv_tlb_flush_ex {
 	u64 gva_list[];
 } __packed;
 
+/* HvGetPartitionId hypercall (output only) */
+struct hv_get_partition_id {
+	u64 partition_id;
+} __packed;
+
 /* HvRetargetDeviceInterrupt hypercall */
 union hv_msi_entry {
 	u64 as_uint64;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH RFC v1 09/18] x86/hyperv: provide a bunch of helper functions
       [not found] <20200914112802.80611-1-wei.liu@kernel.org>
  2020-09-14 11:27 ` [PATCH RFC v1 01/18] asm-generic/hyperv: change HV_CPU_POWER_MANAGEMENT to HV_CPU_MANAGEMENT Wei Liu
  2020-09-14 11:27 ` [PATCH RFC v1 07/18] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary Wei Liu
@ 2020-09-14 11:59 ` Wei Liu
  2020-09-15 11:00   ` Vitaly Kuznetsov
  2020-09-14 11:59 ` [PATCH RFC v1 11/18] asm-generic/hyperv: update hv_msi_entry Wei Liu
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Wei Liu @ 2020-09-14 11:59 UTC (permalink / raw)
  To: Linux on Hyper-V List
  Cc: virtualization, Linux Kernel List, Michael Kelley,
	Vineeth Pillai, Sunil Muthuswamy, Nuno Das Neves, Wei Liu,
	Lillian Grassin-Drake, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, Arnd Bergmann,
	open list:GENERIC INCLUDE/ASM HEADER FILES

They are used to deposit pages into Microsoft Hypervisor and bring up
logical and virtual processors.

Signed-off-by: Lillian Grassin-Drake <ligrassi@microsoft.com>
Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Signed-off-by: Nuno Das Neves <nudasnev@microsoft.com>
Co-Developed-by: Lillian Grassin-Drake <ligrassi@microsoft.com>
Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Co-Developed-by: Nuno Das Neves <nudasnev@microsoft.com>
Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
 arch/x86/hyperv/Makefile          |   2 +-
 arch/x86/hyperv/hv_proc.c         | 209 ++++++++++++++++++++++++++++++
 arch/x86/include/asm/mshyperv.h   |   4 +
 include/asm-generic/hyperv-tlfs.h |  56 ++++++++
 4 files changed, 270 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/hyperv/hv_proc.c

diff --git a/arch/x86/hyperv/Makefile b/arch/x86/hyperv/Makefile
index 89b1f74d3225..565358020921 100644
--- a/arch/x86/hyperv/Makefile
+++ b/arch/x86/hyperv/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-y			:= hv_init.o mmu.o nested.o
-obj-$(CONFIG_X86_64)	+= hv_apic.o
+obj-$(CONFIG_X86_64)	+= hv_apic.o hv_proc.o
 
 ifdef CONFIG_X86_64
 obj-$(CONFIG_PARAVIRT_SPINLOCKS)	+= hv_spinlock.o
diff --git a/arch/x86/hyperv/hv_proc.c b/arch/x86/hyperv/hv_proc.c
new file mode 100644
index 000000000000..847c72465d0e
--- /dev/null
+++ b/arch/x86/hyperv/hv_proc.c
@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/types.h>
+#include <linux/version.h>
+#include <linux/vmalloc.h>
+#include <linux/mm.h>
+#include <linux/clockchips.h>
+#include <linux/acpi.h>
+#include <linux/hyperv.h>
+#include <linux/slab.h>
+#include <linux/cpuhotplug.h>
+#include <asm/hypervisor.h>
+#include <asm/mshyperv.h>
+#include <asm/apic.h>
+
+#include <asm/trace/hyperv.h>
+
+#define HV_DEPOSIT_MAX_ORDER (8)
+#define HV_DEPOSIT_MAX (1 << HV_DEPOSIT_MAX_ORDER)
+
+#define MAX(a, b) ((a) > (b) ? (a) : (b))
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+
+/*
+ * Deposits exact number of pages
+ * Must be called with interrupts enabled
+ * Max 256 pages
+ */
+int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
+{
+	struct page **pages;
+	int *counts;
+	int num_allocations;
+	int i, j, page_count;
+	int order;
+	int desired_order;
+	int status;
+	int ret;
+	u64 base_pfn;
+	struct hv_deposit_memory *input_page;
+	unsigned long flags;
+
+	if (num_pages > HV_DEPOSIT_MAX)
+		return -EINVAL;
+	if (!num_pages)
+		return 0;
+
+	ret = -ENOMEM;
+
+	/* One buffer for page pointers and counts */
+	pages = page_address(alloc_page(GFP_KERNEL));
+	if (!pages)
+		goto free_buf;
+	counts = (int *)&pages[256];
+
+	/* Allocate all the pages before disabling interrupts */
+	num_allocations = 0;
+	i = 0;
+	order = HV_DEPOSIT_MAX_ORDER;
+
+	while (num_pages) {
+		/* Find highest order we can actually allocate */
+		desired_order = 31 - __builtin_clz(num_pages);
+		order = MIN(desired_order, order);
+		do {
+			pages[i] = alloc_pages_node(node, GFP_KERNEL, order);
+			if (!pages[i]) {
+				if (!order) {
+					goto err_free_allocations;
+				}
+				--order;
+			}
+		} while (!pages[i]);
+
+		split_page(pages[i], order);
+		counts[i] = 1 << order;
+		num_pages -= counts[i];
+		i++;
+		num_allocations++;
+	}
+
+	local_irq_save(flags);
+
+	input_page = *this_cpu_ptr(hyperv_pcpu_input_arg);
+
+	input_page->partition_id = partition_id;
+
+	/* Populate gpa_page_list - these will fit on the input page */
+	for (i = 0, page_count = 0; i < num_allocations; ++i) {
+		base_pfn = page_to_pfn(pages[i]);
+		for (j = 0; j < counts[i]; ++j, ++page_count)
+			input_page->gpa_page_list[page_count] = base_pfn + j;
+	}
+	status = hv_do_rep_hypercall(HVCALL_DEPOSIT_MEMORY,
+				     page_count, 0, input_page,
+				     NULL) & HV_HYPERCALL_RESULT_MASK;
+	local_irq_restore(flags);
+
+	if (status != HV_STATUS_SUCCESS) {
+		pr_err("Failed to deposit pages: %d\n", status);
+		ret = status;
+		goto err_free_allocations;
+	}
+
+	ret = 0;
+	goto free_buf;
+
+err_free_allocations:
+	for (i = 0; i < num_allocations; ++i) {
+		base_pfn = page_to_pfn(pages[i]);
+		for (j = 0; j < counts[i]; ++j)
+			__free_page(pfn_to_page(base_pfn + j));
+	}
+
+free_buf:
+	free_page((unsigned long)pages);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(hv_call_deposit_pages);
+
+int hv_call_add_logical_proc(int node, u32 lp_index, u32 apic_id)
+{
+	struct hv_add_logical_processor_in *input;
+	struct hv_add_logical_processor_out *output;
+	int status;
+	unsigned long flags;
+	int ret = 0;
+
+	do {
+		local_irq_save(flags);
+
+		input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+		/* We don't do anything with the output right now */
+		output = *this_cpu_ptr(hyperv_pcpu_output_arg);
+
+		input->lp_index = lp_index;
+		input->apic_id = apic_id;
+		input->flags = 0;
+		input->proximity_domain_info.domain_id = node_to_pxm(node);
+		input->proximity_domain_info.flags.reserved = 0;
+		input->proximity_domain_info.flags.proximity_info_valid = 1;
+		input->proximity_domain_info.flags.proximity_preferred = 1;
+		status = hv_do_hypercall(HVCALL_ADD_LOGICAL_PROCESSOR,
+					 input, output);
+		local_irq_restore(flags);
+
+		if (status != HV_STATUS_INSUFFICIENT_MEMORY) {
+			if (status != HV_STATUS_SUCCESS) {
+				pr_err("%s: cpu %u apic ID %u, %d\n", __func__,
+				       lp_index, apic_id, status);
+				ret = status;
+			}
+			break;
+		}
+		ret = hv_call_deposit_pages(node, hv_current_partition_id, 1);
+
+	} while (!ret);
+
+	return ret;
+}
+
+int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags)
+{
+	struct hv_create_vp *input;
+	int status;
+	unsigned long irq_flags;
+	int ret = 0;
+
+	/* Root VPs don't seem to need pages deposited */
+	if (partition_id != hv_current_partition_id) {
+		ret = hv_call_deposit_pages(node, partition_id, 90);
+		if (ret)
+			return ret;
+	}
+
+	do {
+		local_irq_save(irq_flags);
+
+		input = *this_cpu_ptr(hyperv_pcpu_input_arg);
+
+		input->partition_id = partition_id;
+		input->vp_index = vp_index;
+		input->flags = flags;
+		if (node != NUMA_NO_NODE) {
+			input->proximity_domain_info.domain_id = node_to_pxm(node);
+			input->proximity_domain_info.flags.reserved = 0;
+			input->proximity_domain_info.flags.proximity_info_valid = 1;
+			input->proximity_domain_info.flags.proximity_preferred = 1;
+		} else {
+			input->proximity_domain_info.as_uint64 = 0;
+		}
+		status = hv_do_hypercall(HVCALL_CREATE_VP, input, NULL);
+		local_irq_restore(irq_flags);
+
+		if (status != HV_STATUS_INSUFFICIENT_MEMORY) {
+			if (status != HV_STATUS_SUCCESS) {
+				pr_err("%s: vcpu %u, lp %u, %d\n", __func__,
+				       vp_index, flags, status);
+				ret = status;
+			}
+			break;
+		}
+		ret = hv_call_deposit_pages(node, partition_id, 1);
+
+	} while (!ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(hv_call_create_vp);
+
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 4039302e0ae9..60afc3e417d0 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -67,6 +67,10 @@ extern void  __percpu  **hyperv_pcpu_output_arg;
 
 extern u64 hv_current_partition_id;
 
+int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
+int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
+int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags);
+
 static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
 {
 	u64 input_address = input ? virt_to_phys(input) : 0;
diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
index 87b1a79b19eb..2b05bed712c0 100644
--- a/include/asm-generic/hyperv-tlfs.h
+++ b/include/asm-generic/hyperv-tlfs.h
@@ -142,6 +142,8 @@ struct ms_hyperv_tsc_page {
 #define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX	0x0014
 #define HVCALL_SEND_IPI_EX			0x0015
 #define HVCALL_GET_PARTITION_ID			0x0046
+#define HVCALL_DEPOSIT_MEMORY			0x0048
+#define HVCALL_CREATE_VP			0x004e
 #define HVCALL_GET_VP_REGISTERS			0x0050
 #define HVCALL_SET_VP_REGISTERS			0x0051
 #define HVCALL_POST_MESSAGE			0x005c
@@ -149,6 +151,7 @@ struct ms_hyperv_tsc_page {
 #define HVCALL_POST_DEBUG_DATA			0x0069
 #define HVCALL_RETRIEVE_DEBUG_DATA		0x006a
 #define HVCALL_RESET_DEBUG_SESSION		0x006b
+#define HVCALL_ADD_LOGICAL_PROCESSOR		0x0076
 #define HVCALL_RETARGET_INTERRUPT		0x007e
 #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
 #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
@@ -413,6 +416,59 @@ struct hv_get_partition_id {
 	u64 partition_id;
 } __packed;
 
+/* HvDepositMemory hypercall */
+struct hv_deposit_memory {
+	u64 partition_id;
+	u64 gpa_page_list[];
+};
+
+
+struct hv_proximity_domain_flags {
+	u32 proximity_preferred : 1;
+	u32 reserved : 30;
+	u32 proximity_info_valid : 1;
+};
+
+/* Not a union in windows but useful for zeroing */
+union hv_proximity_domain_info {
+	struct {
+		u32 domain_id;
+		struct hv_proximity_domain_flags flags;
+	};
+	u64 as_uint64;
+};
+
+struct hv_lp_startup_status {
+	u64 hv_status;
+	u64 substatus1;
+	u64 substatus2;
+	u64 substatus3;
+	u64 substatus4;
+	u64 substatus5;
+	u64 substatus6;
+};
+
+/* HvAddLogicalProcessor hypercalls */
+struct hv_add_logical_processor_in {
+	u32 lp_index;
+	u32 apic_id;
+	union hv_proximity_domain_info proximity_domain_info;
+	u64 flags;
+};
+
+struct hv_add_logical_processor_out {
+	struct hv_lp_startup_status startup_status;
+};
+
+/* HvCreateVp hypercall */
+struct hv_create_vp {
+	u64 partition_id;
+	u32 vp_index;
+	u32 padding;
+	union hv_proximity_domain_info proximity_domain_info;
+	u64 flags;
+};
+
 /* HvRetargetDeviceInterrupt hypercall */
 union hv_msi_entry {
 	u64 as_uint64;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH RFC v1 11/18] asm-generic/hyperv: update hv_msi_entry
       [not found] <20200914112802.80611-1-wei.liu@kernel.org>
                   ` (2 preceding siblings ...)
  2020-09-14 11:59 ` [PATCH RFC v1 09/18] x86/hyperv: provide a bunch of helper functions Wei Liu
@ 2020-09-14 11:59 ` Wei Liu
  2020-09-14 11:59 ` [PATCH RFC v1 12/18] asm-generic/hyperv: update hv_interrupt_entry Wei Liu
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Wei Liu @ 2020-09-14 11:59 UTC (permalink / raw)
  To: Linux on Hyper-V List
  Cc: virtualization, Linux Kernel List, Michael Kelley,
	Vineeth Pillai, Sunil Muthuswamy, Nuno Das Neves, Wei Liu,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, Arnd Bergmann,
	open list:GENERIC INCLUDE/ASM HEADER FILES

We will soon need to access fields inside the MSI address and MSI data
fields. Introduce hv_msi_address_register and hv_msi_data_register.

Fix up one user of hv_msi_entry in mshyperv.h.

No functional change expected.

Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
 arch/x86/include/asm/mshyperv.h   |  4 ++--
 include/asm-generic/hyperv-tlfs.h | 28 ++++++++++++++++++++++++++--
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 60afc3e417d0..a4d46ca5a0b1 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -244,8 +244,8 @@ static inline void hv_apic_init(void) {}
 static inline void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry,
 					      struct msi_desc *msi_desc)
 {
-	msi_entry->address = msi_desc->msg.address_lo;
-	msi_entry->data = msi_desc->msg.data;
+	msi_entry->address.as_uint32 = msi_desc->msg.address_lo;
+	msi_entry->data.as_uint32 = msi_desc->msg.data;
 }
 
 #else /* CONFIG_HYPERV */
diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
index 2b05bed712c0..e7e80a27777b 100644
--- a/include/asm-generic/hyperv-tlfs.h
+++ b/include/asm-generic/hyperv-tlfs.h
@@ -469,12 +469,36 @@ struct hv_create_vp {
 	u64 flags;
 };
 
+union hv_msi_address_register {
+	u32 as_uint32;
+	struct {
+		u32 reserved1:2;
+		u32 destination_mode:1;
+		u32 redirection_hint:1;
+		u32 reserved2:8;
+		u32 destination_id:8;
+		u32 msi_base:12;
+	};
+} __packed;
+
+union hv_msi_data_register {
+	u32 as_uint32;
+	struct {
+		u32 vector:8;
+		u32 delivery_mode:3;
+		u32 reserved1:3;
+		u32 level_assert:1;
+		u32 trigger_mode:1;
+		u32 reserved2:16;
+	};
+} __packed;
+
 /* HvRetargetDeviceInterrupt hypercall */
 union hv_msi_entry {
 	u64 as_uint64;
 	struct {
-		u32 address;
-		u32 data;
+		union hv_msi_address_register address;
+		union hv_msi_data_register data;
 	} __packed;
 };
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH RFC v1 12/18] asm-generic/hyperv: update hv_interrupt_entry
       [not found] <20200914112802.80611-1-wei.liu@kernel.org>
                   ` (3 preceding siblings ...)
  2020-09-14 11:59 ` [PATCH RFC v1 11/18] asm-generic/hyperv: update hv_msi_entry Wei Liu
@ 2020-09-14 11:59 ` Wei Liu
  2020-10-01 14:33   ` Rob Herring
  2020-09-14 11:59 ` [PATCH RFC v1 13/18] asm-generic/hyperv: introduce hv_device_id and auxiliary structures Wei Liu
  2020-09-14 11:59 ` [PATCH RFC v1 14/18] asm-generic/hyperv: import data structures for mapping device interrupts Wei Liu
  6 siblings, 1 reply; 15+ messages in thread
From: Wei Liu @ 2020-09-14 11:59 UTC (permalink / raw)
  To: Linux on Hyper-V List
  Cc: virtualization, Linux Kernel List, Michael Kelley,
	Vineeth Pillai, Sunil Muthuswamy, Nuno Das Neves, Wei Liu,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas, Arnd Bergmann,
	open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS,
	open list:GENERIC INCLUDE/ASM HEADER FILES

We will soon use the same structure to handle IO-APIC interrupts as
well. Introduce an enum to identify the source and a data structure for
IO-APIC RTE.

While at it, update pci-hyperv.c to use the enum.

No functional change.

Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
 drivers/pci/controller/pci-hyperv.c |  2 +-
 include/asm-generic/hyperv-tlfs.h   | 36 +++++++++++++++++++++++++++--
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index bf40ff09c99d..b38faa04234b 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1215,7 +1215,7 @@ static void hv_irq_unmask(struct irq_data *data)
 	params = &hbus->retarget_msi_interrupt_params;
 	memset(params, 0, sizeof(*params));
 	params->partition_id = HV_PARTITION_ID_SELF;
-	params->int_entry.source = 1; /* MSI(-X) */
+	params->int_entry.source = HV_INTERRUPT_SOURCE_MSI;
 	hv_set_msi_entry_from_desc(&params->int_entry.msi_entry, msi_desc);
 	params->device_id = (hbus->hdev->dev_instance.b[5] << 24) |
 			   (hbus->hdev->dev_instance.b[4] << 16) |
diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
index e7e80a27777b..83945ada5a50 100644
--- a/include/asm-generic/hyperv-tlfs.h
+++ b/include/asm-generic/hyperv-tlfs.h
@@ -469,6 +469,11 @@ struct hv_create_vp {
 	u64 flags;
 };
 
+enum hv_interrupt_source {
+	HV_INTERRUPT_SOURCE_MSI = 1, /* MSI and MSI-X */
+	HV_INTERRUPT_SOURCE_IOAPIC,
+};
+
 union hv_msi_address_register {
 	u32 as_uint32;
 	struct {
@@ -502,10 +507,37 @@ union hv_msi_entry {
 	} __packed;
 };
 
+union hv_ioapic_rte {
+	u64 as_uint64;
+
+	struct {
+		u32 vector:8;
+		u32 delivery_mode:3;
+		u32 destination_mode:1;
+		u32 delivery_status:1;
+		u32 interrupt_polarity:1;
+		u32 remote_irr:1;
+		u32 trigger_mode:1;
+		u32 interrupt_mask:1;
+		u32 reserved1:15;
+
+		u32 reserved2:24;
+		u32 destination_id:8;
+	};
+
+	struct {
+		u32 low_uint32;
+		u32 high_uint32;
+	};
+} __packed;
+
 struct hv_interrupt_entry {
-	u32 source;			/* 1 for MSI(-X) */
+	u32 source;
 	u32 reserved1;
-	union hv_msi_entry msi_entry;
+	union {
+		union hv_msi_entry msi_entry;
+		union hv_ioapic_rte ioapic_rte;
+	};
 } __packed;
 
 /*
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH RFC v1 13/18] asm-generic/hyperv: introduce hv_device_id and auxiliary structures
       [not found] <20200914112802.80611-1-wei.liu@kernel.org>
                   ` (4 preceding siblings ...)
  2020-09-14 11:59 ` [PATCH RFC v1 12/18] asm-generic/hyperv: update hv_interrupt_entry Wei Liu
@ 2020-09-14 11:59 ` Wei Liu
  2020-09-15 11:16   ` Vitaly Kuznetsov
  2020-09-14 11:59 ` [PATCH RFC v1 14/18] asm-generic/hyperv: import data structures for mapping device interrupts Wei Liu
  6 siblings, 1 reply; 15+ messages in thread
From: Wei Liu @ 2020-09-14 11:59 UTC (permalink / raw)
  To: Linux on Hyper-V List
  Cc: virtualization, Linux Kernel List, Michael Kelley,
	Vineeth Pillai, Sunil Muthuswamy, Nuno Das Neves, Wei Liu,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Arnd Bergmann, open list:GENERIC INCLUDE/ASM HEADER FILES

We will need to identify the device we want Microsoft Hypervisor to
manipulate.  Introduce the data structures for that purpose.

They will be used in a later patch.

Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
 include/asm-generic/hyperv-tlfs.h | 79 +++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
index 83945ada5a50..faf892ce152d 100644
--- a/include/asm-generic/hyperv-tlfs.h
+++ b/include/asm-generic/hyperv-tlfs.h
@@ -612,4 +612,83 @@ struct hv_set_vp_registers_input {
 	} element[];
 } __packed;
 
+enum hv_device_type {
+	HV_DEVICE_TYPE_LOGICAL = 0,
+	HV_DEVICE_TYPE_PCI = 1,
+	HV_DEVICE_TYPE_IOAPIC = 2,
+	HV_DEVICE_TYPE_ACPI = 3,
+};
+
+typedef u16 hv_pci_rid;
+typedef u16 hv_pci_segment;
+typedef u64 hv_logical_device_id;
+union hv_pci_bdf {
+	u16 as_uint16;
+
+	struct {
+		u8 function:3;
+		u8 device:5;
+		u8 bus;
+	};
+} __packed;
+
+union hv_pci_bus_range {
+	u16 as_uint16;
+
+	struct {
+		u8 subordinate_bus;
+		u8 secondary_bus;
+	};
+} __packed;
+
+union hv_device_id {
+	u64 as_uint64;
+
+	struct {
+		u64 :62;
+		u64 device_type:2;
+	};
+
+	// HV_DEVICE_TYPE_LOGICAL
+	struct {
+		u64 id:62;
+		u64 device_type:2;
+	} logical;
+
+	// HV_DEVICE_TYPE_PCI
+	struct {
+		union {
+			hv_pci_rid rid;
+			union hv_pci_bdf bdf;
+		};
+
+		hv_pci_segment segment;
+		union hv_pci_bus_range shadow_bus_range;
+
+		u16 phantom_function_bits:2;
+		u16 source_shadow:1;
+
+		u16 rsvdz0:11;
+		u16 device_type:2;
+	} pci;
+
+	// HV_DEVICE_TYPE_IOAPIC
+	struct {
+		u8 ioapic_id;
+		u8 rsvdz0;
+		u16 rsvdz1;
+		u16 rsvdz2;
+
+		u16 rsvdz3:14;
+		u16 device_type:2;
+	} ioapic;
+
+	// HV_DEVICE_TYPE_ACPI
+	struct {
+		u32 input_mapping_base;
+		u32 input_mapping_count:30;
+		u32 device_type:2;
+	} acpi;
+} __packed;
+
 #endif
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH RFC v1 14/18] asm-generic/hyperv: import data structures for mapping device interrupts
       [not found] <20200914112802.80611-1-wei.liu@kernel.org>
                   ` (5 preceding siblings ...)
  2020-09-14 11:59 ` [PATCH RFC v1 13/18] asm-generic/hyperv: introduce hv_device_id and auxiliary structures Wei Liu
@ 2020-09-14 11:59 ` Wei Liu
  6 siblings, 0 replies; 15+ messages in thread
From: Wei Liu @ 2020-09-14 11:59 UTC (permalink / raw)
  To: Linux on Hyper-V List
  Cc: virtualization, Linux Kernel List, Michael Kelley,
	Vineeth Pillai, Sunil Muthuswamy, Nuno Das Neves, Wei Liu,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, Arnd Bergmann,
	open list:GENERIC INCLUDE/ASM HEADER FILES

Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Signed-off-by: Wei Liu <wei.liu@kernel.org>
---
 arch/x86/include/asm/hyperv-tlfs.h | 13 +++++++++++
 include/asm-generic/hyperv-tlfs.h  | 36 ++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 20d628c1ed50..33210fa9899f 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -559,6 +559,19 @@ struct hv_partition_assist_pg {
 	u32 tlb_lock_count;
 };
 
+enum hv_interrupt_type {
+	HV_X64_INTERRUPT_TYPE_FIXED             = 0x0000,
+	HV_X64_INTERRUPT_TYPE_LOWESTPRIORITY    = 0x0001,
+	HV_X64_INTERRUPT_TYPE_SMI               = 0x0002,
+	HV_X64_INTERRUPT_TYPE_REMOTEREAD        = 0x0003,
+	HV_X64_INTERRUPT_TYPE_NMI               = 0x0004,
+	HV_X64_INTERRUPT_TYPE_INIT              = 0x0005,
+	HV_X64_INTERRUPT_TYPE_SIPI              = 0x0006,
+	HV_X64_INTERRUPT_TYPE_EXTINT            = 0x0007,
+	HV_X64_INTERRUPT_TYPE_LOCALINT0         = 0x0008,
+	HV_X64_INTERRUPT_TYPE_LOCALINT1         = 0x0009,
+	HV_X64_INTERRUPT_TYPE_MAXIMUM           = 0x000A,
+};
 
 #include <asm-generic/hyperv-tlfs.h>
 
diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
index faf892ce152d..410e2dd09a84 100644
--- a/include/asm-generic/hyperv-tlfs.h
+++ b/include/asm-generic/hyperv-tlfs.h
@@ -152,6 +152,8 @@ struct ms_hyperv_tsc_page {
 #define HVCALL_RETRIEVE_DEBUG_DATA		0x006a
 #define HVCALL_RESET_DEBUG_SESSION		0x006b
 #define HVCALL_ADD_LOGICAL_PROCESSOR		0x0076
+#define HVCALL_MAP_DEVICE_INTERRUPT		0x007c
+#define HVCALL_UNMAP_DEVICE_INTERRUPT		0x007d
 #define HVCALL_RETARGET_INTERRUPT		0x007e
 #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
 #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
@@ -691,4 +693,38 @@ union hv_device_id {
 	} acpi;
 } __packed;
 
+enum hv_interrupt_trigger_mode {
+	HV_INTERRUPT_TRIGGER_MODE_EDGE = 0,
+	HV_INTERRUPT_TRIGGER_MODE_LEVEL = 1,
+};
+
+struct hv_device_interrupt_descriptor {
+	u32 interrupt_type;
+	u32 trigger_mode;
+	u32 vector_count;
+	u32 reserved;
+	struct hv_device_interrupt_target target;
+} __packed;
+
+struct hv_input_map_device_interrupt {
+	u64 partition_id;
+	u64 device_id;
+	u64 flags;
+	struct hv_interrupt_entry logical_interrupt_entry;
+	struct hv_device_interrupt_descriptor interrupt_descriptor;
+} __packed;
+
+struct hv_output_map_device_interrupt {
+	struct hv_interrupt_entry interrupt_entry;
+} __packed;
+
+struct hv_input_unmap_device_interrupt {
+	u64 partition_id;
+	u64 device_id;
+	struct hv_interrupt_entry interrupt_entry;
+} __packed;
+
+#define HV_SOURCE_SHADOW_NONE               0x0
+#define HV_SOURCE_SHADOW_BRIDGE_BUS_RANGE   0x1
+
 #endif
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH RFC v1 07/18] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary
  2020-09-14 11:27 ` [PATCH RFC v1 07/18] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary Wei Liu
@ 2020-09-15 10:27   ` Vitaly Kuznetsov
  2020-09-16 16:32     ` Wei Liu
  0 siblings, 1 reply; 15+ messages in thread
From: Vitaly Kuznetsov @ 2020-09-15 10:27 UTC (permalink / raw)
  To: Wei Liu, Linux on Hyper-V List
  Cc: virtualization, Linux Kernel List, Michael Kelley,
	Vineeth Pillai, Sunil Muthuswamy, Nuno Das Neves, Wei Liu,
	Lillian Grassin-Drake, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, Arnd Bergmann,
	open list:GENERIC INCLUDE/ASM HEADER FILES

Wei Liu <wei.liu@kernel.org> writes:

> We will need the partition ID for executing some hypercalls later.
>
> Signed-off-by: Lillian Grassin-Drake <ligrassi@microsoft.com>
> Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
> ---
>  arch/x86/hyperv/hv_init.c         | 26 ++++++++++++++++++++++++++
>  arch/x86/include/asm/mshyperv.h   |  2 ++
>  include/asm-generic/hyperv-tlfs.h |  6 ++++++
>  3 files changed, 34 insertions(+)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index ebba4be4185d..0eec1ed32023 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -30,6 +30,9 @@
>  bool hv_root_partition;
>  EXPORT_SYMBOL_GPL(hv_root_partition);
>  
> +u64 hv_current_partition_id;
> +EXPORT_SYMBOL_GPL(hv_current_partition_id);
> +
>  void *hv_hypercall_pg;
>  EXPORT_SYMBOL_GPL(hv_hypercall_pg);
>  
> @@ -345,6 +348,26 @@ static struct syscore_ops hv_syscore_ops = {
>  	.resume		= hv_resume,
>  };
>  
> +void __init hv_get_partition_id(void)
> +{
> +	struct hv_get_partition_id *output_page;
> +	int status;
> +	unsigned long flags;
> +
> +	local_irq_save(flags);
> +	output_page = *this_cpu_ptr(hyperv_pcpu_output_arg);
> +	status = hv_do_hypercall(HVCALL_GET_PARTITION_ID, NULL, output_page) &
> +		HV_HYPERCALL_RESULT_MASK;

Nit: in this case status is 'u16', we can define it as such (instead of
signed int).

> +	if (status != HV_STATUS_SUCCESS)
> +		pr_err("Failed to get partition ID: %d\n", status);
> +	else
> +		hv_current_partition_id = output_page->partition_id;
> +	local_irq_restore(flags);
> +
> +	/* No point in proceeding if this failed */
> +	BUG_ON(status != HV_STATUS_SUCCESS);
> +}
> +
>  /*
>   * This function is to be invoked early in the boot sequence after the
>   * hypervisor has been detected.
> @@ -440,6 +463,9 @@ void __init hyperv_init(void)
>  
>  	register_syscore_ops(&hv_syscore_ops);
>  
> +	if (hv_root_partition)
> +		hv_get_partition_id();

According to TLFS, partition ID is available when AccessPartitionId
privilege is granted. I'd suggest we check that instead of
hv_root_partition (and we can set hv_current_partition_id to something
like U64_MAX so we know it wasn't acuired). So the BUG_ON condition will
move here:

        hv_get_partition_id();
        BUG_ON(hv_root_partition && hv_current_partition_id == U64_MAX);

> +
>  	return;
>  
>  remove_cpuhp_state:
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index f5c62140f28d..4039302e0ae9 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -65,6 +65,8 @@ extern void *hv_hypercall_pg;
>  extern void  __percpu  **hyperv_pcpu_input_arg;
>  extern void  __percpu  **hyperv_pcpu_output_arg;
>  
> +extern u64 hv_current_partition_id;
> +
>  static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
>  {
>  	u64 input_address = input ? virt_to_phys(input) : 0;
> diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
> index e6903589a82a..87b1a79b19eb 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -141,6 +141,7 @@ struct ms_hyperv_tsc_page {
>  #define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX	0x0013
>  #define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX	0x0014
>  #define HVCALL_SEND_IPI_EX			0x0015
> +#define HVCALL_GET_PARTITION_ID			0x0046
>  #define HVCALL_GET_VP_REGISTERS			0x0050
>  #define HVCALL_SET_VP_REGISTERS			0x0051
>  #define HVCALL_POST_MESSAGE			0x005c
> @@ -407,6 +408,11 @@ struct hv_tlb_flush_ex {
>  	u64 gva_list[];
>  } __packed;
>  
> +/* HvGetPartitionId hypercall (output only) */
> +struct hv_get_partition_id {
> +	u64 partition_id;
> +} __packed;
> +
>  /* HvRetargetDeviceInterrupt hypercall */
>  union hv_msi_entry {
>  	u64 as_uint64;

-- 
Vitaly


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH RFC v1 09/18] x86/hyperv: provide a bunch of helper functions
  2020-09-14 11:59 ` [PATCH RFC v1 09/18] x86/hyperv: provide a bunch of helper functions Wei Liu
@ 2020-09-15 11:00   ` Vitaly Kuznetsov
  2020-10-27 13:10     ` Wei Liu
  0 siblings, 1 reply; 15+ messages in thread
From: Vitaly Kuznetsov @ 2020-09-15 11:00 UTC (permalink / raw)
  To: Wei Liu, Linux on Hyper-V List
  Cc: virtualization, Linux Kernel List, Michael Kelley,
	Vineeth Pillai, Sunil Muthuswamy, Nuno Das Neves, Wei Liu,
	Lillian Grassin-Drake, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, Arnd Bergmann,
	open list:GENERIC INCLUDE/ASM HEADER FILES

Wei Liu <wei.liu@kernel.org> writes:

> They are used to deposit pages into Microsoft Hypervisor and bring up
> logical and virtual processors.
>
> Signed-off-by: Lillian Grassin-Drake <ligrassi@microsoft.com>
> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Signed-off-by: Nuno Das Neves <nudasnev@microsoft.com>
> Co-Developed-by: Lillian Grassin-Drake <ligrassi@microsoft.com>
> Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Co-Developed-by: Nuno Das Neves <nudasnev@microsoft.com>
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
> ---
>  arch/x86/hyperv/Makefile          |   2 +-
>  arch/x86/hyperv/hv_proc.c         | 209 ++++++++++++++++++++++++++++++
>  arch/x86/include/asm/mshyperv.h   |   4 +
>  include/asm-generic/hyperv-tlfs.h |  56 ++++++++
>  4 files changed, 270 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/hyperv/hv_proc.c
>
> diff --git a/arch/x86/hyperv/Makefile b/arch/x86/hyperv/Makefile
> index 89b1f74d3225..565358020921 100644
> --- a/arch/x86/hyperv/Makefile
> +++ b/arch/x86/hyperv/Makefile
> @@ -1,6 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  obj-y			:= hv_init.o mmu.o nested.o
> -obj-$(CONFIG_X86_64)	+= hv_apic.o
> +obj-$(CONFIG_X86_64)	+= hv_apic.o hv_proc.o
>  
>  ifdef CONFIG_X86_64
>  obj-$(CONFIG_PARAVIRT_SPINLOCKS)	+= hv_spinlock.o
> diff --git a/arch/x86/hyperv/hv_proc.c b/arch/x86/hyperv/hv_proc.c
> new file mode 100644
> index 000000000000..847c72465d0e
> --- /dev/null
> +++ b/arch/x86/hyperv/hv_proc.c
> @@ -0,0 +1,209 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/types.h>
> +#include <linux/version.h>
> +#include <linux/vmalloc.h>
> +#include <linux/mm.h>
> +#include <linux/clockchips.h>
> +#include <linux/acpi.h>
> +#include <linux/hyperv.h>
> +#include <linux/slab.h>
> +#include <linux/cpuhotplug.h>
> +#include <asm/hypervisor.h>
> +#include <asm/mshyperv.h>
> +#include <asm/apic.h>
> +
> +#include <asm/trace/hyperv.h>
> +
> +#define HV_DEPOSIT_MAX_ORDER (8)
> +#define HV_DEPOSIT_MAX (1 << HV_DEPOSIT_MAX_ORDER)
> +
> +#define MAX(a, b) ((a) > (b) ? (a) : (b))
> +#define MIN(a, b) ((a) < (b) ? (a) : (b))

Nit: include/linux/kernel.h defines min() and max() macros with type
checking.

> +
> +/*
> + * Deposits exact number of pages
> + * Must be called with interrupts enabled
> + * Max 256 pages
> + */
> +int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
> +{
> +	struct page **pages;
> +	int *counts;
> +	int num_allocations;
> +	int i, j, page_count;
> +	int order;
> +	int desired_order;
> +	int status;
> +	int ret;
> +	u64 base_pfn;
> +	struct hv_deposit_memory *input_page;
> +	unsigned long flags;
> +
> +	if (num_pages > HV_DEPOSIT_MAX)
> +		return -EINVAL;
> +	if (!num_pages)
> +		return 0;
> +
> +	ret = -ENOMEM;
> +
> +	/* One buffer for page pointers and counts */
> +	pages = page_address(alloc_page(GFP_KERNEL));
> +	if (!pages)
> +		goto free_buf;

There is nothing to free, just do 'return -ENOMEM' here;

> +	counts = (int *)&pages[256];
> +

Oh this is weird. So 'pages' is an array of 512 'struct page *' items
and we use its second half (pages[256]) for an array of signed(!)
integers(!). Can we use a locally defined struct or something better for
that?

> +	/* Allocate all the pages before disabling interrupts */
> +	num_allocations = 0;
> +	i = 0;
> +	order = HV_DEPOSIT_MAX_ORDER;
> +
> +	while (num_pages) {
> +		/* Find highest order we can actually allocate */
> +		desired_order = 31 - __builtin_clz(num_pages);
> +		order = MIN(desired_order, order);
> +		do {
> +			pages[i] = alloc_pages_node(node, GFP_KERNEL, order);
> +			if (!pages[i]) {
> +				if (!order) {
> +					goto err_free_allocations;
> +				}
> +				--order;
> +			}
> +		} while (!pages[i]);
> +
> +		split_page(pages[i], order);
> +		counts[i] = 1 << order;
> +		num_pages -= counts[i];
> +		i++;

So here we believe we will never overrun the 2048 bytes we 'allocated'
for 'counts' above. While 'if (num_pages > HV_DEPOSIT_MAX)' presumably
guarantees that, this is not really obvious.

> +		num_allocations++;
> +	}
> +
> +	local_irq_save(flags);
> +
> +	input_page = *this_cpu_ptr(hyperv_pcpu_input_arg);
> +
> +	input_page->partition_id = partition_id;
> +
> +	/* Populate gpa_page_list - these will fit on the input page */
> +	for (i = 0, page_count = 0; i < num_allocations; ++i) {
> +		base_pfn = page_to_pfn(pages[i]);
> +		for (j = 0; j < counts[i]; ++j, ++page_count)
> +			input_page->gpa_page_list[page_count] = base_pfn + j;
> +	}
> +	status = hv_do_rep_hypercall(HVCALL_DEPOSIT_MEMORY,
> +				     page_count, 0, input_page,
> +				     NULL) & HV_HYPERCALL_RESULT_MASK;
> +	local_irq_restore(flags);
> +
> +	if (status != HV_STATUS_SUCCESS) {

Nit: same like in one ov the previous patches, status can be 'u16'.

> +		pr_err("Failed to deposit pages: %d\n", status);
> +		ret = status;
> +		goto err_free_allocations;
> +	}
> +
> +	ret = 0;
> +	goto free_buf;
> +
> +err_free_allocations:
> +	for (i = 0; i < num_allocations; ++i) {
> +		base_pfn = page_to_pfn(pages[i]);
> +		for (j = 0; j < counts[i]; ++j)
> +			__free_page(pfn_to_page(base_pfn + j));
> +	}
> +
> +free_buf:
> +	free_page((unsigned long)pages);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(hv_call_deposit_pages);
> +
> +int hv_call_add_logical_proc(int node, u32 lp_index, u32 apic_id)
> +{
> +	struct hv_add_logical_processor_in *input;
> +	struct hv_add_logical_processor_out *output;
> +	int status;
> +	unsigned long flags;
> +	int ret = 0;
> +
> +	do {
> +		local_irq_save(flags);
> +
> +		input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> +		/* We don't do anything with the output right now */
> +		output = *this_cpu_ptr(hyperv_pcpu_output_arg);
> +
> +		input->lp_index = lp_index;
> +		input->apic_id = apic_id;
> +		input->flags = 0;
> +		input->proximity_domain_info.domain_id = node_to_pxm(node);
> +		input->proximity_domain_info.flags.reserved = 0;
> +		input->proximity_domain_info.flags.proximity_info_valid = 1;
> +		input->proximity_domain_info.flags.proximity_preferred = 1;
> +		status = hv_do_hypercall(HVCALL_ADD_LOGICAL_PROCESSOR,
> +					 input, output);
> +		local_irq_restore(flags);
> +
> +		if (status != HV_STATUS_INSUFFICIENT_MEMORY) {
> +			if (status != HV_STATUS_SUCCESS) {
> +				pr_err("%s: cpu %u apic ID %u, %d\n", __func__,
> +				       lp_index, apic_id, status);
> +				ret = status;
> +			}
> +			break;

So if status == HV_STATUS_SUCCESS we break and avoid
hv_call_deposit_pages() below?

> +		}
> +		ret = hv_call_deposit_pages(node, hv_current_partition_id, 1);
> +
> +	} while (!ret);

And if hv_call_deposit_pages() returns '0' we keep doing something? Sorry
but I'm probably missing something important in the 'depositing'
process, could you please add a comment explaining what's going on here?

> +
> +	return ret;
> +}
> +
> +int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags)
> +{
> +	struct hv_create_vp *input;
> +	int status;
> +	unsigned long irq_flags;
> +	int ret = 0;
> +
> +	/* Root VPs don't seem to need pages deposited */
> +	if (partition_id != hv_current_partition_id) {
> +		ret = hv_call_deposit_pages(node, partition_id, 90);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	do {
> +		local_irq_save(irq_flags);
> +
> +		input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> +
> +		input->partition_id = partition_id;
> +		input->vp_index = vp_index;
> +		input->flags = flags;
> +		if (node != NUMA_NO_NODE) {
> +			input->proximity_domain_info.domain_id = node_to_pxm(node);
> +			input->proximity_domain_info.flags.reserved = 0;
> +			input->proximity_domain_info.flags.proximity_info_valid = 1;
> +			input->proximity_domain_info.flags.proximity_preferred = 1;
> +		} else {
> +			input->proximity_domain_info.as_uint64 = 0;
> +		}
> +		status = hv_do_hypercall(HVCALL_CREATE_VP, input, NULL);
> +		local_irq_restore(irq_flags);
> +
> +		if (status != HV_STATUS_INSUFFICIENT_MEMORY) {
> +			if (status != HV_STATUS_SUCCESS) {
> +				pr_err("%s: vcpu %u, lp %u, %d\n", __func__,
> +				       vp_index, flags, status);
> +				ret = status;
> +			}
> +			break;
> +		}
> +		ret = hv_call_deposit_pages(node, partition_id, 1);
> +
> +	} while (!ret);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(hv_call_create_vp);
> +
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index 4039302e0ae9..60afc3e417d0 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -67,6 +67,10 @@ extern void  __percpu  **hyperv_pcpu_output_arg;
>  
>  extern u64 hv_current_partition_id;
>  
> +int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
> +int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
> +int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags);
> +
>  static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
>  {
>  	u64 input_address = input ? virt_to_phys(input) : 0;
> diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
> index 87b1a79b19eb..2b05bed712c0 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -142,6 +142,8 @@ struct ms_hyperv_tsc_page {
>  #define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX	0x0014
>  #define HVCALL_SEND_IPI_EX			0x0015
>  #define HVCALL_GET_PARTITION_ID			0x0046
> +#define HVCALL_DEPOSIT_MEMORY			0x0048
> +#define HVCALL_CREATE_VP			0x004e
>  #define HVCALL_GET_VP_REGISTERS			0x0050
>  #define HVCALL_SET_VP_REGISTERS			0x0051
>  #define HVCALL_POST_MESSAGE			0x005c
> @@ -149,6 +151,7 @@ struct ms_hyperv_tsc_page {
>  #define HVCALL_POST_DEBUG_DATA			0x0069
>  #define HVCALL_RETRIEVE_DEBUG_DATA		0x006a
>  #define HVCALL_RESET_DEBUG_SESSION		0x006b
> +#define HVCALL_ADD_LOGICAL_PROCESSOR		0x0076
>  #define HVCALL_RETARGET_INTERRUPT		0x007e
>  #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
>  #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
> @@ -413,6 +416,59 @@ struct hv_get_partition_id {
>  	u64 partition_id;
>  } __packed;
>  
> +/* HvDepositMemory hypercall */
> +struct hv_deposit_memory {
> +	u64 partition_id;
> +	u64 gpa_page_list[];
> +};

Other structures above have '__packed' and I remember there were
different opinions if it is needed or not (for properly padded
structures). I'd suggest we stay consitent and keep adding it unless we
decide to get rid of them (but you've added it to the newly introduced
hv_get_partition_id above).
> +
> +
> +struct hv_proximity_domain_flags {
> +	u32 proximity_preferred : 1;
> +	u32 reserved : 30;
> +	u32 proximity_info_valid : 1;
> +};
> +
> +/* Not a union in windows but useful for zeroing */
> +union hv_proximity_domain_info {
> +	struct {
> +		u32 domain_id;
> +		struct hv_proximity_domain_flags flags;
> +	};
> +	u64 as_uint64;
> +};
> +
> +struct hv_lp_startup_status {
> +	u64 hv_status;
> +	u64 substatus1;
> +	u64 substatus2;
> +	u64 substatus3;
> +	u64 substatus4;
> +	u64 substatus5;
> +	u64 substatus6;
> +};
> +
> +/* HvAddLogicalProcessor hypercalls */

s/hypercalls/hypercall/

> +struct hv_add_logical_processor_in {
> +	u32 lp_index;
> +	u32 apic_id;
> +	union hv_proximity_domain_info proximity_domain_info;
> +	u64 flags;
> +};
> +
> +struct hv_add_logical_processor_out {
> +	struct hv_lp_startup_status startup_status;
> +};
> +
> +/* HvCreateVp hypercall */
> +struct hv_create_vp {
> +	u64 partition_id;
> +	u32 vp_index;
> +	u32 padding;
> +	union hv_proximity_domain_info proximity_domain_info;
> +	u64 flags;
> +};
> +
>  /* HvRetargetDeviceInterrupt hypercall */
>  union hv_msi_entry {
>  	u64 as_uint64;

-- 
Vitaly


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH RFC v1 13/18] asm-generic/hyperv: introduce hv_device_id and auxiliary structures
  2020-09-14 11:59 ` [PATCH RFC v1 13/18] asm-generic/hyperv: introduce hv_device_id and auxiliary structures Wei Liu
@ 2020-09-15 11:16   ` Vitaly Kuznetsov
  2020-09-15 11:59     ` Wei Liu
  0 siblings, 1 reply; 15+ messages in thread
From: Vitaly Kuznetsov @ 2020-09-15 11:16 UTC (permalink / raw)
  To: Wei Liu, Linux on Hyper-V List
  Cc: virtualization, Linux Kernel List, Michael Kelley,
	Vineeth Pillai, Sunil Muthuswamy, Nuno Das Neves, Wei Liu,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Arnd Bergmann, open list:GENERIC INCLUDE/ASM HEADER FILES

Wei Liu <wei.liu@kernel.org> writes:

> We will need to identify the device we want Microsoft Hypervisor to
> manipulate.  Introduce the data structures for that purpose.
>
> They will be used in a later patch.
>
> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
> ---
>  include/asm-generic/hyperv-tlfs.h | 79 +++++++++++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
>
> diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
> index 83945ada5a50..faf892ce152d 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -612,4 +612,83 @@ struct hv_set_vp_registers_input {
>  	} element[];
>  } __packed;
>  
> +enum hv_device_type {
> +	HV_DEVICE_TYPE_LOGICAL = 0,
> +	HV_DEVICE_TYPE_PCI = 1,
> +	HV_DEVICE_TYPE_IOAPIC = 2,
> +	HV_DEVICE_TYPE_ACPI = 3,
> +};
> +
> +typedef u16 hv_pci_rid;
> +typedef u16 hv_pci_segment;
> +typedef u64 hv_logical_device_id;
> +union hv_pci_bdf {
> +	u16 as_uint16;
> +
> +	struct {
> +		u8 function:3;
> +		u8 device:5;
> +		u8 bus;
> +	};
> +} __packed;
> +
> +union hv_pci_bus_range {
> +	u16 as_uint16;
> +
> +	struct {
> +		u8 subordinate_bus;
> +		u8 secondary_bus;
> +	};
> +} __packed;
> +
> +union hv_device_id {
> +	u64 as_uint64;
> +
> +	struct {
> +		u64 :62;
> +		u64 device_type:2;
> +	};
> +
> +	// HV_DEVICE_TYPE_LOGICAL

Nit: please no '//' comments.

> +	struct {
> +		u64 id:62;
> +		u64 device_type:2;
> +	} logical;
> +
> +	// HV_DEVICE_TYPE_PCI
> +	struct {
> +		union {
> +			hv_pci_rid rid;
> +			union hv_pci_bdf bdf;
> +		};
> +
> +		hv_pci_segment segment;
> +		union hv_pci_bus_range shadow_bus_range;
> +
> +		u16 phantom_function_bits:2;
> +		u16 source_shadow:1;
> +
> +		u16 rsvdz0:11;
> +		u16 device_type:2;
> +	} pci;
> +
> +	// HV_DEVICE_TYPE_IOAPIC
> +	struct {
> +		u8 ioapic_id;
> +		u8 rsvdz0;
> +		u16 rsvdz1;
> +		u16 rsvdz2;
> +
> +		u16 rsvdz3:14;
> +		u16 device_type:2;
> +	} ioapic;
> +
> +	// HV_DEVICE_TYPE_ACPI
> +	struct {
> +		u32 input_mapping_base;
> +		u32 input_mapping_count:30;
> +		u32 device_type:2;
> +	} acpi;
> +} __packed;
> +
>  #endif

-- 
Vitaly


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH RFC v1 13/18] asm-generic/hyperv: introduce hv_device_id and auxiliary structures
  2020-09-15 11:16   ` Vitaly Kuznetsov
@ 2020-09-15 11:59     ` Wei Liu
  0 siblings, 0 replies; 15+ messages in thread
From: Wei Liu @ 2020-09-15 11:59 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Wei Liu, Linux on Hyper-V List, virtualization,
	Linux Kernel List, Michael Kelley, Vineeth Pillai,
	Sunil Muthuswamy, Nuno Das Neves, K. Y. Srinivasan,
	Haiyang Zhang, Stephen Hemminger, Arnd Bergmann,
	open list:GENERIC INCLUDE/ASM HEADER FILES

On Tue, Sep 15, 2020 at 01:16:59PM +0200, Vitaly Kuznetsov wrote:
[...]
> > +union hv_device_id {
> > +	u64 as_uint64;
> > +
> > +	struct {
> > +		u64 :62;
> > +		u64 device_type:2;
> > +	};
> > +
> > +	// HV_DEVICE_TYPE_LOGICAL
> 
> Nit: please no '//' comments.
> 

Fixed. Thanks.

Wei.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH RFC v1 07/18] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary
  2020-09-15 10:27   ` Vitaly Kuznetsov
@ 2020-09-16 16:32     ` Wei Liu
  2020-10-27 12:19       ` Wei Liu
  0 siblings, 1 reply; 15+ messages in thread
From: Wei Liu @ 2020-09-16 16:32 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Wei Liu, Linux on Hyper-V List, virtualization,
	Linux Kernel List, Michael Kelley, Vineeth Pillai,
	Sunil Muthuswamy, Nuno Das Neves, Lillian Grassin-Drake,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, Arnd Bergmann,
	open list:GENERIC INCLUDE/ASM HEADER FILES

On Tue, Sep 15, 2020 at 12:27:16PM +0200, Vitaly Kuznetsov wrote:
> Wei Liu <wei.liu@kernel.org> writes:
[...]
> >  
> > +void __init hv_get_partition_id(void)
> > +{
> > +	struct hv_get_partition_id *output_page;
> > +	int status;
> > +	unsigned long flags;
> > +
> > +	local_irq_save(flags);
> > +	output_page = *this_cpu_ptr(hyperv_pcpu_output_arg);
> > +	status = hv_do_hypercall(HVCALL_GET_PARTITION_ID, NULL, output_page) &
> > +		HV_HYPERCALL_RESULT_MASK;
> 
> Nit: in this case status is 'u16', we can define it as such (instead of
> signed int).

Fixed.

> 
> > +	if (status != HV_STATUS_SUCCESS)
> > +		pr_err("Failed to get partition ID: %d\n", status);
> > +	else
> > +		hv_current_partition_id = output_page->partition_id;
> > +	local_irq_restore(flags);
> > +
> > +	/* No point in proceeding if this failed */
> > +	BUG_ON(status != HV_STATUS_SUCCESS);
> > +}
> > +
> >  /*
> >   * This function is to be invoked early in the boot sequence after the
> >   * hypervisor has been detected.
> > @@ -440,6 +463,9 @@ void __init hyperv_init(void)
> >  
> >  	register_syscore_ops(&hv_syscore_ops);
> >  
> > +	if (hv_root_partition)
> > +		hv_get_partition_id();
> 
> According to TLFS, partition ID is available when AccessPartitionId
> privilege is granted. I'd suggest we check that instead of
> hv_root_partition (and we can set hv_current_partition_id to something
> like U64_MAX so we know it wasn't acuired). So the BUG_ON condition will
> move here:
> 
>         hv_get_partition_id();
>         BUG_ON(hv_root_partition && hv_current_partition_id == U64_MAX);
> 

Good point. I will reorganize this a bit.

Wei.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH RFC v1 12/18] asm-generic/hyperv: update hv_interrupt_entry
  2020-09-14 11:59 ` [PATCH RFC v1 12/18] asm-generic/hyperv: update hv_interrupt_entry Wei Liu
@ 2020-10-01 14:33   ` Rob Herring
  0 siblings, 0 replies; 15+ messages in thread
From: Rob Herring @ 2020-10-01 14:33 UTC (permalink / raw)
  To: Wei Liu
  Cc: Stephen Hemminger, Sunil Muthuswamy, Lorenzo Pieralisi,
	Arnd Bergmann, virtualization, Bjorn Helgaas, Haiyang Zhang,
	linux-arch, linux-pci, Michael Kelley, Linux on Hyper-V List,
	K. Y. Srinivasan, Nuno Das Neves, Linux Kernel List,
	Vineeth Pillai

On Mon, 14 Sep 2020 11:59:21 +0000, Wei Liu wrote:
> We will soon use the same structure to handle IO-APIC interrupts as
> well. Introduce an enum to identify the source and a data structure for
> IO-APIC RTE.
> 
> While at it, update pci-hyperv.c to use the enum.
> 
> No functional change.
> 
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
> ---
>  drivers/pci/controller/pci-hyperv.c |  2 +-
>  include/asm-generic/hyperv-tlfs.h   | 36 +++++++++++++++++++++++++++--
>  2 files changed, 35 insertions(+), 3 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH RFC v1 07/18] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary
  2020-09-16 16:32     ` Wei Liu
@ 2020-10-27 12:19       ` Wei Liu
  0 siblings, 0 replies; 15+ messages in thread
From: Wei Liu @ 2020-10-27 12:19 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Wei Liu, Linux on Hyper-V List, virtualization,
	Linux Kernel List, Michael Kelley, Vineeth Pillai,
	Sunil Muthuswamy, Nuno Das Neves, Lillian Grassin-Drake,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, Arnd Bergmann,
	open list:GENERIC INCLUDE/ASM HEADER FILES

On Wed, Sep 16, 2020 at 04:32:43PM +0000, Wei Liu wrote:
> On Tue, Sep 15, 2020 at 12:27:16PM +0200, Vitaly Kuznetsov wrote:
> > Wei Liu <wei.liu@kernel.org> writes:
> [...]
> > >  
> > > +void __init hv_get_partition_id(void)
> > > +{
> > > +	struct hv_get_partition_id *output_page;
> > > +	int status;
> > > +	unsigned long flags;
> > > +
> > > +	local_irq_save(flags);
> > > +	output_page = *this_cpu_ptr(hyperv_pcpu_output_arg);
> > > +	status = hv_do_hypercall(HVCALL_GET_PARTITION_ID, NULL, output_page) &
> > > +		HV_HYPERCALL_RESULT_MASK;
> > 
> > Nit: in this case status is 'u16', we can define it as such (instead of
> > signed int).
> 
> Fixed.
> 
> > 
> > > +	if (status != HV_STATUS_SUCCESS)
> > > +		pr_err("Failed to get partition ID: %d\n", status);
> > > +	else
> > > +		hv_current_partition_id = output_page->partition_id;
> > > +	local_irq_restore(flags);
> > > +
> > > +	/* No point in proceeding if this failed */
> > > +	BUG_ON(status != HV_STATUS_SUCCESS);
> > > +}
> > > +
> > >  /*
> > >   * This function is to be invoked early in the boot sequence after the
> > >   * hypervisor has been detected.
> > > @@ -440,6 +463,9 @@ void __init hyperv_init(void)
> > >  
> > >  	register_syscore_ops(&hv_syscore_ops);
> > >  
> > > +	if (hv_root_partition)
> > > +		hv_get_partition_id();
> > 
> > According to TLFS, partition ID is available when AccessPartitionId
> > privilege is granted. I'd suggest we check that instead of
> > hv_root_partition (and we can set hv_current_partition_id to something
> > like U64_MAX so we know it wasn't acuired). So the BUG_ON condition will
> > move here:
> > 
> >         hv_get_partition_id();
> >         BUG_ON(hv_root_partition && hv_current_partition_id == U64_MAX);
> > 
> 
> Good point. I will reorganize this a bit.

Actually, our current code never stashed the feature mask that contains
that privilege anywhere.  Getting access to it will require a few more
extra patches -- I would really like to rename those fields (features,
misc_features) inside ms_hyperv to something more appropriate.

We will gate it wit hv_root_partition anyway, since we a child VM may
not have the privilege, making an unconditional BUG_ON fatal.

All in all, the current code is not too bad. I intend to keep the
current structure for my RFC v2. I will see if I can find some time to
rework the feature mask extraction code and get that upstreamed first.

Wei.

> 
> Wei.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH RFC v1 09/18] x86/hyperv: provide a bunch of helper functions
  2020-09-15 11:00   ` Vitaly Kuznetsov
@ 2020-10-27 13:10     ` Wei Liu
  0 siblings, 0 replies; 15+ messages in thread
From: Wei Liu @ 2020-10-27 13:10 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Wei Liu, Linux on Hyper-V List, virtualization,
	Linux Kernel List, Michael Kelley, Vineeth Pillai,
	Sunil Muthuswamy, Nuno Das Neves, Lillian Grassin-Drake,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, Arnd Bergmann,
	open list:GENERIC INCLUDE/ASM HEADER FILES

On Tue, Sep 15, 2020 at 01:00:55PM +0200, Vitaly Kuznetsov wrote:
> Wei Liu <wei.liu@kernel.org> writes:
> 
> > They are used to deposit pages into Microsoft Hypervisor and bring up
> > logical and virtual processors.
> >
> > Signed-off-by: Lillian Grassin-Drake <ligrassi@microsoft.com>
> > Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> > Signed-off-by: Nuno Das Neves <nudasnev@microsoft.com>
> > Co-Developed-by: Lillian Grassin-Drake <ligrassi@microsoft.com>
> > Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> > Co-Developed-by: Nuno Das Neves <nudasnev@microsoft.com>
> > Signed-off-by: Wei Liu <wei.liu@kernel.org>
> > ---
> >  arch/x86/hyperv/Makefile          |   2 +-
> >  arch/x86/hyperv/hv_proc.c         | 209 ++++++++++++++++++++++++++++++
> >  arch/x86/include/asm/mshyperv.h   |   4 +
> >  include/asm-generic/hyperv-tlfs.h |  56 ++++++++
> >  4 files changed, 270 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/x86/hyperv/hv_proc.c
> >
> > diff --git a/arch/x86/hyperv/Makefile b/arch/x86/hyperv/Makefile
> > index 89b1f74d3225..565358020921 100644
> > --- a/arch/x86/hyperv/Makefile
> > +++ b/arch/x86/hyperv/Makefile
> > @@ -1,6 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> >  obj-y			:= hv_init.o mmu.o nested.o
> > -obj-$(CONFIG_X86_64)	+= hv_apic.o
> > +obj-$(CONFIG_X86_64)	+= hv_apic.o hv_proc.o
> >  
> >  ifdef CONFIG_X86_64
> >  obj-$(CONFIG_PARAVIRT_SPINLOCKS)	+= hv_spinlock.o
> > diff --git a/arch/x86/hyperv/hv_proc.c b/arch/x86/hyperv/hv_proc.c
> > new file mode 100644
> > index 000000000000..847c72465d0e
> > --- /dev/null
> > +++ b/arch/x86/hyperv/hv_proc.c
> > @@ -0,0 +1,209 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <linux/types.h>
> > +#include <linux/version.h>
> > +#include <linux/vmalloc.h>
> > +#include <linux/mm.h>
> > +#include <linux/clockchips.h>
> > +#include <linux/acpi.h>
> > +#include <linux/hyperv.h>
> > +#include <linux/slab.h>
> > +#include <linux/cpuhotplug.h>
> > +#include <asm/hypervisor.h>
> > +#include <asm/mshyperv.h>
> > +#include <asm/apic.h>
> > +
> > +#include <asm/trace/hyperv.h>
> > +
> > +#define HV_DEPOSIT_MAX_ORDER (8)
> > +#define HV_DEPOSIT_MAX (1 << HV_DEPOSIT_MAX_ORDER)
> > +
> > +#define MAX(a, b) ((a) > (b) ? (a) : (b))
> > +#define MIN(a, b) ((a) < (b) ? (a) : (b))
> 
> Nit: include/linux/kernel.h defines min() and max() macros with type
> checking.

Fixed.

> 
> > +
> > +/*
> > + * Deposits exact number of pages
> > + * Must be called with interrupts enabled
> > + * Max 256 pages
> > + */
> > +int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
> > +{
> > +	struct page **pages;
> > +	int *counts;
> > +	int num_allocations;
> > +	int i, j, page_count;
> > +	int order;
> > +	int desired_order;
> > +	int status;
> > +	int ret;
> > +	u64 base_pfn;
> > +	struct hv_deposit_memory *input_page;
> > +	unsigned long flags;
> > +
> > +	if (num_pages > HV_DEPOSIT_MAX)
> > +		return -EINVAL;
> > +	if (!num_pages)
> > +		return 0;
> > +
> > +	ret = -ENOMEM;
> > +
> > +	/* One buffer for page pointers and counts */
> > +	pages = page_address(alloc_page(GFP_KERNEL));
> > +	if (!pages)
> > +		goto free_buf;
> 
> There is nothing to free, just do 'return -ENOMEM' here;
> 
> > +	counts = (int *)&pages[256];
> > +
> 
> Oh this is weird. So 'pages' is an array of 512 'struct page *' items
> and we use its second half (pages[256]) for an array of signed(!)
> integers(!). Can we use a locally defined struct or something better for
> that?
> 

This can be changed. I will make the counts array have its own buffer.

> > +	/* Allocate all the pages before disabling interrupts */
> > +	num_allocations = 0;
> > +	i = 0;
> > +	order = HV_DEPOSIT_MAX_ORDER;
> > +
> > +	while (num_pages) {
> > +		/* Find highest order we can actually allocate */
> > +		desired_order = 31 - __builtin_clz(num_pages);
> > +		order = MIN(desired_order, order);
> > +		do {
> > +			pages[i] = alloc_pages_node(node, GFP_KERNEL, order);
> > +			if (!pages[i]) {
> > +				if (!order) {
> > +					goto err_free_allocations;
> > +				}
> > +				--order;
> > +			}
> > +		} while (!pages[i]);
> > +
> > +		split_page(pages[i], order);
> > +		counts[i] = 1 << order;
> > +		num_pages -= counts[i];
> > +		i++;
> 
> So here we believe we will never overrun the 2048 bytes we 'allocated'
> for 'counts' above. While 'if (num_pages > HV_DEPOSIT_MAX)' presumably
> guarantees that, this is not really obvious.
> 

This is moot since counts is going to have its own buffer allocated with
kcalloc(HV_DEPOSIT_MAX, sizeof(int), ...).

> > +		num_allocations++;
> > +	}
> > +
> > +	local_irq_save(flags);
> > +
> > +	input_page = *this_cpu_ptr(hyperv_pcpu_input_arg);
> > +
> > +	input_page->partition_id = partition_id;
> > +
> > +	/* Populate gpa_page_list - these will fit on the input page */
> > +	for (i = 0, page_count = 0; i < num_allocations; ++i) {
> > +		base_pfn = page_to_pfn(pages[i]);
> > +		for (j = 0; j < counts[i]; ++j, ++page_count)
> > +			input_page->gpa_page_list[page_count] = base_pfn + j;
> > +	}
> > +	status = hv_do_rep_hypercall(HVCALL_DEPOSIT_MEMORY,
> > +				     page_count, 0, input_page,
> > +				     NULL) & HV_HYPERCALL_RESULT_MASK;
> > +	local_irq_restore(flags);
> > +
> > +	if (status != HV_STATUS_SUCCESS) {
> 
> Nit: same like in one ov the previous patches, status can be 'u16'.
> 

Fixed.

> > +		pr_err("Failed to deposit pages: %d\n", status);
> > +		ret = status;
> > +		goto err_free_allocations;
> > +	}
> > +
> > +	ret = 0;
> > +	goto free_buf;
> > +
> > +err_free_allocations:
> > +	for (i = 0; i < num_allocations; ++i) {
> > +		base_pfn = page_to_pfn(pages[i]);
> > +		for (j = 0; j < counts[i]; ++j)
> > +			__free_page(pfn_to_page(base_pfn + j));
> > +	}
> > +
> > +free_buf:
> > +	free_page((unsigned long)pages);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(hv_call_deposit_pages);
> > +
> > +int hv_call_add_logical_proc(int node, u32 lp_index, u32 apic_id)
> > +{
> > +	struct hv_add_logical_processor_in *input;
> > +	struct hv_add_logical_processor_out *output;
> > +	int status;
> > +	unsigned long flags;
> > +	int ret = 0;
> > +
> > +	do {
> > +		local_irq_save(flags);
> > +
> > +		input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> > +		/* We don't do anything with the output right now */
> > +		output = *this_cpu_ptr(hyperv_pcpu_output_arg);
> > +
> > +		input->lp_index = lp_index;
> > +		input->apic_id = apic_id;
> > +		input->flags = 0;
> > +		input->proximity_domain_info.domain_id = node_to_pxm(node);
> > +		input->proximity_domain_info.flags.reserved = 0;
> > +		input->proximity_domain_info.flags.proximity_info_valid = 1;
> > +		input->proximity_domain_info.flags.proximity_preferred = 1;
> > +		status = hv_do_hypercall(HVCALL_ADD_LOGICAL_PROCESSOR,
> > +					 input, output);
> > +		local_irq_restore(flags);
> > +
> > +		if (status != HV_STATUS_INSUFFICIENT_MEMORY) {
> > +			if (status != HV_STATUS_SUCCESS) {
> > +				pr_err("%s: cpu %u apic ID %u, %d\n", __func__,
> > +				       lp_index, apic_id, status);
> > +				ret = status;
> > +			}
> > +			break;
> 
> So if status == HV_STATUS_SUCCESS we break and avoid
> hv_call_deposit_pages() below?
> 

Yes, that means adding the logical processor has succeeded. There is
nothing more to do.

> > +		}
> > +		ret = hv_call_deposit_pages(node, hv_current_partition_id, 1);
> > +
> > +	} while (!ret);
> 
> And if hv_call_deposit_pages() returns '0' we keep doing something? Sorry
> but I'm probably missing something important in the 'depositing'
> process, could you please add a comment explaining what's going on here?
> 

We only get here because 1) there isn't sufficient memory in the last
iteration, 2) we've succeeded in adding a bit more memory. In this case
we will want to retry adding the logical processor.

I will add a comment before the loop.

> > +
> > +	return ret;
> > +}
> > +
[...]
> > diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
> > index 87b1a79b19eb..2b05bed712c0 100644
> > --- a/include/asm-generic/hyperv-tlfs.h
> > +++ b/include/asm-generic/hyperv-tlfs.h
> > @@ -142,6 +142,8 @@ struct ms_hyperv_tsc_page {
> >  #define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX	0x0014
> >  #define HVCALL_SEND_IPI_EX			0x0015
> >  #define HVCALL_GET_PARTITION_ID			0x0046
> > +#define HVCALL_DEPOSIT_MEMORY			0x0048
> > +#define HVCALL_CREATE_VP			0x004e
> >  #define HVCALL_GET_VP_REGISTERS			0x0050
> >  #define HVCALL_SET_VP_REGISTERS			0x0051
> >  #define HVCALL_POST_MESSAGE			0x005c
> > @@ -149,6 +151,7 @@ struct ms_hyperv_tsc_page {
> >  #define HVCALL_POST_DEBUG_DATA			0x0069
> >  #define HVCALL_RETRIEVE_DEBUG_DATA		0x006a
> >  #define HVCALL_RESET_DEBUG_SESSION		0x006b
> > +#define HVCALL_ADD_LOGICAL_PROCESSOR		0x0076
> >  #define HVCALL_RETARGET_INTERRUPT		0x007e
> >  #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
> >  #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
> > @@ -413,6 +416,59 @@ struct hv_get_partition_id {
> >  	u64 partition_id;
> >  } __packed;
> >  
> > +/* HvDepositMemory hypercall */
> > +struct hv_deposit_memory {
> > +	u64 partition_id;
> > +	u64 gpa_page_list[];
> > +};
> 
> Other structures above have '__packed' and I remember there were
> different opinions if it is needed or not (for properly padded
> structures). I'd suggest we stay consitent and keep adding it unless we
> decide to get rid of them (but you've added it to the newly introduced
> hv_get_partition_id above).

Fixed.

> > +
> > +
> > +struct hv_proximity_domain_flags {
> > +	u32 proximity_preferred : 1;
> > +	u32 reserved : 30;
> > +	u32 proximity_info_valid : 1;
> > +};
> > +
> > +/* Not a union in windows but useful for zeroing */
> > +union hv_proximity_domain_info {
> > +	struct {
> > +		u32 domain_id;
> > +		struct hv_proximity_domain_flags flags;
> > +	};
> > +	u64 as_uint64;
> > +};
> > +
> > +struct hv_lp_startup_status {
> > +	u64 hv_status;
> > +	u64 substatus1;
> > +	u64 substatus2;
> > +	u64 substatus3;
> > +	u64 substatus4;
> > +	u64 substatus5;
> > +	u64 substatus6;
> > +};
> > +
> > +/* HvAddLogicalProcessor hypercalls */
> 
> s/hypercalls/hypercall/

Fixed.

Wei.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2020-10-27 13:10 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200914112802.80611-1-wei.liu@kernel.org>
2020-09-14 11:27 ` [PATCH RFC v1 01/18] asm-generic/hyperv: change HV_CPU_POWER_MANAGEMENT to HV_CPU_MANAGEMENT Wei Liu
2020-09-14 11:27 ` [PATCH RFC v1 07/18] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary Wei Liu
2020-09-15 10:27   ` Vitaly Kuznetsov
2020-09-16 16:32     ` Wei Liu
2020-10-27 12:19       ` Wei Liu
2020-09-14 11:59 ` [PATCH RFC v1 09/18] x86/hyperv: provide a bunch of helper functions Wei Liu
2020-09-15 11:00   ` Vitaly Kuznetsov
2020-10-27 13:10     ` Wei Liu
2020-09-14 11:59 ` [PATCH RFC v1 11/18] asm-generic/hyperv: update hv_msi_entry Wei Liu
2020-09-14 11:59 ` [PATCH RFC v1 12/18] asm-generic/hyperv: update hv_interrupt_entry Wei Liu
2020-10-01 14:33   ` Rob Herring
2020-09-14 11:59 ` [PATCH RFC v1 13/18] asm-generic/hyperv: introduce hv_device_id and auxiliary structures Wei Liu
2020-09-15 11:16   ` Vitaly Kuznetsov
2020-09-15 11:59     ` Wei Liu
2020-09-14 11:59 ` [PATCH RFC v1 14/18] asm-generic/hyperv: import data structures for mapping device interrupts Wei Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).