netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 00/12] x86/Hyper-V: Add Hyper-V Isolation VM support
@ 2021-02-28 15:03 Tianyu Lan
  2021-02-28 15:03 ` [RFC PATCH 1/12] x86/Hyper-V: Add visibility parameter for vmbus_establish_gpadl() Tianyu Lan
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Tianyu Lan @ 2021-02-28 15:03 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin, wei.liu, tglx, mingo, bp, x86, hpa,
	davem, kuba, gregkh, arnd, akpm, jejb, martin.petersen
  Cc: Tianyu Lan, linux-arch, linux-hyperv, linux-kernel, linux-mm,
	linux-scsi, netdev, vkuznets, thomas.lendacky, brijesh.singh,
	sunilmut

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

Hyper-V provides two kinds of Isolation VMs. VBS(Virtualization-based
security) and AMD SEV-SNP unenlightened Isolation VMs. This patchset
is to add support for these Isolation VM support in Linux.

The memory of these vms are encrypted and host can't access guest
memory directly. Hyper-V provides new host visibility hvcall and
the guest needs to call new hvcall to mark memory visible to host
before sharing memory with host. For security, all network/storage
stack memory should not be shared with host and so there is bounce
buffer requests.

Vmbus channel ring buffer already plays bounce buffer role because
all data from/to host needs to copy from/to between the ring buffer
and IO stack memory. So mark vmbus channel ring buffer visible.

There are two exceptions - packets sent by vmbus_sendpacket_
pagebuffer() and vmbus_sendpacket_mpb_desc(). These packets
contains IO stack memory address and host will access these memory.
So add allocation bounce buffer support in vmbus for these packets.

For SNP isolation VM, guest needs to access the shared memory via
extra address space which is specified by Hyper-V CPUID HYPERV_CPUID_
ISOLATION_CONFIG. The access physical address of the shared memory
should be bounce buffer memory GPA plus with shared_gpa_boundary
reported by CPUID.

Tianyu Lan (12):
  x86/Hyper-V: Add visibility parameter for vmbus_establish_gpadl()
  x86/Hyper-V: Add new hvcall guest address host visibility support
  x86/HV: Initialize GHCB page and shared memory boundary
  HV: Add Write/Read MSR registers via ghcb
  HV: Add ghcb hvcall support for SNP VM
  HV/Vmbus: Add SNP support for VMbus channel initiate message
  hv/vmbus: Initialize VMbus ring buffer for Isolation VM
  x86/Hyper-V: Initialize bounce buffer page cache and list
  x86/Hyper-V: Add new parameter for
    vmbus_sendpacket_pagebuffer()/mpb_desc()
  HV: Add bounce buffer support for Isolation VM
  HV/Netvsc: Add Isolation VM support for netvsc driver
  HV/Storvsc: Add bounce buffer support for Storvsc

 arch/x86/hyperv/Makefile           |   2 +-
 arch/x86/hyperv/hv_init.c          |  70 +++-
 arch/x86/hyperv/ivm.c              | 257 ++++++++++++
 arch/x86/include/asm/hyperv-tlfs.h |  22 +
 arch/x86/include/asm/mshyperv.h    |  26 +-
 arch/x86/kernel/cpu/mshyperv.c     |   2 +
 drivers/hv/Makefile                |   2 +-
 drivers/hv/channel.c               | 103 ++++-
 drivers/hv/channel_mgmt.c          |  30 +-
 drivers/hv/connection.c            |  68 +++-
 drivers/hv/hv.c                    | 196 ++++++---
 drivers/hv/hv_bounce.c             | 619 +++++++++++++++++++++++++++++
 drivers/hv/hyperv_vmbus.h          |  42 ++
 drivers/hv/ring_buffer.c           |  83 +++-
 drivers/net/hyperv/hyperv_net.h    |   5 +
 drivers/net/hyperv/netvsc.c        | 111 +++++-
 drivers/scsi/storvsc_drv.c         |  46 ++-
 drivers/uio/uio_hv_generic.c       |  13 +-
 include/asm-generic/hyperv-tlfs.h  |   1 +
 include/asm-generic/mshyperv.h     |  24 +-
 include/linux/hyperv.h             |  46 ++-
 mm/ioremap.c                       |   1 +
 mm/vmalloc.c                       |   1 +
 23 files changed, 1614 insertions(+), 156 deletions(-)
 create mode 100644 arch/x86/hyperv/ivm.c
 create mode 100644 drivers/hv/hv_bounce.c

-- 
2.25.1


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

* [RFC PATCH 1/12] x86/Hyper-V: Add visibility parameter for vmbus_establish_gpadl()
  2021-02-28 15:03 [RFC PATCH 00/12] x86/Hyper-V: Add Hyper-V Isolation VM support Tianyu Lan
@ 2021-02-28 15:03 ` Tianyu Lan
  2021-03-03 16:27   ` Vitaly Kuznetsov
  2021-02-28 15:03 ` [RFC PATCH 2/12] x86/Hyper-V: Add new hvcall guest address host visibility support Tianyu Lan
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Tianyu Lan @ 2021-02-28 15:03 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin, wei.liu, tglx, mingo, bp, x86, hpa,
	davem, kuba, gregkh
  Cc: Tianyu Lan, linux-hyperv, linux-kernel, netdev, vkuznets,
	thomas.lendacky, brijesh.singh, sunilmut

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

Add visibility parameter for vmbus_establish_gpadl() and prepare
to change host visibility when create gpadl for buffer.

Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
 arch/x86/include/asm/hyperv-tlfs.h |  9 +++++++++
 drivers/hv/channel.c               | 20 +++++++++++---------
 drivers/net/hyperv/netvsc.c        |  8 ++++++--
 drivers/uio/uio_hv_generic.c       |  7 +++++--
 include/linux/hyperv.h             |  3 ++-
 5 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index e6cd3fee562b..fb1893a4c32b 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -236,6 +236,15 @@ enum hv_isolation_type {
 /* TSC invariant control */
 #define HV_X64_MSR_TSC_INVARIANT_CONTROL	0x40000118
 
+/* Hyper-V GPA map flags */
+#define HV_MAP_GPA_PERMISSIONS_NONE		0x0
+#define HV_MAP_GPA_READABLE			0x1
+#define HV_MAP_GPA_WRITABLE			0x2
+
+#define VMBUS_PAGE_VISIBLE_READ_ONLY HV_MAP_GPA_READABLE
+#define VMBUS_PAGE_VISIBLE_READ_WRITE (HV_MAP_GPA_READABLE|HV_MAP_GPA_WRITABLE)
+#define VMBUS_PAGE_NOT_VISIBLE HV_MAP_GPA_PERMISSIONS_NONE
+
 /*
  * Declare the MSR used to setup pages used to communicate with the hypervisor.
  */
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 0bd202de7960..daa21cc72beb 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -242,7 +242,7 @@ EXPORT_SYMBOL_GPL(vmbus_send_modifychannel);
  */
 static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
 			       u32 size, u32 send_offset,
-			       struct vmbus_channel_msginfo **msginfo)
+			       struct vmbus_channel_msginfo **msginfo, u32 visibility)
 {
 	int i;
 	int pagecount;
@@ -391,7 +391,7 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
 static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
 				   enum hv_gpadl_type type, void *kbuffer,
 				   u32 size, u32 send_offset,
-				   u32 *gpadl_handle)
+				   u32 *gpadl_handle, u32 visibility)
 {
 	struct vmbus_channel_gpadl_header *gpadlmsg;
 	struct vmbus_channel_gpadl_body *gpadl_body;
@@ -405,7 +405,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
 	next_gpadl_handle =
 		(atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1);
 
-	ret = create_gpadl_header(type, kbuffer, size, send_offset, &msginfo);
+	ret = create_gpadl_header(type, kbuffer, size, send_offset,
+				  &msginfo, visibility);
 	if (ret)
 		return ret;
 
@@ -496,10 +497,10 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
  * @gpadl_handle: some funky thing
  */
 int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
-			  u32 size, u32 *gpadl_handle)
+			  u32 size, u32 *gpadl_handle, u32 visibility)
 {
 	return __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer, size,
-				       0U, gpadl_handle);
+				       0U, gpadl_handle, visibility);
 }
 EXPORT_SYMBOL_GPL(vmbus_establish_gpadl);
 
@@ -610,10 +611,11 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
 	newchannel->ringbuffer_gpadlhandle = 0;
 
 	err = __vmbus_establish_gpadl(newchannel, HV_GPADL_RING,
-				      page_address(newchannel->ringbuffer_page),
-				      (send_pages + recv_pages) << PAGE_SHIFT,
-				      newchannel->ringbuffer_send_offset << PAGE_SHIFT,
-				      &newchannel->ringbuffer_gpadlhandle);
+			page_address(newchannel->ringbuffer_page),
+			(send_pages + recv_pages) << PAGE_SHIFT,
+			newchannel->ringbuffer_send_offset << PAGE_SHIFT,
+			&newchannel->ringbuffer_gpadlhandle,
+			VMBUS_PAGE_VISIBLE_READ_WRITE);
 	if (err)
 		goto error_clean_ring;
 
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 2353623259f3..bb72c7578330 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -333,7 +333,8 @@ static int netvsc_init_buf(struct hv_device *device,
 	 */
 	ret = vmbus_establish_gpadl(device->channel, net_device->recv_buf,
 				    buf_size,
-				    &net_device->recv_buf_gpadl_handle);
+				    &net_device->recv_buf_gpadl_handle,
+				    VMBUS_PAGE_VISIBLE_READ_WRITE);
 	if (ret != 0) {
 		netdev_err(ndev,
 			"unable to establish receive buffer's gpadl\n");
@@ -422,10 +423,13 @@ static int netvsc_init_buf(struct hv_device *device,
 	/* Establish the gpadl handle for this buffer on this
 	 * channel.  Note: This call uses the vmbus connection rather
 	 * than the channel to establish the gpadl handle.
+	 * Send buffer should theoretically be only marked as "read-only", but
+	 * the netvsp for some reason needs write capabilities on it.
 	 */
 	ret = vmbus_establish_gpadl(device->channel, net_device->send_buf,
 				    buf_size,
-				    &net_device->send_buf_gpadl_handle);
+				    &net_device->send_buf_gpadl_handle,
+				    VMBUS_PAGE_VISIBLE_READ_WRITE);
 	if (ret != 0) {
 		netdev_err(ndev,
 			   "unable to establish send buffer's gpadl\n");
diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index 0330ba99730e..813a7bee5139 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -29,6 +29,7 @@
 #include <linux/hyperv.h>
 #include <linux/vmalloc.h>
 #include <linux/slab.h>
+#include <asm/mshyperv.h>
 
 #include "../hv/hyperv_vmbus.h"
 
@@ -295,7 +296,8 @@ hv_uio_probe(struct hv_device *dev,
 	}
 
 	ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
-				    RECV_BUFFER_SIZE, &pdata->recv_gpadl);
+				    RECV_BUFFER_SIZE, &pdata->recv_gpadl,
+				    VMBUS_PAGE_VISIBLE_READ_WRITE);
 	if (ret)
 		goto fail_close;
 
@@ -315,7 +317,8 @@ hv_uio_probe(struct hv_device *dev,
 	}
 
 	ret = vmbus_establish_gpadl(channel, pdata->send_buf,
-				    SEND_BUFFER_SIZE, &pdata->send_gpadl);
+				    SEND_BUFFER_SIZE, &pdata->send_gpadl,
+				    VMBUS_PAGE_VISIBLE_READ_ONLY);
 	if (ret)
 		goto fail_close;
 
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index f1d74dcf0353..016fdca20d6e 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1179,7 +1179,8 @@ extern int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
 extern int vmbus_establish_gpadl(struct vmbus_channel *channel,
 				      void *kbuffer,
 				      u32 size,
-				      u32 *gpadl_handle);
+				      u32 *gpadl_handle,
+				      u32 visibility);
 
 extern int vmbus_teardown_gpadl(struct vmbus_channel *channel,
 				     u32 gpadl_handle);
