[RFC PATCH 0/2] Drivers: hv: decrypt netvsc buffers on contiguous direct-map addresses

[RFC PATCH 0/2] Drivers: hv: decrypt netvsc buffers on contiguous direct-map addresses

[Kameron Carr]

Some confidential computing implementations only accept linear-map
addresses in set_memory_decrypted(); for example, Arm CCA Realms reject
vmalloc()/vmap() addresses. netvsc currently allocates its large
send/receive buffers via vzalloc() and relies on vmbus_establish_gpadl()
to decrypt them, which fails on those implementations.

This fix allocates the buffers as a list of physically-contiguous chunks,
decrypts each chunk on its direct-map address, and vmap() them together.
Here netvsc fully owns the encryption lifecycle and the vmbus layer must
not call set_memory_decrypted() on establish or set_memory_encrypted() on
teardown.

Kameron Carr (2):
  Drivers: hv: vmbus: add vmbus_establish_gpadl_caller_decrypted()
  hv_netvsc: back GPADL buffers with kmalloc + decrypt + vmap

 drivers/hv/channel.c            | 123 ++++++++++-----
 drivers/net/hyperv/hyperv_net.h |  18 ++-
 drivers/net/hyperv/netvsc.c     | 271 ++++++++++++++++++++++++++++----
 drivers/net/hyperv/netvsc_drv.c |   6 +
 include/linux/hyperv.h          |  11 ++
 5 files changed, 354 insertions(+), 75 deletions(-)


base-commit: a4ffc59238be84dd1c26bf1c001543e832674fc6
-- 
2.45.4

[RFC PATCH 1/2] Drivers: hv: vmbus: add vmbus_establish_gpadl_caller_decrypted()

[Kameron Carr]

Refactor vmbus_establish_gpadl() to separate the encryption lifecycle
from the rest of the GPADL establishment logic.

Add a new vmbus_establish_gpadl_caller_decrypted() for callers that want
to decrypt their own buffers.

No functional change for existing callers.

Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
---
 drivers/hv/channel.c   | 123 +++++++++++++++++++++++++++--------------
 include/linux/hyperv.h |  11 ++++
 2 files changed, 93 insertions(+), 41 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 6821f225248b1..0166367a4df37 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -426,7 +426,13 @@ static void vmbus_free_channel_msginfo(struct vmbus_channel_msginfo *msginfo)
 }
 
 /*
- * __vmbus_establish_gpadl - Establish a GPADL for a buffer or ringbuffer
+ * __vmbus_establish_gpadl - Establish a GPADL for a buffer or ringbuffer.
+ *
+ * This function only handles the GPADL handshake with the host.  The caller
+ * is responsible for ensuring that @kbuffer is in the encryption state the
+ * host expects (host-visible / "decrypted" for confidential VMs); this
+ * function never calls set_memory_decrypted() or set_memory_encrypted().
+ * It also leaves gpadl->decrypted untouched, so the caller must set it.
  *
  * @channel: a channel
  * @type: the type of the corresponding GPADL, only meaningful for the guest.
@@ -434,7 +440,7 @@ static void vmbus_free_channel_msginfo(struct vmbus_channel_msginfo *msginfo)
  * @size: page-size multiple
  * @send_offset: the offset (in bytes) where the send ring buffer starts,
  *              should be 0 for BUFFER type gpadl
- * @gpadl_handle: some funky thing
+ * @gpadl: out parameter receiving the established GPADL handle/buffer/size
  */
 static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
 				   enum hv_gpadl_type type, void *kbuffer,
@@ -454,30 +460,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
 		(atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1);
 
 	ret = create_gpadl_header(type, kbuffer, size, send_offset, &msginfo);
-	if (ret) {
-		gpadl->decrypted = false;
+	if (ret)
 		return ret;
-	}
-
-	gpadl->decrypted = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
-		(channel->co_ring_buffer && type == HV_GPADL_RING));
-	if (gpadl->decrypted) {
-		/*
-		 * The "decrypted" flag being true assumes that set_memory_decrypted() succeeds.
-		 * But if it fails, the encryption state of the memory is unknown. In that case,
-		 * leave "decrypted" as true to ensure the memory is leaked instead of going back
-		 * on the free list.
-		 */
-		ret = set_memory_decrypted((unsigned long)kbuffer,
-					PFN_UP(size));
-		if (ret) {
-			dev_warn(&channel->device_obj->device,
-				"Failed to set host visibility for new GPADL %d.\n",
-				ret);
-			vmbus_free_channel_msginfo(msginfo);
-			return ret;
-		}
-	}
 
 	init_completion(&msginfo->waitevent);
 	msginfo->waiting_channel = channel;
@@ -553,19 +537,47 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
 	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
 
 	vmbus_free_channel_msginfo(msginfo);
+	return ret;
+}
 
-	if (ret) {
-		/*
-		 * If set_memory_encrypted() fails, the decrypted flag is
-		 * left as true so the memory is leaked instead of being
-		 * put back on the free list.
-		 */
-		if (gpadl->decrypted) {
-			if (!set_memory_encrypted((unsigned long)kbuffer, PFN_UP(size)))
-				gpadl->decrypted = false;
+/*
+ * vmbus_establish_gpadl_and_decrypt - Manage the encryption lifecycle of
+ * @kbuffer around a __vmbus_establish_gpadl() call.
+ *
+ * Decrypts @kbuffer (making it host-visible) unless the channel was created
+ * with confidential memory for @type, then establishes the GPADL.  On
+ * establish failure the buffer is re-encrypted so the caller can free it.
+ *
+ * Sets gpadl->decrypted; see the comment on struct vmbus_gpadl::decrypted.
+ */
+static int vmbus_establish_gpadl_and_decrypt(struct vmbus_channel *channel,
+					     enum hv_gpadl_type type,
+					     void *kbuffer, u32 size,
+					     u32 send_offset,
+					     struct vmbus_gpadl *gpadl)
+{
+	bool decrypt = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
+			 (channel->co_ring_buffer && type == HV_GPADL_RING));
+	int ret;
+
+	if (decrypt) {
+		ret = set_memory_decrypted((unsigned long)kbuffer, PFN_UP(size));
+		if (ret) {
+			dev_warn(&channel->device_obj->device,
+				 "Failed to set host visibility for new GPADL %d.\n",
+				 ret);
+			/* Encryption state unknown; signal caller to leak. */
+			gpadl->decrypted = true;
+			return ret;
 		}
 	}
 
+	gpadl->decrypted = decrypt;
+	ret = __vmbus_establish_gpadl(channel, type, kbuffer, size, send_offset,
+				      gpadl);
+	if (ret && decrypt &&
+	    !set_memory_encrypted((unsigned long)kbuffer, PFN_UP(size)))
+		gpadl->decrypted = false;
 	return ret;
 }
 
@@ -580,11 +592,40 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
 int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
 			  u32 size, struct vmbus_gpadl *gpadl)
 {
-	return __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer, size,
-				       0U, gpadl);
+	return vmbus_establish_gpadl_and_decrypt(channel, HV_GPADL_BUFFER,
+						 kbuffer, size, 0U, gpadl);
 }
 EXPORT_SYMBOL_GPL(vmbus_establish_gpadl);
 
