Hi Chris, Thanks for the patches. Committed as: http://svnweb.freebsd.org/base?view=revision&revision=249396 http://svnweb.freebsd.org/base?view=revision&revision=249351
best Neel On Fri, Apr 5, 2013 at 6:27 PM, Chris Torek <to...@torek.net> wrote: > 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" > _______________________________________________ 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"