Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 20 Feb 2014 01:48:25 +0000 (UTC)
From:      Neel Natu <neel@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r262236 - in head: sys/amd64/include sys/amd64/vmm sys/amd64/vmm/io usr.sbin/bhyve
Message-ID:  <201402200148.s1K1mPPE078159@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: neel
Date: Thu Feb 20 01:48:25 2014
New Revision: 262236
URL: http://svnweb.freebsd.org/changeset/base/262236

Log:
  Simplify APIC mode switching from MMIO to x2APIC. In part this is done to
  simplify the implementation of the x2APIC virtualization assist in VT-x.
  
  Prior to this change the vlapic allowed the guest to change its mode from
  xAPIC to x2APIC. We don't allow that any more and the vlapic mode is locked
  when the virtual machine is created. This is not very constraining because
  operating systems already have to deal with BIOS setting up the APIC in
  x2APIC mode at boot.
  
  Fix a bug in the CPUID emulation where the x2APIC capability was leaking
  from the host to the guest.
  
  Ignore MMIO reads and writes to the vlapic in x2APIC mode. Similarly, ignore
  MSR accesses to the vlapic when it is in xAPIC mode.
  
  The default configuration of the vlapic is xAPIC. The "-x" option to bhyve(8)
  can be used to change the mode to x2APIC instead.
  
  Discussed with:	grehan@

Modified:
  head/sys/amd64/include/vmm.h
  head/sys/amd64/vmm/io/vlapic.c
  head/sys/amd64/vmm/io/vlapic.h
  head/sys/amd64/vmm/vmm.c
  head/sys/amd64/vmm/vmm_lapic.c
  head/sys/amd64/vmm/x86.c
  head/usr.sbin/bhyve/bhyverun.c

Modified: head/sys/amd64/include/vmm.h
==============================================================================
--- head/sys/amd64/include/vmm.h	Wed Feb 19 23:09:25 2014	(r262235)
+++ head/sys/amd64/include/vmm.h	Thu Feb 20 01:48:25 2014	(r262236)
@@ -265,9 +265,8 @@ enum vm_cap_type {
 };
 
 enum x2apic_state {
-	X2APIC_ENABLED,
-	X2APIC_AVAILABLE,
 	X2APIC_DISABLED,
+	X2APIC_ENABLED,
 	X2APIC_STATE_LAST
 };
 

Modified: head/sys/amd64/vmm/io/vlapic.c
==============================================================================
--- head/sys/amd64/vmm/io/vlapic.c	Wed Feb 19 23:09:25 2014	(r262235)
+++ head/sys/amd64/vmm/io/vlapic.c	Thu Feb 20 01:48:25 2014	(r262236)
@@ -1119,12 +1119,31 @@ vlapic_svr_write_handler(struct vlapic *
 }
 
 int
