Index | Thread | Search

From:
Mike Larkin <mlarkin@nested.page>
Subject:
Re: vmm(4) AMD SEV
To:
Alexander Bluhm <bluhm@openbsd.org>
Cc:
Hans-J?rg H?xer <Hans-Joerg_Hoexer@genua.de>, Dave Voutila <dv@sisu.io>, tech@openbsd.org
Date:
Mon, 26 Aug 2024 12:54:26 -0700

Download raw body.

Thread
  • Hans-Jörg Höxer:

    vmm(4) AMD SEV

    • Alexander Bluhm:

      vmm(4) AMD SEV

      • Mike Larkin:

        vmm(4) AMD SEV

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 <hshoexer@genua.de>
> > 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 {
>