two more small improvements
Neel Natu
neelnatu at gmail.com
Fri Apr 12 01:19:00 UTC 2013
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 <torek at 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 at bh1:~ # kldload vmm
> vmx_init: processor does not support VMX operation
> module_register_init: MOD_LOAD (vmm, 0xffffffff81a133b0, 0) error 6
> root at bh1:~ # sysctl hw.vmm
> hw.vmm.destroy: beavis
> hw.vmm.create: beavis
> hw.vmm.pages_allocated: 0
> root at bh1:~ # sysctl hw.vmm.create
> hw.vmm.create: beavis
> root at 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 at bh1:~ # kldload vmm
> vmx_init: processor does not support VMX operation
> module_register_init: MOD_LOAD (vmm, 0xffffffff81a133d0, 0) error 6
> root at bh1:~ # sysctl hw.vmm
> hw.vmm.destroy: beavis
> hw.vmm.create: beavis
> hw.vmm.pages_allocated: 0
> root at bh1:~ # sysctl hw.vmm.create
> hw.vmm.create: beavis
> root at bh1:~ # sysctl hw.vmm.create=
> hw.vmm.create: beavis
> sysctl: hw.vmm.create=: Device not configured
> root at bh1:~ #
>
> Chris
>
> changeset: 1962:7e204f321854
> user: Chris Torek <chris.torek at 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.torek at 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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
> To unsubscribe, send any mail to "
> freebsd-virtualization-unsubscribe at freebsd.org"
>
More information about the freebsd-virtualization
mailing list