From owner-freebsd-virtualization@FreeBSD.ORG Fri Apr 12 01:19:00 2013 Return-Path: Delivered-To: freebsd-virtualization@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 0E159738 for ; Fri, 12 Apr 2013 01:19:00 +0000 (UTC) (envelope-from neelnatu@gmail.com) Received: from mail-ia0-x236.google.com (mail-ia0-x236.google.com [IPv6:2607:f8b0:4001:c02::236]) by mx1.freebsd.org (Postfix) with ESMTP id D7AA376D for ; Fri, 12 Apr 2013 01:18:59 +0000 (UTC) Received: by mail-ia0-f182.google.com with SMTP id u20so1976909iag.27 for ; Thu, 11 Apr 2013 18:18:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type; bh=kf0ivpFqhIzO3xjbR3e/eYp+k4fup1cSPdfL/ML78Aw=; b=lNAADN20Glui1EMKF6fX/B8GWsxADOjr4zYu3Z4M59L/pOgNL9JgYQG9idHCYE2f8j pJsWvX1i33YKbA/7A7sgkJOJdOH6dGCQK4l8DVR5LE4Ys0hCakimZskgQYZFvueJRsM6 8azPwsTSRzjelN0nux7TKxeNy1FlM7Ol0Rra5b7LzAcsvIzDu6tUmPs5f+y5md0BDIZE A6feN3JTyE7GG0YlAM5nCG4gEN/ryR/4O+KglC5oSLhG1Os9CyK7TVaQ64Et7cRrDtMK zSb0ZmKupxJ61Bkp4cvEJRR4z3haxxE/BXBx/U6l/xJDaTy7hD7tyyd227u/CjEiFsbL cnZw== MIME-Version: 1.0 X-Received: by 10.50.213.97 with SMTP id nr1mr455386igc.36.1365729539559; Thu, 11 Apr 2013 18:18:59 -0700 (PDT) Received: by 10.43.9.138 with HTTP; Thu, 11 Apr 2013 18:18:59 -0700 (PDT) In-Reply-To: <201304060127.r361RRGv068417@elf.torek.net> References: <201304060127.r361RRGv068417@elf.torek.net> Date: Thu, 11 Apr 2013 18:18:59 -0700 Message-ID: Subject: Re: two more small improvements From: Neel Natu To: Chris Torek Content-Type: text/plain; charset=ISO-8859-1 X-Content-Filtered-By: Mailman/MimeDel 2.1.14 Cc: "freebsd-virtualization@freebsd.org" X-BeenThere: freebsd-virtualization@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Discussion of various virtualization techniques FreeBSD supports." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Apr 2013 01:19:00 -0000 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 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 > 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 > 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-unsubscribe@freebsd.org" >