In vmm, handle xsetbv like xrstor: instead of trying to prevalidate

the values, just try it and handle the #GP if it faults.

Problem reported by Maxime Villard (max(at)m00nbsd.net)
ok mlarkin@

4 files changed, 40 insertions, 25 deletions

diff --git a/sys/arch/amd64/amd64/locore.S b/sys/arch/amd64/amd64/locore.S
index 01f0d354f5d..67147ed2376 100644
--- a/sys/arch/amd64/amd64/locore.S
+++ b/sys/arch/amd64/amd64/locore.S
@@ -1,4 +1,4 @@
-/*	$OpenBSD: locore.S,v 1.110 2018/10/04 05:00:40 guenther Exp $	*/
+/*	$OpenBSD: locore.S,v 1.111 2018/10/07 22:43:06 guenther Exp $	*/
 /*	$NetBSD: locore.S,v 1.13 2004/03/25 18:33:17 drochner Exp $	*/
 
 /*
@@ -1038,6 +1038,8 @@ warn_once:
  *		save current state, but retain it in the FPU
  *	void fpusavereset(sfp)
  *		save current state and reset FPU to initial/kernel state
+ *	int xsetbv_user(reg, mask)
+ *		load specifed %xcr# register, returns 0/1 if okay/it trapped
  */
 
 ENTRY(xrstor_user)
@@ -1090,6 +1092,24 @@ ENTRY(fpusavereset)
 	ret
 END(fpusavereset)
 
+ENTRY(xsetbv_user)
+	RETGUARD_SETUP(xsetbv_user, r11)
+	movl	%edi, %ecx
+	movq	%rsi, %rdx
+	movl	%esi, %eax
+	shrq	$32, %rdx
+	.globl	xsetbv_fault
+xsetbv_fault:
+	xsetbv
+	xorl	%eax, %eax
+	RETGUARD_CHECK(xsetbv_user, r11)
+	ret
+NENTRY(xsetbv_resume)
+	movl	$1, %eax
+	RETGUARD_CHECK(xsetbv_user, r11)
+	ret
+END(xsetbv_user)
+
 	.section .rodata
 	.globl	_C_LABEL(_xrstor)
 _C_LABEL(_xrstor):
diff --git a/sys/arch/amd64/amd64/vector.S b/sys/arch/amd64/amd64/vector.S
index fb5f02b749f..2cb6035cb47 100644
--- a/sys/arch/amd64/amd64/vector.S
+++ b/sys/arch/amd64/amd64/vector.S
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vector.S,v 1.76 2018/10/04 05:00:40 guenther Exp $	*/
+/*	$OpenBSD: vector.S,v 1.77 2018/10/07 22:43:06 guenther Exp $	*/
 /*	$NetBSD: vector.S,v 1.5 2004/06/28 09:13:11 fvdl Exp $	*/
 
 /*
@@ -284,6 +284,9 @@ IDTVEC(trap0d)
 	leaq	xrstor_fault(%rip),%rcx
 	cmpq	%rcx,%rdx
 	je	.Lhandle_xrstor
+	leaq	xsetbv_fault(%rip),%rcx
+	cmpq	%rcx,%rdx
+	je	.Lhandle_xsetbv
 	popq	%rcx
 	popq	%rdx
 	TRAP(T_PROTFLT)
@@ -293,6 +296,11 @@ IDTVEC(trap0d)
 	leaq	xrstor_resume(%rip),%rcx
 	jmp	1f
 
+.Lhandle_xsetbv:
+	/* xsetbv faulted; just resume in xsetbv_resume */
+	leaq	xsetbv_resume(%rip),%rcx
+	jmp	1f
+
 .Lhandle_doreti:
 	/* iretq faulted; resume in a stub that acts like we got a #GP */
 	leaq	.Lhandle_doreti_resume(%rip),%rcx
