From: Tianyu Lan <ltykernel@xxxxxxxxx> Sent: Friday, August 27, 2021 10:21 AM > > Mark vmbus ring buffer visible with set_memory_decrypted() when > establish gpadl handle. > > Signed-off-by: Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx> > --- > Change since v3: > * Change vmbus_teardown_gpadl() parameter and put gpadl handle, > buffer and buffer size in the struct vmbus_gpadl. > --- > drivers/hv/channel.c | 36 ++++++++++++++++++++++++++++----- > drivers/net/hyperv/hyperv_net.h | 1 + > drivers/net/hyperv/netvsc.c | 16 +++++++++++---- > drivers/uio/uio_hv_generic.c | 14 +++++++++++-- > include/linux/hyperv.h | 8 +++++++- > 5 files changed, 63 insertions(+), 12 deletions(-) > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c > index f3761c73b074..82650beb3af0 100644 > --- a/drivers/hv/channel.c > +++ b/drivers/hv/channel.c > @@ -17,6 +17,7 @@ > #include <linux/hyperv.h> > #include <linux/uio.h> > #include <linux/interrupt.h> > +#include <linux/set_memory.h> > #include <asm/page.h> > #include <asm/mshyperv.h> > > @@ -474,6 +475,13 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel, > if (ret) > return ret; > > + ret = set_memory_decrypted((unsigned long)kbuffer, > + HVPFN_UP(size)); > + if (ret) { > + pr_warn("Failed to set host visibility for new GPADL %d.\n", ret); > + return ret; > + } > + > init_completion(&msginfo->waitevent); > msginfo->waiting_channel = channel; > > @@ -549,6 +557,11 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel, > } > > kfree(msginfo); > + > + if (ret) > + set_memory_encrypted((unsigned long)kbuffer, > + HVPFN_UP(size)); > + > return ret; > } > > @@ -639,6 +652,7 @@ static int __vmbus_open(struct vmbus_channel *newchannel, > struct vmbus_channel_open_channel *open_msg; > struct vmbus_channel_msginfo *open_info = NULL; > struct page *page = newchannel->ringbuffer_page; > + struct vmbus_gpadl gpadl; > u32 send_pages, recv_pages; > unsigned long flags; > int err; > @@ -759,7 +773,10 @@ static int __vmbus_open(struct vmbus_channel *newchannel, > error_free_info: > kfree(open_info); > error_free_gpadl: > - vmbus_teardown_gpadl(newchannel, newchannel->ringbuffer_gpadlhandle); > + gpadl.gpadl_handle = newchannel->ringbuffer_gpadlhandle; > + gpadl.buffer = page_address(newchannel->ringbuffer_page); > + gpadl.size = (send_pages + recv_pages) << PAGE_SHIFT; > + vmbus_teardown_gpadl(newchannel, &gpadl); > newchannel->ringbuffer_gpadlhandle = 0; > error_clean_ring: > hv_ringbuffer_cleanup(&newchannel->outbound); > @@ -806,7 +823,7 @@ 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, struct vmbus_gpadl *gpadl) > { > struct vmbus_channel_gpadl_teardown *msg; > struct vmbus_channel_msginfo *info; > @@ -825,7 +842,7 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle) > > msg->header.msgtype = CHANNELMSG_GPADL_TEARDOWN; > msg->child_relid = channel->offermsg.child_relid; > - msg->gpadl = gpadl_handle; > + msg->gpadl = gpadl->gpadl_handle; > > spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags); > list_add_tail(&info->msglistentry, > @@ -859,6 +876,12 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle) > spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags); > > kfree(info); > + > + ret = set_memory_encrypted((unsigned long)gpadl->buffer, > + HVPFN_UP(gpadl->size)); > + if (ret) > + pr_warn("Fail to set mem host visibility in GPADL teardown %d.\n", ret); > + > return ret; > } > EXPORT_SYMBOL_GPL(vmbus_teardown_gpadl); > @@ -896,6 +919,7 @@ void vmbus_reset_channel_cb(struct vmbus_channel *channel) > static int vmbus_close_internal(struct vmbus_channel *channel) > { > struct vmbus_channel_close_channel *msg; > + struct vmbus_gpadl gpadl; > int ret; > > vmbus_reset_channel_cb(channel); > @@ -934,8 +958,10 @@ 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); > + gpadl.gpadl_handle = channel->ringbuffer_gpadlhandle; > + gpadl.buffer = page_address(channel->ringbuffer_page); > + gpadl.size = channel->ringbuffer_pagecount; > + ret = vmbus_teardown_gpadl(channel, &gpadl); > 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 bc48855dff10..aa7c9962dbd8 100644 > --- a/drivers/net/hyperv/hyperv_net.h > +++ b/drivers/net/hyperv/hyperv_net.h > @@ -1082,6 +1082,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 7bd935412853..f19bffff6a63 100644 > --- a/drivers/net/hyperv/netvsc.c > +++ b/drivers/net/hyperv/netvsc.c > @@ -276,11 +276,14 @@ static void netvsc_teardown_recv_gpadl(struct hv_device *device, > struct netvsc_device *net_device, > struct net_device *ndev) > { > + struct vmbus_gpadl gpadl; > int ret; > > if (net_device->recv_buf_gpadl_handle) { > - ret = vmbus_teardown_gpadl(device->channel, > - net_device->recv_buf_gpadl_handle); > + gpadl.gpadl_handle = net_device->recv_buf_gpadl_handle; > + gpadl.buffer = net_device->recv_buf; > + gpadl.size = net_device->recv_buf_size; > + ret = vmbus_teardown_gpadl(device->channel, &gpadl); > > /* If we failed here, we might as well return and have a leak > * rather than continue and a bugchk > @@ -298,11 +301,15 @@ static void netvsc_teardown_send_gpadl(struct hv_device *device, > struct netvsc_device *net_device, > struct net_device *ndev) > { > + struct vmbus_gpadl gpadl; > int ret; > > if (net_device->send_buf_gpadl_handle) { > - ret = vmbus_teardown_gpadl(device->channel, > - net_device->send_buf_gpadl_handle); > + gpadl.gpadl_handle = net_device->send_buf_gpadl_handle; > + gpadl.buffer = net_device->send_buf; > + gpadl.size = net_device->send_buf_size; > + > + ret = vmbus_teardown_gpadl(device->channel, &gpadl); > > /* If we failed here, we might as well return and have a leak > * rather than continue and a bugchk > @@ -463,6 +470,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 652fe2547587..13c5df8dd11d 100644 > --- a/drivers/uio/uio_hv_generic.c > +++ b/drivers/uio/uio_hv_generic.c > @@ -179,14 +179,24 @@ hv_uio_new_channel(struct vmbus_channel *new_sc) > static void > hv_uio_cleanup(struct hv_device *dev, struct hv_uio_private_data *pdata) > { > + struct vmbus_gpadl gpadl; > + > if (pdata->send_gpadl) { > - vmbus_teardown_gpadl(dev->channel, pdata->send_gpadl); > + gpadl.gpadl_handle = pdata->send_gpadl; > + gpadl.buffer = pdata->send_buf; > + gpadl.size = SEND_BUFFER_SIZE; > + > + vmbus_teardown_gpadl(dev->channel, &gpadl); > pdata->send_gpadl = 0; > vfree(pdata->send_buf); > } > > if (pdata->recv_gpadl) { > - vmbus_teardown_gpadl(dev->channel, pdata->recv_gpadl); > + gpadl.gpadl_handle = pdata->recv_gpadl; > + gpadl.buffer = pdata->recv_buf; > + gpadl.size = RECV_BUFFER_SIZE; > + > + vmbus_teardown_gpadl(dev->channel, &gpadl); > pdata->recv_gpadl = 0; > vfree(pdata->recv_buf); > } > diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h > index ddc8713ce57b..757e09606fd3 100644 > --- a/include/linux/hyperv.h > +++ b/include/linux/hyperv.h > @@ -803,6 +803,12 @@ struct vmbus_device { > > #define VMBUS_DEFAULT_MAX_PKT_SIZE 4096 > > +struct vmbus_gpadl { > + u32 gpadl_handle; > + u32 size; > + void *buffer; > +}; > + > struct vmbus_channel { > struct list_head listentry; > > @@ -1195,7 +1201,7 @@ extern int vmbus_establish_gpadl(struct vmbus_channel *channel, > u32 *gpadl_handle); > > extern int vmbus_teardown_gpadl(struct vmbus_channel *channel, > - u32 gpadl_handle); > + struct vmbus_gpadl *gpadl); > > void vmbus_reset_channel_cb(struct vmbus_channel *channel); > > -- > 2.25.1 This isn't quite what I had in mind in my comments on v3 of this patch series. My idea is to store the full struct vmbus_gpadl data structure in places where previously just the u32 gpadl_handle was stored. Then pass around a pointer to the struct vmbus_gpadl where previously just the gpadl_handle (or a pointer to it) was passed. This lets __vmbus_establish_gpadl() fill in the actual handle value as well the other info (buffer pointer and size) that vmbus_teardown_gpadl() needs. Callers of the gpadl functions don't need to worry about saving or finding the right info. Most of the changes are just tweaking the references to what is now a struct instead of a u32. Here's a diff of what I had in mind. My version also has vmbus_teardown_gpadl() set the handle field to zero, rather than each caller having to do it. The code compiles, but I have not done a runtime test. This diff is a net +21 lines of code, whereas your v3 and v4 patches were both +51 lines of code. diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index f3761c7..fc041ae 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -17,6 +17,7 @@ #include <linux/hyperv.h> #include <linux/uio.h> #include <linux/interrupt.h> +#include <linux/set_memory.h> #include <asm/page.h> #include <asm/mshyperv.h> @@ -456,7 +457,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) + struct vmbus_gpadl *gpadl_handle) { struct vmbus_channel_gpadl_header *gpadlmsg; struct vmbus_channel_gpadl_body *gpadl_body; @@ -474,6 +475,13 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel, if (ret) return ret; + ret = set_memory_decrypted((unsigned long)kbuffer, + HVPFN_UP(size)); + if (ret) { + pr_warn("Failed to set host visibility for new GPADL %d.\n", ret); + return ret; + } + init_completion(&msginfo->waitevent); msginfo->waiting_channel = channel; @@ -537,7 +545,9 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel, } /* At this point, we received the gpadl created msg */ - *gpadl_handle = gpadlmsg->gpadl; + gpadl_handle->handle = gpadlmsg->gpadl; + gpadl_handle->buffer = kbuffer; + gpadl_handle->size = size; cleanup: spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags); @@ -549,6 +559,11 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel, } kfree(msginfo); + + if (ret) + set_memory_encrypted((unsigned long)kbuffer, + HVPFN_UP(size)); + return ret; } @@ -561,7 +576,7 @@ 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, struct vmbus_gpadl *gpadl_handle) { return __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer, size, 0U, gpadl_handle); @@ -675,7 +690,7 @@ static int __vmbus_open(struct vmbus_channel *newchannel, goto error_clean_ring; /* Establish the gpadl for the ring buffer */ - newchannel->ringbuffer_gpadlhandle = 0; + newchannel->ringbuffer_gpadlhandle.handle = 0; err = __vmbus_establish_gpadl(newchannel, HV_GPADL_RING, page_address(newchannel->ringbuffer_page), @@ -701,7 +716,7 @@ static int __vmbus_open(struct vmbus_channel *newchannel, open_msg->header.msgtype = CHANNELMSG_OPENCHANNEL; open_msg->openid = newchannel->offermsg.child_relid; open_msg->child_relid = newchannel->offermsg.child_relid; - open_msg->ringbuffer_gpadlhandle = newchannel->ringbuffer_gpadlhandle; + open_msg->ringbuffer_gpadlhandle = newchannel->ringbuffer_gpadlhandle.handle; /* * The unit of ->downstream_ringbuffer_pageoffset is HV_HYP_PAGE and * the unit of ->ringbuffer_send_offset (i.e. send_pages) is PAGE, so @@ -759,8 +774,7 @@ static int __vmbus_open(struct vmbus_channel *newchannel, error_free_info: kfree(open_info); error_free_gpadl: - vmbus_teardown_gpadl(newchannel, newchannel->ringbuffer_gpadlhandle); - newchannel->ringbuffer_gpadlhandle = 0; + vmbus_teardown_gpadl(newchannel, &newchannel->ringbuffer_gpadlhandle); error_clean_ring: hv_ringbuffer_cleanup(&newchannel->outbound); hv_ringbuffer_cleanup(&newchannel->inbound); @@ -806,7 +820,7 @@ int vmbus_open(struct vmbus_channel *newchannel, /* * 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, struct vmbus_gpadl *gpadl) { struct vmbus_channel_gpadl_teardown *msg; struct vmbus_channel_msginfo *info; @@ -825,7 +839,7 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle) msg->header.msgtype = CHANNELMSG_GPADL_TEARDOWN; msg->child_relid = channel->offermsg.child_relid; - msg->gpadl = gpadl_handle; + msg->gpadl = gpadl->handle; spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags); list_add_tail(&info->msglistentry, @@ -844,6 +858,7 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle) goto post_msg_err; wait_for_completion(&info->waitevent); + gpadl->handle = 0; post_msg_err: /* @@ -859,6 +874,12 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle) spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags); kfree(info); + + ret = set_memory_encrypted((unsigned long)gpadl->buffer, + HVPFN_UP(gpadl->size)); + if (ret) + pr_warn("Fail to set mem host visibility in GPADL teardown %d.\n", ret); + return ret; } EXPORT_SYMBOL_GPL(vmbus_teardown_gpadl); @@ -933,9 +954,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) { + else if (channel->ringbuffer_gpadlhandle.handle) { ret = vmbus_teardown_gpadl(channel, - channel->ringbuffer_gpadlhandle); + &channel->ringbuffer_gpadlhandle); if (ret) { pr_err("Close failed: teardown gpadl return %d\n", ret); /* @@ -943,8 +964,6 @@ static int vmbus_close_internal(struct vmbus_channel *channel) * it is perhaps better to leak memory. */ } - - channel->ringbuffer_gpadlhandle = 0; } if (!ret) diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h index bc48855..54cbce1 100644 --- a/drivers/net/hyperv/hyperv_net.h +++ b/drivers/net/hyperv/hyperv_net.h @@ -1075,14 +1075,14 @@ struct netvsc_device { /* Receive buffer allocated by us but manages by NetVSP */ void *recv_buf; u32 recv_buf_size; /* allocated bytes */ - u32 recv_buf_gpadl_handle; + struct vmbus_gpadl recv_buf_gpadl_handle; u32 recv_section_cnt; u32 recv_section_size; u32 recv_completion_cnt; /* Send buffer allocated by us */ void *send_buf; - u32 send_buf_gpadl_handle; + struct vmbus_gpadl send_buf_gpadl_handle; u32 send_section_cnt; u32 send_section_size; unsigned long *send_section_map; diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index 7bd9354..585974c 100644 --- a/drivers/net/hyperv/netvsc.c +++ b/drivers/net/hyperv/netvsc.c @@ -278,9 +278,9 @@ static void netvsc_teardown_recv_gpadl(struct hv_device *device, { int ret; - if (net_device->recv_buf_gpadl_handle) { + if (net_device->recv_buf_gpadl_handle.handle) { ret = vmbus_teardown_gpadl(device->channel, - net_device->recv_buf_gpadl_handle); + &net_device->recv_buf_gpadl_handle); /* If we failed here, we might as well return and have a leak * rather than continue and a bugchk @@ -290,7 +290,6 @@ static void netvsc_teardown_recv_gpadl(struct hv_device *device, "unable to teardown receive buffer's gpadl\n"); return; } - net_device->recv_buf_gpadl_handle = 0; } } @@ -300,9 +299,9 @@ static void netvsc_teardown_send_gpadl(struct hv_device *device, { int ret; - if (net_device->send_buf_gpadl_handle) { + if (net_device->send_buf_gpadl_handle.handle) { ret = vmbus_teardown_gpadl(device->channel, - net_device->send_buf_gpadl_handle); + &net_device->send_buf_gpadl_handle); /* If we failed here, we might as well return and have a leak * rather than continue and a bugchk @@ -312,7 +311,6 @@ static void netvsc_teardown_send_gpadl(struct hv_device *device, "unable to teardown send buffer's gpadl\n"); return; } - net_device->send_buf_gpadl_handle = 0; } } @@ -380,7 +378,7 @@ static int netvsc_init_buf(struct hv_device *device, memset(init_packet, 0, sizeof(struct nvsp_message)); init_packet->hdr.msg_type = NVSP_MSG1_TYPE_SEND_RECV_BUF; init_packet->msg.v1_msg.send_recv_buf. - gpadl_handle = net_device->recv_buf_gpadl_handle; + gpadl_handle = net_device->recv_buf_gpadl_handle.handle; init_packet->msg.v1_msg. send_recv_buf.id = NETVSC_RECEIVE_BUFFER_ID; @@ -482,7 +480,7 @@ static int netvsc_init_buf(struct hv_device *device, memset(init_packet, 0, sizeof(struct nvsp_message)); init_packet->hdr.msg_type = NVSP_MSG1_TYPE_SEND_SEND_BUF; init_packet->msg.v1_msg.send_send_buf.gpadl_handle = - net_device->send_buf_gpadl_handle; + net_device->send_buf_gpadl_handle.handle; init_packet->msg.v1_msg.send_send_buf.id = NETVSC_SEND_BUFFER_ID; trace_nvsp_send(ndev, init_packet); diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c index 652fe25..97e08e7 100644 --- a/drivers/uio/uio_hv_generic.c +++ b/drivers/uio/uio_hv_generic.c @@ -58,11 +58,11 @@ struct hv_uio_private_data { atomic_t refcnt; void *recv_buf; - u32 recv_gpadl; + struct vmbus_gpadl recv_gpadl; char recv_name[32]; /* "recv_4294967295" */ void *send_buf; - u32 send_gpadl; + struct vmbus_gpadl send_gpadl; char send_name[32]; }; @@ -179,15 +179,13 @@ static int hv_uio_ring_mmap(struct file *filp, struct kobject *kobj, 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); - pdata->send_gpadl = 0; + if (pdata->send_gpadl.handle) { + vmbus_teardown_gpadl(dev->channel, &pdata->send_gpadl); vfree(pdata->send_buf); } - if (pdata->recv_gpadl) { - vmbus_teardown_gpadl(dev->channel, pdata->recv_gpadl); - pdata->recv_gpadl = 0; + if (pdata->recv_gpadl.handle) { + vmbus_teardown_gpadl(dev->channel, &pdata->recv_gpadl); vfree(pdata->recv_buf); } } diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 2e859d2..a0d64c3 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -809,6 +809,12 @@ struct vmbus_device { #define VMBUS_DEFAULT_MAX_PKT_SIZE 4096 +struct vmbus_gpadl { + u32 handle; + u32 size; + void *buffer; +}; + struct vmbus_channel { struct list_head listentry; @@ -828,7 +834,7 @@ struct vmbus_channel { bool rescind_ref; /* got rescind msg, got channel reference */ struct completion rescind_event; - u32 ringbuffer_gpadlhandle; + struct vmbus_gpadl ringbuffer_gpadlhandle; /* Allocated memory for ring buffer */ struct page *ringbuffer_page; @@ -1208,10 +1214,10 @@ 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); + struct vmbus_gpadl *gpadl_handle); extern int vmbus_teardown_gpadl(struct vmbus_channel *channel, - u32 gpadl_handle); + struct vmbus_gpadl *gpadl); void vmbus_reset_channel_cb(struct vmbus_channel *channel);