+/*
+ * vmbus_establish_gpadl_caller_decrypted - Establish a GPADL for a buffer
+ * whose encryption state is managed by the caller.
+ *
+ * @channel: a channel
+ * @kbuffer: a buffer that the caller has already transitioned to host-visible
+ *           (decrypted) via set_memory_decrypted() or an equivalent mechanism.
+ * @size: page-size multiple
+ * @gpadl: out parameter receiving the established GPADL
+ *
+ * Unlike vmbus_establish_gpadl(), this function does not call
+ * set_memory_decrypted() on @kbuffer, and the matching vmbus_teardown_gpadl()
+ * call will not call set_memory_encrypted() on it.  The caller is responsible
+ * for the full encryption lifecycle of @kbuffer; on return gpadl->decrypted
+ * is set to false to record that vmbus does not own the encryption state.
+ */
+int vmbus_establish_gpadl_caller_decrypted(struct vmbus_channel *channel,
+					   void *kbuffer, u32 size,
+					   struct vmbus_gpadl *gpadl)
+{
+	int ret = __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer,
+					  size, 0U, gpadl);
+
+	/* Caller owns @kbuffer's encryption; teardown must not touch it. */
+	gpadl->decrypted = false;
+	return ret;
+}
+EXPORT_SYMBOL_GPL(vmbus_establish_gpadl_caller_decrypted);
+
 /**
  * request_arr_init - Allocates memory for the requestor array. Each slot
  * keeps track of the next available slot in the array. Initially, each
@@ -685,11 +726,11 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
 	/* Establish the gpadl for the ring buffer */
 	newchannel->ringbuffer_gpadlhandle.gpadl_handle = 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);
+	err = vmbus_establish_gpadl_and_decrypt(newchannel, HV_GPADL_RING,
+						page_address(newchannel->ringbuffer_page),
+						(send_pages + recv_pages) << PAGE_SHIFT,
+						newchannel->ringbuffer_send_offset << PAGE_SHIFT,
+						&newchannel->ringbuffer_gpadlhandle);
 	if (err)
 		goto error_clean_ring;
 
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 964f1be8150c5..9c6b1b7794b48 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -781,6 +781,12 @@ struct vmbus_gpadl {
 	u32 gpadl_handle;
 	u32 size;
 	void *buffer;
+	/*
+	 * Only used if the vmbus layer owns the encryption lifecycle of
+	 * @buffer.
+	 * Indicates @buffer must be re-encrypted at teardown or that it
+	 * must be leaked because re-encryption failed.
+	 */
 	bool decrypted;
 };
 
@@ -1205,6 +1211,11 @@ extern int vmbus_establish_gpadl(struct vmbus_channel *channel,
 				      u32 size,
 				      struct vmbus_gpadl *gpadl);
 
+extern int vmbus_establish_gpadl_caller_decrypted(struct vmbus_channel *channel,
+						  void *kbuffer,
+						  u32 size,
+						  struct vmbus_gpadl *gpadl);
+
 extern int vmbus_teardown_gpadl(struct vmbus_channel *channel,
 				     struct vmbus_gpadl *gpadl);
 
-- 
2.45.4

[RFC PATCH 2/2] hv_netvsc: back GPADL buffers with kmalloc + decrypt + vmap

[Kameron Carr]

Allocate the buffer as a list of physically-contiguous chunks using
alloc_pages_node() starting at MAX_PAGE_ORDER and falling back to smaller
orders. Each chunk is transitioned to host-visible via
set_memory_decrypted() on its direct-map address, then all chunks are
stitched together via vmap() into a single virtually-contiguous address.

Use vmbus_establish_gpadl_caller_decrypted() so the vmbus layer doesn't
try to decrypt the virtual address. At teardown, vunmap() the range and
re-encrypt and free each chunk individually; any chunk that fails
re-encryption is leaked to prevent accidentally freeing decrypted memory.

Because vunmap() and set_memory_encrypted() must run in process context,
replace the rcu_head/call_rcu() pair used to defer free_netvsc_device()
with rcu_work/queue_rcu_work().

Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h |  18 ++-
 drivers/net/hyperv/netvsc.c     | 271 ++++++++++++++++++++++++++++----
 drivers/net/hyperv/netvsc_drv.c |   6 +
 3 files changed, 261 insertions(+), 34 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 7397c693f984a..b0c4cb0f7a4ce 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -220,6 +220,8 @@ struct net_device_context;
 
 extern u32 netvsc_ring_bytes;
 
+int netvsc_workqueue_init(void);
+void netvsc_workqueue_destroy(void);
 struct netvsc_device *netvsc_device_add(struct hv_device *device,
 					const struct netvsc_device_info *info);
 int netvsc_alloc_recv_comp_ring(struct netvsc_device *net_device, u32 q_idx);
@@ -1147,6 +1149,16 @@ struct netvsc_channel {
 	struct netvsc_stats_rx rx_stats;
 };
 
