NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: port-amd64/59860: nvmm: support AVX2, AVX512, &c.



The following reply was made to PR port-amd64/59860; it has been noted by GNATS.

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: gnats-bugs%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost
Cc: yamt%NetBSD.org@localhost
Subject: Re: port-amd64/59860: nvmm: support AVX2, AVX512, &c.
Date: Sun, 12 Jul 2026 03:10:02 +0000

 This is a multi-part message in MIME format.
 --=_su1OON3HGWIM8fZuEZsawKeY4iHDSE1Z
 
 The attached patch adds support to nvmm on Intel CPUs for AVX/AVX2 and
 generally any XSAVE components.
 
 Testing and review welcome!
 
 Notes:
 
 - Doesn't expose the AVX-512 CPUID bits yet because (a) there's a lot
   of them to review and (b) I don't have hardware handy to test.
 
 - Not adapted for AMD hardware, for the same reason, but it should
   require essentially the same changes to nvmm_x86_svm.c as I made to
   nvmm_x86_svm.c.
 
 - Doesn't currently expose the XSAVE area state to the userland
   hypervisor software.  I reviewed the way that dfly did this, and
   they just broke the ABI, which is a non-starter.  To do this without
   breaking the ABI, I intend to add an ioctl or something for querying
   the mmap address of the XSAVE area for any vCPU, so you can just
   mmap that like you can already mmap the comm page.  No ABI breakage
   this way.  The mmap addresses can be chosen to be something like:
 
     NVMM_MAX_VCPUS*PAGE_SIZE        // reserved for comm pages
     + MAX_i(x86_xsave_offsets[i] + x86_xsave_sizes[i])*vcpuid
 
   but since userland queries it through ioctl rather than computing it
   directly, the choice of layout carries no ABI implications as long
   as it doesn't overlap with [0, VNMM_MAX_VCPUS*PAGE_SIZE).
 
 --=_su1OON3HGWIM8fZuEZsawKeY4iHDSE1Z
 Content-Type: text/plain; charset="ISO-8859-1"; name="pr59860-nvmmxsave"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename="pr59860-nvmmxsave.patch"
 
 # HG changeset patch
 # User Taylor R Campbell <riastradh%NetBSD.org@localhost>
 # Date 1783811482 0
 #      Sat Jul 11 23:11:22 2026 +0000
 # Branch trunk
 # Node ID 69e5242c35ee9c681e6fe98c101e083a330eeb8b
 # Parent  187abacabbe955f8145e7c12ca1a0909553a72a3
 # EXP-Topic riastradh-pr60426-x86xsavesignal
 WIP: nvmm: Add support for extended CPU state (XSAVE) beyond x87/SSE.
 
 XXX Currently limited to Intel VMX since I have no AMD SVM to test.
 Changes for nvmm_x86_svm.c should be very similar to those for
 nvmm_x86_vmx.c, though.
 
 New machine-dependent x86 vCPU configuration command
 NVMM_VCPU_CONF_XCR0_MASK sets the vCPU's XCR0 mask, that is, the set
 of XSAVE features that the guest sees as supported in the vCPU.  This
 command is advertised by the new machine capability
 NVMM_CAP_ARCH_VCPU_CONF_XCR0_MASK.
 
 This doesn't expose the XSAVE area to userland in the comm page --
 there are machine-independent members in struct nvmm_comm_page at
 fixed offsets past the machine-dependent state so we can't just
 extend struct nvmm_vcpu_state without breaking the ABI, and the XSAVE
 area may exceed a page but the address space for the virtual machine
 uvm object packs comm areas in consecutive pages so we can't put it
 in the comm page at all without breaking the ABI.
 
 Instead, we can expose the XSAVE area in another location in the
 virtual machine uvm object, and create an ioctl for querying its
 object address and size so userland can mmap it.  TBD.
 
 PR port-amd64/59860: nvmm: support AVX2, AVX512, &c.
 
 diff -r 187abacabbe9 -r 69e5242c35ee sys/dev/nvmm/x86/nvmm_x86.c
 --- a/sys/dev/nvmm/x86/nvmm_x86.c	Sat Jul 11 14:02:58 2026 +0000
 +++ b/sys/dev/nvmm/x86/nvmm_x86.c	Sat Jul 11 23:11:22 2026 +0000
 @@ -258,7 +258,7 @@ const struct nvmm_x86_cpuid_mask nvmm_cp
  	    CPUID2_AESNI |
  	    CPUID2_XSAVE |
  	    CPUID2_OSXSAVE |
 -	    /* CPUID2_AVX excluded */
 +	    CPUID2_AVX |
  	    CPUID2_F16C |
  	    CPUID2_RDRAND,
  	    /* CPUID2_RAZ excluded */
 @@ -302,7 +302,7 @@ const struct nvmm_x86_cpuid_mask nvmm_cp
  	    /* CPUID_SEF_SGX excluded */
  	    CPUID_SEF_BMI1 |
  	    /* CPUID_SEF_HLE excluded */
 -	    /* CPUID_SEF_AVX2 excluded */
 +	    CPUID_SEF_AVX2 |
  	    CPUID_SEF_FDPEXONLY |
  	    CPUID_SEF_SMEP |
  	    CPUID_SEF_BMI2 |
 @@ -466,3 +466,24 @@ nvmm_x86_pat_validate(uint64_t val)
 =20
  	return true;
  }
 +
 +uint32_t
 +nvmm_x86_xsave_size(uint64_t xcr0)
 +{
 +	uint32_t size =3D sizeof(struct xsave_header);
 +	unsigned i;
 +
 +	KASSERT((xcr0 & ~__BITS(XSAVE_MAX_COMPONENT, 0)) =3D=3D 0);
 +
 +	CTASSERT(sizeof(struct xsave_header) =3D=3D 512 + 64);
 +	for (i =3D 0; i < XSAVE_MAX_COMPONENT; i++) {
 +		if ((xcr0 & __BIT(i)) =3D=3D 0)
 +			continue;
 +		KASSERT(x86_xsave_sizes[i] <=3D
 +		    UINT32_MAX - x86_xsave_offsets[i]);
 +		if (size < x86_xsave_offsets[i] + x86_xsave_sizes[i])
 +			size =3D x86_xsave_offsets[i] + x86_xsave_sizes[i];
 +	}
 +
 +	return size;
 +}
 diff -r 187abacabbe9 -r 69e5242c35ee sys/dev/nvmm/x86/nvmm_x86.h
 --- a/sys/dev/nvmm/x86/nvmm_x86.h	Sat Jul 11 14:02:58 2026 +0000
 +++ b/sys/dev/nvmm/x86/nvmm_x86.h	Sat Jul 11 23:11:22 2026 +0000
 @@ -132,6 +132,7 @@ struct nvmm_cap_md {
  	uint64_t vcpu_conf_support;
  #define NVMM_CAP_ARCH_VCPU_CONF_CPUID	__BIT(0)
  #define NVMM_CAP_ARCH_VCPU_CONF_TPR	__BIT(1)
 +#define NVMM_CAP_ARCH_VCPU_CONF_XCR0_MASK __BIT(2)
 =20
  	uint64_t xcr0_mask;
  	uint32_t mxcsr_mask;
 @@ -269,6 +270,7 @@ struct nvmm_x64_state {
 =20
  #define NVMM_VCPU_CONF_CPUID	NVMM_VCPU_CONF_MD_BEGIN
  #define NVMM_VCPU_CONF_TPR	(NVMM_VCPU_CONF_MD_BEGIN + 1)
 +#define NVMM_VCPU_CONF_XCR0_MASK (NVMM_VCPU_CONF_MD_BEGIN + 2)
 =20
  struct nvmm_vcpu_conf_cpuid {
  	/* The options. */
 @@ -309,7 +311,7 @@ struct nvmm_vcpu_conf_tpr {
 =20
  #ifdef _KERNEL
  #define NVMM_X86_MACH_NCONF	0
 -#define NVMM_X86_VCPU_NCONF	2
 +#define NVMM_X86_VCPU_NCONF	3
  struct nvmm_x86_cpuid_mask {
  	uint32_t eax;
  	uint32_t ebx;
 @@ -323,6 +325,7 @@ extern const struct nvmm_x86_cpuid_mask=20
  extern const struct nvmm_x86_cpuid_mask nvmm_cpuid_80000007;
  extern const struct nvmm_x86_cpuid_mask nvmm_cpuid_80000008;
  bool nvmm_x86_pat_validate(uint64_t);
 +uint32_t nvmm_x86_xsave_size(uint64_t);
  #endif
 =20
  #endif /* ASM_NVMM */
 diff -r 187abacabbe9 -r 69e5242c35ee sys/dev/nvmm/x86/nvmm_x86_vmx.c
 --- a/sys/dev/nvmm/x86/nvmm_x86_vmx.c	Sat Jul 11 14:02:58 2026 +0000
 +++ b/sys/dev/nvmm/x86/nvmm_x86_vmx.c	Sat Jul 11 23:11:22 2026 +0000
 @@ -717,7 +717,6 @@ static uint8_t *vmx_asidmap __read_mostl
  static uint32_t vmx_maxasid __read_mostly;
  static kmutex_t vmx_asidlock __cacheline_aligned;
 =20
 -#define VMX_XCR0_MASK_DEFAULT	(XCR0_X87|XCR0_SSE)
  static uint64_t vmx_xcr0_mask __read_mostly;
 =20
  #define VMX_NCPUIDS	32
 @@ -775,7 +774,9 @@ static const size_t vmx_vcpu_conf_sizes[
  	[NVMM_VCPU_CONF_MD(NVMM_VCPU_CONF_CPUID)] =3D
  	    sizeof(struct nvmm_vcpu_conf_cpuid),
  	[NVMM_VCPU_CONF_MD(NVMM_VCPU_CONF_TPR)] =3D
 -	    sizeof(struct nvmm_vcpu_conf_tpr)
 +	    sizeof(struct nvmm_vcpu_conf_tpr),
 +	[NVMM_VCPU_CONF_MD(NVMM_VCPU_CONF_XCR0_MASK)] =3D
 +	    sizeof(uint64_t),
  };
 =20
  struct vmx_cpudata {
 @@ -820,12 +821,15 @@ struct vmx_cpudata {
  	uint64_t gprs[NVMM_X64_NGPR];
  	uint64_t drs[NVMM_X64_NDR];
  	uint64_t gtsc;
 -	struct xsave_header gfpu __aligned(64);
 =20
  	/* VCPU configuration. */
  	bool cpuidpresent[VMX_NCPUIDS];
  	struct nvmm_vcpu_conf_cpuid cpuid[VMX_NCPUIDS];
  	struct nvmm_vcpu_conf_tpr tpr;
 +	uint64_t xcr0_mask;
 +
 +	/* Guest XSAVE state. */
 +	struct xsave_header gfpu __aligned(64);
  };
 =20
  static const struct {
 @@ -1318,6 +1322,14 @@ vmx_inkernel_handle_cpuid(struct nvmm_ma
 =20
  		cpudata->gprs[NVMM_X64_GPR_RDX] &=3D nvmm_cpuid_00000001.edx;
 =20
 +		/*
 +		 * CPUID2_AVX depends on XSAVE support for the YMM high
 +		 * halves.
 +		 */
 +		if ((cpudata->xcr0_mask & XCR0_YMM_Hi128) =3D=3D 0) {
 +			cpudata->gprs[NVMM_X64_GPR_RAX] &=3D ~CPUID2_AVX;
 +		}
 +
  		/* CPUID2_OSXSAVE depends on CR4. */
  		cr4 =3D vmx_vmread(VMCS_GUEST_CR4);
  		if (!(cr4 & CR4_OSXSAVE)) {
 @@ -1351,6 +1363,16 @@ vmx_inkernel_handle_cpuid(struct nvmm_ma
  			if (vmx_procbased_ctls2 & PROC_CTLS2_INVPCID_ENABLE) {
  				cpudata->gprs[NVMM_X64_GPR_RBX] |=3D CPUID_SEF_INVPCID;
  			}
 +
 +			/*
 +			 * CPUID_SEF_AVX2 depends on XSAVE support for
 +			 * the YMM high halves.
 +			 */
 +			if ((cpudata->xcr0_mask & XCR0_YMM_Hi128) =3D=3D 0) {
 +				cpudata->gprs[NVMM_X64_GPR_RAX] &=3D
 +				    ~CPUID_SEF_AVX2;
 +			}
 +
  			break;
  		default:
  			cpudata->gprs[NVMM_X64_GPR_RAX] =3D 0;
 @@ -1407,20 +1429,19 @@ vmx_inkernel_handle_cpuid(struct nvmm_ma
  		cpudata->gprs[NVMM_X64_GPR_RDX] =3D 0;
  		break;
  	case 0x0000000D: /* Processor Extended State Enumeration */
 -		if (vmx_xcr0_mask =3D=3D 0) {
 +		if (cpudata->xcr0_mask =3D=3D 0) {
  			break;
  		}
  		switch (ecx) {
  		case 0:
 -			cpudata->gprs[NVMM_X64_GPR_RAX] =3D vmx_xcr0_mask & 0xFFFFFFFF;
 -			if (cpudata->gxcr0 & XCR0_SSE) {
 -				cpudata->gprs[NVMM_X64_GPR_RBX] =3D sizeof(struct fxsave);
 -			} else {
 -				cpudata->gprs[NVMM_X64_GPR_RBX] =3D sizeof(struct save87);
 -			}
 -			cpudata->gprs[NVMM_X64_GPR_RBX] +=3D 64; /* XSAVE header */
 -			cpudata->gprs[NVMM_X64_GPR_RCX] =3D sizeof(struct fxsave) + 64;
 -			cpudata->gprs[NVMM_X64_GPR_RDX] =3D vmx_xcr0_mask >> 32;
 +			cpudata->gprs[NVMM_X64_GPR_RAX] =3D
 +			    cpudata->xcr0_mask & 0xFFFFFFFF;
 +			cpudata->gprs[NVMM_X64_GPR_RBX] =3D
 +			    nvmm_x86_xsave_size(cpudata->gxcr0);
 +			cpudata->gprs[NVMM_X64_GPR_RCX] =3D
 +			    nvmm_x86_xsave_size(cpudata->xcr0_mask);
 +			cpudata->gprs[NVMM_X64_GPR_RDX] =3D
 +			    cpudata->xcr0_mask >> 32;
  			break;
  		case 1:
  			cpudata->gprs[NVMM_X64_GPR_RAX] &=3D
 @@ -1430,6 +1451,21 @@ vmx_inkernel_handle_cpuid(struct nvmm_ma
  			cpudata->gprs[NVMM_X64_GPR_RCX] =3D 0;
  			cpudata->gprs[NVMM_X64_GPR_RDX] =3D 0;
  			break;
 +		case 2 ... 62:
 +			/*
 +			 * CPUID[EAX=3D0x0d,ECX=3Dn], 2 <=3D n <=3D 62: size
 +			 * and offset of nth component in XSAVE area.
 +			 * If the nth bit of XCR0 is disabled in the
 +			 * vCPU configuration, we return all-zero
 +			 * instead.
 +			 */
 +			if ((cpudata->xcr0_mask & __BIT(ecx)) =3D=3D 0) {
 +				cpudata->gprs[NVMM_X64_GPR_RAX] =3D 0;
 +				cpudata->gprs[NVMM_X64_GPR_RBX] =3D 0;
 +				cpudata->gprs[NVMM_X64_GPR_RCX] =3D 0;
 +				cpudata->gprs[NVMM_X64_GPR_RDX] =3D 0;
 +			}
 +			break;
  		default:
  			cpudata->gprs[NVMM_X64_GPR_RAX] =3D 0;
  			cpudata->gprs[NVMM_X64_GPR_RBX] =3D 0;
 @@ -2004,7 +2040,7 @@ vmx_exit_xsetbv(struct nvmm_machine *mac
 =20
  	if (__predict_false(cpudata->gprs[NVMM_X64_GPR_RCX] !=3D 0)) {
  		goto error;
 -	} else if (__predict_false((val & ~vmx_xcr0_mask) !=3D 0)) {
 +	} else if (__predict_false((val & ~cpudata->xcr0_mask) !=3D 0)) {
  		goto error;
  	} else if (__predict_false((val & XCR0_X87) =3D=3D 0)) {
  		goto error;
 @@ -2057,9 +2093,9 @@ vmx_vcpu_guest_fpu_enter(struct nvmm_cpu
 =20
  	fpu_kern_enter();
  	/* TODO: should we use *XSAVE64 here? */
 -	fpu_area_restore(&cpudata->gfpu, vmx_xcr0_mask, false);
 -
 -	if (vmx_xcr0_mask !=3D 0) {
 +	fpu_area_restore(&cpudata->gfpu, cpudata->xcr0_mask, false);
 +
 +	if (cpudata->xcr0_mask !=3D 0) {
  		cpudata->hxcr0 =3D rdxcr(0);
  		wrxcr(0, cpudata->gxcr0);
  	}
 @@ -2070,13 +2106,13 @@ vmx_vcpu_guest_fpu_leave(struct nvmm_cpu
  {
  	struct vmx_cpudata *cpudata =3D vcpu->cpudata;
 =20
 -	if (vmx_xcr0_mask !=3D 0) {
 +	if (cpudata->xcr0_mask !=3D 0) {
  		cpudata->gxcr0 =3D rdxcr(0);
  		wrxcr(0, cpudata->hxcr0);
  	}
 =20
  	/* TODO: should we use *XSAVE64 here? */
 -	fpu_area_save(&cpudata->gfpu, vmx_xcr0_mask, false);
 +	fpu_area_save(&cpudata->gfpu, cpudata->xcr0_mask, false);
  	fpu_kern_leave();
  }
 =20
 @@ -2649,10 +2685,10 @@ vmx_vcpu_setstate(struct nvmm_cpu *vcpu)
 =20
  		cpudata->gcr8 =3D state->crs[NVMM_X64_CR_CR8];
 =20
 -		if (vmx_xcr0_mask !=3D 0) {
 +		if (cpudata->xcr0_mask !=3D 0) {
  			/* Clear illegal XCR0 bits, set mandatory X87 bit. */
  			cpudata->gxcr0 =3D state->crs[NVMM_X64_CR_XCR0];
 -			cpudata->gxcr0 &=3D vmx_xcr0_mask;
 +			cpudata->gxcr0 &=3D cpudata->xcr0_mask;
  			cpudata->gxcr0 |=3D XCR0_X87;
  		}
  	}
 @@ -2731,11 +2767,15 @@ vmx_vcpu_setstate(struct nvmm_cpu *vcpu)
  		fpustate->fx_mxcsr_mask &=3D x86_fpu_mxcsr_mask;
  		fpustate->fx_mxcsr &=3D fpustate->fx_mxcsr_mask;
 =20
 -		if (vmx_xcr0_mask !=3D 0) {
 +		if (cpudata->xcr0_mask !=3D 0) {
  			/* Reset XSTATE_BV, to force a reload. */
 -			cpudata->gfpu.xsh_xstate_bv =3D vmx_xcr0_mask;
 +			cpudata->gfpu.xsh_xstate_bv =3D cpudata->xcr0_mask;
  		}
  	}
 +	/*
 +	 * XXX XSAVE area -- need to allocate and map it separately
 +	 * since it may exceed the comm page size
 +	 */
 =20
  	vmx_vmcs_leave(vcpu);
 =20
 @@ -2836,6 +2876,10 @@ vmx_vcpu_getstate(struct nvmm_cpu *vcpu)
  		memcpy(&state->fpu, cpudata->gfpu.xsh_fxsave,
  		    sizeof(state->fpu));
  	}
 +	/*
 +	 * XXX XSAVE area -- need to allocate and map it separately
 +	 * since it may exceed the comm page size
 +	 */
 =20
  	vmx_vmcs_leave(vcpu);
 =20
 @@ -3010,7 +3054,8 @@ vmx_vcpu_init(struct nvmm_machine *mach,
  	    (IA32_MISC_BTS_UNAVAIL|IA32_MISC_PEBS_UNAVAIL);
 =20
  	/* Init XSAVE header. */
 -	cpudata->gfpu.xsh_xstate_bv =3D vmx_xcr0_mask;
 +	cpudata->xcr0_mask =3D vmx_xcr0_mask;
 +	cpudata->gfpu.xsh_xstate_bv =3D cpudata->xcr0_mask;
  	cpudata->gfpu.xsh_xcomp_bv =3D 0;
 =20
  	/* These MSRs are static. */
 @@ -3032,12 +3077,27 @@ vmx_vcpu_init(struct nvmm_machine *mach,
  static int
  vmx_vcpu_create(struct nvmm_machine *mach, struct nvmm_cpu *vcpu)
  {
 +	size_t xsave_size, cpudata_size;
  	struct vmx_cpudata *cpudata;
  	int error;
 =20
 +	/*
 +	 * Compute the size of the VMX cpudata.  We put the
 +	 * variable-length XSAVE area at the end so if it's small
 +	 * enough, it stays within a single page.  We size the XSAVE
 +	 * area for the maximum set of features supported by the CPU
 +	 * which a guest can enable (which may be more than the NetBSD
 +	 * host enables for itself -- hence we don't use
 +	 * x86_fpu_save_size here!)
 +	 */
 +	xsave_size =3D nvmm_x86_xsave_size(vmx_xcr0_mask);
 +	KASSERT(xsave_size < SIZE_MAX - offsetof(struct vmx_cpudata, gfpu));
 +	cpudata_size =3D MAX(sizeof(*cpudata),
 +	    offsetof(struct vmx_cpudata, gfpu) + xsave_size);
 +
  	/* Allocate the VMX cpudata. */
  	cpudata =3D (struct vmx_cpudata *)uvm_km_alloc(kernel_map,
 -	    roundup(sizeof(*cpudata), PAGE_SIZE), 0,
 +	    roundup(cpudata_size, PAGE_SIZE), 0,
  	    UVM_KMF_WIRED|UVM_KMF_ZERO);
  	vcpu->cpudata =3D cpudata;
 =20
 @@ -3167,6 +3227,18 @@ vmx_vcpu_configure_tpr(struct vmx_cpudat
  }
 =20
  static int
 +vmx_vcpu_configure_xcr0_mask(struct vmx_cpudata *cpudata, void *data)
 +{
 +	const uint64_t *xcr0_maskp =3D data;
 +
 +	if (*xcr0_maskp & ~vmx_xcr0_mask)
 +		return EINVAL;
 +	cpudata->xcr0_mask =3D *xcr0_maskp;
 +	return 0;
 +}
 +
 +
 +static int
  vmx_vcpu_configure(struct nvmm_cpu *vcpu, uint64_t op, void *data)
  {
  	struct vmx_cpudata *cpudata =3D vcpu->cpudata;
 @@ -3176,6 +3248,8 @@ vmx_vcpu_configure(struct nvmm_cpu *vcpu
  		return vmx_vcpu_configure_cpuid(cpudata, data);
  	case NVMM_VCPU_CONF_MD(NVMM_VCPU_CONF_TPR):
  		return vmx_vcpu_configure_tpr(cpudata, data);
 +	case NVMM_VCPU_CONF_MD(NVMM_VCPU_CONF_XCR0_MASK):
 +		return vmx_vcpu_configure_xcr0_mask(cpudata, data);
  	default:
  		return EINVAL;
  	}
 @@ -3589,8 +3663,16 @@ vmx_init(void)
  	/* Init the ASID bitmap (VPID). */
  	vmx_init_asid(VPID_MAX);
 =20
 -	/* Init the XCR0 mask. */
 -	vmx_xcr0_mask =3D VMX_XCR0_MASK_DEFAULT & x86_xsave_features;
 +	/*
 +	 * Init the XCR0 mask.  x86_xsave_features is the cached result
 +	 * of CPUID[EAX=3D0x0000000d,ECX=3D0].EDX:EAX, the set of all
 +	 * supported XCR0 bits for user XSAVE state components on the
 +	 * physical CPU.  Hypervisor software can use
 +	 * nvmm_vcpu_configure(NVMM_VCPU_CONF_XCR0_MASK) to restrict
 +	 * the available features on a per-vCPU basis, e.g. in order to
 +	 * limit guests to compatible features for migration.
 +	 */
 +	vmx_xcr0_mask =3D x86_xsave_features;
 =20
  	/* Init the max basic CPUID leaf. */
  	vmx_cpuid_max_basic =3D uimin(cpuid_level, VMX_CPUID_MAX_BASIC);
 
 --=_su1OON3HGWIM8fZuEZsawKeY4iHDSE1Z--
 



Home | Main Index | Thread Index | Old Index