From: Tianyu Lan <hidden> Date: 2021-07-28 14:53:13
From: Tianyu Lan <redacted>
Hyper-V provides two kinds of Isolation VMs. VBS(Virtualization-based
security) and AMD SEV-SNP unenlightened Isolation VMs. This patchset
is to add support for these Isolation VM support in Linux.
The memory of these vms are encrypted and host can't access guest
memory directly. Hyper-V provides new host visibility hvcall and
the guest needs to call new hvcall to mark memory visible to host
before sharing memory with host. For security, all network/storage
stack memory should not be shared with host and so there is bounce
buffer requests.
Vmbus channel ring buffer already plays bounce buffer role because
all data from/to host needs to copy from/to between the ring buffer
and IO stack memory. So mark vmbus channel ring buffer visible.
There are two exceptions - packets sent by vmbus_sendpacket_
pagebuffer() and vmbus_sendpacket_mpb_desc(). These packets
contains IO stack memory address and host will access these memory.
So add allocation bounce buffer support in vmbus for these packets.
For SNP isolation VM, guest needs to access the shared memory via
extra address space which is specified by Hyper-V CPUID HYPERV_CPUID_
ISOLATION_CONFIG. The access physical address of the shared memory
should be bounce buffer memory GPA plus with shared_gpa_boundary
reported by CPUID.
Change sicne RFC V4:
- Introduce dma map decrypted function to remap bounce buffer
and provide dma map decrypted ops for platform to hook callback.
- Split swiotlb and dma map decrypted change into two patches
- Replace vstart with vaddr in swiotlb changes.
Change since RFC v3:
- Add interface set_memory_decrypted_map() to decrypt memory and
map bounce buffer in extra address space
- Remove swiotlb remap function and store the remap address
returned by set_memory_decrypted_map() in swiotlb mem data structure.
- Introduce hv_set_mem_enc() to make code more readable in the __set_memory_enc_dec().
Change since RFC v2:
- Remove not UIO driver in Isolation VM patch
- Use vmap_pfn() to replace ioremap_page_range function in
order to avoid exposing symbol ioremap_page_range() and
ioremap_page_range()
- Call hv set mem host visibility hvcall in set_memory_encrypted/decrypted()
- Enable swiotlb force mode instead of adding Hyper-V dma map/unmap hook
- Fix code style
Tianyu Lan (13):
x86/HV: Initialize GHCB page in Isolation VM
x86/HV: Initialize shared memory boundary in the Isolation VM.
x86/HV: Add new hvcall guest address host visibility support
HV: Mark vmbus ring buffer visible to host in Isolation VM
HV: Add Write/Read MSR registers via ghcb page
HV: Add ghcb hvcall support for SNP VM
HV/Vmbus: Add SNP support for VMbus channel initiate message
HV/Vmbus: Initialize VMbus ring buffer for Isolation VM
DMA: Add dma_map_decrypted/dma_unmap_encrypted() function
x86/Swiotlb: Add Swiotlb bounce buffer remap function for HV IVM
HV/IOMMU: Enable swiotlb bounce buffer for Isolation VM
HV/Netvsc: Add Isolation VM support for netvsc driver
HV/Storvsc: Add Isolation VM support for storvsc driver
arch/x86/hyperv/Makefile | 2 +-
arch/x86/hyperv/hv_init.c | 87 +++++++--
arch/x86/hyperv/ivm.c | 296 +++++++++++++++++++++++++++++
arch/x86/include/asm/hyperv-tlfs.h | 18 ++
arch/x86/include/asm/mshyperv.h | 86 ++++++++-
arch/x86/include/asm/sev.h | 4 +
arch/x86/kernel/cpu/mshyperv.c | 5 +
arch/x86/kernel/sev-shared.c | 21 +-
arch/x86/mm/pat/set_memory.c | 6 +-
arch/x86/xen/pci-swiotlb-xen.c | 3 +-
drivers/hv/Kconfig | 1 +
drivers/hv/channel.c | 48 ++++-
drivers/hv/connection.c | 71 ++++++-
drivers/hv/hv.c | 129 +++++++++----
drivers/hv/hyperv_vmbus.h | 3 +
drivers/hv/ring_buffer.c | 84 ++++++--
drivers/hv/vmbus_drv.c | 3 +
drivers/iommu/hyperv-iommu.c | 65 +++++++
drivers/net/hyperv/hyperv_net.h | 6 +
drivers/net/hyperv/netvsc.c | 144 +++++++++++++-
drivers/net/hyperv/rndis_filter.c | 2 +
drivers/scsi/storvsc_drv.c | 68 ++++++-
include/asm-generic/hyperv-tlfs.h | 1 +
include/asm-generic/mshyperv.h | 53 +++++-
include/linux/dma-map-ops.h | 9 +
include/linux/hyperv.h | 16 ++
include/linux/swiotlb.h | 4 +
kernel/dma/mapping.c | 22 +++
kernel/dma/swiotlb.c | 11 +-
29 files changed, 1166 insertions(+), 102 deletions(-)
create mode 100644 arch/x86/hyperv/ivm.c
--
2.25.1
From: Tianyu Lan <hidden> Date: 2021-07-28 14:53:18
From: Tianyu Lan <redacted>
Hyper-V exposes GHCB page via SEV ES GHCB MSR for SNP guest
to communicate with hypervisor. Map GHCB page for all
cpus to read/write MSR register and submit hvcall request
via GHCB.
Signed-off-by: Tianyu Lan <redacted>
---
arch/x86/hyperv/hv_init.c | 73 +++++++++++++++++++++++++++++++--
arch/x86/include/asm/mshyperv.h | 2 +
include/asm-generic/mshyperv.h | 2 +
3 files changed, 73 insertions(+), 4 deletions(-)
@@ -75,6 +96,8 @@ static int hv_cpu_init(unsigned int cpu)wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE,val);}+hyperv_init_ghcb();+return0;}
@@ -167,6 +190,31 @@ static int hv_cpu_die(unsigned int cpu){structhv_reenlightenment_controlre_ctrl;unsignedintnew_cpu;+unsignedlongflags;+void**input_arg;+void*pg;+void**ghcb_va=NULL;++local_irq_save(flags);+input_arg=(void**)this_cpu_ptr(hyperv_pcpu_input_arg);+pg=*input_arg;+*input_arg=NULL;++if(hv_root_partition){+void**output_arg;++output_arg=(void**)this_cpu_ptr(hyperv_pcpu_output_arg);+*output_arg=NULL;+}++if(ms_hyperv.ghcb_base){+ghcb_va=(void**)this_cpu_ptr(ms_hyperv.ghcb_base);+if(*ghcb_va)+memunmap(*ghcb_va);+*ghcb_va=NULL;+}++local_irq_restore(flags);hv_common_cpu_die(cpu);
@@ -340,9 +388,22 @@ void __init hyperv_init(void)VMALLOC_END,GFP_KERNEL,PAGE_KERNEL_ROX,VM_FLUSH_RESET_PERMS,NUMA_NO_NODE,__builtin_return_address(0));-if(hv_hypercall_pg==NULL){-wrmsrl(HV_X64_MSR_GUEST_OS_ID,0);-gotoremove_cpuhp_state;+if(hv_hypercall_pg==NULL)+gotoclean_guest_os_id;++if(hv_isolation_type_snp()){+ms_hyperv.ghcb_base=alloc_percpu(void*);+if(!ms_hyperv.ghcb_base)+gotoclean_guest_os_id;++if(hyperv_init_ghcb()){+free_percpu(ms_hyperv.ghcb_base);+ms_hyperv.ghcb_base=NULL;+gotoclean_guest_os_id;+}++/* Hyper-V requires to write guest os id via ghcb in SNP IVM. */+hv_ghcb_msr_write(HV_X64_MSR_GUEST_OS_ID,guest_id);}rdmsrl(HV_X64_MSR_HYPERCALL,hypercall_msr.as_uint64);
@@ -578,4 +583,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/sizeof(u64))-2)++/* HvCallModifySparseGpaPageHostVisibility hypercall */+structhv_gpa_range_for_visibility{+u64partition_id;+u32host_visibility:2;+u32reserved0:30;+u32reserved1;+u64gpa_page_list[HV_MAX_MODIFY_GPA_REP_COUNT];+}__packed;+#endif
@@ -1986,7 +1988,9 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)intret;/* Nothing to do if memory encryption is not active */-if(!mem_encrypt_active())+if(hv_is_isolation_supported())+returnhv_set_mem_enc(addr,numpages,enc);+elseif(!mem_encrypt_active())return0;/* Should not be working on unaligned addresses */
From: Tianyu Lan <hidden> Date: 2021-07-28 14:53:26
From: Tianyu Lan <redacted>
Hyper-V exposes shared memory boundary via cpuid
HYPERV_CPUID_ISOLATION_CONFIG and store it in the
shared_gpa_boundary of ms_hyperv struct. This prepares
to share memory with host for SNP guest.
Signed-off-by: Tianyu Lan <redacted>
---
arch/x86/kernel/cpu/mshyperv.c | 2 ++
include/asm-generic/mshyperv.h | 12 +++++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
@@ -327,6 +327,8 @@ static void __init ms_hyperv_init_platform(void)if(ms_hyperv.priv_high&HV_ISOLATION){ms_hyperv.isolation_config_a=cpuid_eax(HYPERV_CPUID_ISOLATION_CONFIG);ms_hyperv.isolation_config_b=cpuid_ebx(HYPERV_CPUID_ISOLATION_CONFIG);+ms_hyperv.shared_gpa_boundary=+(u64)1<<ms_hyperv.shared_gpa_boundary_bits;pr_info("Hyper-V: Isolation Config: Group A 0x%x, Group B 0x%x\n",ms_hyperv.isolation_config_a,ms_hyperv.isolation_config_b);
@@ -465,7 +466,7 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,structlist_head*curr;u32next_gpadl_handle;unsignedlongflags;-intret=0;+intret=0,index;next_gpadl_handle=(atomic_inc_return(&vmbus_connection.next_gpadl_handle)-1);
@@ -474,6 +475,13 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,if(ret)returnret;+ret=set_memory_decrypted((unsignedlong)kbuffer,+HVPFN_UP(size));+if(ret){+pr_warn("Failed to set host visibility.\n");+returnret;+}+init_completion(&msginfo->waitevent);msginfo->waiting_channel=channel;
@@ -539,6 +547,15 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,/* At this point, we received the gpadl created msg */*gpadl_handle=gpadlmsg->gpadl;+if(type==HV_GPADL_BUFFER)+index=0;+else+index=channel->gpadl_range[1].gpadlhandle?2:1;++channel->gpadl_range[index].size=size;+channel->gpadl_range[index].buffer=kbuffer;+channel->gpadl_range[index].gpadlhandle=*gpadl_handle;+cleanup:spin_lock_irqsave(&vmbus_connection.channelmsg_lock,flags);list_del(&msginfo->msglistentry);
@@ -549,6 +566,11 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,}kfree(msginfo);++if(ret)+set_memory_encrypted((unsignedlong)kbuffer,+HVPFN_UP(size));+returnret;}
@@ -811,7 +833,7 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)structvmbus_channel_gpadl_teardown*msg;structvmbus_channel_msginfo*info;unsignedlongflags;-intret;+intret,i;info=kzalloc(sizeof(*info)+sizeof(structvmbus_channel_gpadl_teardown),GFP_KERNEL);
@@ -859,6 +881,18 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle)spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock,flags);kfree(info);++/* Find gpadl buffer virtual address and size. */+for(i=0;i<VMBUS_GPADL_RANGE_COUNT;i++)+if(channel->gpadl_range[i].gpadlhandle==gpadl_handle)+break;++if(set_memory_encrypted((unsignedlong)channel->gpadl_range[i].buffer,+HVPFN_UP(channel->gpadl_range[i].size)))+pr_warn("Fail to set mem host visibility.\n");++channel->gpadl_range[i].gpadlhandle=0;+returnret;}EXPORT_SYMBOL_GPL(vmbus_teardown_gpadl);
From: Tianyu Lan <hidden> Date: 2021-07-28 14:53:36
From: Tianyu Lan <redacted>
Hyper-V provides GHCB protocol to write Synthetic Interrupt
Controller MSR registers in Isolation VM with AMD SEV SNP
and these registers are emulated by hypervisor directly.
Hyper-V requires to write SINTx MSR registers twice. First
writes MSR via GHCB page to communicate with hypervisor
and then writes wrmsr instruction to talk with paravisor
which runs in VMPL0. Guest OS ID MSR also needs to be set
via GHCB.
Signed-off-by: Tianyu Lan <redacted>
---
arch/x86/hyperv/hv_init.c | 16 +----
arch/x86/hyperv/ivm.c | 114 ++++++++++++++++++++++++++++++
arch/x86/include/asm/mshyperv.h | 78 +++++++++++++++++++-
arch/x86/include/asm/sev.h | 4 ++
arch/x86/kernel/cpu/mshyperv.c | 3 +
arch/x86/kernel/sev-shared.c | 21 ++++--
drivers/hv/hv.c | 121 ++++++++++++++++++++++----------
include/asm-generic/mshyperv.h | 12 +++-
8 files changed, 307 insertions(+), 62 deletions(-)
@@ -13,6 +15,118 @@#include<asm/io.h>#include<asm/mshyperv.h>+voidhv_ghcb_msr_write(u64msr,u64value)+{+unionhv_ghcb*hv_ghcb;+void**ghcb_base;+unsignedlongflags;++if(!ms_hyperv.ghcb_base)+return;++WARN_ON(in_nmi());++local_irq_save(flags);+ghcb_base=(void**)this_cpu_ptr(ms_hyperv.ghcb_base);+hv_ghcb=(unionhv_ghcb*)*ghcb_base;+if(!hv_ghcb){+local_irq_restore(flags);+return;+}++memset(hv_ghcb,0x00,HV_HYP_PAGE_SIZE);++ghcb_set_rcx(&hv_ghcb->ghcb,msr);+ghcb_set_rax(&hv_ghcb->ghcb,lower_32_bits(value));+ghcb_set_rdx(&hv_ghcb->ghcb,value>>32);++if(sev_es_ghcb_hv_call(&hv_ghcb->ghcb,NULL,SVM_EXIT_MSR,1,0))+pr_warn("Fail to write msr via ghcb %llx.\n",msr);++local_irq_restore(flags);+}++voidhv_ghcb_msr_read(u64msr,u64*value)+{+unionhv_ghcb*hv_ghcb;+void**ghcb_base;+unsignedlongflags;++if(!ms_hyperv.ghcb_base)+return;++WARN_ON(in_nmi());++local_irq_save(flags);+ghcb_base=(void**)this_cpu_ptr(ms_hyperv.ghcb_base);+hv_ghcb=(unionhv_ghcb*)*ghcb_base;+if(!hv_ghcb){+local_irq_restore(flags);+return;+}++memset(hv_ghcb,0x00,HV_HYP_PAGE_SIZE);++ghcb_set_rcx(&hv_ghcb->ghcb,msr);+if(sev_es_ghcb_hv_call(&hv_ghcb->ghcb,NULL,SVM_EXIT_MSR,0,0))+pr_warn("Fail to read msr via ghcb %llx.\n",msr);+else+*value=(u64)lower_32_bits(hv_ghcb->ghcb.save.rax)+|((u64)lower_32_bits(hv_ghcb->ghcb.save.rdx)<<32);+local_irq_restore(flags);+}++voidhv_sint_rdmsrl_ghcb(u64msr,u64*value)+{+hv_ghcb_msr_read(msr,value);+}+EXPORT_SYMBOL_GPL(hv_sint_rdmsrl_ghcb);++voidhv_sint_wrmsrl_ghcb(u64msr,u64value)+{+hv_ghcb_msr_write(msr,value);++/* Write proxy bit vua wrmsrl instruction. */+if(msr>=HV_X64_MSR_SINT0&&msr<=HV_X64_MSR_SINT15)+wrmsrl(msr,value|1<<20);+}+EXPORT_SYMBOL_GPL(hv_sint_wrmsrl_ghcb);++voidhv_signal_eom_ghcb(void)+{+hv_sint_wrmsrl_ghcb(HV_X64_MSR_EOM,0);+}+EXPORT_SYMBOL_GPL(hv_signal_eom_ghcb);++enumhv_isolation_typehv_get_isolation_type(void)+{+if(!(ms_hyperv.priv_high&HV_ISOLATION))+returnHV_ISOLATION_TYPE_NONE;+returnFIELD_GET(HV_ISOLATION_TYPE,ms_hyperv.isolation_config_b);+}+EXPORT_SYMBOL_GPL(hv_get_isolation_type);++/*+*hv_is_isolation_supported-ChecksystemrunsintheHyper-V+*isolationVM.+*/+boolhv_is_isolation_supported(void)+{+returnhv_get_isolation_type()!=HV_ISOLATION_TYPE_NONE;+}++DEFINE_STATIC_KEY_FALSE(isolation_type_snp);++/*+*hv_isolation_type_snp-ChecksystemrunsintheAMDSEV-SNPbased+*isolationVM.+*/+boolhv_isolation_type_snp(void)+{+returnstatic_branch_unlikely(&isolation_type_snp);+}+EXPORT_SYMBOL_GPL(hv_isolation_type_snp);+/**hv_mark_gpa_visibility-Setpagesvisibletohostviahvcall.*
@@ -332,6 +332,9 @@ static void __init ms_hyperv_init_platform(void)pr_info("Hyper-V: Isolation Config: Group A 0x%x, Group B 0x%x\n",ms_hyperv.isolation_config_a,ms_hyperv.isolation_config_b);++if(hv_get_isolation_type()==HV_ISOLATION_TYPE_SNP)+static_branch_enable(&isolation_type_snp);}if(hv_max_functions_eax>=HYPERV_CPUID_NESTED_FEATURES){
@@ -120,7 +129,7 @@ static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb,v=info&SVM_EVTINJ_VEC_MASK;/* Check if exception information from hypervisor is sane. */-if((info&SVM_EVTINJ_VALID)&&+if(ctxt&&(info&SVM_EVTINJ_VALID)&&((v==X86_TRAP_GP)||(v==X86_TRAP_UD))&&((info&SVM_EVTINJ_TYPE_MASK)==SVM_EVTINJ_TYPE_EXEPT)){ctxt->fi.vector=v;
@@ -199,26 +214,43 @@ void hv_synic_enable_regs(unsigned int cpu)unionhv_synic_scontrolsctrl;/* Setup the Synic's message page */-simp.as_uint64=hv_get_register(HV_REGISTER_SIMP);+hv_get_simp(simp.as_uint64);simp.simp_enabled=1;-simp.base_simp_gpa=virt_to_phys(hv_cpu->synic_message_page)->>HV_HYP_PAGE_SHIFT;-hv_set_register(HV_REGISTER_SIMP,simp.as_uint64);+if(hv_isolation_type_snp()){+hv_cpu->synic_message_page+=memremap(simp.base_simp_gpa<<HV_HYP_PAGE_SHIFT,+HV_HYP_PAGE_SIZE,MEMREMAP_WB);+if(!hv_cpu->synic_message_page)+pr_err("Fail to map syinc message page.\n");+}else{+simp.base_simp_gpa=virt_to_phys(hv_cpu->synic_message_page)+>>HV_HYP_PAGE_SHIFT;+}++hv_set_simp(simp.as_uint64);/* Setup the Synic's event page */-siefp.as_uint64=hv_get_register(HV_REGISTER_SIEFP);+hv_get_siefp(siefp.as_uint64);siefp.siefp_enabled=1;-siefp.base_siefp_gpa=virt_to_phys(hv_cpu->synic_event_page)->>HV_HYP_PAGE_SHIFT;-hv_set_register(HV_REGISTER_SIEFP,siefp.as_uint64);+if(hv_isolation_type_snp()){+hv_cpu->synic_event_page=+memremap(siefp.base_siefp_gpa<<HV_HYP_PAGE_SHIFT,+HV_HYP_PAGE_SIZE,MEMREMAP_WB);++if(!hv_cpu->synic_event_page)+pr_err("Fail to map syinc event page.\n");+}else{+siefp.base_siefp_gpa=virt_to_phys(hv_cpu->synic_event_page)+>>HV_HYP_PAGE_SHIFT;+}+hv_set_siefp(siefp.as_uint64);/* Setup the shared SINT. */if(vmbus_irq!=-1)enable_percpu_irq(vmbus_irq,0);-shared_sint.as_uint64=hv_get_register(HV_REGISTER_SINT0+-VMBUS_MESSAGE_SINT);+hv_get_synint_state(VMBUS_MESSAGE_SINT,shared_sint.as_uint64);shared_sint.vector=vmbus_interrupt;shared_sint.masked=false;
@@ -233,14 +265,12 @@ void hv_synic_enable_regs(unsigned int cpu)#elseshared_sint.auto_eoi=0;#endif-hv_set_register(HV_REGISTER_SINT0+VMBUS_MESSAGE_SINT,-shared_sint.as_uint64);+hv_set_synint_state(VMBUS_MESSAGE_SINT,shared_sint.as_uint64);/* Enable the global synic bit */-sctrl.as_uint64=hv_get_register(HV_REGISTER_SCONTROL);+hv_get_synic_state(sctrl.as_uint64);sctrl.enable=1;--hv_set_register(HV_REGISTER_SCONTROL,sctrl.as_uint64);+hv_set_synic_state(sctrl.as_uint64);}inthv_synic_init(unsignedintcpu)
@@ -257,37 +287,50 @@ int hv_synic_init(unsigned int cpu)*/voidhv_synic_disable_regs(unsignedintcpu){+structhv_per_cpu_context*hv_cpu+=per_cpu_ptr(hv_context.cpu_context,cpu);unionhv_synic_sintshared_sint;unionhv_synic_simpsimp;unionhv_synic_siefpsiefp;unionhv_synic_scontrolsctrl;-shared_sint.as_uint64=hv_get_register(HV_REGISTER_SINT0+-VMBUS_MESSAGE_SINT);-+hv_get_synint_state(VMBUS_MESSAGE_SINT,shared_sint.as_uint64);shared_sint.masked=1;+hv_set_synint_state(VMBUS_MESSAGE_SINT,shared_sint.as_uint64);+/* Need to correctly cleanup in the case of SMP!!! *//* Disable the interrupt */-hv_set_register(HV_REGISTER_SINT0+VMBUS_MESSAGE_SINT,-shared_sint.as_uint64);+hv_get_simp(simp.as_uint64);-simp.as_uint64=hv_get_register(HV_REGISTER_SIMP);+/*+*InIsolationVM,simandsiefpagesareallocatedby+*paravisor.Thesepagesalsowillbeusedbykdump+*kernel.Sojustresetenablebithereandkeeppage+*addresses.+*/simp.simp_enabled=0;-simp.base_simp_gpa=0;+if(hv_isolation_type_snp())+memunmap(hv_cpu->synic_message_page);+else+simp.base_simp_gpa=0;-hv_set_register(HV_REGISTER_SIMP,simp.as_uint64);+hv_set_simp(simp.as_uint64);-siefp.as_uint64=hv_get_register(HV_REGISTER_SIEFP);+hv_get_siefp(siefp.as_uint64);siefp.siefp_enabled=0;-siefp.base_siefp_gpa=0;-hv_set_register(HV_REGISTER_SIEFP,siefp.as_uint64);+if(hv_isolation_type_snp())+memunmap(hv_cpu->synic_event_page);+else+siefp.base_siefp_gpa=0;++hv_set_siefp(siefp.as_uint64);/* Disable the global synic bit */-sctrl.as_uint64=hv_get_register(HV_REGISTER_SCONTROL);+hv_get_synic_state(sctrl.as_uint64);sctrl.enable=0;-hv_set_register(HV_REGISTER_SCONTROL,sctrl.as_uint64);+hv_set_synic_state(sctrl.as_uint64);if(vmbus_irq!=-1)disable_percpu_irq(vmbus_irq);
@@ -98,7 +98,13 @@ int hv_post_message(union hv_connection_id connection_id,aligned_msg->payload_size=payload_size;memcpy((void*)aligned_msg->payload,payload,payload_size);-status=hv_do_hypercall(HVCALL_POST_MESSAGE,aligned_msg,NULL);+if(hv_isolation_type_snp())+status=hv_ghcb_hypercall(HVCALL_POST_MESSAGE,+(void*)aligned_msg,NULL,+sizeof(structhv_input_post_message));+else+status=hv_do_hypercall(HVCALL_POST_MESSAGE,+aligned_msg,NULL);/* Preemption must remain disabled until after the hypercall*sosomeotherthreadcan'tgetscheduledontothiscpuand
From: Tianyu Lan <hidden> Date: 2021-07-28 14:53:58
From: Tianyu Lan <redacted>
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).
Signed-off-by: Tianyu Lan <redacted>
---
drivers/hv/connection.c | 65 +++++++++++++++++++++++++++++++++++++++
drivers/hv/hyperv_vmbus.h | 1 +
2 files changed, 66 insertions(+)
From: Tianyu Lan <hidden> Date: 2021-07-28 14:54:11
From: Tianyu Lan <redacted>
VMbus ring buffer are shared with host and it's need to
be accessed via extra address space of Isolation VM with
SNP support. This patch is to map the ring buffer
address in extra address space via ioremap(). HV host
visibility hvcall smears data in the ring buffer and
so reset the ring buffer memory to zero after calling
visibility hvcall.
Signed-off-by: Tianyu Lan <redacted>
---
drivers/hv/Kconfig | 1 +
drivers/hv/channel.c | 10 +++++
drivers/hv/hyperv_vmbus.h | 2 +
drivers/hv/ring_buffer.c | 84 ++++++++++++++++++++++++++++++---------
4 files changed, 79 insertions(+), 18 deletions(-)
@@ -707,6 +707,16 @@ static int __vmbus_open(struct vmbus_channel *newchannel,if(err)gotoerror_clean_ring;+err=hv_ringbuffer_post_init(&newchannel->outbound,+page,send_pages);+if(err)+gotoerror_free_gpadl;++err=hv_ringbuffer_post_init(&newchannel->inbound,+&page[send_pages],recv_pages);+if(err)+gotoerror_free_gpadl;+/* Create and init the channel open message */open_info=kzalloc(sizeof(*open_info)+sizeof(structvmbus_channel_open_channel),
@@ -179,43 +181,89 @@ void hv_ringbuffer_pre_init(struct vmbus_channel *channel)mutex_init(&channel->outbound.ring_buffer_mutex);}-/* Initialize the ring buffer. */-inthv_ringbuffer_init(structhv_ring_buffer_info*ring_info,-structpage*pages,u32page_cnt,u32max_pkt_size)+inthv_ringbuffer_post_init(structhv_ring_buffer_info*ring_info,+structpage*pages,u32page_cnt){+u64physic_addr=page_to_pfn(pages)<<PAGE_SHIFT;+unsignedlong*pfns_wraparound;+void*vaddr;inti;-structpage**pages_wraparound;-BUILD_BUG_ON((sizeof(structhv_ring_buffer)!=PAGE_SIZE));+if(!hv_isolation_type_snp())+return0;++physic_addr+=ms_hyperv.shared_gpa_boundary;/**Firstpageholdsstructhv_ring_buffer,dowraparoundmappingfor*therest.*/-pages_wraparound=kcalloc(page_cnt*2-1,sizeof(structpage*),+pfns_wraparound=kcalloc(page_cnt*2-1,sizeof(unsignedlong),GFP_KERNEL);-if(!pages_wraparound)+if(!pfns_wraparound)return-ENOMEM;-pages_wraparound[0]=pages;+pfns_wraparound[0]=physic_addr>>PAGE_SHIFT;for(i=0;i<2*(page_cnt-1);i++)-pages_wraparound[i+1]=&pages[i%(page_cnt-1)+1];--ring_info->ring_buffer=(structhv_ring_buffer*)-vmap(pages_wraparound,page_cnt*2-1,VM_MAP,PAGE_KERNEL);--kfree(pages_wraparound);+pfns_wraparound[i+1]=(physic_addr>>PAGE_SHIFT)++i%(page_cnt-1)+1;--if(!ring_info->ring_buffer)+vaddr=vmap_pfn(pfns_wraparound,page_cnt*2-1,PAGE_KERNEL_IO);+kfree(pfns_wraparound);+if(!vaddr)return-ENOMEM;-ring_info->ring_buffer->read_index=-ring_info->ring_buffer->write_index=0;+/* Clean memory after setting host visibility. */+memset((void*)vaddr,0x00,page_cnt*PAGE_SIZE);++ring_info->ring_buffer=(structhv_ring_buffer*)vaddr;+ring_info->ring_buffer->read_index=0;+ring_info->ring_buffer->write_index=0;/* Set the feature bit for enabling flow control. */ring_info->ring_buffer->feature_bits.value=1;+return0;+}++/* Initialize the ring buffer. */+inthv_ringbuffer_init(structhv_ring_buffer_info*ring_info,+structpage*pages,u32page_cnt,u32max_pkt_size)+{+inti;+structpage**pages_wraparound;++BUILD_BUG_ON((sizeof(structhv_ring_buffer)!=PAGE_SIZE));++if(!hv_isolation_type_snp()){+/*+*Firstpageholdsstructhv_ring_buffer,dowraparoundmappingfor+*therest.+*/+pages_wraparound=kcalloc(page_cnt*2-1,sizeof(structpage*),+GFP_KERNEL);+if(!pages_wraparound)+return-ENOMEM;++pages_wraparound[0]=pages;+for(i=0;i<2*(page_cnt-1);i++)+pages_wraparound[i+1]=&pages[i%(page_cnt-1)+1];++ring_info->ring_buffer=(structhv_ring_buffer*)+vmap(pages_wraparound,page_cnt*2-1,VM_MAP,PAGE_KERNEL);++kfree(pages_wraparound);++if(!ring_info->ring_buffer)+return-ENOMEM;++ring_info->ring_buffer->read_index=+ring_info->ring_buffer->write_index=0;++/* Set the feature bit for enabling flow control. */+ring_info->ring_buffer->feature_bits.value=1;+}+ring_info->ring_size=page_cnt<<PAGE_SHIFT;ring_info->ring_size_div10_reciprocal=reciprocal_value(ring_info->ring_size/10);
From: Tianyu Lan <hidden> Date: 2021-07-28 14:54:21
From: Tianyu Lan <redacted>
In Hyper-V Isolation VM with AMD SEV, swiotlb boucne buffer
needs to be mapped into address space above vTOM and so
introduce dma_map_decrypted/dma_unmap_encrypted() to map/unmap
bounce buffer memory. The platform can populate man/unmap callback
in the dma memory decrypted ops.
Signed-off-by: Tianyu Lan <redacted>
---
include/linux/dma-map-ops.h | 9 +++++++++
kernel/dma/mapping.c | 22 ++++++++++++++++++++++
2 files changed, 31 insertions(+)
From: Tianyu Lan <hidden> Date: 2021-07-28 14:54:25
From: Tianyu Lan <redacted>
In Isolation VM with AMD SEV, bounce buffer needs to be accessed via
extra address space which is above shared_gpa_boundary
(E.G 39 bit address line) reported by Hyper-V CPUID ISOLATION_CONFIG.
The access physical address will be original physical address +
shared_gpa_boundary. The shared_gpa_boundary in the AMD SEV SNP
spec is called virtual top of memory(vTOM). Memory addresses below
vTOM are automatically treated as private while memory above
vTOM is treated as shared.
Use dma_map_decrypted() in the swiotlb code, store remap address returned
and use the remap address to copy data from/to swiotlb bounce buffer.
Signed-off-by: Tianyu Lan <redacted>
---
include/linux/swiotlb.h | 4 ++++
kernel/dma/swiotlb.c | 11 ++++++++---
2 files changed, 12 insertions(+), 3 deletions(-)
From: Tianyu Lan <hidden> Date: 2021-07-28 14:54:29
From: Tianyu Lan <redacted>
In Isolation VM, all shared memory with host needs to mark visible
to host via hvcall. vmbus_establish_gpadl() has already done it for
netvsc rx/tx ring buffer. The page buffer used by vmbus_sendpacket_
pagebuffer() still need to handle. Use DMA API to map/umap these
memory during sending/receiving packet and Hyper-V DMA ops callback
will use swiotlb function to allocate bounce buffer and copy data
from/to bounce buffer.
Signed-off-by: Tianyu Lan <redacted>
---
drivers/net/hyperv/hyperv_net.h | 6 ++
drivers/net/hyperv/netvsc.c | 144 +++++++++++++++++++++++++++++-
drivers/net/hyperv/rndis_filter.c | 2 +
include/linux/hyperv.h | 5 ++
4 files changed, 154 insertions(+), 3 deletions(-)
@@ -1074,6 +1075,7 @@ struct netvsc_device {/* Receive buffer allocated by us but manages by NetVSP */void*recv_buf;+void*recv_original_buf;u32recv_buf_size;/* allocated bytes */u32recv_buf_gpadl_handle;u32recv_section_cnt;
@@ -1082,6 +1084,8 @@ struct netvsc_device {/* Send buffer allocated by us */void*send_buf;+void*send_original_buf;+u32send_buf_size;u32send_buf_gpadl_handle;u32send_section_cnt;u32send_section_size;
@@ -330,6 +343,27 @@ int netvsc_alloc_recv_comp_ring(struct netvsc_device *net_device, u32 q_idx)returnnvchan->mrc.slots?0:-ENOMEM;}+staticvoid*netvsc_remap_buf(void*buf,unsignedlongsize)+{+unsignedlong*pfns;+void*vaddr;+inti;++pfns=kcalloc(size/HV_HYP_PAGE_SIZE,sizeof(unsignedlong),+GFP_KERNEL);+if(!pfns)+returnNULL;++for(i=0;i<size/HV_HYP_PAGE_SIZE;i++)+pfns[i]=virt_to_hvpfn(buf+i*HV_HYP_PAGE_SIZE)++(ms_hyperv.shared_gpa_boundary>>HV_HYP_PAGE_SHIFT);++vaddr=vmap_pfn(pfns,size/HV_HYP_PAGE_SIZE,PAGE_KERNEL_IO);+kfree(pfns);++returnvaddr;+}+staticintnetvsc_init_buf(structhv_device*device,structnetvsc_device*net_device,conststructnetvsc_device_info*device_info)
@@ -340,6 +374,7 @@ static int netvsc_init_buf(struct hv_device *device,unsignedintbuf_size;size_tmap_words;inti,ret=0;+void*vaddr;/* Get receive buffer area. */buf_size=device_info->recv_sections*device_info->recv_section_size;
@@ -375,6 +410,15 @@ static int netvsc_init_buf(struct hv_device *device,gotocleanup;}+if(hv_isolation_type_snp()){+vaddr=netvsc_remap_buf(net_device->recv_buf,buf_size);+if(!vaddr)+gotocleanup;++net_device->recv_original_buf=net_device->recv_buf;+net_device->recv_buf=vaddr;+}+/* Notify the NetVsp of the gpadl handle */init_packet=&net_device->channel_init_pkt;memset(init_packet,0,sizeof(structnvsp_message));
@@ -477,6 +521,15 @@ static int netvsc_init_buf(struct hv_device *device,gotocleanup;}+if(hv_isolation_type_snp()){+vaddr=netvsc_remap_buf(net_device->send_buf,buf_size);+if(!vaddr)+gotocleanup;++net_device->send_original_buf=net_device->send_buf;+net_device->send_buf=vaddr;+}+/* Notify the NetVsp of the gpadl handle */init_packet=&net_device->channel_init_pkt;memset(init_packet,0,sizeof(structnvsp_message));
@@ -767,7 +820,7 @@ static void netvsc_send_tx_complete(struct net_device *ndev,/* Notify the layer above us */if(likely(skb)){-conststructhv_netvsc_packet*packet+structhv_netvsc_packet*packet=(structhv_netvsc_packet*)skb->cb;u32send_index=packet->send_buf_index;structnetvsc_stats*tx_stats;
From: Tianyu Lan <hidden> Date: 2021-07-28 14:54:31
From: Tianyu Lan <redacted>
Hyper-V Isolation VM requires bounce buffer support to copy
data from/to encrypted memory and so enable swiotlb force
mode to use swiotlb bounce buffer for DMA transaction.
In Isolation VM with AMD SEV, the bounce buffer needs to be
accessed via extra address space which is above shared_gpa_boundary
(E.G 39 bit address line) reported by Hyper-V CPUID ISOLATION_CONFIG.
The access physical address will be original physical address +
shared_gpa_boundary. The shared_gpa_boundary in the AMD SEV SNP
spec is called virtual top of memory(vTOM). Memory addresses below
vTOM are automatically treated as private while memory above
vTOM is treated as shared.
Swiotlb bounce buffer code calls dma_map_decrypted()
to mark bounce buffer visible to host and map it in extra
address space. Populate dma memory decrypted ops with hv
map/unmap function.
Hyper-V initalizes swiotlb bounce buffer and default swiotlb
needs to be disabled. pci_swiotlb_detect_override() and
pci_swiotlb_detect_4gb() enable the default one. To override
the setting, hyperv_swiotlb_detect() needs to run before
these detect functions which depends on the pci_xen_swiotlb_
init(). Make pci_xen_swiotlb_init() depends on the hyperv_swiotlb
_detect() to keep the order.
The map function vmap_pfn() can't work in the early place
hyperv_iommu_swiotlb_init() and so initialize swiotlb bounce
buffer in the hyperv_iommu_swiotlb_later_init().
Signed-off-by: Tianyu Lan <redacted>
---
arch/x86/hyperv/ivm.c | 28 ++++++++++++++
arch/x86/include/asm/mshyperv.h | 2 +
arch/x86/xen/pci-swiotlb-xen.c | 3 +-
drivers/hv/vmbus_drv.c | 3 ++
drivers/iommu/hyperv-iommu.c | 65 +++++++++++++++++++++++++++++++++
include/linux/hyperv.h | 1 +
6 files changed, 101 insertions(+), 1 deletion(-)
@@ -266,3 +266,31 @@ int hv_set_mem_enc(unsigned long addr, int numpages, bool enc)enc?VMBUS_PAGE_NOT_VISIBLE:VMBUS_PAGE_VISIBLE_READ_WRITE);}++/*+*hv_map_memory-mapmemorytoextraspaceintheAMDSEV-SNPIsolationVM.+*/+void*hv_map_memory(void*addr,unsignedlongsize)+{+unsignedlong*pfns=kcalloc(size/HV_HYP_PAGE_SIZE,+sizeof(unsignedlong),GFP_KERNEL);+void*vaddr;+inti;++if(!pfns)+return(unsignedlong)NULL;++for(i=0;i<size/HV_HYP_PAGE_SIZE;i++)+pfns[i]=virt_to_hvpfn(addr+i*HV_HYP_PAGE_SIZE)++(ms_hyperv.shared_gpa_boundary>>HV_HYP_PAGE_SHIFT);++vaddr=vmap_pfn(pfns,size/HV_HYP_PAGE_SIZE,PAGE_KERNEL_IO);+kfree(pfns);++returnvaddr;+}++voidhv_unmap_memory(void*addr)+{+vunmap(addr);+}
@@ -249,6 +249,8 @@ int hv_map_ioapic_interrupt(int ioapic_id, bool level, int vcpu, int vector,inthv_unmap_ioapic_interrupt(intioapic_id,structhv_interrupt_entry*entry);inthv_mark_gpa_visibility(u16count,constu64pfn[],u32visibility);inthv_set_mem_enc(unsignedlongaddr,intnumpages,boolenc);+void*hv_map_memory(void*addr,unsignedlongsize);+voidhv_unmap_memory(void*addr);voidhv_sint_wrmsrl_ghcb(u64msr,u64value);voidhv_sint_rdmsrl_ghcb(u64msr,u64*value);voidhv_signal_eom_ghcb(void);
From: Tianyu Lan <hidden> Date: 2021-07-28 14:54:37
From: Tianyu Lan <redacted>
In Isolation VM, all shared memory with host needs to mark visible
to host via hvcall. vmbus_establish_gpadl() has already done it for
storvsc rx/tx ring buffer. The page buffer used by vmbus_sendpacket_
mpb_desc() still need to handle. Use DMA API to map/umap these
memory during sending/receiving packet and Hyper-V DMA ops callback
will use swiotlb function to allocate bounce buffer and copy data
from/to bounce buffer.
Signed-off-by: Tianyu Lan <redacted>
---
drivers/scsi/storvsc_drv.c | 68 +++++++++++++++++++++++++++++++++++---
1 file changed, 63 insertions(+), 5 deletions(-)
@@ -1860,13 +1911,20 @@ static int storvsc_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scmnd)put_cpu();if(ret==-EAGAIN){-if(payload_sz>sizeof(cmd_request->mpb))-kfree(payload);/* no more space */-returnSCSI_MLQUEUE_DEVICE_BUSY;+ret=SCSI_MLQUEUE_DEVICE_BUSY;+gotofree_dma_range;}return0;++free_dma_range:+kfree(cmd_request->dma_range);++free_payload:+if(payload_sz>sizeof(cmd_request->mpb))+kfree(payload);+returnret;}staticstructscsi_host_templatescsi_driver={
From: Dave Hansen <hidden> Date: 2021-07-28 15:29:54
On 7/28/21 7:52 AM, Tianyu Lan wrote:
quoted hunk
@@ -1986,7 +1988,9 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc) int ret; /* Nothing to do if memory encryption is not active */- if (!mem_encrypt_active())+ if (hv_is_isolation_supported())+ return hv_set_mem_enc(addr, numpages, enc);+ else if (!mem_encrypt_active()) return 0;
__set_memory_enc_dec() is turning into a real mess. SEV, TDX and now
Hyper-V are messing around in here.
It doesn't help that these additions are totally uncommented. Even
worse is that hv_set_mem_enc() was intentionally named "enc" when it
presumably has nothing to do with encryption.
This needs to be refactored. The current __set_memory_enc_dec() can
become __set_memory_enc_pgtable(). It gets used for the hypervisors
that get informed about "encryption" status via page tables: SEV and TDX.
Then, rename hv_set_mem_enc() to hv_set_visible_hcall(). You'll end up
with:
int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
{
if (hv_is_isolation_supported())
return hv_set_visible_hcall(...);
if (mem_encrypt_active() || ...)
return __set_memory_enc_pgtable();
/* Nothing to do */
return 0;
}
That tells the story pretty effectively, in code.
From: Dave Hansen <hidden> Date: 2021-07-28 17:07:22
On 7/28/21 7:52 AM, Tianyu Lan wrote:
quoted hunk
@@ -1986,7 +1988,9 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc) int ret; /* Nothing to do if memory encryption is not active */- if (!mem_encrypt_active())+ if (hv_is_isolation_supported())+ return hv_set_mem_enc(addr, numpages, enc);+ else if (!mem_encrypt_active()) return 0;
One more thing. If you're going to be patching generic code, please
start using feature checks that can get optimized away at runtime.
hv_is_isolation_supported() doesn't look like the world's cheapest
check. It can't be inlined and costs at least a function call.
These checks could, with basically no effort be wrapped in a header like
this:
static inline bool hv_is_isolation_supported(void)
{
if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
return 0;
// out of line function call:
return __hv_is_isolation_supported();
}
I don't think it would be the end of the world to add an
X86_FEATURE_HYPERV_GUEST, either. There are plenty of bits allocated
for Xen and VMWare.
From: Tianyu Lan <hidden> Date: 2021-07-29 12:54:38
Hi Dave:
Thanks for your review.
On 7/28/2021 11:29 PM, Dave Hansen wrote:
On 7/28/21 7:52 AM, Tianyu Lan wrote:
quoted
@@ -1986,7 +1988,9 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc) int ret; /* Nothing to do if memory encryption is not active */- if (!mem_encrypt_active())+ if (hv_is_isolation_supported())+ return hv_set_mem_enc(addr, numpages, enc);+ else if (!mem_encrypt_active()) return 0;
__set_memory_enc_dec() is turning into a real mess. SEV, TDX and now
Hyper-V are messing around in here.
It doesn't help that these additions are totally uncommented. Even
worse is that hv_set_mem_enc() was intentionally named "enc" when it
presumably has nothing to do with encryption.
This needs to be refactored. The current __set_memory_enc_dec() can
become __set_memory_enc_pgtable(). It gets used for the hypervisors
that get informed about "encryption" status via page tables: SEV and TDX.
Then, rename hv_set_mem_enc() to hv_set_visible_hcall(). You'll end up
with:
int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
{
if (hv_is_isolation_supported())
return hv_set_visible_hcall(...);
if (mem_encrypt_active() || ...)
return __set_memory_enc_pgtable();
/* Nothing to do */
return 0;
}
That tells the story pretty effectively, in code.
Yes, this is good idea. Thanks for your suggestion.
From: Tianyu Lan <hidden> Date: 2021-07-29 13:02:11
On 7/29/2021 1:06 AM, Dave Hansen wrote:
On 7/28/21 7:52 AM, Tianyu Lan wrote:
quoted
@@ -1986,7 +1988,9 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc) int ret; /* Nothing to do if memory encryption is not active */- if (!mem_encrypt_active())+ if (hv_is_isolation_supported())+ return hv_set_mem_enc(addr, numpages, enc);+ else if (!mem_encrypt_active()) return 0;
One more thing. If you're going to be patching generic code, please
start using feature checks that can get optimized away at runtime.
hv_is_isolation_supported() doesn't look like the world's cheapest
check. It can't be inlined and costs at least a function call.
Yes, you are right. How about adding a static branch key for the check
of isolation VM? This may reduce the check cost.
From: Dave Hansen <hidden> Date: 2021-07-29 14:12:25
On 7/29/21 6:01 AM, Tianyu Lan wrote:
On 7/29/2021 1:06 AM, Dave Hansen wrote:
quoted
On 7/28/21 7:52 AM, Tianyu Lan wrote:
quoted
@@ -1986,7 +1988,9 @@ static int __set_memory_enc_dec(unsigned long
addr, int numpages, bool enc)
int ret;
/* Nothing to do if memory encryption is not active */
- if (!mem_encrypt_active())
+ if (hv_is_isolation_supported())
+ return hv_set_mem_enc(addr, numpages, enc);
+ else if (!mem_encrypt_active())
return 0;
One more thing. If you're going to be patching generic code, please
start using feature checks that can get optimized away at runtime.
hv_is_isolation_supported() doesn't look like the world's cheapest
check. It can't be inlined and costs at least a function call.
Yes, you are right. How about adding a static branch key for the check
of isolation VM? This may reduce the check cost.
I don't think you need a static key.
There are basically three choices:
1. Use an existing X86_FEATURE bit. I think there's already one for
when you are running under a hypervisor. It's not super precise,
but it's better than what you have.
2. Define a new X86_FEATURE bit for when you are running under
Hyper-V.
3. Define a new X86_FEATURE bit specifically for Hyper-V isolation VM
support. This particular feature might be a little uncommon to
deserve its own bit.
I'd probably just do #2.
From: Tianyu Lan <hidden> Date: 2021-07-29 15:02:39
On 7/29/2021 10:09 PM, Dave Hansen wrote:
On 7/29/21 6:01 AM, Tianyu Lan wrote:
quoted
On 7/29/2021 1:06 AM, Dave Hansen wrote:
quoted
On 7/28/21 7:52 AM, Tianyu Lan wrote:
quoted
@@ -1986,7 +1988,9 @@ static int __set_memory_enc_dec(unsigned long
addr, int numpages, bool enc)
int ret;
/* Nothing to do if memory encryption is not active */
- if (!mem_encrypt_active())
+ if (hv_is_isolation_supported())
+ return hv_set_mem_enc(addr, numpages, enc);
+ else if (!mem_encrypt_active())
return 0;
One more thing. If you're going to be patching generic code, please
start using feature checks that can get optimized away at runtime.
hv_is_isolation_supported() doesn't look like the world's cheapest
check. It can't be inlined and costs at least a function call.
Yes, you are right. How about adding a static branch key for the check
of isolation VM? This may reduce the check cost.
I don't think you need a static key.
There are basically three choices:
1. Use an existing X86_FEATURE bit. I think there's already one for
when you are running under a hypervisor. It's not super precise,
but it's better than what you have.
2. Define a new X86_FEATURE bit for when you are running under
Hyper-V.
3. Define a new X86_FEATURE bit specifically for Hyper-V isolation VM
support. This particular feature might be a little uncommon to
deserve its own bit.
I'd probably just do #2.
There is x86_hyper_type to identify hypervisor type and we may check
this variable after checking X86_FEATURE_HYPERVISOR.
static inline bool hv_is_isolation_supported(void)
{
if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
return 0;
if (x86_hyper_type != X86_HYPER_MS_HYPERV)
return 0;
// out of line function call:
return __hv_is_isolation_supported();
}
From: Tianyu Lan <hidden> Date: 2021-07-29 15:13:32
Hi Christoph:
Could you have a look at this patch and the following patch
"[PATCH 10/13] x86/Swiotlb: Add Swiotlb bounce buffer remap function
for HV IVM" These two patches follows your previous comments and add
dma_map_decrypted/dma_unmap_decrypted(). I don't add arch prefix because
each platform may populate their callbacks into dma memory decrypted ops.
Thanks.
On 7/28/2021 10:52 PM, Tianyu Lan wrote:
quoted hunk
From: Tianyu Lan <redacted>
In Hyper-V Isolation VM with AMD SEV, swiotlb boucne buffer
needs to be mapped into address space above vTOM and so
introduce dma_map_decrypted/dma_unmap_encrypted() to map/unmap
bounce buffer memory. The platform can populate man/unmap callback
in the dma memory decrypted ops.
Signed-off-by: Tianyu Lan <redacted>
---
include/linux/dma-map-ops.h | 9 +++++++++
kernel/dma/mapping.c | 22 ++++++++++++++++++++++
2 files changed, 31 insertions(+)
From: Dave Hansen <hidden> Date: 2021-07-29 16:07:59
On 7/29/21 8:02 AM, Tianyu Lan wrote:
quoted
There is x86_hyper_type to identify hypervisor type and we may check
this variable after checking X86_FEATURE_HYPERVISOR.
static inline bool hv_is_isolation_supported(void)
{
if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
return 0;
if (x86_hyper_type != X86_HYPER_MS_HYPERV)
return 0;
// out of line function call:
return __hv_is_isolation_supported();
}
Looks fine. You just might want to use this existing helper:
static inline bool hypervisor_is_type(enum x86_hypervisor_type type)
{
return x86_hyper_type == type;
}
From: Konrad Rzeszutek Wilk <hidden> Date: 2021-07-29 16:30:50
On Wed, Jul 28, 2021 at 10:52:25AM -0400, Tianyu Lan wrote:
quoted hunk
From: Tianyu Lan <redacted>
In Isolation VM with AMD SEV, bounce buffer needs to be accessed via
extra address space which is above shared_gpa_boundary
(E.G 39 bit address line) reported by Hyper-V CPUID ISOLATION_CONFIG.
The access physical address will be original physical address +
shared_gpa_boundary. The shared_gpa_boundary in the AMD SEV SNP
spec is called virtual top of memory(vTOM). Memory addresses below
vTOM are automatically treated as private while memory above
vTOM is treated as shared.
Use dma_map_decrypted() in the swiotlb code, store remap address returned
and use the remap address to copy data from/to swiotlb bounce buffer.
Signed-off-by: Tianyu Lan <redacted>
---
include/linux/swiotlb.h | 4 ++++
kernel/dma/swiotlb.c | 11 ++++++++---
2 files changed, 12 insertions(+), 3 deletions(-)
I am wondering if it would be worth returning an error code in this
function instead of just printing an error?
For this patch I think it is Ok, but perhaps going forward this would be
better done as I am thinking - is there some global guest->hyperv
reporting mechanism so that if this fails - it ends up being bubbled up
to the HyperV console-ish?
And ditto for other hypervisors?
quoted hunk
+ return;
+ }
+
+ memset(mem->vaddr, 0, bytes);
}
int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
From: Tianyu Lan <hidden> Date: 2021-07-30 02:53:04
On 7/30/2021 12:05 AM, Dave Hansen wrote:
On 7/29/21 8:02 AM, Tianyu Lan wrote:
quoted
quoted
There is x86_hyper_type to identify hypervisor type and we may check
this variable after checking X86_FEATURE_HYPERVISOR.
static inline bool hv_is_isolation_supported(void)
{
if (!cpu_feature_enabled(X86_FEATURE_HYPERVISOR))
return 0;
if (x86_hyper_type != X86_HYPER_MS_HYPERV)
return 0;
// out of line function call:
return __hv_is_isolation_supported();
}
Looks fine. You just might want to use this existing helper:
static inline bool hypervisor_is_type(enum x86_hypervisor_type type)
{
return x86_hyper_type == type;
}
Yes,thanks for suggestion and will update in the next version.
I am wondering if it would be worth returning an error code in this
function instead of just printing an error?
Yes, this is good idea and will update in the next version.
For this patch I think it is Ok, but perhaps going forward this would be
better done as I am thinking - is there some global guest->hyperv
reporting mechanism so that if this fails - it ends up being bubbled up
to the HyperV console-ish?
Hyper-V has such panic page report mechanism. Guest can pass one page
log to host during crash.
This deserves a comment. As I understand it, the GHCB pa is set by
Hyper-V or the paravisor, so the page does not need to be allocated by
Linux.
And it is not mapped unencrypted because the GHCB page is allocated
above the VTOM boundary?
quoted hunk
@@ -167,6 +190,31 @@ static int hv_cpu_die(unsigned int cpu) { struct hv_reenlightenment_control re_ctrl; unsigned int new_cpu;+ unsigned long flags;+ void **input_arg;+ void *pg;+ void **ghcb_va = NULL;++ local_irq_save(flags);+ input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);+ pg = *input_arg;
On Wed, Jul 28, 2021 at 08:29:41AM -0700, Dave Hansen wrote:
__set_memory_enc_dec() is turning into a real mess. SEV, TDX and now
Hyper-V are messing around in here.
I was going to suggest a PV_OPS call where the fitting implementation
for the guest environment can be plugged in at boot. There is TDX and an
SEV(-SNP) case, a Hyper-V case, and likely more coming up from other
cloud/hypervisor vendors. Hiding all these behind feature checks is not
going to make things cleaner.
Regards,
Joerg
On Wed, Jul 28, 2021 at 10:52:19AM -0400, Tianyu Lan wrote:
+ if (type == HV_GPADL_BUFFER)
+ index = 0;
+ else
+ index = channel->gpadl_range[1].gpadlhandle ? 2 : 1;
Hmm... This doesn't look very robust. Can you set fixed indexes for
different buffer types? HV_GPADL_BUFFER already has fixed index 0. But
as it is implemented here you risk that index 2 gets overwritten by
subsequent calls.
Do you really need to zero out the whole 4k? The validation bitmap
should be enough, there are no secrets on the page anyway.
Same in hv_ghcb_msr_read().
@@ -109,7 +109,16 @@ static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, ghcb_set_sw_exit_info_1(ghcb, exit_info_1); ghcb_set_sw_exit_info_2(ghcb, exit_info_2);- sev_es_wr_ghcb_msr(__pa(ghcb));+ /*+ * Hyper-V runs paravisor with SEV. Ghcb page is allocated by+ * paravisor and not needs to be updated in the Linux guest.+ * Otherwise, the ghcb page's PA reported by paravisor is above+ * VTOM. Hyper-V use this function with NULL for ctxt point and+ * skip setting ghcb page in such case.+ */+ if (ctxt)+ sev_es_wr_ghcb_msr(__pa(ghcb));
No, do not make this function work with ctxt==NULL. Instead, factor out
a helper function which contains what Hyper-V needs and use that in
sev_es_ghcb_hv_call() and Hyper-V code.
This deserves a comment. As I understand it, the GHCB pa is set by
Hyper-V or the paravisor, so the page does not need to be allocated by
Linux.
And it is not mapped unencrypted because the GHCB page is allocated
above the VTOM boundary?
You are right. The ghdb page is allocated by paravisor and its physical
address is above VTOM boundary. Will add a comment to describe this.
Thanks for suggestion.
quoted
@@ -167,6 +190,31 @@ static int hv_cpu_die(unsigned int cpu) { struct hv_reenlightenment_control re_ctrl; unsigned int new_cpu;+ unsigned long flags;+ void **input_arg;+ void *pg;+ void **ghcb_va = NULL;++ local_irq_save(flags);+ input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);+ pg = *input_arg;
Pg is never used later on, why is it set?
Sorry for noise. This should be removed during rebase and will fix in
the next version.
From: Tianyu Lan <hidden> Date: 2021-08-02 12:56:53
On 8/2/2021 8:07 PM, Joerg Roedel wrote:
On Wed, Jul 28, 2021 at 10:52:19AM -0400, Tianyu Lan wrote:
quoted
+ if (type == HV_GPADL_BUFFER)
+ index = 0;
+ else
+ index = channel->gpadl_range[1].gpadlhandle ? 2 : 1;
Hmm... This doesn't look very robust. Can you set fixed indexes for
different buffer types? HV_GPADL_BUFFER already has fixed index 0. But
as it is implemented here you risk that index 2 gets overwritten by
subsequent calls.
Both second and third are HV_GPADL_RING type. One is send ring and the
other is receive ring. The driver keeps the order to allocate rx and
tx buffer. You are right this is not robust and will add a mutex to keep
the order.
Okay, let me see if I got this right. In Hyper-V Isolation VMs, when the
guest wants to make memory shared, it does":
- Call to the Hypervisor the mark the pages shared. The
Hypervisor will do the RMP update and remap the pages at
(VTOM + pa)
- The guest maps the memory again into its page-table, so that
the entries point to the correct GPA (which is above VTOM
now).
Or in other words, Hyper-V implements a hardware-independent and
configurable c-bit position, as the VTOM value is always power-of-two
aligned. Is that correct?
This would at least explain why there is no separate
allocation/dealloction of memory for the shared range.
Thanks,
Joerg
On Mon, Aug 02, 2021 at 08:56:29PM +0800, Tianyu Lan wrote:
Both second and third are HV_GPADL_RING type. One is send ring and the
other is receive ring. The driver keeps the order to allocate rx and
tx buffer. You are right this is not robust and will add a mutex to keep
the order.
Or you introduce fixed indexes for the RX and TX buffers?
From: Tianyu Lan <hidden> Date: 2021-08-02 13:00:18
On 8/2/2021 8:01 PM, Joerg Roedel wrote:
On Wed, Jul 28, 2021 at 08:29:41AM -0700, Dave Hansen wrote:
quoted
__set_memory_enc_dec() is turning into a real mess. SEV, TDX and now
Hyper-V are messing around in here.
I was going to suggest a PV_OPS call where the fitting implementation
for the guest environment can be plugged in at boot. There is TDX and an
SEV(-SNP) case, a Hyper-V case, and likely more coming up from other
cloud/hypervisor vendors. Hiding all these behind feature checks is not
going to make things cleaner.
Yes, that makes sense. I will do this in the next version.
From: Tianyu Lan <hidden> Date: 2021-08-02 13:09:14
On 8/2/2021 8:59 PM, Joerg Roedel wrote:
On Mon, Aug 02, 2021 at 08:56:29PM +0800, Tianyu Lan wrote:
quoted
Both second and third are HV_GPADL_RING type. One is send ring and the
other is receive ring. The driver keeps the order to allocate rx and
tx buffer. You are right this is not robust and will add a mutex to keep
the order.
Or you introduce fixed indexes for the RX and TX buffers?
The interface just allocates a buffer and driver will continue to
configure the buffer to be rx or tx after calling.
On Wed, Jul 28, 2021 at 08:29:41AM -0700, Dave Hansen wrote:
quoted
__set_memory_enc_dec() is turning into a real mess. SEV, TDX and now
Hyper-V are messing around in here.
I was going to suggest a PV_OPS call where the fitting implementation
for the guest environment can be plugged in at boot. There is TDX and an
SEV(-SNP) case, a Hyper-V case, and likely more coming up from other
cloud/hypervisor vendors. Hiding all these behind feature checks is not
going to make things cleaner.
As those cases are all mutually exclusive, wouldn't a static_call() be
the appropriate solution?
Juergen
Do you really need to zero out the whole 4k? The validation bitmap
should be enough, there are no secrets on the page anyway.
Same in hv_ghcb_msr_read().
@@ -109,7 +109,16 @@ static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, ghcb_set_sw_exit_info_1(ghcb, exit_info_1); ghcb_set_sw_exit_info_2(ghcb, exit_info_2);- sev_es_wr_ghcb_msr(__pa(ghcb));+ /*+ * Hyper-V runs paravisor with SEV. Ghcb page is allocated by+ * paravisor and not needs to be updated in the Linux guest.+ * Otherwise, the ghcb page's PA reported by paravisor is above+ * VTOM. Hyper-V use this function with NULL for ctxt point and+ * skip setting ghcb page in such case.+ */+ if (ctxt)+ sev_es_wr_ghcb_msr(__pa(ghcb));
No, do not make this function work with ctxt==NULL. Instead, factor out
a helper function which contains what Hyper-V needs and use that in
sev_es_ghcb_hv_call() and Hyper-V code.
I am curious what this will end up being good for.
Hyper-V introduces a specific hypercall request in GHCB page and use
same union in the Linux Hyper-V code to read/write MSR and call the new
hypercall request.
On Wed, Jul 28, 2021 at 10:52:28AM -0400, Tianyu Lan wrote:
In Isolation VM, all shared memory with host needs to mark visible
to host via hvcall. vmbus_establish_gpadl() has already done it for
storvsc rx/tx ring buffer. The page buffer used by vmbus_sendpacket_
mpb_desc() still need to handle. Use DMA API to map/umap these
memory during sending/receiving packet and Hyper-V DMA ops callback
will use swiotlb function to allocate bounce buffer and copy data
from/to bounce buffer.
I am wondering why you dont't use DMA-API unconditionally? It provides
enough abstraction to do the right thing for isolated and legacy VMs.
Regards,
Joerg
From: Tianyu Lan <hidden> Date: 2021-08-02 14:22:39
On 8/2/2021 9:20 PM, Joerg Roedel wrote:
On Wed, Jul 28, 2021 at 10:52:28AM -0400, Tianyu Lan wrote:
quoted
In Isolation VM, all shared memory with host needs to mark visible
to host via hvcall. vmbus_establish_gpadl() has already done it for
storvsc rx/tx ring buffer. The page buffer used by vmbus_sendpacket_
mpb_desc() still need to handle. Use DMA API to map/umap these
memory during sending/receiving packet and Hyper-V DMA ops callback
will use swiotlb function to allocate bounce buffer and copy data
from/to bounce buffer.
I am wondering why you dont't use DMA-API unconditionally? It provides
enough abstraction to do the right thing for isolated and legacy VMs.
In VMbus, there is already a similar bounce buffer design and so there
is no need to call DMA-API for such buffer. Calling DMA-API is to use
swiotlb bounce buffer for those buffer which hasn't been covered. This
is why need to conditionally call DMA API.