Attachment 176039 Details for Bug 213689 – The patch (locking fixed)

[patch] The patch (locking fixed)

bhyve.patch (text/plain), 10.36 KB, created by Vasily Postnicov on 2016-10-21 20:11:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Vasily Postnicov

Created: 2016-10-21 20:11:16 UTC

Size: 10.36 KB

patch

obsolete

>diff --git a/lib/libvmmapi/vmmapi.c b/lib/libvmmapi/vmmapi.c
>index 3a6f210..6d3d7c2 100644
>--- a/lib/libvmmapi/vmmapi.c
>+++ b/lib/libvmmapi/vmmapi.c
>@@ -77,9 +77,6 @@ struct vmctx {
> 	char	*name;
> };
> 
>-#define	CREATE(x)  sysctlbyname("hw.vmm.create", NULL, NULL, (x), strlen((x)))
>-#define	DESTROY(x) sysctlbyname("hw.vmm.destroy", NULL, NULL, (x), strlen((x)))
>-
> static int
> vm_device_open(const char *name)
> {
>@@ -101,8 +98,13 @@ vm_device_open(const char *name)
> int
> vm_create(const char *name)
> {
>+	int fd = open("/dev/vmmctl", O_RDWR);
>+	if (fd == -1)
>+		return fd;
> 
>-	return (CREATE((char *)name));
>+	ioctl(fd, VMCTL_CREATE, name);
>+	close(fd);
>+	return 0;
> }
> 
> struct vmctx *
>@@ -128,16 +130,22 @@ err:
> 	return (NULL);
> }
> 
>-void
>+int
> vm_destroy(struct vmctx *vm)
> {
>+	int fd;
>+
> 	assert(vm != NULL);
>+	fd = open("/dev/vmmctl", O_RDWR);
>+	if (fd == -1)
>+		return fd;
> 
> 	if (vm->fd >= 0)
> 		close(vm->fd);
>-	DESTROY(vm->name);
>-
>+	ioctl(fd, VMCTL_DESTROY, vm->name);
>+	close(fd);
> 	free(vm);
>+	return 0;
> }
> 
> int
>diff --git a/lib/libvmmapi/vmmapi.h b/lib/libvmmapi/vmmapi.h
>index 98eee60..fc571a5 100644
>--- a/lib/libvmmapi/vmmapi.h
>+++ b/lib/libvmmapi/vmmapi.h
>@@ -103,7 +103,7 @@ int	vm_mmap_memseg(struct vmctx *ctx, vm_paddr_t gpa, int segid,
> 
> int	vm_create(const char *name);
> struct vmctx *vm_open(const char *name);
>-void	vm_destroy(struct vmctx *ctx);
>+int	vm_destroy(struct vmctx *ctx);
> int	vm_parse_memsize(const char *optarg, size_t *memsize);
> int	vm_setup_memory(struct vmctx *ctx, size_t len, enum vm_mmap_style s);
> void	*vm_map_gpa(struct vmctx *ctx, vm_paddr_t gaddr, size_t len);
>diff --git a/sys/amd64/include/vmm_dev.h b/sys/amd64/include/vmm_dev.h
>index 1af75a3..1905ebe 100644
>--- a/sys/amd64/include/vmm_dev.h
>+++ b/sys/amd64/include/vmm_dev.h
>@@ -382,4 +382,9 @@ enum {
> 	_IOR('v', IOCNUM_RTC_GETTIME, struct vm_rtc_time)
> #define	VM_RESTART_INSTRUCTION \
> 	_IOW('v', IOCNUM_RESTART_INSTRUCTION, int)
>+
>+#define VMCTL_CREATE \
>+	_IOW('v', 1, char*)
>+#define VMCTL_DESTROY \
>+	_IOW('v', 2, char*)
> #endif
>diff --git a/sys/amd64/vmm/vmm_dev.c b/sys/amd64/vmm/vmm_dev.c
>index 5cb4150..6c69732 100644
>--- a/sys/amd64/vmm/vmm_dev.c
>+++ b/sys/amd64/vmm/vmm_dev.c
>@@ -36,6 +36,7 @@ __FBSDID("$FreeBSD$");
> #include <sys/mutex.h>
> #include <sys/malloc.h>
> #include <sys/conf.h>
>+#include <sys/proc.h>
> #include <sys/sysctl.h>
> #include <sys/libkern.h>
> #include <sys/ioccom.h>
>@@ -746,59 +747,6 @@ vmmdev_destroy(void *arg)
> 	free(sc, M_VMMDEV);
> }
> 
>-static int
>-sysctl_vmm_destroy(SYSCTL_HANDLER_ARGS)
>-{
>-	int error;
>-	char buf[VM_MAX_NAMELEN];
>-	struct devmem_softc *dsc;
>-	struct vmmdev_softc *sc;
>-	struct cdev *cdev;
>-
>-	strlcpy(buf, "beavis", sizeof(buf));
>-	error = sysctl_handle_string(oidp, buf, sizeof(buf), req);
>-	if (error != 0 || req->newptr == NULL)
>-		return (error);
>-
>-	mtx_lock(&vmmdev_mtx);
>-	sc = vmmdev_lookup(buf);
>-	if (sc == NULL || sc->cdev == NULL) {
>-		mtx_unlock(&vmmdev_mtx);
>-		return (EINVAL);
>-	}
>-
>-	/*
>-	 * The 'cdev' will be destroyed asynchronously when 'si_threadcount'
>-	 * goes down to 0 so we should not do it again in the callback.
>-	 *
>-	 * Setting 'sc->cdev' to NULL is also used to indicate that the VM
>-	 * is scheduled for destruction.
>-	 */
>-	cdev = sc->cdev;
>-	sc->cdev = NULL;		
>-	mtx_unlock(&vmmdev_mtx);
>-
>-	/*
>-	 * Schedule all cdevs to be destroyed:
>-	 *
>-	 * - any new operations on the 'cdev' will return an error (ENXIO).
>-	 *
>-	 * - when the 'si_threadcount' dwindles down to zero the 'cdev' will
>-	 *   be destroyed and the callback will be invoked in a taskqueue
>-	 *   context.
>-	 *
>-	 * - the 'devmem' cdevs are destroyed before the virtual machine 'cdev'
>-	 */
>-	SLIST_FOREACH(dsc, &sc->devmem, link) {
>-		KASSERT(dsc->cdev != NULL, ("devmem cdev already destroyed"));
>-		destroy_dev_sched_cb(dsc->cdev, devmem_destroy, dsc);
>-	}
>-	destroy_dev_sched_cb(cdev, vmmdev_destroy, sc);
>-	return (0);
>-}
>-SYSCTL_PROC(_hw_vmm, OID_AUTO, destroy, CTLTYPE_STRING | CTLFLAG_RW,
>-	    NULL, 0, sysctl_vmm_destroy, "A", NULL);
>-
> static struct cdevsw vmmdevsw = {
> 	.d_name		= "vmmdev",
> 	.d_version	= D_VERSION,
>@@ -808,68 +756,6 @@ static struct cdevsw vmmdevsw = {
> 	.d_write	= vmmdev_rw,
> };
> 
>-static int
>-sysctl_vmm_create(SYSCTL_HANDLER_ARGS)
>-{
>-	int error;
>-	struct vm *vm;
>-	struct cdev *cdev;
>-	struct vmmdev_softc *sc, *sc2;
>-	char buf[VM_MAX_NAMELEN];
>-
>-	strlcpy(buf, "beavis", sizeof(buf));
>-	error = sysctl_handle_string(oidp, buf, sizeof(buf), req);
>-	if (error != 0 || req->newptr == NULL)
>-		return (error);
>-
>-	mtx_lock(&vmmdev_mtx);
>-	sc = vmmdev_lookup(buf);
>-	mtx_unlock(&vmmdev_mtx);
>-	if (sc != NULL)
>-		return (EEXIST);
>-
>-	error = vm_create(buf, &vm);
>-	if (error != 0)
>-		return (error);
>-
>-	sc = malloc(sizeof(struct vmmdev_softc), M_VMMDEV, M_WAITOK | M_ZERO);
>-	sc->vm = vm;
>-	SLIST_INIT(&sc->devmem);
>-
>-	/*
>-	 * Lookup the name again just in case somebody sneaked in when we
>-	 * dropped the lock.
>-	 */
>-	mtx_lock(&vmmdev_mtx);
>-	sc2 = vmmdev_lookup(buf);
>-	if (sc2 == NULL) {
>-		SLIST_INSERT_HEAD(&head, sc, link);
>-		sc->flags |= VSC_LINKED;
>-	}
>-	mtx_unlock(&vmmdev_mtx);
>-
>-	if (sc2 != NULL) {
>-		vmmdev_destroy(sc);
>-		return (EEXIST);
>-	}
>-
>-	error = make_dev_p(MAKEDEV_CHECKNAME, &cdev, &vmmdevsw, NULL,
>-			   UID_ROOT, GID_WHEEL, 0600, "vmm/%s", buf);
>-	if (error != 0) {
>-		vmmdev_destroy(sc);
>-		return (error);
>-	}
>-
>-	mtx_lock(&vmmdev_mtx);
>-	sc->cdev = cdev;
>-	sc->cdev->si_drv1 = sc;
>-	mtx_unlock(&vmmdev_mtx);
>-
>-	return (0);
>-}
>-SYSCTL_PROC(_hw_vmm, OID_AUTO, create, CTLTYPE_STRING | CTLFLAG_RW,
>-	    NULL, 0, sysctl_vmm_create, "A", NULL);
>-
> void
> vmmdev_init(void)
> {
>@@ -941,22 +827,33 @@ devmem_create_cdev(const char *vmname, int segid, char *devname)
> 	struct vmmdev_softc *sc;
> 	struct cdev *cdev;
> 	int error;
>+	uid_t uid;
>+
>+	mtx_lock(&vmmdev_mtx);
>+	sc = vmmdev_lookup(vmname);
>+	KASSERT(sc != NULL, ("%s: vm %s softc not found", __func__, vmname));
>+	if (sc->cdev == NULL) {
>+		/* virtual machine is being created or destroyed */
>+		mtx_unlock(&vmmdev_mtx);
>+		return (ENODEV);
>+	}
>+	uid = sc->cdev->si_uid;
>+	mtx_unlock(&vmmdev_mtx);
> 
> 	error = make_dev_p(MAKEDEV_CHECKNAME, &cdev, &devmemsw, NULL,
>-	    UID_ROOT, GID_WHEEL, 0600, "vmm.io/%s.%s", vmname, devname);
>+		uid, GID_WHEEL, 0600, "vmm.io/%s.%s", vmname, devname);
> 	if (error)
> 		return (error);
>-
> 	dsc = malloc(sizeof(struct devmem_softc), M_VMMDEV, M_WAITOK | M_ZERO);
> 
>+	/* The lock was released, check again, if the machine is being destroyed */
> 	mtx_lock(&vmmdev_mtx);
> 	sc = vmmdev_lookup(vmname);
> 	KASSERT(sc != NULL, ("%s: vm %s softc not found", __func__, vmname));
> 	if (sc->cdev == NULL) {
>-		/* virtual machine is being created or destroyed */
> 		mtx_unlock(&vmmdev_mtx);
> 		free(dsc, M_VMMDEV);
>-		destroy_dev_sched_cb(cdev, NULL, 0);
>+		destroy_dev_sched(cdev);
> 		return (ENODEV);
> 	}
> 
>@@ -981,3 +878,168 @@ devmem_destroy(void *arg)
> 	dsc->cdev = NULL;
> 	dsc->sc = NULL;
> }
>+
>+/* ARGSUSED */
>+static int
>+vmmctl_rw(struct cdev *cdev __unused, struct uio *uio __unused, int flags __unused)
>+{
>+	return EOPNOTSUPP;
>+}
>+
>+static int
>+vmmctl_create (char *name, uid_t uid)
>+{
>+	int error;
>+	struct vmmdev_softc *sc, *sc2;
>+	struct cdev *cdev;
>+	struct vm *vm;
>+
>+	mtx_lock(&vmmdev_mtx);
>+	sc = vmmdev_lookup(name);
>+	mtx_unlock(&vmmdev_mtx);
>+	if (sc != NULL)
>+		return (EEXIST);
>+
>+	error = vm_create(name, &vm);
>+	if (error != 0)
>+		return error;
>+
>+	sc = malloc(sizeof(struct vmmdev_softc), M_VMMDEV, M_WAITOK | M_ZERO);
>+	sc->vm = vm;
>+	SLIST_INIT(&sc->devmem);
>+
>+	/*
>+	 * Lookup the name again just in case somebody sneaked in when we
>+	 * dropped the lock.
>+	 */
>+	mtx_lock(&vmmdev_mtx);
>+	sc2 = vmmdev_lookup(name);
>+	if (sc2 == NULL) {
>+		SLIST_INSERT_HEAD(&head, sc, link);
>+			sc->flags |= VSC_LINKED;
>+	}
>+	mtx_unlock(&vmmdev_mtx);
>+
>+	if (sc2 != NULL) {
>+		vmmdev_destroy(sc);
>+		return EEXIST;
>+	}
>+
>+	error = make_dev_p(MAKEDEV_CHECKNAME, &cdev, &vmmdevsw, NULL,
>+					   uid, GID_WHEEL, 0600, "vmm/%s", name);
>+	if (error != 0) {
>+		vmmdev_destroy(sc);
>+		return error;
>+	}
>+
>+	mtx_lock(&vmmdev_mtx);
>+	sc->cdev = cdev;
>+	sc->cdev->si_drv1 = sc;
>+	mtx_unlock(&vmmdev_mtx);
>+
>+	return 0;
>+}
>+
>+static int
>+vmmctl_destroy (char *name)
>+{
>+	struct devmem_softc *dsc;
>+	struct vmmdev_softc *sc;
>+	struct cdev *cdev;
>+
>+	mtx_lock(&vmmdev_mtx);
>+	sc = vmmdev_lookup(name);
>+	if (sc == NULL || sc->cdev == NULL) {
>+		mtx_unlock(&vmmdev_mtx);
>+		return EINVAL;
>+	}
>+	/*
>+	 * The 'cdev' will be destroyed asynchronously when 'si_threadcount'
>+	 * goes down to 0 so we should not do it again in the callback.
>+	 *
>+	 * Setting 'sc->cdev' to NULL is also used to indicate that the VM
>+	 * is scheduled for destruction.
>+	 */
>+	cdev = sc->cdev;
>+	sc->cdev = NULL;
>+	mtx_unlock(&vmmdev_mtx);
>+
>+	SLIST_FOREACH(dsc, &sc->devmem, link) {
>+		KASSERT(dsc->cdev != NULL, ("devmem cdev already destroyed"));
>+		destroy_dev_sched_cb(dsc->cdev, devmem_destroy, dsc);
>+	}
>+	destroy_dev_sched_cb(cdev, vmmdev_destroy, sc);
>+	return 0;
>+}
>+
>+/* ARGSUSED */
>+static int
>+vmmctl_ioctl(struct cdev *dev __unused, u_long cmd, caddr_t data,
>+	int flags __unused, struct thread *td)
>+{
>+	int error;
>+	char *name;
>+	int len;
>+
>+	error = 0;
>+
>+	switch (cmd) {
>+	case VMCTL_CREATE:
>+		name = (char*) data;
>+		len = strnlen (name, VM_MAX_NAMELEN);
>+		if (len == VM_MAX_NAMELEN) {
>+			error = EINVAL;
>+			goto done;
>+		}
>+		printf ("Creating VM with name: %s\n", name);
>+		error = vmmctl_create (name, td->td_ucred->cr_uid);
>+		break;
>+
>+	case VMCTL_DESTROY:
>+		name = (char*) data;
>+		len = strnlen (name, VM_MAX_NAMELEN);
>+		if (len == VM_MAX_NAMELEN) {
>+			error = EINVAL;
>+			goto done;
>+		}
>+		printf ("Destroying VM with name: %s\n", (char*)data);
>+		error = vmmctl_destroy (name);
>+		break;
>+
>+	default:
>+		error = ENOIOCTL;
>+	}
>+done:
>+	/* Make sure that no handler returns a bogus value like ERESTART */
>+	KASSERT(error >= 0, ("vmmdev_ioctl: invalid error return %d", error));
>+	return (error);
>+}
>+
>+static struct cdevsw vmmctl_cdevsw = {
>+	.d_version =	D_VERSION,
>+	.d_open =	(d_open_t*) nullop,
>+	.d_read =	vmmctl_rw,
>+	.d_write =	vmmctl_rw,
>+	.d_ioctl = vmmctl_ioctl,
>+	.d_name =	"vmmctl",
>+};
>+
>+static struct cdev *vmmctl_dev;
>+
>+static void
>+vmmctl_sysinit(void *p)
>+{
>+	vmmctl_dev = make_dev(&vmmctl_cdevsw, 0,
>+		UID_ROOT, GID_WHEEL, 0660, "vmmctl");
>+}
>+SYSINIT(vmmctl_sysinit, SI_SUB_DEVFS, SI_ORDER_ANY, vmmctl_sysinit, NULL);
>+
>+static void
>+vmmctl_sysuninit(void *p)
>+{
>+	if (vmmctl_dev != NULL) {
>+		/* destroy_dev() will wait for all references to go away */
>+		destroy_dev(vmmctl_dev);
>+	}
>+}
>+SYSUNINIT(vmmctl_sysuninit, SI_SUB_DEVFS, SI_ORDER_ANY, vmmctl_sysuninit, NULL);

Actions: View | Diff

Attachments on bug 213689: 176037 | 176039 | 176218