+/* A physically-contiguous chunk of netvsc buffer.
+ *
+ * The chunk list is preserved so each chunk can be individually freed at
+ * teardown.
+ */
+struct netvsc_buf_chunk {
+	struct page *page;
+	unsigned int order;
+};
+
 /* Per netvsc device */
 struct netvsc_device {
 	u32 nvsp_version;
@@ -1158,6 +1170,8 @@ struct netvsc_device {
 	/* Receive buffer allocated by us but manages by NetVSP */
 	void *recv_buf;
 	u32 recv_buf_size; /* allocated bytes */
+	struct netvsc_buf_chunk *recv_buf_chunks;
+	u32 recv_buf_chunk_cnt;
 	struct vmbus_gpadl recv_buf_gpadl_handle;
 	u32 recv_section_cnt;
 	u32 recv_section_size;
@@ -1166,6 +1180,8 @@ struct netvsc_device {
 	/* Send buffer allocated by us */
 	void *send_buf;
 	u32 send_buf_size;
+	struct netvsc_buf_chunk *send_buf_chunks;
+	u32 send_buf_chunk_cnt;
 	struct vmbus_gpadl send_buf_gpadl_handle;
 	u32 send_section_cnt;
 	u32 send_section_size;
@@ -1193,7 +1209,7 @@ struct netvsc_device {
 
 	struct netvsc_channel chan_table[VRSS_CHANNEL_MAX];
 
-	struct rcu_head rcu;
+	struct rcu_work rwork;
 };
 
 /* NdisInitialize message */
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 59e95341f9b1e..2505d10e22010 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -14,6 +14,8 @@
 #include <linux/mm.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/log2.h>
+#include <linux/set_memory.h>
 #include <linux/slab.h>
 #include <linux/netdevice.h>
 #include <linux/if_ether.h>
@@ -28,6 +30,8 @@
 #include "hyperv_net.h"
 #include "netvsc_trace.h"
 
+static struct workqueue_struct *netvsc_wq;
+
 /*
  * Switch the data path from the synthetic interface to the VF
  * interface.
@@ -125,6 +129,194 @@ static void netvsc_subchan_work(struct work_struct *w)
 	rtnl_unlock();
 }
 
+/*
+ * netvsc_free_buf_pages - release a netvsc send/receive buffer.
+ *
+ * @addr: buffer address, or NULL if none was allocated (e.g. cleanup from a
+ *        failed allocation)
+ * @chunks: chunks array from netvsc_alloc_buf_pages(), or NULL
+ * @chunk_cnt: number of entries in @chunks
+ *
+ * When @chunks is NULL the buffer is a plain vzalloc() allocation.
+ *
+ * Otherwise tear down the vmap, and for each chunk re-encrypt and free
+ * the underlying pages. Any chunk that cannot be re-encrypted is leaked.
+ */
+static void netvsc_free_buf_pages(void *addr,
+				  struct netvsc_buf_chunk *chunks,
+				  u32 chunk_cnt)
+{
+	u32 i;
+
+	if (!chunks) {
+		vfree(addr);
+		return;
+	}
+
+	vunmap(addr);
+
+	for (i = 0; i < chunk_cnt; i++) {
+		unsigned long vaddr =
+			(unsigned long)page_address(chunks[i].page);
+		unsigned int order = chunks[i].order;
+
+		if (set_memory_encrypted(vaddr, 1U << order))
+			continue;
+		__free_pages(chunks[i].page, order);
+	}
+
+	kvfree(chunks);
+}
+
+/*
+ * netvsc_alloc_buf_pages - allocate a virtually-contiguous, host-visible
+ * buffer for the netvsc send/receive area.
+ *
+ * @node: NUMA node hint (NUMA_NO_NODE for any node)
+ * @size: requested buffer size in bytes (rounded up to PAGE_SIZE)
+ * @chunks_out: on success, set to the array of underlying chunks
+ * @chunk_cnt_out: on success, set to the number of chunks
+ *
+ * Allocates the buffer as a series of physically-contiguous chunks,
+ * starting at MAX_PAGE_ORDER and falling back to smaller orders on
+ * allocation failure. Each chunk is transitioned to host-visible via
+ * set_memory_decrypted() on its direct-map address, then all chunks are
+ * combined into a virtually-contiguous range via vmap().
+ *
+ * Return: the vmap()ed virtual address, or NULL on failure.
+ */
+static void *netvsc_alloc_buf_pages(int node, u32 size,
+				    struct netvsc_buf_chunk **chunks_out,
+				    u32 *chunk_cnt_out)
+{
+	u32 nr_pages = PFN_UP(size);
+	struct netvsc_buf_chunk *chunks = NULL;
+	struct page **pages = NULL;
+	unsigned int order;
+	u32 chunk_cnt = 0;
+	u32 page_idx = 0;
+	u32 remaining = nr_pages;
+	void *addr;
+	u32 i;
+	int ret;
+
+	*chunks_out = NULL;
+	*chunk_cnt_out = 0;
+
+	if (!nr_pages)
+		return NULL;
+
+	/* Worst case: every chunk is a single page. */
+	chunks = kvmalloc_array(nr_pages, sizeof(*chunks),
+				GFP_KERNEL | __GFP_ZERO);
+	if (!chunks)
+		goto err;
+
+	pages = kvmalloc_array(nr_pages, sizeof(*pages), GFP_KERNEL);
+	if (!pages)
+		goto err;
+
+	/*
+	 * @order monotonically decreases across iterations
+	 *
+	 * Use __GFP_NORETRY | __GFP_NOWARN to avoid OOM-killing, but try
+	 * harder at order 0 since that is the final fallback.
+	 */
+	order = min_t(unsigned int, MAX_PAGE_ORDER, ilog2(nr_pages));
+	while (remaining) {
+		struct page *page;
+		gfp_t gfp;
+
+		order = min_t(unsigned int, order, ilog2(remaining));
+
+		for (;;) {
+			gfp = GFP_KERNEL | __GFP_ZERO;
+			if (order)
+				gfp |= __GFP_NORETRY | __GFP_NOWARN;
+			page = alloc_pages_node(node, gfp, order);
+			if (page)
+				break;
+			if (!order)
+				goto err;
+			order--;
+		}
+
+		ret = set_memory_decrypted((unsigned long)page_address(page),
+					   1U << order);
+		if (ret) {
+			/*
+			 * set_memory_decrypted() failed; the page state is
+			 * unknown so it must be leaked rather than freed.
+			 */
+			goto err;
+		}
+
+		chunks[chunk_cnt].page = page;
+		chunks[chunk_cnt].order = order;
+		chunk_cnt++;
+
+		for (i = 0; i < (1U << order); i++)
+			pages[page_idx++] = page + i;
+
+		remaining -= 1U << order;
+	}
+
+	addr = vmap(pages, nr_pages, VM_MAP, pgprot_decrypted(PAGE_KERNEL));
+	if (!addr)
+		goto err;
+
+	kvfree(pages);
+	*chunks_out = chunks;
+	*chunk_cnt_out = chunk_cnt;
+	return addr;
+
+err:
+	kvfree(pages);
+	netvsc_free_buf_pages(NULL, chunks, chunk_cnt);
+	return NULL;
+}
+
+static void __free_netvsc_device(struct netvsc_device *nvdev)
+{
+	int i;
+
+	kfree(nvdev->extension);
+
+	netvsc_free_buf_pages(nvdev->recv_buf, nvdev->recv_buf_chunks,
+			      nvdev->recv_buf_chunk_cnt);
+	netvsc_free_buf_pages(nvdev->send_buf, nvdev->send_buf_chunks,
+			      nvdev->send_buf_chunk_cnt);
+	bitmap_free(nvdev->send_section_map);
+
+	for (i = 0; i < VRSS_CHANNEL_MAX; i++) {
+		xdp_rxq_info_unreg(&nvdev->chan_table[i].xdp_rxq);
+		kfree(nvdev->chan_table[i].recv_buf);
+		vfree(nvdev->chan_table[i].mrc.slots);
+	}
+
+	kfree(nvdev);
+}
+
+static void free_netvsc_device(struct work_struct *w)
+{
+	struct rcu_work *rwork = to_rcu_work(w);
+
+	__free_netvsc_device(container_of(rwork, struct netvsc_device, rwork));
+}
+
+int netvsc_workqueue_init(void)
+{
+	netvsc_wq = alloc_workqueue("hv_netvsc", WQ_UNBOUND, 0);
+
+	return netvsc_wq ? 0 : -ENOMEM;
+}
+
+void netvsc_workqueue_destroy(void)
+{
+	rcu_barrier();
+	destroy_workqueue(netvsc_wq);
+}
+
 static struct netvsc_device *alloc_net_device(void)
 {
 	struct netvsc_device *net_device;
@@ -143,36 +335,18 @@ static struct netvsc_device *alloc_net_device(void)
 	init_completion(&net_device->channel_init_wait);
 	init_waitqueue_head(&net_device->subchan_open);
 	INIT_WORK(&net_device->subchan_work, netvsc_subchan_work);
+	INIT_RCU_WORK(&net_device->rwork, free_netvsc_device);
 
 	return net_device;
 }
 
-static void free_netvsc_device(struct rcu_head *head)
-{
-	struct netvsc_device *nvdev
-		= container_of(head, struct netvsc_device, rcu);
-	int i;
-
-	kfree(nvdev->extension);
-
-	if (!nvdev->recv_buf_gpadl_handle.decrypted)
-		vfree(nvdev->recv_buf);
-	if (!nvdev->send_buf_gpadl_handle.decrypted)
-		vfree(nvdev->send_buf);
-	bitmap_free(nvdev->send_section_map);
-
-	for (i = 0; i < VRSS_CHANNEL_MAX; i++) {
-		xdp_rxq_info_unreg(&nvdev->chan_table[i].xdp_rxq);
-		kfree(nvdev->chan_table[i].recv_buf);
-		vfree(nvdev->chan_table[i].mrc.slots);
-	}
-
-	kfree(nvdev);
-}
-
 static void free_netvsc_device_rcu(struct netvsc_device *nvdev)
 {
-	call_rcu(&nvdev->rcu, free_netvsc_device);
+	/*
+	 * Defer the actual free to process context: vunmap() and
+	 * set_memory_encrypted() cannot run from RCU softirq context.
+	 */
+	queue_rcu_work(netvsc_wq, &nvdev->rwork);
 }
 
 static void netvsc_revoke_recv_buf(struct hv_device *device,
@@ -351,7 +525,20 @@ static int netvsc_init_buf(struct hv_device *device,
 		buf_size = min_t(unsigned int, buf_size,
 				 NETVSC_RECEIVE_BUFFER_SIZE_LEGACY);
 
-	net_device->recv_buf = vzalloc(buf_size);
+	if (device->channel->co_external_memory) {
+		/* Confidential VM Bus leaves buffer encrypted */
+		net_device->recv_buf_chunks = NULL;
+		net_device->recv_buf_chunk_cnt = 0;
+		net_device->recv_buf = vzalloc(buf_size);
+	} else {
+		/* Otherwise, allocate decrypted buffer */
+		net_device->recv_buf =
+			netvsc_alloc_buf_pages(cpu_to_node(device->channel->target_cpu),
+					       buf_size,
+					       &net_device->recv_buf_chunks,
+					       &net_device->recv_buf_chunk_cnt);
+	}
+
 	if (!net_device->recv_buf) {
 		netdev_err(ndev,
 			   "unable to allocate receive buffer of size %u\n",
@@ -367,9 +554,10 @@ static int netvsc_init_buf(struct hv_device *device,
 	 * channel.  Note: This call uses the vmbus connection rather
 	 * than the channel to establish the gpadl handle.
 	 */
-	ret = vmbus_establish_gpadl(device->channel, net_device->recv_buf,
-				    buf_size,
-				    &net_device->recv_buf_gpadl_handle);
+	ret = vmbus_establish_gpadl_caller_decrypted(device->channel,
+						     net_device->recv_buf,
+						     buf_size,
+						     &net_device->recv_buf_gpadl_handle);
 	if (ret != 0) {
 		netdev_err(ndev,
 			"unable to establish receive buffer's gpadl\n");
@@ -457,7 +645,19 @@ static int netvsc_init_buf(struct hv_device *device,
 	buf_size = device_info->send_sections * device_info->send_section_size;
 	buf_size = round_up(buf_size, PAGE_SIZE);
 
-	net_device->send_buf = vzalloc(buf_size);
+	if (device->channel->co_external_memory) {
+		/* Confidential VM Bus leaves buffer encrypted */
+		net_device->send_buf_chunks = NULL;
+		net_device->send_buf_chunk_cnt = 0;
+		net_device->send_buf = vzalloc(buf_size);
+	} else {
+		/* Otherwise, allocate decrypted buffer */
+		net_device->send_buf =
+			netvsc_alloc_buf_pages(cpu_to_node(device->channel->target_cpu),
+					       buf_size,
+					       &net_device->send_buf_chunks,
+					       &net_device->send_buf_chunk_cnt);
+	}
 	if (!net_device->send_buf) {
 		netdev_err(ndev, "unable to allocate send buffer of size %u\n",
 			   buf_size);
@@ -470,9 +670,10 @@ static int netvsc_init_buf(struct hv_device *device,
 	 * channel.  Note: This call uses the vmbus connection rather
 	 * than the channel to establish the gpadl handle.
 	 */
-	ret = vmbus_establish_gpadl(device->channel, net_device->send_buf,
-				    buf_size,
-				    &net_device->send_buf_gpadl_handle);
+	ret = vmbus_establish_gpadl_caller_decrypted(device->channel,
+						     net_device->send_buf,
+						     buf_size,
+						     &net_device->send_buf_gpadl_handle);
 	if (ret != 0) {
 		netdev_err(ndev,
 			   "unable to establish send buffer's gpadl\n");
@@ -1863,7 +2064,11 @@ struct netvsc_device *netvsc_device_add(struct hv_device *device,
 	netif_napi_del(&net_device->chan_table[0].napi);
 
 cleanup2:
-	free_netvsc_device(&net_device->rcu);
+	/*
+	 * net_device was never published, so we don't need to wait for an
+	 * RCU grace period -- call the free routine synchronously.
+	 */
+	__free_netvsc_device(net_device);
 
 	return ERR_PTR(ret);
 }
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index ee5ab5ceb2be2..1d43c73fd73f1 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2867,12 +2867,17 @@ static void __exit netvsc_drv_exit(void)
 {
 	unregister_netdevice_notifier(&netvsc_netdev_notifier);
 	vmbus_driver_unregister(&netvsc_drv);
+	netvsc_workqueue_destroy();
 }
 
 static int __init netvsc_drv_init(void)
 {
 	int ret;
 
+	ret = netvsc_workqueue_init();
+	if (ret)
+		return ret;
+
 	if (ring_size < RING_SIZE_MIN) {
 		ring_size = RING_SIZE_MIN;
 		pr_info("Increased ring_size to %u (min allowed)\n",
@@ -2890,6 +2895,7 @@ static int __init netvsc_drv_init(void)
 
 err_vmbus_reg:
 	unregister_netdevice_notifier(&netvsc_netdev_notifier);
+	netvsc_workqueue_destroy();
 	return ret;
 }
 
-- 
2.45.4

RE: [RFC PATCH 1/2] Drivers: hv: vmbus: add vmbus_establish_gpadl_caller_decrypted()

[Michael Kelley]

From: Kameron Carr <kameroncarr@linux.microsoft.com> Sent: Tuesday, July 21, 2026 12:57 PM
> 
> Refactor vmbus_establish_gpadl() to separate the encryption lifecycle
> from the rest of the GPADL establishment logic.
> 
> Add a new vmbus_establish_gpadl_caller_decrypted() for callers that want
> to decrypt their own buffers.
> 
> No functional change for existing callers.

I have a suggestion for a different way to accomplish this. I haven't coded
it, but I think it will result in less code churn and be simpler overall.

1) In enum hv_gpadl_type, add value HV_GPADL_BUFFER_DECRYPTED

2) Update hv_gpadl_size() and hv_gpadl_hvpfn() to treat the new enum
value just like HV_GPADL_BUFFER

3) In __vmbus_establish_gpadl(), change this code

        gpadl->decrypted = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
                (channel->co_ring_buffer && type == HV_GPADL_RING));

to

        gpadl->decrypted = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
                (channel->co_ring_buffer && type == HV_GPADL_RING) ||
	  (type == HV_GPADL_BUFFER_DECRYPTED));

4) Add the function vmbus_establish_gpadl_caller_decrypted() like you
have in this patch. It just calls __vmbus_establish_gpadl(), passing
HV_GPADL_BUFFER_DECRYPTED.

I think that's all that is needed, though I haven't gone through
everything rigorously and might be missing something.

FWIW, the meaning of the "decrypted" field in struct vmbus_gpadl is
a bit abused, but it was already that way. In current code, it initially
means "decryption needed". Then after the gpadl is created, it
means "was decrypted" so that gpadl teardown will re-encrypt.
But my suggested approach doesn't really make the abuse any worse.  

> 
> Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
> ---
>  drivers/hv/channel.c   | 123 +++++++++++++++++++++++++++--------------
>  include/linux/hyperv.h |  11 ++++
>  2 files changed, 93 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 6821f225248b1..0166367a4df37 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -426,7 +426,13 @@ static void vmbus_free_channel_msginfo(struct vmbus_channel_msginfo *msginfo)
>  }
> 
>  /*
> - * __vmbus_establish_gpadl - Establish a GPADL for a buffer or ringbuffer
> + * __vmbus_establish_gpadl - Establish a GPADL for a buffer or ringbuffer.
> + *
> + * This function only handles the GPADL handshake with the host.  The caller
> + * is responsible for ensuring that @kbuffer is in the encryption state the
> + * host expects (host-visible / "decrypted" for confidential VMs); this
> + * function never calls set_memory_decrypted() or set_memory_encrypted().
> + * It also leaves gpadl->decrypted untouched, so the caller must set it.
>   *
>   * @channel: a channel
>   * @type: the type of the corresponding GPADL, only meaningful for the guest.
> @@ -434,7 +440,7 @@ static void vmbus_free_channel_msginfo(struct vmbus_channel_msginfo *msginfo)
>   * @size: page-size multiple
>   * @send_offset: the offset (in bytes) where the send ring buffer starts,
>   *              should be 0 for BUFFER type gpadl
> - * @gpadl_handle: some funky thing
> + * @gpadl: out parameter receiving the established GPADL handle/buffer/size
>   */
>  static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>  				   enum hv_gpadl_type type, void *kbuffer,
> @@ -454,30 +460,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>  		(atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1);
> 
>  	ret = create_gpadl_header(type, kbuffer, size, send_offset, &msginfo);
> -	if (ret) {
> -		gpadl->decrypted = false;
> +	if (ret)
>  		return ret;
> -	}
> -
> -	gpadl->decrypted = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
> -		(channel->co_ring_buffer && type == HV_GPADL_RING));
> -	if (gpadl->decrypted) {
> -		/*
> -		 * The "decrypted" flag being true assumes that set_memory_decrypted() succeeds.
> -		 * But if it fails, the encryption state of the memory is unknown. In that case,
> -		 * leave "decrypted" as true to ensure the memory is leaked instead of going back
> -		 * on the free list.
> -		 */
> -		ret = set_memory_decrypted((unsigned long)kbuffer,
> -					PFN_UP(size));
> -		if (ret) {
> -			dev_warn(&channel->device_obj->device,
> -				"Failed to set host visibility for new GPADL %d.\n",
> -				ret);
> -			vmbus_free_channel_msginfo(msginfo);
> -			return ret;
> -		}
> -	}
> 
>  	init_completion(&msginfo->waitevent);
>  	msginfo->waiting_channel = channel;
> @@ -553,19 +537,47 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>  	spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
> 
>  	vmbus_free_channel_msginfo(msginfo);
> +	return ret;
> +}
> 
> -	if (ret) {
> -		/*
> -		 * If set_memory_encrypted() fails, the decrypted flag is
> -		 * left as true so the memory is leaked instead of being
> -		 * put back on the free list.
> -		 */
> -		if (gpadl->decrypted) {
> -			if (!set_memory_encrypted((unsigned long)kbuffer, PFN_UP(size)))
> -				gpadl->decrypted = false;
> +/*
> + * vmbus_establish_gpadl_and_decrypt - Manage the encryption lifecycle of
> + * @kbuffer around a __vmbus_establish_gpadl() call.
> + *
> + * Decrypts @kbuffer (making it host-visible) unless the channel was created
> + * with confidential memory for @type, then establishes the GPADL.  On
> + * establish failure the buffer is re-encrypted so the caller can free it.
> + *
> + * Sets gpadl->decrypted; see the comment on struct vmbus_gpadl::decrypted.
> + */
> +static int vmbus_establish_gpadl_and_decrypt(struct vmbus_channel *channel,
> +					     enum hv_gpadl_type type,
> +					     void *kbuffer, u32 size,
> +					     u32 send_offset,
> +					     struct vmbus_gpadl *gpadl)
> +{
> +	bool decrypt = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
> +			 (channel->co_ring_buffer && type == HV_GPADL_RING));
> +	int ret;
> +
> +	if (decrypt) {
> +		ret = set_memory_decrypted((unsigned long)kbuffer, PFN_UP(size));
> +		if (ret) {
> +			dev_warn(&channel->device_obj->device,
> +				 "Failed to set host visibility for new GPADL %d.\n",
> +				 ret);
> +			/* Encryption state unknown; signal caller to leak. */
> +			gpadl->decrypted = true;
> +			return ret;
>  		}
>  	}
> 
> +	gpadl->decrypted = decrypt;
> +	ret = __vmbus_establish_gpadl(channel, type, kbuffer, size, send_offset,
> +				      gpadl);
> +	if (ret && decrypt &&
> +	    !set_memory_encrypted((unsigned long)kbuffer, PFN_UP(size)))
> +		gpadl->decrypted = false;
>  	return ret;
>  }
> 
> @@ -580,11 +592,40 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>  int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
>  			  u32 size, struct vmbus_gpadl *gpadl)
>  {
> -	return __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer, size,
> -				       0U, gpadl);
> +	return vmbus_establish_gpadl_and_decrypt(channel, HV_GPADL_BUFFER,
> +						 kbuffer, size, 0U, gpadl);
>  }
>  EXPORT_SYMBOL_GPL(vmbus_establish_gpadl);
> 
> +/*
> + * vmbus_establish_gpadl_caller_decrypted - Establish a GPADL for a buffer
> + * whose encryption state is managed by the caller.
> + *
> + * @channel: a channel
> + * @kbuffer: a buffer that the caller has already transitioned to host-visible
> + *           (decrypted) via set_memory_decrypted() or an equivalent mechanism.
> + * @size: page-size multiple
> + * @gpadl: out parameter receiving the established GPADL
> + *
> + * Unlike vmbus_establish_gpadl(), this function does not call
> + * set_memory_decrypted() on @kbuffer, and the matching vmbus_teardown_gpadl()
> + * call will not call set_memory_encrypted() on it.  The caller is responsible
> + * for the full encryption lifecycle of @kbuffer; on return gpadl->decrypted
> + * is set to false to record that vmbus does not own the encryption state.
> + */
> +int vmbus_establish_gpadl_caller_decrypted(struct vmbus_channel *channel,
> +					   void *kbuffer, u32 size,
> +					   struct vmbus_gpadl *gpadl)
> +{
> +	int ret = __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer,
> +					  size, 0U, gpadl);
> +
> +	/* Caller owns @kbuffer's encryption; teardown must not touch it. */
> +	gpadl->decrypted = false;
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(vmbus_establish_gpadl_caller_decrypted);
> +
>  /**
>   * request_arr_init - Allocates memory for the requestor array. Each slot
>   * keeps track of the next available slot in the array. Initially, each
> @@ -685,11 +726,11 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
>  	/* Establish the gpadl for the ring buffer */
>  	newchannel->ringbuffer_gpadlhandle.gpadl_handle = 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);
> +	err = vmbus_establish_gpadl_and_decrypt(newchannel, HV_GPADL_RING,
> +						page_address(newchannel->ringbuffer_page),
> +						(send_pages + recv_pages) << PAGE_SHIFT,
> +						newchannel->ringbuffer_send_offset << PAGE_SHIFT,
> +						&newchannel->ringbuffer_gpadlhandle);
>  	if (err)
>  		goto error_clean_ring;
> 
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 964f1be8150c5..9c6b1b7794b48 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -781,6 +781,12 @@ struct vmbus_gpadl {
>  	u32 gpadl_handle;
>  	u32 size;
>  	void *buffer;
> +	/*
> +	 * Only used if the vmbus layer owns the encryption lifecycle of

s/VMBus/vmbus/

> +	 * @buffer.
> +	 * Indicates @buffer must be re-encrypted at teardown or that it
> +	 * must be leaked because re-encryption failed.

Overall, this comment would be a bit different with my suggested approach.

> +	 */
>  	bool decrypted;
>  };
> 
> @@ -1205,6 +1211,11 @@ extern int vmbus_establish_gpadl(struct vmbus_channel *channel,
>  				      u32 size,
>  				      struct vmbus_gpadl *gpadl);
> 
> +extern int vmbus_establish_gpadl_caller_decrypted(struct vmbus_channel *channel,
> +						  void *kbuffer,
> +						  u32 size,
> +						  struct vmbus_gpadl *gpadl);
> +
>  extern int vmbus_teardown_gpadl(struct vmbus_channel *channel,
>  				     struct vmbus_gpadl *gpadl);
> 
> --
> 2.45.4
> 

RE: [RFC PATCH 1/2] Drivers: hv: vmbus: add vmbus_establish_gpadl_caller_decrypted()

[Kameron Carr]

On Thursday, July 23, 2026 10:53 AM, Michael Kelley wrote:
> From: Kameron Carr <kameroncarr@linux.microsoft.com> Sent: Tuesday, July
21, 2026 12:57 PM
> >
> > Refactor vmbus_establish_gpadl() to separate the encryption lifecycle
> > from the rest of the GPADL establishment logic.
> >
> > Add a new vmbus_establish_gpadl_caller_decrypted() for callers that want
> > to decrypt their own buffers.
> >
> > No functional change for existing callers.
> 
> I have a suggestion for a different way to accomplish this. I haven't
coded
> it, but I think it will result in less code churn and be simpler overall.
> 
> 1) In enum hv_gpadl_type, add value HV_GPADL_BUFFER_DECRYPTED
> 
> 2) Update hv_gpadl_size() and hv_gpadl_hvpfn() to treat the new enum
> value just like HV_GPADL_BUFFER
> 
> 3) In __vmbus_establish_gpadl(), change this code
> 
>         gpadl->decrypted = !((channel->co_external_memory && type ==
HV_GPADL_BUFFER) ||
>                 (channel->co_ring_buffer && type == HV_GPADL_RING));
> 
> to
> 
>         gpadl->decrypted = !((channel->co_external_memory && type ==
HV_GPADL_BUFFER) ||
>                 (channel->co_ring_buffer && type == HV_GPADL_RING) ||
> 	  (type == HV_GPADL_BUFFER_DECRYPTED));
> 
> 4) Add the function vmbus_establish_gpadl_caller_decrypted() like you
> have in this patch. It just calls __vmbus_establish_gpadl(), passing
> HV_GPADL_BUFFER_DECRYPTED.