-vlapic_read(struct vlapic *vlapic, uint64_t offset, uint64_t *data, bool *retu)
+vlapic_read(struct vlapic *vlapic, int mmio_access, uint64_t offset,
+    uint64_t *data, bool *retu)
 {
 	struct LAPIC	*lapic = vlapic->apic_page;
 	uint32_t	*reg;
 	int		 i;
 
+	/* Ignore MMIO accesses in x2APIC mode */
+	if (x2apic(vlapic) && mmio_access) {
+		VLAPIC_CTR1(vlapic, "MMIO read from offset %#lx in x2APIC mode",
+		    offset);
+		*data = 0;
+		goto done;
+	}
+
+	if (!x2apic(vlapic) && !mmio_access) {
+		/*
+		 * XXX Generate GP fault for MSR accesses in xAPIC mode
+		 */
+		VLAPIC_CTR1(vlapic, "x2APIC MSR read from offset %#lx in "
+		    "xAPIC mode", offset);
+		*data = 0;
+		goto done;
+	}
+
 	if (offset > sizeof(*lapic)) {
 		*data = 0;
 		goto done;
@@ -1221,7 +1240,8 @@ done:
 }
 
 int
-vlapic_write(struct vlapic *vlapic, uint64_t offset, uint64_t data, bool *retu)
+vlapic_write(struct vlapic *vlapic, int mmio_access, uint64_t offset,
+    uint64_t data, bool *retu)
 {
 	struct LAPIC	*lapic = vlapic->apic_page;
 	uint32_t	*regptr;
@@ -1230,10 +1250,26 @@ vlapic_write(struct vlapic *vlapic, uint
 	KASSERT((offset & 0xf) == 0 && offset < PAGE_SIZE,
 	    ("vlapic_write: invalid offset %#lx", offset));
 
-	VLAPIC_CTR2(vlapic, "vlapic write offset %#x, data %#lx", offset, data);
+	VLAPIC_CTR2(vlapic, "vlapic write offset %#lx, data %#lx",
+	    offset, data);
 
-	if (offset > sizeof(*lapic)) {
-		return 0;
+	if (offset > sizeof(*lapic))
+		return (0);
+
+	/* Ignore MMIO accesses in x2APIC mode */
+	if (x2apic(vlapic) && mmio_access) {
+		VLAPIC_CTR2(vlapic, "MMIO write of %#lx to offset %#lx "
+		    "in x2APIC mode", data, offset);
+		return (0);
+	}
+
+	/*
+	 * XXX Generate GP fault for MSR accesses in xAPIC mode
+	 */
+	if (!x2apic(vlapic) && !mmio_access) {
+		VLAPIC_CTR2(vlapic, "x2APIC MSR write of %#lx to offset %#lx "
+		    "in xAPIC mode", data, offset);
+		return (0);
 	}
 
 	retval = 0;
@@ -1380,50 +1416,47 @@ vlapic_get_apicbase(struct vlapic *vlapi
 	return (vlapic->msr_apicbase);
 }
 
-void
+int
 vlapic_set_apicbase(struct vlapic *vlapic, uint64_t new)
 {
-	struct LAPIC *lapic;
-	enum x2apic_state state;
-	uint64_t old;
-	int err;
-
-	err = vm_get_x2apic_state(vlapic->vm, vlapic->vcpuid, &state);
-	if (err)
-		panic("vlapic_set_apicbase: err %d fetching x2apic state", err);
-
-	if (state == X2APIC_DISABLED)
-		new &= ~APICBASE_X2APIC;
-
-	old = vlapic->msr_apicbase;
-	vlapic->msr_apicbase = new;
 
-	/*
-	 * If the vlapic is switching between xAPIC and x2APIC modes then
-	 * reset the mode-dependent registers.
-	 */
-	if ((old ^ new) & APICBASE_X2APIC) {
-		lapic = vlapic->apic_page;
-		lapic->id = vlapic_get_id(vlapic);
-		if (x2apic(vlapic)) {
-			lapic->ldr = x2apic_ldr(vlapic);
-			lapic->dfr = 0;
-		} else {
-			lapic->ldr = 0;
-			lapic->dfr = 0xffffffff;
-		}
+	if (vlapic->msr_apicbase != new) {
+		VLAPIC_CTR2(vlapic, "Changing APIC_BASE MSR from %#lx to %#lx "
+		    "not supported", vlapic->msr_apicbase, new);
+		return (-1);
 	}
+
+	return (0);
 }
 
 void
 vlapic_set_x2apic_state(struct vm *vm, int vcpuid, enum x2apic_state state)
 {
 	struct vlapic *vlapic;
+	struct LAPIC *lapic;
 
 	vlapic = vm_lapic(vm, vcpuid);
 
 	if (state == X2APIC_DISABLED)
 		vlapic->msr_apicbase &= ~APICBASE_X2APIC;
+	else
+		vlapic->msr_apicbase |= APICBASE_X2APIC;
+
+	/*
+	 * Reset the local APIC registers whose values are mode-dependent.
+	 *
+	 * XXX this works because the APIC mode can be changed only at vcpu
+	 * initialization time.
+	 */
+	lapic = vlapic->apic_page;
+	lapic->id = vlapic_get_id(vlapic);
+	if (x2apic(vlapic)) {
+		lapic->ldr = x2apic_ldr(vlapic);
+		lapic->dfr = 0;
+	} else {
+		lapic->ldr = 0;
+		lapic->dfr = 0xffffffff;
+	}
 }
 
 void

Modified: head/sys/amd64/vmm/io/vlapic.h
==============================================================================
--- head/sys/amd64/vmm/io/vlapic.h	Wed Feb 19 23:09:25 2014	(r262235)
+++ head/sys/amd64/vmm/io/vlapic.h	Thu Feb 20 01:48:25 2014	(r262236)
@@ -32,10 +32,10 @@
 struct vm;
 enum x2apic_state;
 
-int vlapic_write(struct vlapic *vlapic, uint64_t offset, uint64_t data,
-    bool *retu);
-int vlapic_read(struct vlapic *vlapic, uint64_t offset, uint64_t *data,
-    bool *retu);
+int vlapic_write(struct vlapic *vlapic, int mmio_access, uint64_t offset,
+    uint64_t data, bool *retu);
+int vlapic_read(struct vlapic *vlapic, int mmio_access, uint64_t offset,
+    uint64_t *data, bool *retu);
 
 /*
  * Returns 0 if there is no eligible vector that can be delivered to the
@@ -74,7 +74,7 @@ void vlapic_fire_cmci(struct vlapic *vla
 int vlapic_trigger_lvt(struct vlapic *vlapic, int vector);
 
 uint64_t vlapic_get_apicbase(struct vlapic *vlapic);
-void vlapic_set_apicbase(struct vlapic *vlapic, uint64_t val);
+int vlapic_set_apicbase(struct vlapic *vlapic, uint64_t val);
 void vlapic_set_x2apic_state(struct vm *vm, int vcpuid, enum x2apic_state s);
 bool vlapic_enabled(struct vlapic *vlapic);
 

Modified: head/sys/amd64/vmm/vmm.c
==============================================================================
--- head/sys/amd64/vmm/vmm.c	Wed Feb 19 23:09:25 2014	(r262235)
+++ head/sys/amd64/vmm/vmm.c	Thu Feb 20 01:48:25 2014	(r262236)
@@ -206,7 +206,7 @@ vcpu_init(struct vm *vm, uint32_t vcpu_i
 	vcpu->hostcpu = NOCPU;
 	vcpu->vcpuid = vcpu_id;
 	vcpu->vlapic = VLAPIC_INIT(vm->cookie, vcpu_id);
-	vm_set_x2apic_state(vm, vcpu_id, X2APIC_ENABLED);
+	vm_set_x2apic_state(vm, vcpu_id, X2APIC_DISABLED);
 	vcpu->guest_xcr0 = XFEATURE_ENABLED_X87;
 	vcpu->guestfpu = fpu_save_area_alloc();
 	fpu_save_area_reset(vcpu->guestfpu);

Modified: head/sys/amd64/vmm/vmm_lapic.c
==============================================================================
--- head/sys/amd64/vmm/vmm_lapic.c	Wed Feb 19 23:09:25 2014	(r262235)
+++ head/sys/amd64/vmm/vmm_lapic.c	Thu Feb 20 01:48:25 2014	(r262236)
@@ -172,7 +172,7 @@ lapic_rdmsr(struct vm *vm, int cpu, u_in
 		error = 0;
 	} else {
 		offset = x2apic_msr_to_regoff(msr);
-		error = vlapic_read(vlapic, offset, rval, retu);
+		error = vlapic_read(vlapic, 0, offset, rval, retu);
 	}
 
 	return (error);
@@ -188,11 +188,10 @@ lapic_wrmsr(struct vm *vm, int cpu, u_in
 	vlapic = vm_lapic(vm, cpu);
 
 	if (msr == MSR_APICBASE) {
-		vlapic_set_apicbase(vlapic, val);
-		error = 0;
+		error = vlapic_set_apicbase(vlapic, val);
 	} else {
 		offset = x2apic_msr_to_regoff(msr);
-		error = vlapic_write(vlapic, offset, val, retu);
+		error = vlapic_write(vlapic, 0, offset, val, retu);
 	}
 
 	return (error);
@@ -216,7 +215,7 @@ lapic_mmio_write(void *vm, int cpu, uint
 		return (EINVAL);
 
 	vlapic = vm_lapic(vm, cpu);
-	error = vlapic_write(vlapic, off, wval, arg);
+	error = vlapic_write(vlapic, 1, off, wval, arg);
 	return (error);
 }
 
@@ -238,6 +237,6 @@ lapic_mmio_read(void *vm, int cpu, uint6
 		return (EINVAL);
 
 	vlapic = vm_lapic(vm, cpu);
-	error = vlapic_read(vlapic, off, rval, arg);
+	error = vlapic_read(vlapic, 1, off, rval, arg);
 	return (error);
 }

Modified: head/sys/amd64/vmm/x86.c
==============================================================================
--- head/sys/amd64/vmm/x86.c	Wed Feb 19 23:09:25 2014	(r262235)
+++ head/sys/amd64/vmm/x86.c	Thu Feb 20 01:48:25 2014	(r262236)
@@ -149,6 +149,8 @@ x86_emulate_cpuid(struct vm *vm, int vcp
 
 			if (x2apic_state != X2APIC_DISABLED)
 				regs[2] |= CPUID2_X2APIC;
+			else
+				regs[2] &= ~CPUID2_X2APIC;
 
 			/*
 			 * Only advertise CPUID2_XSAVE in the guest if

Modified: head/usr.sbin/bhyve/bhyverun.c
==============================================================================
--- head/usr.sbin/bhyve/bhyverun.c	Wed Feb 19 23:09:25 2014	(r262235)
+++ head/usr.sbin/bhyve/bhyverun.c	Thu Feb 20 01:48:25 2014	(r262236)
@@ -84,8 +84,9 @@ char *vmname;
 int guest_ncpus;
 
 static int pincpu = -1;
-static int guest_vmexit_on_hlt, guest_vmexit_on_pause, disable_x2apic;
+static int guest_vmexit_on_hlt, guest_vmexit_on_pause;
 static int virtio_msix = 1;
+static int x2apic_mode = 0;	/* default is xAPIC */
 
 static int strictio;
 static int strictmsr = 1;
@@ -126,7 +127,7 @@ usage(int code)
         fprintf(stderr,
                 "Usage: %s [-aehwAHIPW] [-g <gdb port>] [-s <pci>]\n"
 		"       %*s [-c vcpus] [-p pincpu] [-m mem] [-l <lpc>] <vm>\n"
-		"       -a: local apic is in XAPIC mode (default is X2APIC)\n"
+		"       -a: local apic is in xAPIC mode (deprecated)\n"
 		"       -A: create an ACPI table\n"
 		"       -g: gdb port\n"
 		"       -c: # cpus (default 1)\n"
@@ -139,7 +140,8 @@ usage(int code)
 		"       -s: <slot,driver,configinfo> PCI slot config\n"
 		"       -l: LPC device configuration\n"
 		"       -m: memory size in MB\n"
-		"       -w: ignore unimplemented MSRs\n",
+		"       -w: ignore unimplemented MSRs\n"
+		"       -x: local apic is in x2APIC mode\n",
 		progname, (int)strlen(progname), "");
 
 	exit(code);
@@ -153,13 +155,6 @@ paddr_guest2host(struct vmctx *ctx, uint
 }
 
 int
-fbsdrun_disable_x2apic(void)
-{
-
-	return (disable_x2apic);
-}
-
-int
 fbsdrun_vmexit_on_pause(void)
 {
 
@@ -570,10 +565,10 @@ fbsdrun_set_capabilities(struct vmctx *c
 			handler[VM_EXITCODE_PAUSE] = vmexit_pause;
         }
 
-	if (fbsdrun_disable_x2apic())
-		err = vm_set_x2apic_state(ctx, cpu, X2APIC_DISABLED);
-	else
+	if (x2apic_mode)
 		err = vm_set_x2apic_state(ctx, cpu, X2APIC_ENABLED);
+	else
+		err = vm_set_x2apic_state(ctx, cpu, X2APIC_DISABLED);
 
 	if (err) {
 		fprintf(stderr, "Unable to set x2apic state (%d)\n", err);
@@ -598,10 +593,10 @@ main(int argc, char *argv[])
 	guest_ncpus = 1;
 	memsize = 256 * MB;
 
-	while ((c = getopt(argc, argv, "abehwAHIPWp:g:c:s:m:l:")) != -1) {
+	while ((c = getopt(argc, argv, "abehwxAHIPWp:g:c:s:m:l:")) != -1) {
 		switch (c) {
 		case 'a':
-			disable_x2apic = 1;
+			x2apic_mode = 0;
 			break;
 		case 'A':
 			acpi = 1;
@@ -658,6 +653,9 @@ main(int argc, char *argv[])
 		case 'W':
 			virtio_msix = 0;
 			break;
+		case 'x':
+			x2apic_mode = 1;
+			break;
 		case 'h':
 			usage(0);			
 		default:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201402200148.s1K1mPPE078159>