-- 
2.25.1


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

* [RFC PATCH 2/12] x86/Hyper-V: Add new hvcall guest address host visibility support
  2021-02-28 15:03 [RFC PATCH 00/12] x86/Hyper-V: Add Hyper-V Isolation VM support Tianyu Lan
  2021-02-28 15:03 ` [RFC PATCH 1/12] x86/Hyper-V: Add visibility parameter for vmbus_establish_gpadl() Tianyu Lan
@ 2021-02-28 15:03 ` Tianyu Lan
  2021-03-03 16:58   ` Vitaly Kuznetsov
  2021-02-28 15:03 ` [RFC PATCH 9/12] x86/Hyper-V: Add new parameter for vmbus_sendpacket_pagebuffer()/mpb_desc() Tianyu Lan
  2021-02-28 15:03 ` [RFC PATCH 11/12] HV/Netvsc: Add Isolation VM support for netvsc driver Tianyu Lan
  3 siblings, 1 reply; 9+ messages in thread
From: Tianyu Lan @ 2021-02-28 15:03 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin, wei.liu, tglx, mingo, bp, x86, hpa,
	davem, kuba, gregkh, arnd
  Cc: Tianyu Lan, linux-hyperv, linux-kernel, netdev, linux-arch,
	vkuznets, thomas.lendacky, brijesh.singh, sunilmut

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

Add new hvcall guest address host visibility support. Mark vmbus
ring buffer visible to host when create gpadl buffer and mark back
to not visible when tear down gpadl buffer.

Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
 arch/x86/include/asm/hyperv-tlfs.h | 13 ++++++++
 arch/x86/include/asm/mshyperv.h    |  4 +--
 arch/x86/kernel/cpu/mshyperv.c     | 46 ++++++++++++++++++++++++++
 drivers/hv/channel.c               | 53 ++++++++++++++++++++++++++++--
 drivers/net/hyperv/hyperv_net.h    |  1 +
 drivers/net/hyperv/netvsc.c        |  9 +++--
 drivers/uio/uio_hv_generic.c       |  6 ++--
 include/asm-generic/hyperv-tlfs.h  |  1 +
 include/linux/hyperv.h             |  3 +-
 9 files changed, 126 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index fb1893a4c32b..d22b1c3f425a 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -573,4 +573,17 @@ enum hv_interrupt_type {
 
 #include <asm-generic/hyperv-tlfs.h>
 
+/* All input parameters should be in single page. */
+#define HV_MAX_MODIFY_GPA_REP_COUNT		\
+	((PAGE_SIZE - 2 * sizeof(u64)) / (sizeof(u64)))
+
+/* HvCallModifySparseGpaPageHostVisibility hypercall */
+struct hv_input_modify_sparse_gpa_page_host_visibility {
+	u64 partition_id;
+	u32 host_visibility:2;
+	u32 reserved0:30;
+	u32 reserved1;
+	u64 gpa_page_list[HV_MAX_MODIFY_GPA_REP_COUNT];
+} __packed;
+
 #endif
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index ccf60a809a17..1e8275d35c1f 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -262,13 +262,13 @@ static inline void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry,
 	msi_entry->address.as_uint32 = msi_desc->msg.address_lo;
 	msi_entry->data.as_uint32 = msi_desc->msg.data;
 }
-
 struct irq_domain *hv_create_pci_msi_domain(void);
 
 int hv_map_ioapic_interrupt(int ioapic_id, bool level, int vcpu, int vector,
 		struct hv_interrupt_entry *entry);
 int hv_unmap_ioapic_interrupt(int ioapic_id, struct hv_interrupt_entry *entry);
-
+int hv_set_mem_host_visibility(void *kbuffer, u32 size, u32 visibility);
+int hv_mark_gpa_visibility(u16 count, const u64 pfn[], u32 visibility);
 #else /* CONFIG_HYPERV */
 static inline void hyperv_init(void) {}
 static inline void hyperv_setup_mmu_ops(void) {}
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index e88bc296afca..347c32eac8fd 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -37,6 +37,8 @@
 bool hv_root_partition;
 EXPORT_SYMBOL_GPL(hv_root_partition);
 
+#define HV_PARTITION_ID_SELF ((u64)-1)
+
 struct ms_hyperv_info ms_hyperv;
 EXPORT_SYMBOL_GPL(ms_hyperv);
 
@@ -477,3 +479,47 @@ const __initconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
 	.init.msi_ext_dest_id	= ms_hyperv_msi_ext_dest_id,
 	.init.init_platform	= ms_hyperv_init_platform,
 };
+
+int hv_mark_gpa_visibility(u16 count, const u64 pfn[], u32 visibility)
+{
+	struct hv_input_modify_sparse_gpa_page_host_visibility **input_pcpu;
+	struct hv_input_modify_sparse_gpa_page_host_visibility *input;
+	u16 pages_processed;
+	u64 hv_status;
+	unsigned long flags;
+
+	/* no-op if partition isolation is not enabled */
+	if (!hv_is_isolation_supported())
+		return 0;
+
+	if (count > HV_MAX_MODIFY_GPA_REP_COUNT) {
+		pr_err("Hyper-V: GPA count:%d exceeds supported:%lu\n", count,
+			HV_MAX_MODIFY_GPA_REP_COUNT);
+		return -EINVAL;
+	}
+
+	local_irq_save(flags);
+	input_pcpu = (struct hv_input_modify_sparse_gpa_page_host_visibility **)
+			this_cpu_ptr(hyperv_pcpu_input_arg);
+	input = *input_pcpu;
+	if (unlikely(!input)) {
+		local_irq_restore(flags);
+		return -1;
+	}
+
+	input->partition_id = HV_PARTITION_ID_SELF;
+	input->host_visibility = visibility;
+	input->reserved0 = 0;
+	input->reserved1 = 0;
+	memcpy((void *)input->gpa_page_list, pfn, count * sizeof(*pfn));
+	hv_status = hv_do_rep_hypercall(
+			HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY, count,
+			0, input, &pages_processed);
+	local_irq_restore(flags);
+
+	if (!(hv_status & HV_HYPERCALL_RESULT_MASK))
+		return 0;
+
+	return -EFAULT;
+}
+EXPORT_SYMBOL(hv_mark_gpa_visibility);
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index daa21cc72beb..204e6f3598a5 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -237,6 +237,38 @@ int vmbus_send_modifychannel(u32 child_relid, u32 target_vp)
 }
 EXPORT_SYMBOL_GPL(vmbus_send_modifychannel);
 
+/*
+ * hv_set_mem_host_visibility - Set host visibility for specified memory.
+ */
+int hv_set_mem_host_visibility(void *kbuffer, u32 size, u32 visibility)
+{
+	int i, pfn;
+	int pagecount = size >> HV_HYP_PAGE_SHIFT;
+	u64 *pfn_array;
+	int ret = 0;
+
+	if (!hv_isolation_type_snp())
+		return 0;
+
+	pfn_array = vzalloc(HV_HYP_PAGE_SIZE);
+	if (!pfn_array)
+		return -ENOMEM;
+
+	for (i = 0, pfn = 0; i < pagecount; i++) {
+		pfn_array[pfn] = virt_to_hvpfn(kbuffer + i * HV_HYP_PAGE_SIZE);
+		pfn++;
+
+		if (pfn == HV_MAX_MODIFY_GPA_REP_COUNT || i == pagecount - 1) {
+			ret |= hv_mark_gpa_visibility(pfn, pfn_array, visibility);
+			pfn = 0;
+		}
+	}
+
+	vfree(pfn_array);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(hv_set_mem_host_visibility);
+
 /*
  * create_gpadl_header - Creates a gpadl for the specified buffer
  */
@@ -410,6 +442,12 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
 	if (ret)
 		return ret;
 
+	ret = hv_set_mem_host_visibility(kbuffer, size, visibility);
+	if (ret) {
+		pr_warn("Failed to set host visibility.\n");
+		return ret;
+	}
+
 	init_completion(&msginfo->waitevent);
 	msginfo->waiting_channel = channel;
 
@@ -693,7 +731,9 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
 error_free_info:
 	kfree(open_info);
 error_free_gpadl:
-	vmbus_teardown_gpadl(newchannel, newchannel->ringbuffer_gpadlhandle);
+	vmbus_teardown_gpadl(newchannel, newchannel->ringbuffer_gpadlhandle,
+			     page_address(newchannel->ringbuffer_page),
+			     newchannel->ringbuffer_pagecount << PAGE_SHIFT);
 	newchannel->ringbuffer_gpadlhandle = 0;
 error_clean_ring:
 	hv_ringbuffer_cleanup(&newchannel->outbound);
@@ -740,7 +780,8 @@ EXPORT_SYMBOL_GPL(vmbus_open);
 /*
  * vmbus_teardown_gpadl -Teardown the specified GPADL handle
  */
-int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)
+int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle,
+			 void *kbuffer, u32 size)
 {
 	struct vmbus_channel_gpadl_teardown *msg;
 	struct vmbus_channel_msginfo *info;
@@ -793,6 +834,10 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)
 	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
 
 	kfree(info);
+
+	if (hv_set_mem_host_visibility(kbuffer, size, VMBUS_PAGE_NOT_VISIBLE))
+		pr_warn("Fail to set mem host visibility.\n");
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(vmbus_teardown_gpadl);
@@ -869,7 +914,9 @@ static int vmbus_close_internal(struct vmbus_channel *channel)
 	/* Tear down the gpadl for the channel's ring buffer */
 	else if (channel->ringbuffer_gpadlhandle) {
 		ret = vmbus_teardown_gpadl(channel,
-					   channel->ringbuffer_gpadlhandle);
+					   channel->ringbuffer_gpadlhandle,
+					   page_address(channel->ringbuffer_page),
+					   channel->ringbuffer_pagecount << PAGE_SHIFT);
 		if (ret) {
 			pr_err("Close failed: teardown gpadl return %d\n", ret);
 			/*
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 2a87cfa27ac0..b3a43c4ec8ab 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -1034,6 +1034,7 @@ struct netvsc_device {
 
 	/* Send buffer allocated by us */
 	void *send_buf;
+	u32 send_buf_size;
 	u32 send_buf_gpadl_handle;
 	u32 send_section_cnt;
 	u32 send_section_size;
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index bb72c7578330..08d73401bb28 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -245,7 +245,9 @@ static void netvsc_teardown_recv_gpadl(struct hv_device *device,
 
 	if (net_device->recv_buf_gpadl_handle) {
 		ret = vmbus_teardown_gpadl(device->channel,
-					   net_device->recv_buf_gpadl_handle);
+					   net_device->recv_buf_gpadl_handle,
+					   net_device->recv_buf,
+					   net_device->recv_buf_size);
 
 		/* If we failed here, we might as well return and have a leak
 		 * rather than continue and a bugchk
@@ -267,7 +269,9 @@ static void netvsc_teardown_send_gpadl(struct hv_device *device,
 
 	if (net_device->send_buf_gpadl_handle) {
 		ret = vmbus_teardown_gpadl(device->channel,
-					   net_device->send_buf_gpadl_handle);
+					   net_device->send_buf_gpadl_handle,
+					   net_device->send_buf,
+					   net_device->send_buf_size);
 
 		/* If we failed here, we might as well return and have a leak
 		 * rather than continue and a bugchk
@@ -419,6 +423,7 @@ static int netvsc_init_buf(struct hv_device *device,
 		ret = -ENOMEM;
 		goto cleanup;
 	}
+	net_device->send_buf_size = buf_size;
 
 	/* Establish the gpadl handle for this buffer on this
 	 * channel.  Note: This call uses the vmbus connection rather
diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index 813a7bee5139..c8d4704fc90c 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -181,13 +181,15 @@ static void
 hv_uio_cleanup(struct hv_device *dev, struct hv_uio_private_data *pdata)
 {
 	if (pdata->send_gpadl) {
-		vmbus_teardown_gpadl(dev->channel, pdata->send_gpadl);
+		vmbus_teardown_gpadl(dev->channel, pdata->send_gpadl,
+				     pdata->send_buf, SEND_BUFFER_SIZE);
 		pdata->send_gpadl = 0;
 		vfree(pdata->send_buf);
 	}
 
 	if (pdata->recv_gpadl) {
-		vmbus_teardown_gpadl(dev->channel, pdata->recv_gpadl);
+		vmbus_teardown_gpadl(dev->channel, pdata->recv_gpadl,
+				     pdata->recv_buf, RECV_BUFFER_SIZE);
 		pdata->recv_gpadl = 0;
 		vfree(pdata->recv_buf);
 	}
diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
index 83448e837ded..ad19f4199f90 100644
--- a/include/asm-generic/hyperv-tlfs.h
+++ b/include/asm-generic/hyperv-tlfs.h
@@ -158,6 +158,7 @@ struct ms_hyperv_tsc_page {
 #define HVCALL_RETARGET_INTERRUPT		0x007e
 #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
 #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
+#define HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY 0x00db
 
 #define HV_FLUSH_ALL_PROCESSORS			BIT(0)
 #define HV_FLUSH_ALL_VIRTUAL_ADDRESS_SPACES	BIT(1)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 016fdca20d6e..41cbaa2db567 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1183,7 +1183,8 @@ extern int vmbus_establish_gpadl(struct vmbus_channel *channel,
 				      u32 visibility);
 
 extern int vmbus_teardown_gpadl(struct vmbus_channel *channel,
-				     u32 gpadl_handle);
+				u32 gpadl_handle,
+				void *kbuffer, u32 size);
 
 void vmbus_reset_channel_cb(struct vmbus_channel *channel);
 
-- 
2.25.1


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

* [RFC PATCH 9/12] x86/Hyper-V: Add new parameter for vmbus_sendpacket_pagebuffer()/mpb_desc()
  2021-02-28 15:03 [RFC PATCH 00/12] x86/Hyper-V: Add Hyper-V Isolation VM support Tianyu Lan
  2021-02-28 15:03 ` [RFC PATCH 1/12] x86/Hyper-V: Add visibility parameter for vmbus_establish_gpadl() Tianyu Lan
  2021-02-28 15:03 ` [RFC PATCH 2/12] x86/Hyper-V: Add new hvcall guest address host visibility support Tianyu Lan
@ 2021-02-28 15:03 ` Tianyu Lan
  2021-02-28 15:03 ` [RFC PATCH 11/12] HV/Netvsc: Add Isolation VM support for netvsc driver Tianyu Lan
  3 siblings, 0 replies; 9+ messages in thread
From: Tianyu Lan @ 2021-02-28 15:03 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin, wei.liu, davem, kuba, jejb, martin.petersen
  Cc: Tianyu Lan, linux-hyperv, linux-kernel, netdev, linux-scsi,
	vkuznets, thomas.lendacky, brijesh.singh, sunilmut

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

Add new parameter io_type and struct bounce_pkt for vmbus_sendpacket_pagebuffer()
and vmbus_sendpacket_mpb_desc() in order to add bounce buffer support
later.

Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
 drivers/hv/channel.c            |  7 +++++--
 drivers/hv/hyperv_vmbus.h       | 12 ++++++++++++
 drivers/net/hyperv/hyperv_net.h |  1 +
 drivers/net/hyperv/netvsc.c     |  5 ++++-
 drivers/scsi/storvsc_drv.c      | 23 +++++++++++++++++------
 include/linux/hyperv.h          | 16 ++++++++++++++--
 6 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 4c05b1488649..976ef99dda28 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -1044,7 +1044,8 @@ EXPORT_SYMBOL(vmbus_sendpacket);
 int vmbus_sendpacket_pagebuffer(struct vmbus_channel *channel,
 				struct hv_page_buffer pagebuffers[],
 				u32 pagecount, void *buffer, u32 bufferlen,
-				u64 requestid)
+				u64 requestid, u8 io_type,
+				struct hv_bounce_pkt **bounce_pkt)
 {
 	int i;
 	struct vmbus_channel_packet_page_buffer desc;
@@ -1101,7 +1102,9 @@ EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer);
 int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
 			      struct vmbus_packet_mpb_array *desc,
 			      u32 desc_size,
-			      void *buffer, u32 bufferlen, u64 requestid)
+			      void *buffer, u32 bufferlen, u64 requestid,
+			      u32 pfn_count, u8 io_type,
+			      struct hv_bounce_pkt **bounce_pkt)
 {
 	u32 packetlen;
 	u32 packetlen_aligned;
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 7edf2be60d2c..7677f083d33a 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -57,6 +57,18 @@ union hv_monitor_trigger_state {
 	};
 };
 
+/*
+ * Hyper-V bounce packet. Each in-use bounce packet is mapped to a vmbus
+ * transaction and contains a list of bounce pages for that transaction.
+ */
+struct hv_bounce_pkt {
+	/* Link to the next bounce packet, when it is in the free list */
+	struct list_head link;
+	struct list_head bounce_page_head;
+	u32 flags;
+};
+
+
 /*
  * All vmbus channels initially start with zero bounce pages and are required
  * to set any non-zero size, if needed.
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index b3a43c4ec8ab..11266b92bcf0 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -130,6 +130,7 @@ struct hv_netvsc_packet {
 	u32 total_bytes;
 	u32 send_buf_index;
 	u32 total_data_buflen;
+	struct hv_bounce_pkt *bounce_pkt;
 };
 
 #define NETVSC_HASH_KEYLEN 40
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 08d73401bb28..77657c5acc65 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -926,14 +926,17 @@ static inline int netvsc_send_pkt(
 
 	trace_nvsp_send_pkt(ndev, out_channel, rpkt);
 
+	packet->bounce_pkt = NULL;
 	if (packet->page_buf_cnt) {
 		if (packet->cp_partial)
 			pb += packet->rmsg_pgcnt;
 
+		/* The I/O type is always 'write' for netvsc */
 		ret = vmbus_sendpacket_pagebuffer(out_channel,
 						  pb, packet->page_buf_cnt,
 						  &nvmsg, sizeof(nvmsg),
-						  req_id);
+						  req_id, IO_TYPE_WRITE,
+						  &packet->bounce_pkt);
 	} else {
 		ret = vmbus_sendpacket(out_channel,
 				       &nvmsg, sizeof(nvmsg),
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 2e4fa77445fd..c5b4974eb41f 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -31,6 +31,7 @@
 #include <scsi/scsi_dbg.h>
 #include <scsi/scsi_transport_fc.h>
 #include <scsi/scsi_transport.h>
+#include <asm/mshyperv.h>
 
 /*
  * All wire protocol details (storage protocol between the guest and the host)
@@ -427,6 +428,7 @@ struct storvsc_cmd_request {
 	u32 payload_sz;
 
 	struct vstor_packet vstor_packet;
+	struct hv_bounce_pkt *bounce_pkt;
 };
 
 
@@ -1390,7 +1392,8 @@ static struct vmbus_channel *get_og_chn(struct storvsc_device *stor_device,
 
 
 static int storvsc_do_io(struct hv_device *device,
-			 struct storvsc_cmd_request *request, u16 q_num)
+			 struct storvsc_cmd_request *request, u16 q_num,
+			 u32 pfn_count)
 {
 	struct storvsc_device *stor_device;
 	struct vstor_packet *vstor_packet;
@@ -1493,14 +1496,18 @@ static int storvsc_do_io(struct hv_device *device,
 
 	vstor_packet->operation = VSTOR_OPERATION_EXECUTE_SRB;
 
+	request->bounce_pkt = NULL;
 	if (request->payload->range.len) {
+		struct vmscsi_request *vm_srb = &request->vstor_packet.vm_srb;
 
 		ret = vmbus_sendpacket_mpb_desc(outgoing_channel,
 				request->payload, request->payload_sz,
 				vstor_packet,
 				(sizeof(struct vstor_packet) -
 				vmscsi_size_delta),
-				(unsigned long)request);
+				(unsigned long)request,
+				pfn_count,
+				vm_srb->data_in, &request->bounce_pkt);
 	} else {
 		ret = vmbus_sendpacket(outgoing_channel, vstor_packet,
 			       (sizeof(struct vstor_packet) -
@@ -1510,8 +1517,10 @@ static int storvsc_do_io(struct hv_device *device,
 			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
 	}
 
-	if (ret != 0)
+	if (ret != 0) {
+		request->bounce_pkt = NULL;
 		return ret;
+	}
 
 	atomic_inc(&stor_device->num_outstanding_req);
 
@@ -1825,14 +1834,16 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd)
 	cmd_request->payload_sz = payload_sz;
 
 	/* Invokes the vsc to start an IO */
-	ret = storvsc_do_io(dev, cmd_request, get_cpu());
+	ret = storvsc_do_io(dev, cmd_request, get_cpu(), sg_count);
 	put_cpu();
 
-	if (ret == -EAGAIN) {
+	if (ret) {
 		if (payload_sz > sizeof(cmd_request->mpb))
 			kfree(payload);
 		/* no more space */
-		return SCSI_MLQUEUE_DEVICE_BUSY;
+		if (ret == -EAGAIN || ret == -ENOSPC)
+			return SCSI_MLQUEUE_DEVICE_BUSY;
+		return ret;
 	}
 
 	return 0;
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index d518aba17565..d1a936091665 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1184,19 +1184,31 @@ extern int vmbus_sendpacket(struct vmbus_channel *channel,
 				  enum vmbus_packet_type type,
 				  u32 flags);
 
+#define IO_TYPE_WRITE	0
+#define IO_TYPE_READ	1
+#define IO_TYPE_UNKNOWN 2
+
+struct hv_bounce_pkt;
+
 extern int vmbus_sendpacket_pagebuffer(struct vmbus_channel *channel,
 					    struct hv_page_buffer pagebuffers[],
 					    u32 pagecount,
 					    void *buffer,
 					    u32 bufferlen,
-					    u64 requestid);
+					    u64 requestid,
+					    u8 io_type,
+					    struct hv_bounce_pkt **bounce_pkt);
 
 extern int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
 				     struct vmbus_packet_mpb_array *mpb,
 				     u32 desc_size,
 				     void *buffer,
 				     u32 bufferlen,
-				     u64 requestid);
+				     u64 requestid,
+				     u32 pfn_count,
+				     u8 io_type,
+				     struct hv_bounce_pkt **bounce_pkt);
+
 
 extern int vmbus_establish_gpadl(struct vmbus_channel *channel,
 				      void *kbuffer,
-- 
2.25.1


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

* [RFC PATCH 11/12] HV/Netvsc: Add Isolation VM support for netvsc driver
  2021-02-28 15:03 [RFC PATCH 00/12] x86/Hyper-V: Add Hyper-V Isolation VM support Tianyu Lan
                   ` (2 preceding siblings ...)
  2021-02-28 15:03 ` [RFC PATCH 9/12] x86/Hyper-V: Add new parameter for vmbus_sendpacket_pagebuffer()/mpb_desc() Tianyu Lan
@ 2021-02-28 15:03 ` Tianyu Lan
  3 siblings, 0 replies; 9+ messages in thread