Ack. Thank you for the feedback. I will use this approach in v2.

Regards,
Kameron

> I think that's all that is needed, though I haven't gone through
> everything rigorously and might be missing something.
> 
> FWIW, the meaning of the "decrypted" field in struct vmbus_gpadl is
> a bit abused, but it was already that way. In current code, it initially
> means "decryption needed". Then after the gpadl is created, it
> means "was decrypted" so that gpadl teardown will re-encrypt.
> But my suggested approach doesn't really make the abuse any worse.
[...]

RE: [RFC PATCH 2/2] hv_netvsc: back GPADL buffers with kmalloc + decrypt + vmap

[Michael Kelley]

From: Kameron Carr <kameroncarr@linux.microsoft.com> Sent: Tuesday, July 21, 2026 12:57 PM
> 
> Allocate the buffer as a list of physically-contiguous chunks using
> alloc_pages_node() starting at MAX_PAGE_ORDER and falling back to smaller
> orders. Each chunk is transitioned to host-visible via
> set_memory_decrypted() on its direct-map address, then all chunks are
> stitched together via vmap() into a single virtually-contiguous address.

The commit message should describe the "why" and not just the "what".
So mention the high-level motivations for this approach:

1) Avoids the need for set_memory_decrypted()/encrypted() to work
on vmalloc'ed memory, since (so far) that requirement is specific to
the Hyper-V netvsc driver.

