| Message ID | 20210914133916.1440931-8-ltykernel@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86/Hyper-V: Add Hyper-V Isolation VM support | expand |
From: Tianyu Lan <ltykernel@gmail.com> Sent: Tuesday, September 14, 2021 6:39 AM > > The monitor pages in the CHANNELMSG_INITIATE_CONTACT msg are shared > with host in Isolation VM and so it's necessary to use hvcall to set > them visible to host. In Isolation VM with AMD SEV SNP, the access > address should be in the extra space which is above shared gpa > boundary. So remap these pages into the extra address(pa + > shared_gpa_boundary). > > Introduce monitor_pages_original[] in the struct vmbus_connection > to store monitor page virtual address returned by hv_alloc_hyperv_ > zeroed_page() and free monitor page via monitor_pages_original in > the vmbus_disconnect(). The monitor_pages[] is to used to access > monitor page and it is initialized to be equal with monitor_pages_ > original. The monitor_pages[] will be overridden in the isolation VM > with va of extra address. Introduce monitor_pages_pa[] to store > monitor pages' physical address and use it to populate pa in the > initiate msg. > > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com> > --- > Change since v4: > * Introduce monitor_pages_pa[] to store monitor pages' physical > address and use it to populate pa in the initiate msg. > * Move code of mapping moniter pages in extra address into > vmbus_connect(). > > Change since v3: > * Rename monitor_pages_va with monitor_pages_original > * free monitor page via monitor_pages_original and > monitor_pages is used to access monitor page. > > Change since v1: > * Not remap monitor pages in the non-SNP isolation VM. > --- > drivers/hv/connection.c | 90 ++++++++++++++++++++++++++++++++++++--- > drivers/hv/hyperv_vmbus.h | 2 + > 2 files changed, 86 insertions(+), 6 deletions(-) > > diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c > index 8820ae68f20f..edd8f7dd169f 100644 > --- a/drivers/hv/connection.c > +++ b/drivers/hv/connection.c > @@ -19,6 +19,8 @@ > #include <linux/vmalloc.h> > #include <linux/hyperv.h> > #include <linux/export.h> > +#include <linux/io.h> > +#include <linux/set_memory.h> > #include <asm/mshyperv.h> > > #include "hyperv_vmbus.h" > @@ -102,8 +104,9 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version) > vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID; > } > > - msg->monitor_page1 = virt_to_phys(vmbus_connection.monitor_pages[0]); > - msg->monitor_page2 = virt_to_phys(vmbus_connection.monitor_pages[1]); > + msg->monitor_page1 = vmbus_connection.monitor_pages_pa[0]; > + msg->monitor_page2 = vmbus_connection.monitor_pages_pa[1]; > + > msg->target_vcpu = hv_cpu_number_to_vp_number(VMBUS_CONNECT_CPU); > > /* > @@ -216,6 +219,65 @@ int vmbus_connect(void) > goto cleanup; > } > > + vmbus_connection.monitor_pages_original[0] > + = vmbus_connection.monitor_pages[0]; > + vmbus_connection.monitor_pages_original[1] > + = vmbus_connection.monitor_pages[1]; > + vmbus_connection.monitor_pages_pa[0] > + = virt_to_phys(vmbus_connection.monitor_pages[0]); > + vmbus_connection.monitor_pages_pa[1] > + = virt_to_phys(vmbus_connection.monitor_pages[1]); > + > + if (hv_is_isolation_supported()) { > + vmbus_connection.monitor_pages_pa[0] += > + ms_hyperv.shared_gpa_boundary; > + vmbus_connection.monitor_pages_pa[1] += > + ms_hyperv.shared_gpa_boundary; > + > + ret = set_memory_decrypted((unsigned long) > + vmbus_connection.monitor_pages[0], > + 1); > + ret |= set_memory_decrypted((unsigned long) > + vmbus_connection.monitor_pages[1], > + 1); > + if (ret) > + goto cleanup; > + > + /* > + * Isolation VM with AMD SNP needs to access monitor page via > + * address space above shared gpa boundary. > + */ > + if (hv_isolation_type_snp()) { > + vmbus_connection.monitor_pages[0] > + = memremap(vmbus_connection.monitor_pages_pa[0], > + HV_HYP_PAGE_SIZE, > + MEMREMAP_WB); > + if (!vmbus_connection.monitor_pages[0]) { > + ret = -ENOMEM; > + goto cleanup; > + } > + > + vmbus_connection.monitor_pages[1] > + = memremap(vmbus_connection.monitor_pages_pa[1], > + HV_HYP_PAGE_SIZE, > + MEMREMAP_WB); > + if (!vmbus_connection.monitor_pages[1]) { > + ret = -ENOMEM; > + goto cleanup; > + } > + } > + > + /* > + * Set memory host visibility hvcall smears memory > + * and so zero monitor pages here. > + */ > + memset(vmbus_connection.monitor_pages[0], 0x00, > + HV_HYP_PAGE_SIZE); > + memset(vmbus_connection.monitor_pages[1], 0x00, > + HV_HYP_PAGE_SIZE); > + > + } > + This all looks good. To me, this is a lot clearer to have all the mapping and encryption/decryption handled in one place. > msginfo = kzalloc(sizeof(*msginfo) + > sizeof(struct vmbus_channel_initiate_contact), > GFP_KERNEL); > @@ -303,10 +365,26 @@ void vmbus_disconnect(void) > vmbus_connection.int_page = NULL; > } > > - hv_free_hyperv_page((unsigned long)vmbus_connection.monitor_pages[0]); > - hv_free_hyperv_page((unsigned long)vmbus_connection.monitor_pages[1]); > - vmbus_connection.monitor_pages[0] = NULL; > - vmbus_connection.monitor_pages[1] = NULL; > + if (hv_is_isolation_supported()) { > + memunmap(vmbus_connection.monitor_pages[0]); > + memunmap(vmbus_connection.monitor_pages[1]); > + > + set_memory_encrypted((unsigned long) > + vmbus_connection.monitor_pages_original[0], > + 1); > + set_memory_encrypted((unsigned long) > + vmbus_connection.monitor_pages_original[1], > + 1); > + } > + > + hv_free_hyperv_page((unsigned long) > + vmbus_connection.monitor_pages_original[0]); > + hv_free_hyperv_page((unsigned long) > + vmbus_connection.monitor_pages_original[1]); > + vmbus_connection.monitor_pages_original[0] = > + vmbus_connection.monitor_pages[0] = NULL; > + vmbus_connection.monitor_pages_original[1] = > + vmbus_connection.monitor_pages[1] = NULL; > } > > /* > diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h > index 42f3d9d123a1..560cba916d1d 100644 > --- a/drivers/hv/hyperv_vmbus.h > +++ b/drivers/hv/hyperv_vmbus.h > @@ -240,6 +240,8 @@ struct vmbus_connection { > * is child->parent notification > */ > struct hv_monitor_page *monitor_pages[2]; > + void *monitor_pages_original[2]; > + unsigned long monitor_pages_pa[2]; The type of this field really should be phys_addr_t. In addition to just making semantic sense, then it will match the return type from virt_to_phys() and the input arg to memremap() since resource_size_t is typedef'ed as phys_addr_t. > struct list_head chn_msg_list; > spinlock_t channelmsg_lock; > > -- > 2.25.1
On 9/15/2021 11:41 PM, Michael Kelley wrote: >> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h >> index 42f3d9d123a1..560cba916d1d 100644 >> --- a/drivers/hv/hyperv_vmbus.h >> +++ b/drivers/hv/hyperv_vmbus.h >> @@ -240,6 +240,8 @@ struct vmbus_connection { >> * is child->parent notification >> */ >> struct hv_monitor_page *monitor_pages[2]; >> + void *monitor_pages_original[2]; >> + unsigned long monitor_pages_pa[2]; > The type of this field really should be phys_addr_t. In addition to > just making semantic sense, then it will match the return type from > virt_to_phys() and the input arg to memremap() since resource_size_t > is typedef'ed as phys_addr_t. > OK. Will update in the next version. Thanks.
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c index 8820ae68f20f..edd8f7dd169f 100644 --- a/drivers/hv/connection.c +++ b/drivers/hv/connection.c @@ -19,6 +19,8 @@ #include <linux/vmalloc.h> #include <linux/hyperv.h> #include <linux/export.h> +#include <linux/io.h> +#include <linux/set_memory.h> #include <asm/mshyperv.h> #include "hyperv_vmbus.h" @@ -102,8 +104,9 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version) vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID; } - msg->monitor_page1 = virt_to_phys(vmbus_connection.monitor_pages[0]); - msg->monitor_page2 = virt_to_phys(vmbus_connection.monitor_pages[1]); + msg->monitor_page1 = vmbus_connection.monitor_pages_pa[0]; + msg->monitor_page2 = vmbus_connection.monitor_pages_pa[1]; + msg->target_vcpu = hv_cpu_number_to_vp_number(VMBUS_CONNECT_CPU); /* @@ -216,6 +219,65 @@ int vmbus_connect(void) goto cleanup; } + vmbus_connection.monitor_pages_original[0] + = vmbus_connection.monitor_pages[0]; + vmbus_connection.monitor_pages_original[1] + = vmbus_connection.monitor_pages[1]; + vmbus_connection.monitor_pages_pa[0] + = virt_to_phys(vmbus_connection.monitor_pages[0]); + vmbus_connection.monitor_pages_pa[1] + = virt_to_phys(vmbus_connection.monitor_pages[1]); + + if (hv_is_isolation_supported()) { + vmbus_connection.monitor_pages_pa[0] += + ms_hyperv.shared_gpa_boundary; + vmbus_connection.monitor_pages_pa[1] += + ms_hyperv.shared_gpa_boundary; + + ret = set_memory_decrypted((unsigned long) + vmbus_connection.monitor_pages[0], + 1); + ret |= set_memory_decrypted((unsigned long) + vmbus_connection.monitor_pages[1], + 1); + if (ret) + goto cleanup; + + /* + * Isolation VM with AMD SNP needs to access monitor page via + * address space above shared gpa boundary. + */ + if (hv_isolation_type_snp()) { + vmbus_connection.monitor_pages[0] + = memremap(vmbus_connection.monitor_pages_pa[0], + HV_HYP_PAGE_SIZE, + MEMREMAP_WB); + if (!vmbus_connection.monitor_pages[0]) { + ret = -ENOMEM; + goto cleanup; + } + + vmbus_connection.monitor_pages[1] + = memremap(vmbus_connection.monitor_pages_pa[1], + HV_HYP_PAGE_SIZE, + MEMREMAP_WB); + if (!vmbus_connection.monitor_pages[1]) { + ret = -ENOMEM; + goto cleanup; + } + } + + /* + * Set memory host visibility hvcall smears memory + * and so zero monitor pages here. + */ + memset(vmbus_connection.monitor_pages[0], 0x00, + HV_HYP_PAGE_SIZE); + memset(vmbus_connection.monitor_pages[1], 0x00, + HV_HYP_PAGE_SIZE); + + } + msginfo = kzalloc(sizeof(*msginfo) + sizeof(struct vmbus_channel_initiate_contact), GFP_KERNEL); @@ -303,10 +365,26 @@ void vmbus_disconnect(void) vmbus_connection.int_page = NULL; } - hv_free_hyperv_page((unsigned long)vmbus_connection.monitor_pages[0]); - hv_free_hyperv_page((unsigned long)vmbus_connection.monitor_pages[1]); - vmbus_connection.monitor_pages[0] = NULL; - vmbus_connection.monitor_pages[1] = NULL; + if (hv_is_isolation_supported()) { + memunmap(vmbus_connection.monitor_pages[0]); + memunmap(vmbus_connection.monitor_pages[1]); + + set_memory_encrypted((unsigned long) + vmbus_connection.monitor_pages_original[0], + 1); + set_memory_encrypted((unsigned long) + vmbus_connection.monitor_pages_original[1], + 1); + } + + hv_free_hyperv_page((unsigned long) + vmbus_connection.monitor_pages_original[0]); + hv_free_hyperv_page((unsigned long) + vmbus_connection.monitor_pages_original[1]); + vmbus_connection.monitor_pages_original[0] = + vmbus_connection.monitor_pages[0] = NULL; + vmbus_connection.monitor_pages_original[1] = + vmbus_connection.monitor_pages[1] = NULL; } /* diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 42f3d9d123a1..560cba916d1d 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -240,6 +240,8 @@ struct vmbus_connection { * is child->parent notification */ struct hv_monitor_page *monitor_pages[2]; + void *monitor_pages_original[2]; + unsigned long monitor_pages_pa[2]; struct list_head chn_msg_list; spinlock_t channelmsg_lock;