From: Tianyu Lan @ 2021-02-28 15:03 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin, wei.liu, davem, kuba
  Cc: Tianyu Lan, linux-hyperv, netdev, linux-kernel, vkuznets,
	thomas.lendacky, brijesh.singh, sunilmut

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

Add Isolation VM support for netvsc driver. Map send/receive
ring buffer in extra address space in SNP isolation VM, reserve
bounce buffer for packets sent via vmbus_sendpacket_pagebuffer()
and release bounce buffer via hv_pkt_bounce() when get send
complete response from host.

Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h |  3 +
 drivers/net/hyperv/netvsc.c     | 97 ++++++++++++++++++++++++++++++---
 2 files changed, 92 insertions(+), 8 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 11266b92bcf0..45d5838ff128 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -1027,14 +1027,17 @@ struct netvsc_device {
 
 	/* Receive buffer allocated by us but manages by NetVSP */
 	void *recv_buf;
+	void *recv_original_buf;
 	u32 recv_buf_size; /* allocated bytes */
 	u32 recv_buf_gpadl_handle;
 	u32 recv_section_cnt;
 	u32 recv_section_size;
 	u32 recv_completion_cnt;
 
+
 	/* Send buffer allocated by us */
 	void *send_buf;
+	void *send_original_buf;
 	u32 send_buf_size;
 	u32 send_buf_gpadl_handle;
 	u32 send_section_cnt;
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 77657c5acc65..171af85e055d 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -26,7 +26,7 @@
 
 #include "hyperv_net.h"
 #include "netvsc_trace.h"
-
+#include "../../hv/hyperv_vmbus.h"
 /*
  * Switch the data path from the synthetic interface to the VF
  * interface.
@@ -119,8 +119,21 @@ static void free_netvsc_device(struct rcu_head *head)
 	int i;
 
 	kfree(nvdev->extension);
-	vfree(nvdev->recv_buf);
-	vfree(nvdev->send_buf);
+
+	if (nvdev->recv_original_buf) {
+		iounmap(nvdev->recv_buf);
+		vfree(nvdev->recv_original_buf);
+	} else {
+		vfree(nvdev->recv_buf);
+	}
+
+	if (nvdev->send_original_buf) {
+		iounmap(nvdev->send_buf);
+		vfree(nvdev->send_original_buf);
+	} else {
+		vfree(nvdev->send_buf);
+	}
+
 	kfree(nvdev->send_section_map);
 
 	for (i = 0; i < VRSS_CHANNEL_MAX; i++) {
@@ -241,13 +254,18 @@ static void netvsc_teardown_recv_gpadl(struct hv_device *device,
 				       struct netvsc_device *net_device,
 				       struct net_device *ndev)
 {
+	void *recv_buf;
 	int ret;
 
 	if (net_device->recv_buf_gpadl_handle) {
+		if (net_device->recv_original_buf)
+			recv_buf = net_device->recv_original_buf;
+		else
+			recv_buf = net_device->recv_buf;
+
 		ret = vmbus_teardown_gpadl(device->channel,
 					   net_device->recv_buf_gpadl_handle,
-					   net_device->recv_buf,
-					   net_device->recv_buf_size);
+					   recv_buf, net_device->recv_buf_size);
 
 		/* If we failed here, we might as well return and have a leak
 		 * rather than continue and a bugchk
@@ -265,13 +283,18 @@ static void netvsc_teardown_send_gpadl(struct hv_device *device,
 				       struct netvsc_device *net_device,
 				       struct net_device *ndev)
 {
+	void *send_buf;
 	int ret;
 
 	if (net_device->send_buf_gpadl_handle) {
+		if (net_device->send_original_buf)
+			send_buf = net_device->send_original_buf;
+		else
+			send_buf = net_device->send_buf;
+
 		ret = vmbus_teardown_gpadl(device->channel,
 					   net_device->send_buf_gpadl_handle,
-					   net_device->send_buf,
-					   net_device->send_buf_size);
+					   send_buf, net_device->send_buf_size);
 
 		/* If we failed here, we might as well return and have a leak
 		 * rather than continue and a bugchk
@@ -306,9 +329,19 @@ static int netvsc_init_buf(struct hv_device *device,
 	struct nvsp_1_message_send_receive_buffer_complete *resp;
 	struct net_device *ndev = hv_get_drvdata(device);
 	struct nvsp_message *init_packet;
+	struct vm_struct *area;
+	u64 extra_phys;
 	unsigned int buf_size;
+	unsigned long vaddr;
 	size_t map_words;
-	int ret = 0;
+	int ret = 0, i;
+
+	ret = hv_bounce_resources_reserve(device->channel,
+			PAGE_SIZE * 1024);
+	if (ret) {
+		pr_warn("Fail to reserve bounce buffer.\n");
+		return -ENOMEM;
+	}
 
 	/* Get receive buffer area. */
 	buf_size = device_info->recv_sections * device_info->recv_section_size;
@@ -345,6 +378,28 @@ static int netvsc_init_buf(struct hv_device *device,
 		goto cleanup;
 	}
 
+	if (hv_isolation_type_snp()) {
+		area = get_vm_area(buf_size, VM_IOREMAP);
+		if (!area)
+			goto cleanup;
+
+		vaddr = (unsigned long)area->addr;
+		for (i = 0; i < buf_size / HV_HYP_PAGE_SIZE; i++) {
+			extra_phys = (virt_to_hvpfn(net_device->recv_buf + i * HV_HYP_PAGE_SIZE)
+				<< HV_HYP_PAGE_SHIFT) + ms_hyperv.shared_gpa_boundary;
+			ret |= ioremap_page_range(vaddr + i * HV_HYP_PAGE_SIZE,
+					   vaddr + (i + 1) * HV_HYP_PAGE_SIZE,
+					   extra_phys, PAGE_KERNEL_IO);
+		}
+
+		if (ret)
+			goto cleanup;
+
+		net_device->recv_original_buf = net_device->recv_buf;
+		net_device->recv_buf = (void*)vaddr;
+	}
+
+
 	/* Notify the NetVsp of the gpadl handle */
 	init_packet = &net_device->channel_init_pkt;
 	memset(init_packet, 0, sizeof(struct nvsp_message));
@@ -435,12 +490,36 @@ static int netvsc_init_buf(struct hv_device *device,
 				    buf_size,
 				    &net_device->send_buf_gpadl_handle,
 				    VMBUS_PAGE_VISIBLE_READ_WRITE);
+	
 	if (ret != 0) {
 		netdev_err(ndev,
 			   "unable to establish send buffer's gpadl\n");
 		goto cleanup;
 	}
 
+	if (hv_isolation_type_snp()) {
+		area = get_vm_area(buf_size , VM_IOREMAP);
+		if (!area)
+			goto cleanup;
+
+		vaddr = (unsigned long)area->addr;
+	
+		for (i = 0; i < buf_size / HV_HYP_PAGE_SIZE; i++) {
+			extra_phys = (virt_to_hvpfn(net_device->send_buf + i * HV_HYP_PAGE_SIZE)
+				<< HV_HYP_PAGE_SHIFT) + ms_hyperv.shared_gpa_boundary;
+			ret |= ioremap_page_range(vaddr + i * HV_HYP_PAGE_SIZE,
+					   vaddr + (i + 1) * HV_HYP_PAGE_SIZE,
+					   extra_phys, PAGE_KERNEL_IO);
+		}
+
+		if (ret)
+			goto cleanup;
+
+		net_device->send_original_buf = net_device->send_buf;
+		net_device->send_buf = (void*)vaddr;	
+	}
+
+	
 	/* Notify the NetVsp of the gpadl handle */
 	init_packet = &net_device->channel_init_pkt;
 	memset(init_packet, 0, sizeof(struct nvsp_message));
@@ -747,6 +826,8 @@ static void netvsc_send_tx_complete(struct net_device *ndev,
 		tx_stats->bytes += packet->total_bytes;
 		u64_stats_update_end(&tx_stats->syncp);
 
+		if (desc->type == VM_PKT_COMP && packet->bounce_pkt)
+			hv_pkt_bounce(channel, packet->bounce_pkt);
 		napi_consume_skb(skb, budget);
 	}
 
-- 
2.25.1


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

* Re: [RFC PATCH 1/12] x86/Hyper-V: Add visibility parameter for vmbus_establish_gpadl()
  2021-02-28 15:03 ` [RFC PATCH 1/12] x86/Hyper-V: Add visibility parameter for vmbus_establish_gpadl() Tianyu Lan
@ 2021-03-03 16:27   ` Vitaly Kuznetsov
  2021-03-05  6:06     ` Tianyu Lan
  0 siblings, 1 reply; 9+ messages in thread
From: Vitaly Kuznetsov @ 2021-03-03 16:27 UTC (permalink / raw)
  To: Tianyu Lan
  Cc: Tianyu Lan, linux-hyperv, linux-kernel, netdev, thomas.lendacky,
	brijesh.singh, sunilmut, kys, haiyangz, sthemmin, wei.liu, tglx,
	mingo, bp, x86, hpa, davem, kuba, gregkh

Tianyu Lan <ltykernel@gmail.com> writes:

> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>
> Add visibility parameter for vmbus_establish_gpadl() and prepare
> to change host visibility when create gpadl for buffer.
>

"No functional change" as you don't actually use the parameter.

> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>

Nit: Sunil's SoB looks misleading because the patch is from you,
Co-Developed-by should be sufficient.

> ---
>  arch/x86/include/asm/hyperv-tlfs.h |  9 +++++++++
>  drivers/hv/channel.c               | 20 +++++++++++---------
>  drivers/net/hyperv/netvsc.c        |  8 ++++++--
>  drivers/uio/uio_hv_generic.c       |  7 +++++--
>  include/linux/hyperv.h             |  3 ++-
>  5 files changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index e6cd3fee562b..fb1893a4c32b 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -236,6 +236,15 @@ enum hv_isolation_type {
>  /* TSC invariant control */
>  #define HV_X64_MSR_TSC_INVARIANT_CONTROL	0x40000118
>  
> +/* Hyper-V GPA map flags */
> +#define HV_MAP_GPA_PERMISSIONS_NONE		0x0
> +#define HV_MAP_GPA_READABLE			0x1
> +#define HV_MAP_GPA_WRITABLE			0x2
> +
> +#define VMBUS_PAGE_VISIBLE_READ_ONLY HV_MAP_GPA_READABLE
> +#define VMBUS_PAGE_VISIBLE_READ_WRITE (HV_MAP_GPA_READABLE|HV_MAP_GPA_WRITABLE)
> +#define VMBUS_PAGE_NOT_VISIBLE HV_MAP_GPA_PERMISSIONS_NONE
> +

Are these x86-only? If not, then we should probably move these defines
to include/asm-generic/hyperv-tlfs.h. In case they are, we should do
something as we're using them from arch neutral places.

Also, could you please add a comment stating that these flags define
host's visibility of a page and not guest's (this seems to be not
obvious at least to me).

>  /*
>   * Declare the MSR used to setup pages used to communicate with the hypervisor.
>   */
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 0bd202de7960..daa21cc72beb 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -242,7 +242,7 @@ EXPORT_SYMBOL_GPL(vmbus_send_modifychannel);
>   */
>  static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
>  			       u32 size, u32 send_offset,
> -			       struct vmbus_channel_msginfo **msginfo)
> +			       struct vmbus_channel_msginfo **msginfo, u32 visibility)
>  {
>  	int i;
>  	int pagecount;
> @@ -391,7 +391,7 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
>  static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>  				   enum hv_gpadl_type type, void *kbuffer,
>  				   u32 size, u32 send_offset,
> -				   u32 *gpadl_handle)
> +				   u32 *gpadl_handle, u32 visibility)
>  {
>  	struct vmbus_channel_gpadl_header *gpadlmsg;
>  	struct vmbus_channel_gpadl_body *gpadl_body;
> @@ -405,7 +405,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>  	next_gpadl_handle =
>  		(atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1);
>  
> -	ret = create_gpadl_header(type, kbuffer, size, send_offset, &msginfo);
> +	ret = create_gpadl_header(type, kbuffer, size, send_offset,
> +				  &msginfo, visibility);
>  	if (ret)
>  		return ret;
>  
> @@ -496,10 +497,10 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>   * @gpadl_handle: some funky thing
>   */
>  int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
> -			  u32 size, u32 *gpadl_handle)
> +			  u32 size, u32 *gpadl_handle, u32 visibility)
>  {
>  	return __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer, size,
> -				       0U, gpadl_handle);
> +				       0U, gpadl_handle, visibility);
>  }
>  EXPORT_SYMBOL_GPL(vmbus_establish_gpadl);
>  
> @@ -610,10 +611,11 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
>  	newchannel->ringbuffer_gpadlhandle = 0;
>  
>  	err = __vmbus_establish_gpadl(newchannel, HV_GPADL_RING,
> -				      page_address(newchannel->ringbuffer_page),
> -				      (send_pages + recv_pages) << PAGE_SHIFT,
> -				      newchannel->ringbuffer_send_offset << PAGE_SHIFT,
> -				      &newchannel->ringbuffer_gpadlhandle);
> +			page_address(newchannel->ringbuffer_page),
> +			(send_pages + recv_pages) << PAGE_SHIFT,
> +			newchannel->ringbuffer_send_offset << PAGE_SHIFT,
> +			&newchannel->ringbuffer_gpadlhandle,
> +			VMBUS_PAGE_VISIBLE_READ_WRITE);

Nit: I liked the original alignment more and we can avoid the unneeded
code churn.

>  	if (err)
>  		goto error_clean_ring;
>  
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 2353623259f3..bb72c7578330 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -333,7 +333,8 @@ static int netvsc_init_buf(struct hv_device *device,
>  	 */
>  	ret = vmbus_establish_gpadl(device->channel, net_device->recv_buf,
>  				    buf_size,
> -				    &net_device->recv_buf_gpadl_handle);
> +				    &net_device->recv_buf_gpadl_handle,
> +				    VMBUS_PAGE_VISIBLE_READ_WRITE);
>  	if (ret != 0) {
>  		netdev_err(ndev,
>  			"unable to establish receive buffer's gpadl\n");
> @@ -422,10 +423,13 @@ static int netvsc_init_buf(struct hv_device *device,
>  	/* Establish the gpadl handle for this buffer on this
>  	 * channel.  Note: This call uses the vmbus connection rather
>  	 * than the channel to establish the gpadl handle.
> +	 * Send buffer should theoretically be only marked as "read-only", but
> +	 * the netvsp for some reason needs write capabilities on it.
>  	 */
>  	ret = vmbus_establish_gpadl(device->channel, net_device->send_buf,
>  				    buf_size,
> -				    &net_device->send_buf_gpadl_handle);
> +				    &net_device->send_buf_gpadl_handle,
> +				    VMBUS_PAGE_VISIBLE_READ_WRITE);
>  	if (ret != 0) {
>  		netdev_err(ndev,
>  			   "unable to establish send buffer's gpadl\n");
> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
> index 0330ba99730e..813a7bee5139 100644
> --- a/drivers/uio/uio_hv_generic.c
> +++ b/drivers/uio/uio_hv_generic.c
> @@ -29,6 +29,7 @@
>  #include <linux/hyperv.h>
>  #include <linux/vmalloc.h>
>  #include <linux/slab.h>
> +#include <asm/mshyperv.h>
>  
>  #include "../hv/hyperv_vmbus.h"
>  
> @@ -295,7 +296,8 @@ hv_uio_probe(struct hv_device *dev,
>  	}
>  
>  	ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
> -				    RECV_BUFFER_SIZE, &pdata->recv_gpadl);
> +				    RECV_BUFFER_SIZE, &pdata->recv_gpadl,
> +				    VMBUS_PAGE_VISIBLE_READ_WRITE);
>  	if (ret)
>  		goto fail_close;
>  
> @@ -315,7 +317,8 @@ hv_uio_probe(struct hv_device *dev,
>  	}
>  
>  	ret = vmbus_establish_gpadl(channel, pdata->send_buf,
> -				    SEND_BUFFER_SIZE, &pdata->send_gpadl);
> +				    SEND_BUFFER_SIZE, &pdata->send_gpadl,
> +				    VMBUS_PAGE_VISIBLE_READ_ONLY);

Actually, this is the only place where you use 'READ_ONLY' mapping --
which makes me wonder if it's actually worth it or we can hard-code
VMBUS_PAGE_VISIBLE_READ_WRITE for now and avoid this additional
parameter.

>  	if (ret)
>  		goto fail_close;
>  
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index f1d74dcf0353..016fdca20d6e 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1179,7 +1179,8 @@ extern int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
>  extern int vmbus_establish_gpadl(struct vmbus_channel *channel,
>  				      void *kbuffer,
>  				      u32 size,
> -				      u32 *gpadl_handle);
> +				      u32 *gpadl_handle,
> +				      u32 visibility);
>  
>  extern int vmbus_teardown_gpadl(struct vmbus_channel *channel,
>  				     u32 gpadl_handle);

-- 
Vitaly


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

* Re: [RFC PATCH 2/12] x86/Hyper-V: Add new hvcall guest address host visibility support
  2021-02-28 15:03 ` [RFC PATCH 2/12] x86/Hyper-V: Add new hvcall guest address host visibility support Tianyu Lan
@ 2021-03-03 16:58   ` Vitaly Kuznetsov
  2021-03-05  6:23     ` Tianyu Lan
  0 siblings, 1 reply; 9+ messages in thread
From: Vitaly Kuznetsov @ 2021-03-03 16:58 UTC (permalink / raw)
  To: Tianyu Lan
  Cc: Tianyu Lan, linux-hyperv, linux-kernel, netdev, linux-arch,
	thomas.lendacky, brijesh.singh, sunilmut, kys, haiyangz,
	sthemmin, wei.liu, tglx, mingo, bp, x86, hpa, davem, kuba,
	gregkh, arnd

Tianyu Lan <ltykernel@gmail.com> writes:

> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>
> Add new hvcall guest address host visibility support. Mark vmbus
> ring buffer visible to host when create gpadl buffer and mark back
> to not visible when tear down gpadl buffer.
>
> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
>  arch/x86/include/asm/hyperv-tlfs.h | 13 ++++++++
>  arch/x86/include/asm/mshyperv.h    |  4 +--
>  arch/x86/kernel/cpu/mshyperv.c     | 46 ++++++++++++++++++++++++++
>  drivers/hv/channel.c               | 53 ++++++++++++++++++++++++++++--
>  drivers/net/hyperv/hyperv_net.h    |  1 +
>  drivers/net/hyperv/netvsc.c        |  9 +++--
>  drivers/uio/uio_hv_generic.c       |  6 ++--
>  include/asm-generic/hyperv-tlfs.h  |  1 +
>  include/linux/hyperv.h             |  3 +-
>  9 files changed, 126 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index fb1893a4c32b..d22b1c3f425a 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -573,4 +573,17 @@ enum hv_interrupt_type {
>  
>  #include <asm-generic/hyperv-tlfs.h>
>  
> +/* All input parameters should be in single page. */
> +#define HV_MAX_MODIFY_GPA_REP_COUNT		\
> +	((PAGE_SIZE - 2 * sizeof(u64)) / (sizeof(u64)))

Would it be easier to express this as '((PAGE_SIZE / sizeof(u64)) - 2' ?

> +
> +/* HvCallModifySparseGpaPageHostVisibility hypercall */
> +struct hv_input_modify_sparse_gpa_page_host_visibility {
> +	u64 partition_id;
> +	u32 host_visibility:2;
> +	u32 reserved0:30;
> +	u32 reserved1;
> +	u64 gpa_page_list[HV_MAX_MODIFY_GPA_REP_COUNT];
> +} __packed;
> +
>  #endif
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index ccf60a809a17..1e8275d35c1f 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -262,13 +262,13 @@ static inline void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry,
>  	msi_entry->address.as_uint32 = msi_desc->msg.address_lo;
>  	msi_entry->data.as_uint32 = msi_desc->msg.data;
>  }
> -

stray change

>  struct irq_domain *hv_create_pci_msi_domain(void);
>  
>  int hv_map_ioapic_interrupt(int ioapic_id, bool level, int vcpu, int vector,
>  		struct hv_interrupt_entry *entry);
>  int hv_unmap_ioapic_interrupt(int ioapic_id, struct hv_interrupt_entry *entry);
> -
> +int hv_set_mem_host_visibility(void *kbuffer, u32 size, u32 visibility);
> +int hv_mark_gpa_visibility(u16 count, const u64 pfn[], u32 visibility);
>  #else /* CONFIG_HYPERV */
>  static inline void hyperv_init(void) {}
>  static inline void hyperv_setup_mmu_ops(void) {}
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index e88bc296afca..347c32eac8fd 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -37,6 +37,8 @@
>  bool hv_root_partition;
>  EXPORT_SYMBOL_GPL(hv_root_partition);
>  
> +#define HV_PARTITION_ID_SELF ((u64)-1)
> +

We seem to have this already:

include/asm-generic/hyperv-tlfs.h:#define HV_PARTITION_ID_SELF          ((u64)-1)

>  struct ms_hyperv_info ms_hyperv;
>  EXPORT_SYMBOL_GPL(ms_hyperv);
>  
> @@ -477,3 +479,47 @@ const __initconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
>  	.init.msi_ext_dest_id	= ms_hyperv_msi_ext_dest_id,
>  	.init.init_platform	= ms_hyperv_init_platform,
>  };
> +
> +int hv_mark_gpa_visibility(u16 count, const u64 pfn[], u32 visibility)
> +{
> +	struct hv_input_modify_sparse_gpa_page_host_visibility **input_pcpu;
> +	struct hv_input_modify_sparse_gpa_page_host_visibility *input;
> +	u16 pages_processed;
> +	u64 hv_status;
> +	unsigned long flags;
> +
> +	/* no-op if partition isolation is not enabled */
> +	if (!hv_is_isolation_supported())
> +		return 0;
> +
> +	if (count > HV_MAX_MODIFY_GPA_REP_COUNT) {
> +		pr_err("Hyper-V: GPA count:%d exceeds supported:%lu\n", count,
> +			HV_MAX_MODIFY_GPA_REP_COUNT);
> +		return -EINVAL;
> +	}
> +
> +	local_irq_save(flags);
> +	input_pcpu = (struct hv_input_modify_sparse_gpa_page_host_visibility **)
> +			this_cpu_ptr(hyperv_pcpu_input_arg);
> +	input = *input_pcpu;
> +	if (unlikely(!input)) {
> +		local_irq_restore(flags);
> +		return -1;

-EFAULT/-ENOMEM/... maybe ?

> +	}
> +
> +	input->partition_id = HV_PARTITION_ID_SELF;
> +	input->host_visibility = visibility;
> +	input->reserved0 = 0;
> +	input->reserved1 = 0;
> +	memcpy((void *)input->gpa_page_list, pfn, count * sizeof(*pfn));
> +	hv_status = hv_do_rep_hypercall(
> +			HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY, count,
> +			0, input, &pages_processed);
> +	local_irq_restore(flags);
> +
> +	if (!(hv_status & HV_HYPERCALL_RESULT_MASK))
> +		return 0;
> +
> +	return -EFAULT;

Could we just propagate "hv_status & HV_HYPERCALL_RESULT_MASK" maybe?

> +}
> +EXPORT_SYMBOL(hv_mark_gpa_visibility);
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index daa21cc72beb..204e6f3598a5 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -237,6 +237,38 @@ int vmbus_send_modifychannel(u32 child_relid, u32 target_vp)
>  }
>  EXPORT_SYMBOL_GPL(vmbus_send_modifychannel);
>  
> +/*
> + * hv_set_mem_host_visibility - Set host visibility for specified memory.
> + */
> +int hv_set_mem_host_visibility(void *kbuffer, u32 size, u32 visibility)
> +{
> +	int i, pfn;
> +	int pagecount = size >> HV_HYP_PAGE_SHIFT;
> +	u64 *pfn_array;
> +	int ret = 0;
> +
> +	if (!hv_isolation_type_snp())
> +		return 0;
> +
> +	pfn_array = vzalloc(HV_HYP_PAGE_SIZE);
> +	if (!pfn_array)
> +		return -ENOMEM;
> +
> +	for (i = 0, pfn = 0; i < pagecount; i++) {
> +		pfn_array[pfn] = virt_to_hvpfn(kbuffer + i * HV_HYP_PAGE_SIZE);
> +		pfn++;
> +
> +		if (pfn == HV_MAX_MODIFY_GPA_REP_COUNT || i == pagecount - 1) {
> +			ret |= hv_mark_gpa_visibility(pfn, pfn_array, visibility);
> +			pfn = 0;

hv_mark_gpa_visibility() return different error codes and aggregating
them with 

 ret |= ...

will have an unpredictable result. I'd suggest bail immediately instead:

 if (ret)
     goto err_free_pfn_array;

> +		}
> +	}
> +

err_free_pfn_array:

> +	vfree(pfn_array);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(hv_set_mem_host_visibility);
> +
>  /*
>   * create_gpadl_header - Creates a gpadl for the specified buffer
>   */
> @@ -410,6 +442,12 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>  	if (ret)
>  		return ret;
>  
> +	ret = hv_set_mem_host_visibility(kbuffer, size, visibility);
> +	if (ret) {
> +		pr_warn("Failed to set host visibility.\n");
> +		return ret;
> +	}
> +
>  	init_completion(&msginfo->waitevent);
>  	msginfo->waiting_channel = channel;
>  
> @@ -693,7 +731,9 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
>  error_free_info:
>  	kfree(open_info);
>  error_free_gpadl:
> -	vmbus_teardown_gpadl(newchannel, newchannel->ringbuffer_gpadlhandle);
> +	vmbus_teardown_gpadl(newchannel, newchannel->ringbuffer_gpadlhandle,
> +			     page_address(newchannel->ringbuffer_page),
> +			     newchannel->ringbuffer_pagecount << PAGE_SHIFT);

Instead of modifying vmbus_teardown_gpadl() interface and all its call
sites, could we just keep track of all established gpadls and then get 
the required data from there? I.e. make vmbus_establish_gpadl() save
kbuffer/size to some internal structure associated with 'gpadl_handle'.

>  	newchannel->ringbuffer_gpadlhandle = 0;
>  error_clean_ring:
>  	hv_ringbuffer_cleanup(&newchannel->outbound);
> @@ -740,7 +780,8 @@ EXPORT_SYMBOL_GPL(vmbus_open);
>  /*
>   * vmbus_teardown_gpadl -Teardown the specified GPADL handle
>   */
> -int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)
> +int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle,
> +			 void *kbuffer, u32 size)

