From: Hans-Jörg Höxer Subject: Re: vmm(4) AMD SEV To: Dave Voutila Cc: Alexander Bluhm , , Date: Mon, 26 Aug 2024 10:55:02 +0200 Hi, thanks for the feedback! On Fri, Aug 23, 2024 at 03:06:42PM -0400, Dave Voutila wrote: > > +int vcpu_init_vmx(struct vcpu *, struct vm_create_params *); > > +int vcpu_init_svm(struct vcpu *, struct vm_create_params *); > > I don't see a benefit in changing the signature of vcpu_init_vmx to keep > it similar to vcpu_init_svm. Since we're not doing anything new on VMX > with this diff, I'd rather keep that code unchanged for now until we > have a reason to do so. ok. > > + /* Inform vmd(8) about ASID and C bit position. */ > > + vcp->vcp_poscbit = amd64_pos_cbit; > > + vcp->vcp_asid[vcpu->vc_parent->vm_vcpu_ct] = vcpu->vc_vpid; > > + > > This looks odd and looking more closely at vmm, this is a future > footgun. I'd rather the index be vcpu->vc_id, which semantically makes > most sense. > > To do this, it looks like prior to vcpu_init() is called in vm_create(), > you need to set vcpu->vc_id, so moving the lines like: > > vcpu->vc_id = vm->vm_vcpu_ct; > vm->vm_vcpu_ct++; > > to prior to calling vcpu_init(vcpu). ok! Updated diff below. Take care! ----------------------------------------------------------------- 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 {