From: Mike Larkin Subject: Re: vmm(4) AMD SEV To: Alexander Bluhm Cc: Hans-J?rg H?xer , Dave Voutila , tech@openbsd.org Date: Mon, 26 Aug 2024 12:54:26 -0700 On Mon, Aug 26, 2024 at 04:13:46PM +0200, Alexander Bluhm wrote: > On Mon, Aug 26, 2024 at 10:55:02AM +0200, Hans-J?rg H?xer wrote: > > Updated diff below. > > I have repeated my tests with this diff, still running fine. > > ok? > I'm happy with this. ok mlarkin > bluhm > > > ----------------------------------------------------------------- > > commit 659582f61af2e7b9fe67e83436d5bcae0295f7d8 > > Author: Hans-Joerg Hoexer > > Date: Thu Jul 11 16:23:00 2024 +0200 > > > > vmm(4): SEV support > > > > This changes enables SEV support in vmm(4): > > > > - emulate cpuid 0x8000001f so the guest can discover SEV features > > - allow vmd(8) to enable SEV on VM creation > > - inform vmd(8) about the c-bit position and ASID assigned to each VCPU > > > > diff --git a/sys/arch/amd64/amd64/identcpu.c b/sys/arch/amd64/amd64/identcpu.c > > index a6e31e50255..91d267fff64 100644 > > --- a/sys/arch/amd64/amd64/identcpu.c > > +++ b/sys/arch/amd64/amd64/identcpu.c > > @@ -66,7 +66,7 @@ char cpu_model[48]; > > int cpuspeed; > > > > int amd64_has_xcrypt; > > -int amd64_pos_cbit; > > +int amd64_pos_cbit; /* C bit position for SEV */ > > int has_rdrand; > > int has_rdseed; > > > > diff --git a/sys/arch/amd64/amd64/vmm_machdep.c b/sys/arch/amd64/amd64/vmm_machdep.c > > index 0df3d8b070f..eb6e9258366 100644 > > --- a/sys/arch/amd64/amd64/vmm_machdep.c > > +++ b/sys/arch/amd64/amd64/vmm_machdep.c > > @@ -82,9 +82,9 @@ int vcpu_reset_regs(struct vcpu *, struct vcpu_reg_state *); > > int vcpu_reset_regs_vmx(struct vcpu *, struct vcpu_reg_state *); > > int vcpu_reset_regs_svm(struct vcpu *, struct vcpu_reg_state *); > > int vcpu_reload_vmcs_vmx(struct vcpu *); > > -int vcpu_init(struct vcpu *); > > +int vcpu_init(struct vcpu *, struct vm_create_params *); > > int vcpu_init_vmx(struct vcpu *); > > -int vcpu_init_svm(struct vcpu *); > > +int vcpu_init_svm(struct vcpu *, struct vm_create_params *); > > int vcpu_run_vmx(struct vcpu *, struct vm_run_params *); > > int vcpu_run_svm(struct vcpu *, struct vm_run_params *); > > void vcpu_deinit(struct vcpu *); > > @@ -1890,7 +1890,6 @@ vcpu_reset_regs_svm(struct vcpu *vcpu, struct vcpu_reg_state *vrs) > > { > > struct vmcb *vmcb; > > int ret; > > - uint16_t asid; > > > > vmcb = (struct vmcb *)vcpu->vc_control_va; > > > > @@ -1963,14 +1962,7 @@ vcpu_reset_regs_svm(struct vcpu *vcpu, struct vcpu_reg_state *vrs) > > svm_setmsrbr(vcpu, MSR_PSTATEDEF(0)); > > > > /* Guest VCPU ASID */ > > - if (vmm_alloc_vpid(&asid)) { > > - DPRINTF("%s: could not allocate asid\n", __func__); > > - ret = EINVAL; > > - goto exit; > > - } > > - > > - vmcb->v_asid = asid; > > - vcpu->vc_vpid = asid; > > + vmcb->v_asid = vcpu->vc_vpid; > > > > /* TLB Control - First time in, flush all*/ > > vmcb->v_tlb_control = SVM_TLB_CONTROL_FLUSH_ALL; > > @@ -1985,9 +1977,13 @@ vcpu_reset_regs_svm(struct vcpu *vcpu, struct vcpu_reg_state *vrs) > > PATENTRY(6, PAT_UCMINUS) | PATENTRY(7, PAT_UC); > > > > /* NPT */ > > - vmcb->v_np_enable = 1; > > + vmcb->v_np_enable = SVM_ENABLE_NP; > > vmcb->v_n_cr3 = vcpu->vc_parent->vm_map->pmap->pm_pdirpa; > > > > + /* SEV */ > > + if (vcpu->vc_sev) > > + vmcb->v_np_enable |= SVM_ENABLE_SEV; > > + > > /* Enable SVME in EFER (must always be set) */ > > vmcb->v_efer |= EFER_SVME; > > > > @@ -1998,7 +1994,6 @@ vcpu_reset_regs_svm(struct vcpu *vcpu, struct vcpu_reg_state *vrs) > > > > vcpu->vc_parent->vm_map->pmap->eptp = 0; > > > > -exit: > > return ret; > > } > > > > @@ -3086,6 +3081,7 @@ vcpu_reset_regs(struct vcpu *vcpu, struct vcpu_reg_state *vrs) > > * > > * Parameters: > > * vcpu: the VCPU structure being initialized > > + * vcp: parameters provided by vmd(8) > > * > > * Return values: > > * 0: the VCPU was initialized successfully > > @@ -3093,8 +3089,9 @@ vcpu_reset_regs(struct vcpu *vcpu, struct vcpu_reg_state *vrs) > > * EINVAL: an error occurred during VCPU initialization > > */ > > int > > -vcpu_init_svm(struct vcpu *vcpu) > > +vcpu_init_svm(struct vcpu *vcpu, struct vm_create_params *vcp) > > { > > + uint16_t asid; > > int ret = 0; > > > > /* Allocate VMCB VA */ > > @@ -3176,6 +3173,21 @@ vcpu_init_svm(struct vcpu *vcpu) > > (uint64_t)vcpu->vc_svm_ioio_va, > > (uint64_t)vcpu->vc_svm_ioio_pa); > > > > + /* Guest VCPU ASID */ > > + if (vmm_alloc_vpid(&asid)) { > > + DPRINTF("%s: could not allocate asid\n", __func__); > > + ret = EINVAL; > > + goto exit; > > + } > > + vcpu->vc_vpid = asid; > > + > > + /* Shall we enable SEV? */ > > + vcpu->vc_sev = vcp->vcp_sev; > > + > > + /* Inform vmd(8) about ASID and C bit position. */ > > + vcp->vcp_poscbit = amd64_pos_cbit; > > + vcp->vcp_asid[vcpu->vc_id] = vcpu->vc_vpid; > > + > > exit: > > if (ret) > > vcpu_deinit_svm(vcpu); > > @@ -3189,7 +3201,7 @@ exit: > > * Calls the architecture-specific VCPU init routine > > */ > > int > > -vcpu_init(struct vcpu *vcpu) > > +vcpu_init(struct vcpu *vcpu, struct vm_create_params *vcp) > > { > > int ret = 0; > > > > @@ -3207,7 +3219,7 @@ vcpu_init(struct vcpu *vcpu) > > if (vmm_softc->mode == VMM_MODE_EPT) > > ret = vcpu_init_vmx(vcpu); > > else if (vmm_softc->mode == VMM_MODE_RVI) > > - ret = vcpu_init_svm(vcpu); > > + ret = vcpu_init_svm(vcpu, vcp); > > else > > panic("%s: unknown vmm mode: %d", __func__, vmm_softc->mode); > > > > @@ -6285,7 +6297,7 @@ vmm_handle_cpuid(struct vcpu *vcpu) > > *rdx = 0; > > break; > > case 0x80000000: /* Extended function level */ > > - *rax = 0x80000008; /* curcpu()->ci_pnfeatset */ > > + *rax = 0x8000001f; /* curcpu()->ci_pnfeatset */ > > *rbx = 0; > > *rcx = 0; > > *rdx = 0; > > @@ -6345,6 +6357,12 @@ vmm_handle_cpuid(struct vcpu *vcpu) > > *rcx = ecx; > > *rdx = edx; > > break; > > + case 0x8000001f: /* encryption features (AMD) */ > > + *rax = eax; > > + *rbx = ebx; > > + *rcx = ecx; > > + *rdx = edx; > > + break; > > default: > > DPRINTF("%s: unsupported rax=0x%llx\n", __func__, *rax); > > *rax = 0; > > diff --git a/sys/arch/amd64/include/cpu.h b/sys/arch/amd64/include/cpu.h > > index f5aada4d4d3..67cef9c37b8 100644 > > --- a/sys/arch/amd64/include/cpu.h > > +++ b/sys/arch/amd64/include/cpu.h > > @@ -415,6 +415,7 @@ void x86_print_cacheinfo(struct cpu_info *); > > void identifycpu(struct cpu_info *); > > int cpu_amd64speed(int *); > > extern int cpuspeed; > > +extern int amd64_pos_cbit; > > > > /* machdep.c */ > > void dumpconf(void); > > diff --git a/sys/arch/amd64/include/vmmvar.h b/sys/arch/amd64/include/vmmvar.h > > index 3f35e520926..855855ad2a7 100644 > > --- a/sys/arch/amd64/include/vmmvar.h > > +++ b/sys/arch/amd64/include/vmmvar.h > > @@ -624,6 +624,7 @@ enum { > > > > /* Forward declarations */ > > struct vm; > > +struct vm_create_params; > > > > /* > > * Implementation-specific cpu state > > @@ -636,6 +637,9 @@ struct vmcb_segment { > > uint64_t vs_base; /* 008h */ > > }; > > > > +#define SVM_ENABLE_NP (1ULL << 0) > > +#define SVM_ENABLE_SEV (1ULL << 1) > > + > > struct vmcb { > > union { > > struct { > > @@ -893,6 +897,7 @@ struct vcpu { > > paddr_t vc_svm_hsa_pa; > > vaddr_t vc_svm_ioio_va; > > paddr_t vc_svm_ioio_pa; > > + int vc_sev; /* [I] */ > > }; > > > > SLIST_HEAD(vcpu_head, vcpu); > > @@ -921,7 +926,7 @@ int vmm_start(void); > > int vmm_stop(void); > > int vm_impl_init(struct vm *, struct proc *); > > void vm_impl_deinit(struct vm *); > > -int vcpu_init(struct vcpu *); > > +int vcpu_init(struct vcpu *, struct vm_create_params *); > > void vcpu_deinit(struct vcpu *); > > int vm_rwregs(struct vm_rwregs_params *, int); > > int vcpu_reset_regs(struct vcpu *, struct vcpu_reg_state *); > > diff --git a/sys/dev/vmm/vmm.c b/sys/dev/vmm/vmm.c > > index 4d4866f70dc..35c00943302 100644 > > --- a/sys/dev/vmm/vmm.c > > +++ b/sys/dev/vmm/vmm.c > > @@ -399,13 +399,13 @@ vm_create(struct vm_create_params *vcp, struct proc *p) > > vcpu = pool_get(&vcpu_pool, PR_WAITOK | PR_ZERO); > > > > vcpu->vc_parent = vm; > > - if ((ret = vcpu_init(vcpu)) != 0) { > > + vcpu->vc_id = vm->vm_vcpu_ct; > > + vm->vm_vcpu_ct++; > > + if ((ret = vcpu_init(vcpu, vcp)) != 0) { > > printf("failed to init vcpu %d for vm %p\n", i, vm); > > vm_teardown(&vm); > > return (ret); > > } > > - vcpu->vc_id = vm->vm_vcpu_ct; > > - vm->vm_vcpu_ct++; > > /* Publish vcpu to list, inheriting the reference. */ > > SLIST_INSERT_HEAD(&vm->vm_vcpu_list, vcpu, vc_vcpu_link); > > } > > diff --git a/sys/dev/vmm/vmm.h b/sys/dev/vmm/vmm.h > > index ac682bc2c45..677885c8fc0 100644 > > --- a/sys/dev/vmm/vmm.h > > +++ b/sys/dev/vmm/vmm.h > > @@ -49,9 +49,12 @@ struct vm_create_params { > > size_t vcp_ncpus; > > struct vm_mem_range vcp_memranges[VMM_MAX_MEM_RANGES]; > > char vcp_name[VMM_MAX_NAME_LEN]; > > + int vcp_sev; > > > > /* Output parameter from VMM_IOC_CREATE */ > > uint32_t vcp_id; > > + uint32_t vcp_poscbit; > > + uint32_t vcp_asid[VMM_MAX_VCPUS]; > > }; > > > > struct vm_info_result { >