This probably doesn't matter but why not 'u64 size'?

>  {
>  	struct vmbus_channel_gpadl_teardown *msg;
>  	struct vmbus_channel_msginfo *info;
> @@ -793,6 +834,10 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)
>  	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
>  
>  	kfree(info);
> +
> +	if (hv_set_mem_host_visibility(kbuffer, size, VMBUS_PAGE_NOT_VISIBLE))
> +		pr_warn("Fail to set mem host visibility.\n");

pr_err() maybe?

> +
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(vmbus_teardown_gpadl);
> @@ -869,7 +914,9 @@ static int vmbus_close_internal(struct vmbus_channel *channel)
>  	/* Tear down the gpadl for the channel's ring buffer */
>  	else if (channel->ringbuffer_gpadlhandle) {
>  		ret = vmbus_teardown_gpadl(channel,
> -					   channel->ringbuffer_gpadlhandle);
> +					   channel->ringbuffer_gpadlhandle,
> +					   page_address(channel->ringbuffer_page),
> +					   channel->ringbuffer_pagecount << PAGE_SHIFT);
>  		if (ret) {
>  			pr_err("Close failed: teardown gpadl return %d\n", ret);
>  			/*
> diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
> index 2a87cfa27ac0..b3a43c4ec8ab 100644
> --- a/drivers/net/hyperv/hyperv_net.h
> +++ b/drivers/net/hyperv/hyperv_net.h
> @@ -1034,6 +1034,7 @@ struct netvsc_device {
>  
>  	/* Send buffer allocated by us */
>  	void *send_buf;
> +	u32 send_buf_size;
>  	u32 send_buf_gpadl_handle;
>  	u32 send_section_cnt;
>  	u32 send_section_size;
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index bb72c7578330..08d73401bb28 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -245,7 +245,9 @@ static void netvsc_teardown_recv_gpadl(struct hv_device *device,
>  
>  	if (net_device->recv_buf_gpadl_handle) {
>  		ret = vmbus_teardown_gpadl(device->channel,
> -					   net_device->recv_buf_gpadl_handle);
> +					   net_device->recv_buf_gpadl_handle,
> +					   net_device->recv_buf,
> +					   net_device->recv_buf_size);
>  
>  		/* If we failed here, we might as well return and have a leak
>  		 * rather than continue and a bugchk
> @@ -267,7 +269,9 @@ static void netvsc_teardown_send_gpadl(struct hv_device *device,
>  
>  	if (net_device->send_buf_gpadl_handle) {
>  		ret = vmbus_teardown_gpadl(device->channel,
> -					   net_device->send_buf_gpadl_handle);
> +					   net_device->send_buf_gpadl_handle,
> +					   net_device->send_buf,
> +					   net_device->send_buf_size);
>  
>  		/* If we failed here, we might as well return and have a leak
>  		 * rather than continue and a bugchk
> @@ -419,6 +423,7 @@ static int netvsc_init_buf(struct hv_device *device,
>  		ret = -ENOMEM;
>  		goto cleanup;
>  	}
> +	net_device->send_buf_size = buf_size;
>  
>  	/* Establish the gpadl handle for this buffer on this
>  	 * channel.  Note: This call uses the vmbus connection rather
> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
> index 813a7bee5139..c8d4704fc90c 100644
> --- a/drivers/uio/uio_hv_generic.c
> +++ b/drivers/uio/uio_hv_generic.c
> @@ -181,13 +181,15 @@ static void
>  hv_uio_cleanup(struct hv_device *dev, struct hv_uio_private_data *pdata)
>  {
>  	if (pdata->send_gpadl) {
> -		vmbus_teardown_gpadl(dev->channel, pdata->send_gpadl);
> +		vmbus_teardown_gpadl(dev->channel, pdata->send_gpadl,
> +				     pdata->send_buf, SEND_BUFFER_SIZE);
>  		pdata->send_gpadl = 0;
>  		vfree(pdata->send_buf);
>  	}
>  
>  	if (pdata->recv_gpadl) {
> -		vmbus_teardown_gpadl(dev->channel, pdata->recv_gpadl);
> +		vmbus_teardown_gpadl(dev->channel, pdata->recv_gpadl,
> +				     pdata->recv_buf, RECV_BUFFER_SIZE);
>  		pdata->recv_gpadl = 0;
>  		vfree(pdata->recv_buf);
>  	}
> diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
> index 83448e837ded..ad19f4199f90 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -158,6 +158,7 @@ struct ms_hyperv_tsc_page {
>  #define HVCALL_RETARGET_INTERRUPT		0x007e
>  #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
>  #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
> +#define HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY 0x00db
>  
>  #define HV_FLUSH_ALL_PROCESSORS			BIT(0)
>  #define HV_FLUSH_ALL_VIRTUAL_ADDRESS_SPACES	BIT(1)
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 016fdca20d6e..41cbaa2db567 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1183,7 +1183,8 @@ extern int vmbus_establish_gpadl(struct vmbus_channel *channel,
>  				      u32 visibility);
>  
>  extern int vmbus_teardown_gpadl(struct vmbus_channel *channel,
> -				     u32 gpadl_handle);
> +				u32 gpadl_handle,
> +				void *kbuffer, u32 size);
>  
>  void vmbus_reset_channel_cb(struct vmbus_channel *channel);

-- 
Vitaly


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

* Re: [RFC PATCH 1/12] x86/Hyper-V: Add visibility parameter for vmbus_establish_gpadl()
  2021-03-03 16:27   ` Vitaly Kuznetsov
@ 2021-03-05  6:06     ` Tianyu Lan
  0 siblings, 0 replies; 9+ messages in thread
From: Tianyu Lan @ 2021-03-05  6:06 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Tianyu Lan, linux-hyperv, linux-kernel, netdev, thomas.lendacky,
	brijesh.singh, sunilmut, kys, haiyangz, sthemmin, wei.liu, tglx,
	mingo, bp, x86, hpa, davem, kuba, gregkh

Hi Vitaly:
      Thanks for your review.

On 3/4/2021 12:27 AM, Vitaly Kuznetsov wrote:
> Tianyu Lan <ltykernel@gmail.com> writes:
> 
>> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>>
>> Add visibility parameter for vmbus_establish_gpadl() and prepare
>> to change host visibility when create gpadl for buffer.
>>
> 
> "No functional change" as you don't actually use the parameter.

Yes, will add it into commit log.

> 
>> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
>> Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
>> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> 
> Nit: Sunil's SoB looks misleading because the patch is from you,
> Co-Developed-by should be sufficient.
> 

Will update.

>> ---
>>   arch/x86/include/asm/hyperv-tlfs.h |  9 +++++++++
>>   drivers/hv/channel.c               | 20 +++++++++++---------
>>   drivers/net/hyperv/netvsc.c        |  8 ++++++--
>>   drivers/uio/uio_hv_generic.c       |  7 +++++--
>>   include/linux/hyperv.h             |  3 ++-
>>   5 files changed, 33 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
>> index e6cd3fee562b..fb1893a4c32b 100644
>> --- a/arch/x86/include/asm/hyperv-tlfs.h
>> +++ b/arch/x86/include/asm/hyperv-tlfs.h
>> @@ -236,6 +236,15 @@ enum hv_isolation_type {
>>   /* TSC invariant control */
>>   #define HV_X64_MSR_TSC_INVARIANT_CONTROL	0x40000118
>>   
>> +/* Hyper-V GPA map flags */
>> +#define HV_MAP_GPA_PERMISSIONS_NONE		0x0
>> +#define HV_MAP_GPA_READABLE			0x1
>> +#define HV_MAP_GPA_WRITABLE			0x2
>> +
>> +#define VMBUS_PAGE_VISIBLE_READ_ONLY HV_MAP_GPA_READABLE
>> +#define VMBUS_PAGE_VISIBLE_READ_WRITE (HV_MAP_GPA_READABLE|HV_MAP_GPA_WRITABLE)
>> +#define VMBUS_PAGE_NOT_VISIBLE HV_MAP_GPA_PERMISSIONS_NONE
>> +
> 
> Are these x86-only? If not, then we should probably move these defines
> to include/asm-generic/hyperv-tlfs.h. In case they are, we should do
> something as we're using them from arch neutral places.
> 
> Also, could you please add a comment stating that these flags define
> host's visibility of a page and not guest's (this seems to be not
> obvious at least to me).
>




>>   /*
>>    * Declare the MSR used to setup pages used to communicate with the hypervisor.
>>    */
>> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
>> index 0bd202de7960..daa21cc72beb 100644
>> --- a/drivers/hv/channel.c
>> +++ b/drivers/hv/channel.c
>> @@ -242,7 +242,7 @@ EXPORT_SYMBOL_GPL(vmbus_send_modifychannel);
>>    */
>>   static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
>>   			       u32 size, u32 send_offset,
>> -			       struct vmbus_channel_msginfo **msginfo)
>> +			       struct vmbus_channel_msginfo **msginfo, u32 visibility)
>>   {
>>   	int i;
>>   	int pagecount;
>> @@ -391,7 +391,7 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
>>   static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>>   				   enum hv_gpadl_type type, void *kbuffer,
>>   				   u32 size, u32 send_offset,
>> -				   u32 *gpadl_handle)
>> +				   u32 *gpadl_handle, u32 visibility)
>>   {
>>   	struct vmbus_channel_gpadl_header *gpadlmsg;
>>   	struct vmbus_channel_gpadl_body *gpadl_body;
>> @@ -405,7 +405,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>>   	next_gpadl_handle =
>>   		(atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1);
>>   
>> -	ret = create_gpadl_header(type, kbuffer, size, send_offset, &msginfo);
>> +	ret = create_gpadl_header(type, kbuffer, size, send_offset,
>> +				  &msginfo, visibility);
>>   	if (ret)
>>   		return ret;
>>   
>> @@ -496,10 +497,10 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>>    * @gpadl_handle: some funky thing
>>    */
>>   int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
>> -			  u32 size, u32 *gpadl_handle)
>> +			  u32 size, u32 *gpadl_handle, u32 visibility)
>>   {
>>   	return __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer, size,
>> -				       0U, gpadl_handle);
>> +				       0U, gpadl_handle, visibility);
>>   }
>>   EXPORT_SYMBOL_GPL(vmbus_establish_gpadl);
>>   
>> @@ -610,10 +611,11 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
>>   	newchannel->ringbuffer_gpadlhandle = 0;
>>   
>>   	err = __vmbus_establish_gpadl(newchannel, HV_GPADL_RING,
>> -				      page_address(newchannel->ringbuffer_page),
>> -				      (send_pages + recv_pages) << PAGE_SHIFT,
>> -				      newchannel->ringbuffer_send_offset << PAGE_SHIFT,
>> -				      &newchannel->ringbuffer_gpadlhandle);
>> +			page_address(newchannel->ringbuffer_page),
>> +			(send_pages + recv_pages) << PAGE_SHIFT,
>> +			newchannel->ringbuffer_send_offset << PAGE_SHIFT,
>> +			&newchannel->ringbuffer_gpadlhandle,
>> +			VMBUS_PAGE_VISIBLE_READ_WRITE);
> 
> Nit: I liked the original alignment more and we can avoid the unneeded
> code churn.
> 
>>   	if (err)
>>   		goto error_clean_ring;
>>   
>> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
>> index 2353623259f3..bb72c7578330 100644
>> --- a/drivers/net/hyperv/netvsc.c
>> +++ b/drivers/net/hyperv/netvsc.c
>> @@ -333,7 +333,8 @@ static int netvsc_init_buf(struct hv_device *device,
>>   	 */
>>   	ret = vmbus_establish_gpadl(device->channel, net_device->recv_buf,
>>   				    buf_size,
>> -				    &net_device->recv_buf_gpadl_handle);
>> +				    &net_device->recv_buf_gpadl_handle,
>> +				    VMBUS_PAGE_VISIBLE_READ_WRITE);
>>   	if (ret != 0) {
>>   		netdev_err(ndev,
>>   			"unable to establish receive buffer's gpadl\n");
>> @@ -422,10 +423,13 @@ static int netvsc_init_buf(struct hv_device *device,
>>   	/* Establish the gpadl handle for this buffer on this
>>   	 * channel.  Note: This call uses the vmbus connection rather
>>   	 * than the channel to establish the gpadl handle.
>> +	 * Send buffer should theoretically be only marked as "read-only", but
>> +	 * the netvsp for some reason needs write capabilities on it.
>>   	 */
>>   	ret = vmbus_establish_gpadl(device->channel, net_device->send_buf,
>>   				    buf_size,
>> -				    &net_device->send_buf_gpadl_handle);
>> +				    &net_device->send_buf_gpadl_handle,
>> +				    VMBUS_PAGE_VISIBLE_READ_WRITE);
>>   	if (ret != 0) {
>>   		netdev_err(ndev,
>>   			   "unable to establish send buffer's gpadl\n");
>> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
>> index 0330ba99730e..813a7bee5139 100644
>> --- a/drivers/uio/uio_hv_generic.c
>> +++ b/drivers/uio/uio_hv_generic.c
>> @@ -29,6 +29,7 @@
>>   #include <linux/hyperv.h>
>>   #include <linux/vmalloc.h>
>>   #include <linux/slab.h>
>> +#include <asm/mshyperv.h>
>>   
>>   #include "../hv/hyperv_vmbus.h"
>>   
>> @@ -295,7 +296,8 @@ hv_uio_probe(struct hv_device *dev,
>>   	}
>>   
>>   	ret = vmbus_establish_gpadl(channel, pdata->recv_buf,
>> -				    RECV_BUFFER_SIZE, &pdata->recv_gpadl);
>> +				    RECV_BUFFER_SIZE, &pdata->recv_gpadl,
>> +				    VMBUS_PAGE_VISIBLE_READ_WRITE);
>>   	if (ret)
>>   		goto fail_close;
>>   
>> @@ -315,7 +317,8 @@ hv_uio_probe(struct hv_device *dev,
>>   	}
>>   
>>   	ret = vmbus_establish_gpadl(channel, pdata->send_buf,
>> -				    SEND_BUFFER_SIZE, &pdata->send_gpadl);
>> +				    SEND_BUFFER_SIZE, &pdata->send_gpadl,
>> +				    VMBUS_PAGE_VISIBLE_READ_ONLY);
> 
> Actually, this is the only place where you use 'READ_ONLY' mapping --
> which makes me wonder if it's actually worth it or we can hard-code
> VMBUS_PAGE_VISIBLE_READ_WRITE for now and avoid this additional
> parameter.
> 

