When I first went to run bhyve on one hardware box I had the
entire system crash.  This turned out to be due to the BIOS
disabling VMX.  A message came out on the console, but the scripts
continued anyway, and then, boom.  I first looked at the wrong bit
of code for fixing this, but I *think* I improved it anyway. :-)
That's the first patch below.

The second patch below (note: I'll put them somewhere on torek.net
if that's better than just including them here) fixes the crash.
The easiest way to reproduce it is to attempt to create a VM
within a bhyve VM ("bh1" is a running VM):

[before patch -- note, some wraparound fixed]

        root@bh1:~ # kldload vmm
        vmx_init: processor does not support VMX operation
        module_register_init: MOD_LOAD (vmm, 0xffffffff81a133b0, 0) error 6
        root@bh1:~ # sysctl hw.vmm
        hw.vmm.destroy: beavis
        hw.vmm.create: beavis
        hw.vmm.pages_allocated: 0
        root@bh1:~ # sysctl hw.vmm.create
        hw.vmm.create: beavis
        root@bh1:~ # sysctl hw.vmm.create=
        hw.vmm.create: beavisvm exit[1]
        reason          VMX
        rip             0xffffffff81a1932c
        inst_length     6
        error           0
        exit_reason     50
        qualification   0xfffffffffffffff0
        vm exit[0]
        reason          VMX
        rip             0xffffffff81a1932c
        inst_length     6
        error           0
        exit_reason     50
        qualification   0xfffffffffffffff0

[after patch]

        root@bh1:~ # kldload vmm
        vmx_init: processor does not support VMX operation
        module_register_init: MOD_LOAD (vmm, 0xffffffff81a133d0, 0) error 6
        root@bh1:~ # sysctl hw.vmm
        hw.vmm.destroy: beavis
        hw.vmm.create: beavis
        hw.vmm.pages_allocated: 0
        root@bh1:~ # sysctl hw.vmm.create
        hw.vmm.create: beavis
        root@bh1:~ # sysctl hw.vmm.create=
        hw.vmm.create: beavis
        sysctl: hw.vmm.create=: Device not configured
        root@bh1:~ #

Chris

changeset:   1962:7e204f321854
user:        Chris Torek <chris.to...@gmail.com>
date:        Fri Apr 05 19:10:09 2013 -0600
files:       sys/amd64/vmm/intel/vmx.c
description:
improve test for VMX-mode

We only need VMX extensions to be enabled, not locked.
Moreover, we should not care about any VMX-in-SMX settings.


diff --git a/sys/amd64/vmm/intel/vmx.c b/sys/amd64/vmm/intel/vmx.c
--- a/sys/amd64/vmm/intel/vmx.c
+++ b/sys/amd64/vmm/intel/vmx.c
@@ -436,14 +436,33 @@
                return (ENXIO);
        }
 
+/*
+ * Bits in MSR_IA32_FEATURE_CONTROL register.
+ *
+ * XXX move these to machine/specialreg.h
+ */
+#define        MSR_IA32_FEAT_CTL_LOCK          0x01    /* locks the control 
reg */
+#define        MSR_IA32_FEAT_CTL_VMX_SMX_EN    0x02    /* enable VMXON in SMX 
mode */
+#define        MSR_IA32_FEAT_CTL_VMX_EN        0x04    /* enable VMXON 
(non-SMX) */
        /*
-        * Verify that MSR_IA32_FEATURE_CONTROL lock and VMXON enable bits
-        * are set (bits 0 and 2 respectively).
+        * Verify that VMXON is allowed.
+        *
+        * According to Intel docs, we just need VMX_EN to be
+        * set.  If the LOCK bit is not set we can set or clear
+        * VMX_EN ourselves.  Once the LOCK bit is set no more
+        * changes are possible without a processor reset.
+        *
+        * Existing BIOSes currently set-and-lock the feature, but
+        * this code should work with BIOSes that don't lock it.
         */
        feature_control = rdmsr(MSR_IA32_FEATURE_CONTROL);
