Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix FPU DNA exception on NetBSD #168

Merged
merged 1 commit into from Feb 14, 2019

Conversation

@krytarowski
Copy link
Contributor

commented Feb 12, 2019

Call clts (Clear Task-Switch) bit before calling FPU functions.
This prevents FPU Device Not Available exceptions.

Store the original CR0_TS of host before entering FPU state and
restore it on FPU exit.

Closes #166

@krytarowski krytarowski force-pushed the krytarowski:netbsd-5 branch from 1e05b13 to 9d5a232 Feb 12, 2019

@raphaelning

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2019

Thanks for fixing the issue! IIUC, the NetBSD guest runs into #NM because guest CR0.TS is set, and in this PR you are making sure host CR0.TS is clear right before every VM entry.

So my question is: what's the relationship between host CR0.TS and guest CR0.TS? Why does clearing the former result in the latter being cleared?

Note that there is a GUEST_CR0 field in VMCS. According to Intel SDM 26.3.2.1 (Loading Guest Control Registers, Debug Registers, and MSRs):

CR0 is loaded from the CR0 field with the exception of the following bits, which are never modified on VM entry: ET (bit 4); reserved bits 15:6, 17, and 28:19; NW (bit 29) and CD (bit 30). The values of these bits in the CR0 field are ignored.

So have you looked into how HAXM updates the TS bit (bit 3) of GUEST_CR0? In addition, there are also the VMX_CR0_MASK and VMX_CR0_READ_SHADOW fields in VMCS (cf. vmwrite_cr() in core/vcpu.c), whose exact values seem to depend on the host CPU model. Could you try to print them?

@krytarowski

This comment has been minimized.

Copy link
Contributor Author

commented Feb 13, 2019

According to my understanding the NetBSD kernel runs with CR0_TS set and we must explicitly disable this bit before any FPU operation in order to prevent FPU DNA. This is done in other parts of the NetBSD kernel whenever we do anything with FPU.

A similar magic is done in NVMM:

   1093 static void
   1094 svm_vcpu_guest_fpu_enter(struct nvmm_cpu *vcpu)
   1095 {
   1096 	struct svm_cpudata *cpudata = vcpu->cpudata;
   1097 
   1098 	cpudata->ts_set = (rcr0() & CR0_TS) != 0;
   1099 
   1100 	fpu_area_save(&cpudata->hfpu, svm_xcr0_mask); // implicit clts()
   1101 	fpu_area_restore(&cpudata->gfpu, svm_xcr0_mask); // implicit clts()
   1102 
   1103 	if (svm_xcr0_mask != 0) {
   1104 		cpudata->hxcr0 = rdxcr(0);
   1105 		wrxcr(0, cpudata->gxcr0);
   1106 	}
   1107 }
   1108 
   1109 static void
   1110 svm_vcpu_guest_fpu_leave(struct nvmm_cpu *vcpu)
   1111 {
   1112 	struct svm_cpudata *cpudata = vcpu->cpudata;
   1113 
   1114 	if (svm_xcr0_mask != 0) {
   1115 		cpudata->gxcr0 = rdxcr(0);
   1116 		wrxcr(0, cpudata->hxcr0);
   1117 	}
   1118 
   1119 	fpu_area_save(&cpudata->gfpu, svm_xcr0_mask); // implicit clts()
   1120 	fpu_area_restore(&cpudata->hfpu, svm_xcr0_mask); // implicit clts()
   1121 
   1122 	if (cpudata->ts_set) {
   1123 		stts();
   1124 	}
   1125 }

https://nxr.netbsd.org/xref/src/sys/dev/nvmm/x86/nvmm_x86_svm.c#1093

@raphaelning

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2019

Oh I see! Sorry, my earlier interpretation was completely off. It's not about the guest at all--this is just to make sure HAXM's own FXSAVE/FXRSTOR instructions don't cause a host #NM. Now I can review the details of the code :-)

core/include/vcpu.h Outdated Show resolved Hide resolved
core/vcpu.c Show resolved Hide resolved
@raphaelning

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2019

BTW, just to answer my own question:

So my question is: what's the relationship between host CR0.TS and guest CR0.TS?

The two are completely separate. At least on my Haswell host, VMX allows the guest full control over CR0.TS, so this bit is mapped to bit 3 of GUEST_CR0 and has nothing to do with host CR0.TS.

Fix FPU DNA exception on NetBSD
Call clts (Clear Task-Switch) bit before calling FPU functions.
This prevents FPU Device Not Available exceptions.

Store the original CR0_TS of host before entering FPU state and
restore it on FPU exit.

Closes #166

Signed-off-by: Kamil Rytarowski <n54@gmx.com>

@krytarowski krytarowski force-pushed the krytarowski:netbsd-5 branch from 707dd8e to b5b2356 Feb 14, 2019

@krytarowski

This comment has been minimized.

Copy link
Contributor Author

commented Feb 14, 2019

Done!

@raphaelning

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2019

Thanks! We'll test this on Windows and macOS.

@raphaelning raphaelning merged commit 3bdfd1a into intel:master Feb 14, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.