Another option is to set host visibility out of vmbus_establish_gpadl(). 
There are three places calling vmbus_establish_gpadl(). Vmbus, netvsc 
and uio drivers.

>>   	if (ret)
>>   		goto fail_close;
>>   
>> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
>> index f1d74dcf0353..016fdca20d6e 100644
>> --- a/include/linux/hyperv.h
>> +++ b/include/linux/hyperv.h
>> @@ -1179,7 +1179,8 @@ extern int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
>>   extern int vmbus_establish_gpadl(struct vmbus_channel *channel,
>>   				      void *kbuffer,
>>   				      u32 size,
>> -				      u32 *gpadl_handle);
>> +				      u32 *gpadl_handle,
>> +				      u32 visibility);
>>   
>>   extern int vmbus_teardown_gpadl(struct vmbus_channel *channel,
>>   				     u32 gpadl_handle);
> 

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

* Re: [RFC PATCH 2/12] x86/Hyper-V: Add new hvcall guest address host visibility support
  2021-03-03 16:58   ` Vitaly Kuznetsov
@ 2021-03-05  6:23     ` Tianyu Lan
  0 siblings, 0 replies; 9+ messages in thread
From: Tianyu Lan @ 2021-03-05  6:23 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: Tianyu Lan, linux-hyperv, linux-kernel, netdev, linux-arch,
	thomas.lendacky, brijesh.singh, sunilmut, kys, haiyangz,
	sthemmin, wei.liu, tglx, mingo, bp, x86, hpa, davem, kuba,
	gregkh, arnd