-       if ((feature_control & 0x5) != 0x5) {
-               printf("vmx_init: VMX operation disabled by BIOS\n");
-               return (ENXIO);
+       if ((feature_control & MSR_IA32_FEAT_CTL_VMX_EN) == 0) {
+               if (feature_control & MSR_IA32_FEAT_CTL_LOCK) {
+                       printf("vmx_init: VMX operation disabled by BIOS\n");
+                       return (ENXIO);
+               }
+               feature_control |= MSR_IA32_FEAT_CTL_VMX_EN;
+               wrmsr(MSR_IA32_FEATURE_CONTROL, feature_control);
        }
 
        /* Check support for primary processor-based VM-execution controls */

changeset:   1963:6cea2a2ed727
tag:         tip
user:        Chris Torek <chris.to...@gmail.com>
date:        Fri Apr 05 19:14:05 2013 -0600
files:       sys/amd64/include/vmm.h sys/amd64/vmm/vmm.c sys/amd64/vmm/vmm_dev.c
description:
prevent host OS crash when VMX is disabled

If VMX is disabled in the BIOS, vmm_init correctly returns an
error, but this still leaves the module loaded and running, and a
later sysctl to create a virtual machine crashed the host.
Have vm_create() check, and return an error for this condition.


diff --git a/sys/amd64/include/vmm.h b/sys/amd64/include/vmm.h
--- a/sys/amd64/include/vmm.h
+++ b/sys/amd64/include/vmm.h
@@ -87,7 +87,7 @@
 extern struct vmm_ops vmm_ops_intel;
 extern struct vmm_ops vmm_ops_amd;
 
-struct vm *vm_create(const char *name);
+int vm_create(const char *name, struct vm **retval);
 void vm_destroy(struct vm *vm);
 const char *vm_name(struct vm *vm);
 int vm_malloc(struct vm *vm, vm_paddr_t gpa, size_t len);
diff --git a/sys/amd64/vmm/vmm.c b/sys/amd64/vmm/vmm.c
--- a/sys/amd64/vmm/vmm.c
+++ b/sys/amd64/vmm/vmm.c
@@ -213,6 +213,15 @@
                vmmdev_init();
                iommu_init();
                error = vmm_init();
+               if (error) {
+                       /*
+                        * Returning an error here does not force
+                        * the module to be unloaded, so we have
+                        * to make sure that vm_create()s will not
+                        * proceed.
+                        */
+                       ops = NULL;
+               }
                break;
        case MOD_UNLOAD:
                error = vmmdev_cleanup();
@@ -249,8 +258,8 @@
 
 SYSCTL_NODE(_hw, OID_AUTO, vmm, CTLFLAG_RW, NULL, NULL);
 
-struct vm *
-vm_create(const char *name)
+int
+vm_create(const char *name, struct vm **retval)
 {
        int i;
        struct vm *vm;
@@ -258,8 +267,11 @@
 
        const int BSP = 0;
 
+       if (ops == NULL)
+               return (ENXIO);
+
        if (name == NULL || strlen(name) >= VM_MAX_NAMELEN)
-               return (NULL);
+               return (EINVAL);
 
        vm = malloc(sizeof(struct vm), M_VM, M_WAITOK | M_ZERO);
        strcpy(vm->name, name);
@@ -274,7 +286,8 @@
        vm->iommu = iommu_create_domain(maxaddr);
        vm_activate_cpu(vm, BSP);
 
-       return (vm);
+       *retval = vm;
+       return (0);
 }
 
 static void
diff --git a/sys/amd64/vmm/vmm_dev.c b/sys/amd64/vmm/vmm_dev.c
--- a/sys/amd64/vmm/vmm_dev.c
+++ b/sys/amd64/vmm/vmm_dev.c
@@ -475,9 +475,9 @@
        if (sc != NULL)
                return (EEXIST);
 
-       vm = vm_create(buf);
-       if (vm == NULL)
-               return (EINVAL);
+       error = vm_create(buf, &vm);
+       if (error)
+               return (error);
 
        sc = malloc(sizeof(struct vmmdev_softc), M_VMMDEV, M_WAITOK | M_ZERO);
        sc->vm = vm;

_______________________________________________
freebsd-virtualization@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"

Reply via email to