2) Minimizes scattering of decrypted 4 KiB pages through the kernel
direct map, with the resulting impact of shattering large page mappings.

3) Meets the requirement to allocate the large netvsc buffers if a
synthetic NIC is added after the kernel has been running a while and
memory is fragmented to the point that high order allocations fail.

> 
> Use vmbus_establish_gpadl_caller_decrypted() so the vmbus layer doesn't
> try to decrypt the virtual address. At teardown, vunmap() the range and
> re-encrypt and free each chunk individually; any chunk that fails
> re-encryption is leaked to prevent accidentally freeing decrypted memory.
> 
> Because vunmap() and set_memory_encrypted() must run in process context,
> replace the rcu_head/call_rcu() pair used to defer free_netvsc_device()
> with rcu_work/queue_rcu_work().

This special RCU handling points out a problem with the current
upstream code. I don't know the exact motivation, but presumably
the freeing of the large buffers is done after the RCU grace period
because the buffers might still be accessed by code in the networking
stack. In current code, tearing down the GPADL is done before doing
the RCU free. But tearing down the GPADL re-encrypts the large
buffer. Re-encryption is a multi-step process, and if the networking
stack should access the buffer at certain points in that process,
unhanded faults will occur. It's a small timing window on a
relatively infrequent operation, but it's a risk nonetheless.
This patch fixes the race condition.