On 3/4/2021 12:58 AM, Vitaly Kuznetsov wrote:
> Tianyu Lan <ltykernel@gmail.com> writes:
> 
>> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
>>
>> Add new hvcall guest address host visibility support. Mark vmbus
>> ring buffer visible to host when create gpadl buffer and mark back
>> to not visible when tear down gpadl buffer.
>>
>> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
>> Co-Developed-by: Sunil Muthuswamy <sunilmut@microsoft.com>
>> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
>> ---
>>   arch/x86/include/asm/hyperv-tlfs.h | 13 ++++++++
>>   arch/x86/include/asm/mshyperv.h    |  4 +--
>>   arch/x86/kernel/cpu/mshyperv.c     | 46 ++++++++++++++++++++++++++
>>   drivers/hv/channel.c               | 53 ++++++++++++++++++++++++++++--
>>   drivers/net/hyperv/hyperv_net.h    |  1 +
>>   drivers/net/hyperv/netvsc.c        |  9 +++--
>>   drivers/uio/uio_hv_generic.c       |  6 ++--
>>   include/asm-generic/hyperv-tlfs.h  |  1 +
>>   include/linux/hyperv.h             |  3 +-
>>   9 files changed, 126 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
>> index fb1893a4c32b..d22b1c3f425a 100644
>> --- a/arch/x86/include/asm/hyperv-tlfs.h
>> +++ b/arch/x86/include/asm/hyperv-tlfs.h
>> @@ -573,4 +573,17 @@ enum hv_interrupt_type {
>>   
>>   #include <asm-generic/hyperv-tlfs.h>
>>   
>> +/* All input parameters should be in single page. */
>> +#define HV_MAX_MODIFY_GPA_REP_COUNT		\
>> +	((PAGE_SIZE - 2 * sizeof(u64)) / (sizeof(u64)))
> 
> Would it be easier to express this as '((PAGE_SIZE / sizeof(u64)) - 2'
Yes, will update. Thanks.

>> +
>> +/* HvCallModifySparseGpaPageHostVisibility hypercall */
>> +struct hv_input_modify_sparse_gpa_page_host_visibility {
>> +	u64 partition_id;
>> +	u32 host_visibility:2;
>> +	u32 reserved0:30;
>> +	u32 reserved1;
>> +	u64 gpa_page_list[HV_MAX_MODIFY_GPA_REP_COUNT];
>> +} __packed;
>> +
>>   #endif
>> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
>> index ccf60a809a17..1e8275d35c1f 100644
>> --- a/arch/x86/include/asm/mshyperv.h
>> +++ b/arch/x86/include/asm/mshyperv.h
>> @@ -262,13 +262,13 @@ static inline void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry,
>>   	msi_entry->address.as_uint32 = msi_desc->msg.address_lo;
>>   	msi_entry->data.as_uint32 = msi_desc->msg.data;
>>   }
>> -
> 
> stray change
> 
>>   struct irq_domain *hv_create_pci_msi_domain(void);
>>   
>>   int hv_map_ioapic_interrupt(int ioapic_id, bool level, int vcpu, int vector,
>>   		struct hv_interrupt_entry *entry);
>>   int hv_unmap_ioapic_interrupt(int ioapic_id, struct hv_interrupt_entry *entry);
>> -
>> +int hv_set_mem_host_visibility(void *kbuffer, u32 size, u32 visibility);
>> +int hv_mark_gpa_visibility(u16 count, const u64 pfn[], u32 visibility);
>>   #else /* CONFIG_HYPERV */
>>   static inline void hyperv_init(void) {}
>>   static inline void hyperv_setup_mmu_ops(void) {}
>> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
>> index e88bc296afca..347c32eac8fd 100644
>> --- a/arch/x86/kernel/cpu/mshyperv.c
>> +++ b/arch/x86/kernel/cpu/mshyperv.c
>> @@ -37,6 +37,8 @@
>>   bool hv_root_partition;
>>   EXPORT_SYMBOL_GPL(hv_root_partition);
>>   
>> +#define HV_PARTITION_ID_SELF ((u64)-1)
>> +
> 
> We seem to have this already:
> 
> include/asm-generic/hyperv-tlfs.h:#define HV_PARTITION_ID_SELF          ((u64)-1)