diff --git a/sys/arch/amd64/amd64/vmm.c b/sys/arch/amd64/amd64/vmm.c
index 266a5333e56..0c25d915f82 100644
--- a/sys/arch/amd64/amd64/vmm.c
+++ b/sys/arch/amd64/amd64/vmm.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmm.c,v 1.220 2018/09/20 14:32:59 brynet Exp $	*/
+/*	$OpenBSD: vmm.c,v 1.221 2018/10/07 22:43:06 guenther Exp $	*/
 /*
  * Copyright (c) 2014 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -3851,14 +3851,6 @@ vmm_fpurestore(struct vcpu *vcpu)
 	}
 
 	if (vcpu->vc_fpuinited) {
-		/* Restore guest XCR0 and FPU context */
-		if (vcpu->vc_gueststate.vg_xcr0 & ~xsave_mask) {
-			DPRINTF("%s: guest attempted to set invalid bits in "
-			    "xcr0 (guest %%xcr0=0x%llx, host mask=0x%llx)\n",
-			    __func__, vcpu->vc_gueststate.vg_xcr0, ~xsave_mask);
-			return EINVAL;
-		}
-
 		if (xrstor_user(&vcpu->vc_g_fpu, xsave_mask)) {
 			DPRINTF("%s: guest attempted to set invalid %s\n",
 			    __func__, "xsave/xrstor state");
@@ -3868,7 +3860,12 @@ vmm_fpurestore(struct vcpu *vcpu)
 
 	if (xsave_mask) {
 		/* Restore guest %xcr0 */
-		xsetbv(0, vcpu->vc_gueststate.vg_xcr0);
+		if (xsetbv_user(0, vcpu->vc_gueststate.vg_xcr0)) {
+			DPRINTF("%s: guest attempted to set invalid bits in "
+			    "xcr0 (guest %%xcr0=0x%llx, host %%xcr0=0x%llx)\n",
+			    __func__, vcpu->vc_gueststate.vg_xcr0, xsave_mask);
+			return EINVAL;
+		}
 	}
 
 	return 0;
@@ -5741,18 +5738,7 @@ vmm_handle_xsetbv(struct vcpu *vcpu, uint64_t *rax)
 		return (EINVAL);
 	}
 
-	/*
-	 * No bits in %edx are currently supported. Check this, and validate
-	 * against the host mask.
-	 */
-	if (*rdx != 0 || (*rax & ~xsave_mask)) {
-		DPRINTF("%s: guest specified invalid xcr0 content "
-		    "(0x%llx:0x%llx)\n", __func__, *rdx, *rax);
-		/* XXX this should #GP(0) instead of killing the guest */
-		return (EINVAL);
-	}
-
-	vcpu->vc_gueststate.vg_xcr0 = *rax;
+	vcpu->vc_gueststate.vg_xcr0 = *rax + (*rdx << 32);
 
 	return (0);
 }
diff --git a/sys/arch/amd64/include/fpu.h b/sys/arch/amd64/include/fpu.h
index 67232001c8f..017a0e9f080 100644
--- a/sys/arch/amd64/include/fpu.h
+++ b/sys/arch/amd64/include/fpu.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: fpu.h,v 1.15 2018/06/24 00:49:25 guenther Exp $	*/
+/*	$OpenBSD: fpu.h,v 1.16 2018/10/07 22:43:06 guenther Exp $	*/
 /*	$NetBSD: fpu.h,v 1.1 2003/04/26 18:39:40 fvdl Exp $	*/
 
 #ifndef	_MACHINE_FPU_H_
@@ -73,6 +73,7 @@ void fpu_kernel_exit(void);
 int	xrstor_user(struct savefpu *_addr, uint64_t _mask);
 #define	fpureset() \
 	xrstor_user(&proc0.p_addr->u_pcb.pcb_savefpu, xsave_mask)
+int	xsetbv_user(uint32_t _reg, uint64_t _mask);
 
 #define fninit()		__asm("fninit")
 #define fwait()			__asm("fwait")