On 3/4/2021 12:58 AM, Vitaly Kuznetsov wrote:
Tianyu Lan <ltykernel@xxxxxxxxx> writes:From: Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx> Co-Developed-by: Sunil Muthuswamy <sunilmut@xxxxxxxxxxxxx> Signed-off-by: Tianyu Lan <Tianyu.Lan@xxxxxxxxxxxxx> --- 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 changestruct 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);