>>   struct ms_hyperv_info ms_hyperv;
>>   EXPORT_SYMBOL_GPL(ms_hyperv);
>>   
>> @@ -477,3 +479,47 @@ const __initconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
>>   	.init.msi_ext_dest_id	= ms_hyperv_msi_ext_dest_id,
>>   	.init.init_platform	= ms_hyperv_init_platform,
>>   };
>> +
>> +int hv_mark_gpa_visibility(u16 count, const u64 pfn[], u32 visibility)
>> +{
>> +	struct hv_input_modify_sparse_gpa_page_host_visibility **input_pcpu;
>> +	struct hv_input_modify_sparse_gpa_page_host_visibility *input;
>> +	u16 pages_processed;
>> +	u64 hv_status;
>> +	unsigned long flags;
>> +
>> +	/* no-op if partition isolation is not enabled */
>> +	if (!hv_is_isolation_supported())
>> +		return 0;
>> +
>> +	if (count > HV_MAX_MODIFY_GPA_REP_COUNT) {
>> +		pr_err("Hyper-V: GPA count:%d exceeds supported:%lu\n", count,
>> +			HV_MAX_MODIFY_GPA_REP_COUNT);
>> +		return -EINVAL;
>> +	}
>> +
>> +	local_irq_save(flags);
>> +	input_pcpu = (struct hv_input_modify_sparse_gpa_page_host_visibility **)
>> +			this_cpu_ptr(hyperv_pcpu_input_arg);
>> +	input = *input_pcpu;
>> +	if (unlikely(!input)) {
>> +		local_irq_restore(flags);
>> +		return -1;
> 
> -EFAULT/-ENOMEM/... maybe ?

Yes, will update.
> 
>> +	}
>> +
>> +	input->partition_id = HV_PARTITION_ID_SELF;
>> +	input->host_visibility = visibility;
>> +	input->reserved0 = 0;
>> +	input->reserved1 = 0;
>> +	memcpy((void *)input->gpa_page_list, pfn, count * sizeof(*pfn));
>> +	hv_status = hv_do_rep_hypercall(
>> +			HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY, count,
>> +			0, input, &pages_processed);
>> +	local_irq_restore(flags);
>> +
>> +	if (!(hv_status & HV_HYPERCALL_RESULT_MASK))
>> +		return 0;
>> +
>> +	return -EFAULT;
> 
> Could we just propagate "hv_status & HV_HYPERCALL_RESULT_MASK" maybe?

Yes. will update.

> 
>> +}
>> +EXPORT_SYMBOL(hv_mark_gpa_visibility);
>> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
>> index daa21cc72beb..204e6f3598a5 100644
>> --- a/drivers/hv/channel.c
>> +++ b/drivers/hv/channel.c
>> @@ -237,6 +237,38 @@ int vmbus_send_modifychannel(u32 child_relid, u32 target_vp)
>>   }
>>   EXPORT_SYMBOL_GPL(vmbus_send_modifychannel);
>>   
>> +/*
>> + * hv_set_mem_host_visibility - Set host visibility for specified memory.
>> + */
>> +int hv_set_mem_host_visibility(void *kbuffer, u32 size, u32 visibility)
>> +{
>> +	int i, pfn;
>> +	int pagecount = size >> HV_HYP_PAGE_SHIFT;
>> +	u64 *pfn_array;
>> +	int ret = 0;
>> +
>> +	if (!hv_isolation_type_snp())
>> +		return 0;
>> +
>> +	pfn_array = vzalloc(HV_HYP_PAGE_SIZE);
>> +	if (!pfn_array)
>> +		return -ENOMEM;
>> +
>> +	for (i = 0, pfn = 0; i < pagecount; i++) {
>> +		pfn_array[pfn] = virt_to_hvpfn(kbuffer + i * HV_HYP_PAGE_SIZE);
>> +		pfn++;
>> +
>> +		if (pfn == HV_MAX_MODIFY_GPA_REP_COUNT || i == pagecount - 1) {
>> +			ret |= hv_mark_gpa_visibility(pfn, pfn_array, visibility);
>> +			pfn = 0;
> 
> hv_mark_gpa_visibility() return different error codes and aggregating
> them with
> 
>   ret |= ...
> 
> will have an unpredictable result. I'd suggest bail immediately instead:
> 
>   if (ret)
>       goto err_free_pfn_array;

Yes, this makes sense. Thanks.
> 
>> +		}
>> +	}
>> +
> 
> err_free_pfn_array:
> 
>> +	vfree(pfn_array);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(hv_set_mem_host_visibility);
>> +
>>   /*
>>    * create_gpadl_header - Creates a gpadl for the specified buffer
>>    */
>> @@ -410,6 +442,12 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>>   	if (ret)
>>   		return ret;
>>   
>> +	ret = hv_set_mem_host_visibility(kbuffer, size, visibility);
>> +	if (ret) {
>> +		pr_warn("Failed to set host visibility.\n");
>> +		return ret;
>> +	}
>> +
>>   	init_completion(&msginfo->waitevent);
>>   	msginfo->waiting_channel = channel;
>>   
>> @@ -693,7 +731,9 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
>>   error_free_info:
>>   	kfree(open_info);
>>   error_free_gpadl:
>> -	vmbus_teardown_gpadl(newchannel, newchannel->ringbuffer_gpadlhandle);
>> +	vmbus_teardown_gpadl(newchannel, newchannel->ringbuffer_gpadlhandle,
>> +			     page_address(newchannel->ringbuffer_page),
>> +			     newchannel->ringbuffer_pagecount << PAGE_SHIFT);
> 
> Instead of modifying vmbus_teardown_gpadl() interface and all its call
> sites, could we just keep track of all established gpadls and then get
> the required data from there? I.e. make vmbus_establish_gpadl() save
> kbuffer/size to some internal structure associated with 'gpadl_handle'.
> 

Yes, that's another approach. Add an array or list in struct vmbus_channel.

>>   	newchannel->ringbuffer_gpadlhandle = 0;
>>   error_clean_ring:
>>   	hv_ringbuffer_cleanup(&newchannel->outbound);
>> @@ -740,7 +780,8 @@ EXPORT_SYMBOL_GPL(vmbus_open);
>>   /*
>>    * vmbus_teardown_gpadl -Teardown the specified GPADL handle
>>    */
>> -int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)
>> +int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle,
>> +			 void *kbuffer, u32 size)
> 
> This probably doesn't matter but why not 'u64 size'?
> 
>>   {
>>   	struct vmbus_channel_gpadl_teardown *msg;
>>   	struct vmbus_channel_msginfo *info;
>> @@ -793,6 +834,10 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)
>>   	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
>>   
>>   	kfree(info);
>> +
>> +	if (hv_set_mem_host_visibility(kbuffer, size, VMBUS_PAGE_NOT_VISIBLE))
>> +		pr_warn("Fail to set mem host visibility.\n");
> 
> pr_err() maybe?
> 