> 
> Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
> ---
>  drivers/net/hyperv/hyperv_net.h |  18 ++-
>  drivers/net/hyperv/netvsc.c     | 271 ++++++++++++++++++++++++++++----
>  drivers/net/hyperv/netvsc_drv.c |   6 +
>  3 files changed, 261 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
> index 7397c693f984a..b0c4cb0f7a4ce 100644
> --- a/drivers/net/hyperv/hyperv_net.h
> +++ b/drivers/net/hyperv/hyperv_net.h
> @@ -220,6 +220,8 @@ struct net_device_context;
> 
>  extern u32 netvsc_ring_bytes;
> 
> +int netvsc_workqueue_init(void);
> +void netvsc_workqueue_destroy(void);
>  struct netvsc_device *netvsc_device_add(struct hv_device *device,
>  					const struct netvsc_device_info *info);
>  int netvsc_alloc_recv_comp_ring(struct netvsc_device *net_device, u32 q_idx);
> @@ -1147,6 +1149,16 @@ struct netvsc_channel {
>  	struct netvsc_stats_rx rx_stats;
>  };
> 
> +/* A physically-contiguous chunk of netvsc buffer.
> + *
> + * The chunk list is preserved so each chunk can be individually freed at
> + * teardown.
> + */
> +struct netvsc_buf_chunk {
> +	struct page *page;
> +	unsigned int order;
> +};
> +
>  /* Per netvsc device */
>  struct netvsc_device {
>  	u32 nvsp_version;
> @@ -1158,6 +1170,8 @@ struct netvsc_device {
>  	/* Receive buffer allocated by us but manages by NetVSP */
>  	void *recv_buf;
>  	u32 recv_buf_size; /* allocated bytes */
> +	struct netvsc_buf_chunk *recv_buf_chunks;
> +	u32 recv_buf_chunk_cnt;
>  	struct vmbus_gpadl recv_buf_gpadl_handle;
>  	u32 recv_section_cnt;
>  	u32 recv_section_size;
> @@ -1166,6 +1180,8 @@ struct netvsc_device {
>  	/* Send buffer allocated by us */
>  	void *send_buf;
>  	u32 send_buf_size;
> +	struct netvsc_buf_chunk *send_buf_chunks;
> +	u32 send_buf_chunk_cnt;
>  	struct vmbus_gpadl send_buf_gpadl_handle;
>  	u32 send_section_cnt;
>  	u32 send_section_size;
> @@ -1193,7 +1209,7 @@ struct netvsc_device {
> 
>  	struct netvsc_channel chan_table[VRSS_CHANNEL_MAX];
> 
> -	struct rcu_head rcu;
> +	struct rcu_work rwork;
>  };
> 
>  /* NdisInitialize message */
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 59e95341f9b1e..2505d10e22010 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -14,6 +14,8 @@
>  #include <linux/mm.h>
>  #include <linux/delay.h>
>  #include <linux/io.h>
> +#include <linux/log2.h>
> +#include <linux/set_memory.h>
>  #include <linux/slab.h>
>  #include <linux/netdevice.h>
>  #include <linux/if_ether.h>
> @@ -28,6 +30,8 @@
>  #include "hyperv_net.h"
>  #include "netvsc_trace.h"
> 
> +static struct workqueue_struct *netvsc_wq;
> +
>  /*
>   * Switch the data path from the synthetic interface to the VF
>   * interface.
> @@ -125,6 +129,194 @@ static void netvsc_subchan_work(struct work_struct *w)
>  	rtnl_unlock();
>  }
> 
> +/*
> + * netvsc_free_buf_pages - release a netvsc send/receive buffer.
> + *
> + * @addr: buffer address, or NULL if none was allocated (e.g. cleanup from a
> + *        failed allocation)
> + * @chunks: chunks array from netvsc_alloc_buf_pages(), or NULL
> + * @chunk_cnt: number of entries in @chunks
> + *
> + * When @chunks is NULL the buffer is a plain vzalloc() allocation.
> + *
> + * Otherwise tear down the vmap, and for each chunk re-encrypt and free
> + * the underlying pages. Any chunk that cannot be re-encrypted is leaked.
> + */
> +static void netvsc_free_buf_pages(void *addr,
> +				  struct netvsc_buf_chunk *chunks,
> +				  u32 chunk_cnt)
> +{
> +	u32 i;
> +
> +	if (!chunks) {
> +		vfree(addr);
> +		return;
> +	}
> +
> +	vunmap(addr);
> +
> +	for (i = 0; i < chunk_cnt; i++) {
> +		unsigned long vaddr =
> +			(unsigned long)page_address(chunks[i].page);
> +		unsigned int order = chunks[i].order;
> +
> +		if (set_memory_encrypted(vaddr, 1U << order))
> +			continue;
> +		__free_pages(chunks[i].page, order);
> +	}
> +
> +	kvfree(chunks);
> +}
> +
> +/*
> + * netvsc_alloc_buf_pages - allocate a virtually-contiguous, host-visible
> + * buffer for the netvsc send/receive area.
> + *
> + * @node: NUMA node hint (NUMA_NO_NODE for any node)
> + * @size: requested buffer size in bytes (rounded up to PAGE_SIZE)

A slight ambiguity about rounding up. Is the text here saying that the
requested size must already be a multiple of PAGE_SIZE, or that this
function will round up the requested size? I think it's the latter, so
maybe "(will be rounded up to PAGE_SIZE)" is more explicit and clear.

> + * @chunks_out: on success, set to the array of underlying chunks
> + * @chunk_cnt_out: on success, set to the number of chunks
> + *
> + * Allocates the buffer as a series of physically-contiguous chunks,
> + * starting at MAX_PAGE_ORDER and falling back to smaller orders on
> + * allocation failure. Each chunk is transitioned to host-visible via
> + * set_memory_decrypted() on its direct-map address, then all chunks are
> + * combined into a virtually-contiguous range via vmap().
> + *
> + * Return: the vmap()ed virtual address, or NULL on failure.
> + */
> +static void *netvsc_alloc_buf_pages(int node, u32 size,
> +				    struct netvsc_buf_chunk **chunks_out,
> +				    u32 *chunk_cnt_out)
> +{
> +	u32 nr_pages = PFN_UP(size);
> +	struct netvsc_buf_chunk *chunks = NULL;
> +	struct page **pages = NULL;
> +	unsigned int order;
> +	u32 chunk_cnt = 0;
> +	u32 page_idx = 0;
> +	u32 remaining = nr_pages;
> +	void *addr;
> +	u32 i;
> +	int ret;
> +
> +	*chunks_out = NULL;
> +	*chunk_cnt_out = 0;
> +
> +	if (!nr_pages)
> +		return NULL;
> +
> +	/* Worst case: every chunk is a single page. */
> +	chunks = kvmalloc_array(nr_pages, sizeof(*chunks),
> +				GFP_KERNEL | __GFP_ZERO);
> +	if (!chunks)
> +		goto err;
> +
> +	pages = kvmalloc_array(nr_pages, sizeof(*pages), GFP_KERNEL);
> +	if (!pages)
> +		goto err;
> +
> +	/*
> +	 * @order monotonically decreases across iterations
> +	 *
> +	 * Use __GFP_NORETRY | __GFP_NOWARN to avoid OOM-killing, but try
> +	 * harder at order 0 since that is the final fallback.
> +	 */
> +	order = min_t(unsigned int, MAX_PAGE_ORDER, ilog2(nr_pages));

Prefer using min() instead of min_t(). For normal integer values,
min() should work correctly.

> +	while (remaining) {
> +		struct page *page;
> +		gfp_t gfp;
> +
> +		order = min_t(unsigned int, order, ilog2(remaining));

Same here.

> +
> +		for (;;) {
> +			gfp = GFP_KERNEL | __GFP_ZERO;
> +			if (order)
> +				gfp |= __GFP_NORETRY | __GFP_NOWARN;
> +			page = alloc_pages_node(node, gfp, order);
> +			if (page)
> +				break;
> +			if (!order)
> +				goto err;
> +			order--;
> +		}
> +
> +		ret = set_memory_decrypted((unsigned long)page_address(page),
> +					   1U << order);
> +		if (ret) {
> +			/*
> +			 * set_memory_decrypted() failed; the page state is
> +			 * unknown so it must be leaked rather than freed.
> +			 */
> +			goto err;
> +		}
> +
> +		chunks[chunk_cnt].page = page;
> +		chunks[chunk_cnt].order = order;

Is it necessary to explicitly save "order" in the chunks array? For
contiguously allocated memory like this, where "page" is a
struct page *, I think netvsc_free_buf_pages() can do

    folio_order(page_folio(page))

to get the order.  

> +		chunk_cnt++;
> +
> +		for (i = 0; i < (1U << order); i++)
> +			pages[page_idx++] = page + i;
> +
> +		remaining -= 1U << order;
> +	}

The memory was zero'ed before decryption. But after decryption, it
may no longer be zero, depending on the underlying hardware. TDX
zero's the memory after decryption. But SEV-SNP does not. I'm unsure
about CCA.

It's not clear to me that these buffers really *need* to be zero'ed
for netvsc's purposes, but historically they have been, so you would
need to zero the full buffer again. It's probably best to do it a few lines
below as a single operation using the vmap() address.

> +
> +	addr = vmap(pages, nr_pages, VM_MAP, pgprot_decrypted(PAGE_KERNEL));
> +	if (!addr)
> +		goto err;
> +
> +	kvfree(pages);
> +	*chunks_out = chunks;
> +	*chunk_cnt_out = chunk_cnt;
> +	return addr;
> +
> +err:
> +	kvfree(pages);
> +	netvsc_free_buf_pages(NULL, chunks, chunk_cnt);
> +	return NULL;
> +}
> +
> +static void __free_netvsc_device(struct netvsc_device *nvdev)
> +{
> +	int i;
> +
> +	kfree(nvdev->extension);
> +
> +	netvsc_free_buf_pages(nvdev->recv_buf, nvdev->recv_buf_chunks,
> +			      nvdev->recv_buf_chunk_cnt);
> +	netvsc_free_buf_pages(nvdev->send_buf, nvdev->send_buf_chunks,
> +			      nvdev->send_buf_chunk_cnt);
> +	bitmap_free(nvdev->send_section_map);
> +
> +	for (i = 0; i < VRSS_CHANNEL_MAX; i++) {
> +		xdp_rxq_info_unreg(&nvdev->chan_table[i].xdp_rxq);
> +		kfree(nvdev->chan_table[i].recv_buf);
> +		vfree(nvdev->chan_table[i].mrc.slots);
> +	}
> +
> +	kfree(nvdev);
> +}
> +
> +static void free_netvsc_device(struct work_struct *w)
> +{
> +	struct rcu_work *rwork = to_rcu_work(w);
> +
> +	__free_netvsc_device(container_of(rwork, struct netvsc_device, rwork));
> +}
> +
> +int netvsc_workqueue_init(void)
> +{
> +	netvsc_wq = alloc_workqueue("hv_netvsc", WQ_UNBOUND, 0);
> +
> +	return netvsc_wq ? 0 : -ENOMEM;
> +}
> +
> +void netvsc_workqueue_destroy(void)
> +{
> +	rcu_barrier();
> +	destroy_workqueue(netvsc_wq);
> +}
> +
>  static struct netvsc_device *alloc_net_device(void)
>  {
>  	struct netvsc_device *net_device;
> @@ -143,36 +335,18 @@ static struct netvsc_device *alloc_net_device(void)
>  	init_completion(&net_device->channel_init_wait);
>  	init_waitqueue_head(&net_device->subchan_open);
>  	INIT_WORK(&net_device->subchan_work, netvsc_subchan_work);
> +	INIT_RCU_WORK(&net_device->rwork, free_netvsc_device);
> 
>  	return net_device;
>  }
> 
> -static void free_netvsc_device(struct rcu_head *head)
> -{
> -	struct netvsc_device *nvdev
> -		= container_of(head, struct netvsc_device, rcu);
> -	int i;
> -
> -	kfree(nvdev->extension);
> -
> -	if (!nvdev->recv_buf_gpadl_handle.decrypted)
> -		vfree(nvdev->recv_buf);
> -	if (!nvdev->send_buf_gpadl_handle.decrypted)
> -		vfree(nvdev->send_buf);
> -	bitmap_free(nvdev->send_section_map);
> -
> -	for (i = 0; i < VRSS_CHANNEL_MAX; i++) {
> -		xdp_rxq_info_unreg(&nvdev->chan_table[i].xdp_rxq);
> -		kfree(nvdev->chan_table[i].recv_buf);
> -		vfree(nvdev->chan_table[i].mrc.slots);
> -	}
> -
> -	kfree(nvdev);
> -}
> -
>  static void free_netvsc_device_rcu(struct netvsc_device *nvdev)
>  {
> -	call_rcu(&nvdev->rcu, free_netvsc_device);
> +	/*
> +	 * Defer the actual free to process context: vunmap() and
> +	 * set_memory_encrypted() cannot run from RCU softirq context.
> +	 */
> +	queue_rcu_work(netvsc_wq, &nvdev->rwork);
>  }
> 
>  static void netvsc_revoke_recv_buf(struct hv_device *device,
> @@ -351,7 +525,20 @@ static int netvsc_init_buf(struct hv_device *device,
>  		buf_size = min_t(unsigned int, buf_size,
>  				 NETVSC_RECEIVE_BUFFER_SIZE_LEGACY);
> 
> -	net_device->recv_buf = vzalloc(buf_size);
> +	if (device->channel->co_external_memory) {
> +		/* Confidential VM Bus leaves buffer encrypted */
> +		net_device->recv_buf_chunks = NULL;
> +		net_device->recv_buf_chunk_cnt = 0;
> +		net_device->recv_buf = vzalloc(buf_size);
> +	} else {
> +		/* Otherwise, allocate decrypted buffer */
> +		net_device->recv_buf =
> +			netvsc_alloc_buf_pages(cpu_to_node(device->channel->target_cpu),
> +					       buf_size,
> +					       &net_device->recv_buf_chunks,
> +					       &net_device->recv_buf_chunk_cnt);
> +	}
> +

I have several thoughts about this code, and the similar code below
for the send buffer.

Deciding whether to allocate decrypted or not in the netvsc driver
seems out-of-place. That decision should be in the VMBus code. I'd
recommend renaming and moving netvsc_free_buf_pages() and
netvsc_alloc_buf_pages() to drivers/hv/channel.c where the GPADL
code is. Then make the decision about decrypting in the new
vmbus_alloc_buf_pages() function and fallback to vzalloc() when
not decrypting. That way vmbus_alloc_buf_pages() will more
closely parallel vmbus_free_buf_pages(), which already has
code for the vfree() case. The recv_buf_chunks and
recv_buf_chunk_cnt fields stay in struct netvsc_device, and are
passed into vmbus_alloc/free_buf_pages().

In the new vmbus_alloc_buf_pages(), the decision to decrypt or not
is more complicated than you have here. A "normal" VM (i.e., not
Confidential) has co_external_memory set to false, so as coded here,
a normal VM takes the decrypted path instead of doing just vzalloc().
That actually works because set_memory_decrypted() and
pgprot_decrypted() are no-op's in a normal VM. Still, I think it's
probably better to do just vzalloc() in a normal VM and let vzalloc()
handle populating the pages, since there's no scattering problem
to ameliorate.

That's my comments for now. Next week, I may look at some
areas more closely, but don't necessarily wait for those
additional comments before spinning a new version.

Michael


>  	if (!net_device->recv_buf) {
>  		netdev_err(ndev,
>  			   "unable to allocate receive buffer of size %u\n",
> @@ -367,9 +554,10 @@ static int netvsc_init_buf(struct hv_device *device,
>  	 * channel.  Note: This call uses the vmbus connection rather
>  	 * than the channel to establish the gpadl handle.	
>  	 */
> -	ret = vmbus_establish_gpadl(device->channel, net_device->recv_buf,
> -				    buf_size,
> -				    &net_device->recv_buf_gpadl_handle);
> +	ret = vmbus_establish_gpadl_caller_decrypted(device->channel,
> +						     net_device->recv_buf,
> +						     buf_size,
> +						     &net_device->recv_buf_gpadl_handle);
>  	if (ret != 0) {
>  		netdev_err(ndev,
>  			"unable to establish receive buffer's gpadl\n");
> @@ -457,7 +645,19 @@ static int netvsc_init_buf(struct hv_device *device,
>  	buf_size = device_info->send_sections * device_info->send_section_size;
>  	buf_size = round_up(buf_size, PAGE_SIZE);
> 
> -	net_device->send_buf = vzalloc(buf_size);
> +	if (device->channel->co_external_memory) {
> +		/* Confidential VM Bus leaves buffer encrypted */
> +		net_device->send_buf_chunks = NULL;
> +		net_device->send_buf_chunk_cnt = 0;
> +		net_device->send_buf = vzalloc(buf_size);
> +	} else {
> +		/* Otherwise, allocate decrypted buffer */
> +		net_device->send_buf =
> +			netvsc_alloc_buf_pages(cpu_to_node(device->channel->target_cpu),
> +					       buf_size,
> +					       &net_device->send_buf_chunks,
> +					       &net_device->send_buf_chunk_cnt);
> +	}
>  	if (!net_device->send_buf) {
>  		netdev_err(ndev, "unable to allocate send buffer of size %u\n",
>  			   buf_size);
> @@ -470,9 +670,10 @@ static int netvsc_init_buf(struct hv_device *device,
>  	 * channel.  Note: This call uses the vmbus connection rather
>  	 * than the channel to establish the gpadl handle.
>  	 */
> -	ret = vmbus_establish_gpadl(device->channel, net_device->send_buf,
> -				    buf_size,
> -				    &net_device->send_buf_gpadl_handle);
> +	ret = vmbus_establish_gpadl_caller_decrypted(device->channel,
> +						     net_device->send_buf,
> +						     buf_size,
> +						     &net_device-
> >send_buf_gpadl_handle);
>  	if (ret != 0) {
>  		netdev_err(ndev,
>  			   "unable to establish send buffer's gpadl\n");
> @@ -1863,7 +2064,11 @@ struct netvsc_device *netvsc_device_add(struct hv_device *device,
>  	netif_napi_del(&net_device->chan_table[0].napi);
> 
>  cleanup2:
> -	free_netvsc_device(&net_device->rcu);
> +	/*
> +	 * net_device was never published, so we don't need to wait for an
> +	 * RCU grace period -- call the free routine synchronously.
> +	 */
> +	__free_netvsc_device(net_device);
> 
>  	return ERR_PTR(ret);
>  }
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index ee5ab5ceb2be2..1d43c73fd73f1 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -2867,12 +2867,17 @@ static void __exit netvsc_drv_exit(void)
>  {
>  	unregister_netdevice_notifier(&netvsc_netdev_notifier);
>  	vmbus_driver_unregister(&netvsc_drv);
> +	netvsc_workqueue_destroy();
>  }
> 
>  static int __init netvsc_drv_init(void)
>  {
>  	int ret;
> 
> +	ret = netvsc_workqueue_init();
> +	if (ret)
> +		return ret;
> +
>  	if (ring_size < RING_SIZE_MIN) {
>  		ring_size = RING_SIZE_MIN;
>  		pr_info("Increased ring_size to %u (min allowed)\n",
> @@ -2890,6 +2895,7 @@ static int __init netvsc_drv_init(void)
> 
>  err_vmbus_reg:
>  	unregister_netdevice_notifier(&netvsc_netdev_notifier);
> +	netvsc_workqueue_destroy();
>  	return ret;
>  }
> 
> --
> 2.45.4
> 

RE: [RFC PATCH 2/2] hv_netvsc: back GPADL buffers with kmalloc + decrypt + vmap

[Kameron Carr]

On Friday, July 24, 2026 12:37 PM, Michael Kelley wrote:
> From: Kameron Carr <kameroncarr@linux.microsoft.com> Sent: Tuesday, July
21, 2026 12:57 PM
[...]
> > +	/*
> > +	 * @order monotonically decreases across iterations
> > +	 *
> > +	 * Use __GFP_NORETRY | __GFP_NOWARN to avoid OOM-killing, but try
> > +	 * harder at order 0 since that is the final fallback.
> > +	 */
> > +	order = min_t(unsigned int, MAX_PAGE_ORDER, ilog2(nr_pages));
> 
> Prefer using min() instead of min_t(). For normal integer values,
> min() should work correctly.

Using min() compiled fine on ARM, but x86 is throwing a compile error.

    drivers/hv/channel.c: In function 'vmbus_alloc_buffer':
    ././include/linux/compiler_types.h:699:45: error: call to
    '__compiletime_assert_518' declared with attribute error: min(order, (
    __builtin_constant_p(remaining) ? ((remaining) < 2 ? 0 : 63 -
    __builtin_clzll(remaining)) : (sizeof(remaining) <= 4) ?
    __ilog2_u32(remaining) : __ilog2_u64(remaining) )) signedness error

In my v3 I may go back to using min_t. Please let me know if a cast (or
some other method) is preferred.

-Kameron