Yes, will update.

>> +
>>   	return ret;
>>   }
>>   EXPORT_SYMBOL_GPL(vmbus_teardown_gpadl);
>> @@ -869,7 +914,9 @@ static int vmbus_close_internal(struct vmbus_channel *channel)
>>   	/* Tear down the gpadl for the channel's ring buffer */
>>   	else if (channel->ringbuffer_gpadlhandle) {
>>   		ret = vmbus_teardown_gpadl(channel,
>> -					   channel->ringbuffer_gpadlhandle);
>> +					   channel->ringbuffer_gpadlhandle,
>> +					   page_address(channel->ringbuffer_page),
>> +					   channel->ringbuffer_pagecount << PAGE_SHIFT);
>>   		if (ret) {
>>   			pr_err("Close failed: teardown gpadl return %d\n", ret);
>>   			/*
>> diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
>> index 2a87cfa27ac0..b3a43c4ec8ab 100644
>> --- a/drivers/net/hyperv/hyperv_net.h
>> +++ b/drivers/net/hyperv/hyperv_net.h
>> @@ -1034,6 +1034,7 @@ struct netvsc_device {
>>   
>>   	/* Send buffer allocated by us */
>>   	void *send_buf;
>> +	u32 send_buf_size;
>>   	u32 send_buf_gpadl_handle;
>>   	u32 send_section_cnt;
>>   	u32 send_section_size;
>> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
>> index bb72c7578330..08d73401bb28 100644
>> --- a/drivers/net/hyperv/netvsc.c
>> +++ b/drivers/net/hyperv/netvsc.c
>> @@ -245,7 +245,9 @@ static void netvsc_teardown_recv_gpadl(struct hv_device *device,
>>   
>>   	if (net_device->recv_buf_gpadl_handle) {
>>   		ret = vmbus_teardown_gpadl(device->channel,
>> -					   net_device->recv_buf_gpadl_handle);
>> +					   net_device->recv_buf_gpadl_handle,
>> +					   net_device->recv_buf,
>> +					   net_device->recv_buf_size);
>>   
>>   		/* If we failed here, we might as well return and have a leak
>>   		 * rather than continue and a bugchk
>> @@ -267,7 +269,9 @@ static void netvsc_teardown_send_gpadl(struct hv_device *device,
>>   
>>   	if (net_device->send_buf_gpadl_handle) {
>>   		ret = vmbus_teardown_gpadl(device->channel,
>> -					   net_device->send_buf_gpadl_handle);
>> +					   net_device->send_buf_gpadl_handle,
>> +					   net_device->send_buf,
>> +					   net_device->send_buf_size);
>>   
>>   		/* If we failed here, we might as well return and have a leak
>>   		 * rather than continue and a bugchk
>> @@ -419,6 +423,7 @@ static int netvsc_init_buf(struct hv_device *device,
>>   		ret = -ENOMEM;
>>   		goto cleanup;
>>   	}
>> +	net_device->send_buf_size = buf_size;
>>   
>>   	/* Establish the gpadl handle for this buffer on this
>>   	 * channel.  Note: This call uses the vmbus connection rather
>> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
>> index 813a7bee5139..c8d4704fc90c 100644
>> --- a/drivers/uio/uio_hv_generic.c
>> +++ b/drivers/uio/uio_hv_generic.c
>> @@ -181,13 +181,15 @@ static void
>>   hv_uio_cleanup(struct hv_device *dev, struct hv_uio_private_data *pdata)
>>   {
>>   	if (pdata->send_gpadl) {
>> -		vmbus_teardown_gpadl(dev->channel, pdata->send_gpadl);
>> +		vmbus_teardown_gpadl(dev->channel, pdata->send_gpadl,
>> +				     pdata->send_buf, SEND_BUFFER_SIZE);
>>   		pdata->send_gpadl = 0;
>>   		vfree(pdata->send_buf);
>>   	}
>>   
>>   	if (pdata->recv_gpadl) {
>> -		vmbus_teardown_gpadl(dev->channel, pdata->recv_gpadl);
>> +		vmbus_teardown_gpadl(dev->channel, pdata->recv_gpadl,
>> +				     pdata->recv_buf, RECV_BUFFER_SIZE);
>>   		pdata->recv_gpadl = 0;
>>   		vfree(pdata->recv_buf);
>>   	}
>> diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
>> index 83448e837ded..ad19f4199f90 100644
>> --- a/include/asm-generic/hyperv-tlfs.h
>> +++ b/include/asm-generic/hyperv-tlfs.h
>> @@ -158,6 +158,7 @@ struct ms_hyperv_tsc_page {
>>   #define HVCALL_RETARGET_INTERRUPT		0x007e
>>   #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_SPACE 0x00af
>>   #define HVCALL_FLUSH_GUEST_PHYSICAL_ADDRESS_LIST 0x00b0
>> +#define HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY 0x00db
>>   
>>   #define HV_FLUSH_ALL_PROCESSORS			BIT(0)
>>   #define HV_FLUSH_ALL_VIRTUAL_ADDRESS_SPACES	BIT(1)
>> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
>> index 016fdca20d6e..41cbaa2db567 100644
>> --- a/include/linux/hyperv.h
>> +++ b/include/linux/hyperv.h
>> @@ -1183,7 +1183,8 @@ extern int vmbus_establish_gpadl(struct vmbus_channel *channel,
>>   				      u32 visibility);
>>   
>>   extern int vmbus_teardown_gpadl(struct vmbus_channel *channel,
>> -				     u32 gpadl_handle);
>> +				u32 gpadl_handle,
>> +				void *kbuffer, u32 size);
>>   
>>   void vmbus_reset_channel_cb(struct vmbus_channel *channel);
> 

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

end of thread, other threads:[~2021-03-05  6:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-28 15:03 [RFC PATCH 00/12] x86/Hyper-V: Add Hyper-V Isolation VM support Tianyu Lan
2021-02-28 15:03 ` [RFC PATCH 1/12] x86/Hyper-V: Add visibility parameter for vmbus_establish_gpadl() Tianyu Lan
2021-03-03 16:27   ` Vitaly Kuznetsov
2021-03-05  6:06     ` Tianyu Lan
2021-02-28 15:03 ` [RFC PATCH 2/12] x86/Hyper-V: Add new hvcall guest address host visibility support Tianyu Lan
2021-03-03 16:58   ` Vitaly Kuznetsov
2021-03-05  6:23     ` Tianyu Lan
2021-02-28 15:03 ` [RFC PATCH 9/12] x86/Hyper-V: Add new parameter for vmbus_sendpacket_pagebuffer()/mpb_desc() Tianyu Lan
2021-02-28 15:03 ` [RFC PATCH 11/12] HV/Netvsc: Add Isolation VM support for netvsc driver Tianyu Lan

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).