Changeset View
Changeset View
Standalone View
Standalone View
sys/amd64/vmm/vmm_dev.c
Show First 20 Lines • Show All 239 Lines • ▼ Show 20 Lines | if (hpa == NULL) { | ||||
error = uiomove(hpa, c, uio); | error = uiomove(hpa, c, uio); | ||||
vm_gpa_release(cookie); | vm_gpa_release(cookie); | ||||
} | } | ||||
} | } | ||||
vcpu_unlock_one(sc, lastcpu); | vcpu_unlock_one(sc, lastcpu); | ||||
return (error); | return (error); | ||||
} | } | ||||
CTASSERT(sizeof(((struct vm_memseg *)0)->name) >= SPECNAMELEN + 1); | CTASSERT(sizeof(((struct vm_memseg *)0)->name) >= VM_MAX_SUFFIXLEN + 1); | ||||
static int | static int | ||||
get_memseg(struct vmmdev_softc *sc, struct vm_memseg *mseg) | get_memseg(struct vmmdev_softc *sc, struct vm_memseg *mseg) | ||||
{ | { | ||||
struct devmem_softc *dsc; | struct devmem_softc *dsc; | ||||
int error; | int error; | ||||
bool sysmem; | bool sysmem; | ||||
error = vm_get_memseg(sc->vm, mseg->segid, &mseg->len, &sysmem, NULL); | error = vm_get_memseg(sc->vm, mseg->segid, &mseg->len, &sysmem, NULL); | ||||
if (error || mseg->len == 0) | if (error || mseg->len == 0) | ||||
return (error); | return (error); | ||||
if (!sysmem) { | if (!sysmem) { | ||||
SLIST_FOREACH(dsc, &sc->devmem, link) { | SLIST_FOREACH(dsc, &sc->devmem, link) { | ||||
if (dsc->segid == mseg->segid) | if (dsc->segid == mseg->segid) | ||||
break; | break; | ||||
} | } | ||||
KASSERT(dsc != NULL, ("%s: devmem segment %d not found", | KASSERT(dsc != NULL, ("%s: devmem segment %d not found", | ||||
__func__, mseg->segid)); | __func__, mseg->segid)); | ||||
error = copystr(dsc->name, mseg->name, SPECNAMELEN + 1, NULL); | error = copystr(dsc->name, mseg->name, sizeof(mseg->name), | ||||
NULL); | |||||
jhb: It might be nicer to use sizeof() instead of the constants as then there'd be less to change… | |||||
} else { | } else { | ||||
bzero(mseg->name, sizeof(mseg->name)); | bzero(mseg->name, sizeof(mseg->name)); | ||||
} | } | ||||
return (error); | return (error); | ||||
} | } | ||||
static int | static int | ||||
alloc_memseg(struct vmmdev_softc *sc, struct vm_memseg *mseg) | alloc_memseg(struct vmmdev_softc *sc, struct vm_memseg *mseg) | ||||
{ | { | ||||
char *name; | char *name; | ||||
int error; | int error; | ||||
bool sysmem; | bool sysmem; | ||||
error = 0; | error = 0; | ||||
name = NULL; | name = NULL; | ||||
sysmem = true; | sysmem = true; | ||||
/* | |||||
* The allocation is lengthened by 1 to hold a terminating NUL. It'll | |||||
* by stripped off when devfs processes the full string. | |||||
*/ | |||||
if (VM_MEMSEG_NAME(mseg)) { | if (VM_MEMSEG_NAME(mseg)) { | ||||
sysmem = false; | sysmem = false; | ||||
name = malloc(SPECNAMELEN + 1, M_VMMDEV, M_WAITOK); | name = malloc(sizeof(mseg->name) M_VMMDEV, M_WAITOK); | ||||
error = copystr(mseg->name, name, SPECNAMELEN + 1, 0); | error = copystr(mseg->name, name, sizeof(mseg->name), NULL); | ||||
Done Inline ActionsI would use sizeof() here as well. devfs just ignores the trailing nul, it doesn't actively try to resize the allocation? jhb: I would use sizeof() here as well. devfs just ignores the trailing nul, it doesn't actively… | |||||
Done Inline ActionsAre you saying that we should drop the +1? scottl: Are you saying that we should drop the +1? | |||||
if (error) | if (error) | ||||
goto done; | goto done; | ||||
} | } | ||||
error = vm_alloc_memseg(sc->vm, mseg->segid, mseg->len, sysmem); | error = vm_alloc_memseg(sc->vm, mseg->segid, mseg->len, sysmem); | ||||
if (error) | if (error) | ||||
goto done; | goto done; | ||||
▲ Show 20 Lines • Show All 589 Lines • ▼ Show 20 Lines | if ((sc->flags & VSC_LINKED) != 0) { | ||||
mtx_unlock(&vmmdev_mtx); | mtx_unlock(&vmmdev_mtx); | ||||
} | } | ||||
free(sc, M_VMMDEV); | free(sc, M_VMMDEV); | ||||
} | } | ||||
static int | static int | ||||
sysctl_vmm_destroy(SYSCTL_HANDLER_ARGS) | sysctl_vmm_destroy(SYSCTL_HANDLER_ARGS) | ||||
{ | { | ||||
int error; | |||||
char buf[VM_MAX_NAMELEN]; | |||||
struct devmem_softc *dsc; | struct devmem_softc *dsc; | ||||
Not Done Inline ActionsFWIW, even with the larger size, it's probably ok for this to be on the stack still. sysctl handlers are called from a pretty shallow stack (sys__sysctl -> userland_sysctl -> sysctl_root -> sysctl_root_handler_locked -> handler). It's also fine to malloc. It's not a critical path so the cost of malloc doesn't matter. jhb: FWIW, even with the larger size, it's probably ok for this to be on the stack still. sysctl… | |||||
struct vmmdev_softc *sc; | struct vmmdev_softc *sc; | ||||
struct cdev *cdev; | struct cdev *cdev; | ||||
char *buf; | |||||
int error, buflen; | |||||
error = vmm_priv_check(req->td->td_ucred); | error = vmm_priv_check(req->td->td_ucred); | ||||
if (error) | if (error) | ||||
return (error); | return (error); | ||||
strlcpy(buf, "beavis", sizeof(buf)); | buflen = VM_MAX_NAMELEN + 1; | ||||
error = sysctl_handle_string(oidp, buf, sizeof(buf), req); | buf = malloc(buflen, M_VMMDEV, M_WAITOK | M_ZERO); | ||||
strlcpy(buf, "beavis", buflen); | |||||
error = sysctl_handle_string(oidp, buf, buflen, req); | |||||
Done Inline ActionsShouldn't we add one byte for a nul terminator here? VM_MAX_NAMELEN, like SPECNAMELEN, doesn't count the nul terminator anymore with this change. markj: Shouldn't we add one byte for a nul terminator here? VM_MAX_NAMELEN, like SPECNAMELEN, doesn't… | |||||
if (error != 0 || req->newptr == NULL) | if (error != 0 || req->newptr == NULL) | ||||
return (error); | goto out; | ||||
mtx_lock(&vmmdev_mtx); | mtx_lock(&vmmdev_mtx); | ||||
sc = vmmdev_lookup(buf); | sc = vmmdev_lookup(buf); | ||||
if (sc == NULL || sc->cdev == NULL) { | if (sc == NULL || sc->cdev == NULL) { | ||||
mtx_unlock(&vmmdev_mtx); | mtx_unlock(&vmmdev_mtx); | ||||
return (EINVAL); | error = EINVAL; | ||||
goto out; | |||||
} | } | ||||
/* | /* | ||||
* The 'cdev' will be destroyed asynchronously when 'si_threadcount' | * The 'cdev' will be destroyed asynchronously when 'si_threadcount' | ||||
* goes down to 0 so we should not do it again in the callback. | * 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 | * Setting 'sc->cdev' to NULL is also used to indicate that the VM | ||||
* is scheduled for destruction. | * is scheduled for destruction. | ||||
Show All 13 Lines | sysctl_vmm_destroy(SYSCTL_HANDLER_ARGS) | ||||
* | * | ||||
* - the 'devmem' cdevs are destroyed before the virtual machine 'cdev' | * - the 'devmem' cdevs are destroyed before the virtual machine 'cdev' | ||||
*/ | */ | ||||
SLIST_FOREACH(dsc, &sc->devmem, link) { | SLIST_FOREACH(dsc, &sc->devmem, link) { | ||||
KASSERT(dsc->cdev != NULL, ("devmem cdev already destroyed")); | KASSERT(dsc->cdev != NULL, ("devmem cdev already destroyed")); | ||||
destroy_dev_sched_cb(dsc->cdev, devmem_destroy, dsc); | destroy_dev_sched_cb(dsc->cdev, devmem_destroy, dsc); | ||||
} | } | ||||
destroy_dev_sched_cb(cdev, vmmdev_destroy, sc); | destroy_dev_sched_cb(cdev, vmmdev_destroy, sc); | ||||
return (0); | error = 0; | ||||
markjUnsubmitted Not Done Inline ActionsIt is redundant to set error here. markj: It is redundant to set `error` here. | |||||
out: | |||||
free(buf, M_VMMDEV); | |||||
return (error); | |||||
} | } | ||||
SYSCTL_PROC(_hw_vmm, OID_AUTO, destroy, | SYSCTL_PROC(_hw_vmm, OID_AUTO, destroy, | ||||
CTLTYPE_STRING | CTLFLAG_RW | CTLFLAG_PRISON, | CTLTYPE_STRING | CTLFLAG_RW | CTLFLAG_PRISON, | ||||
NULL, 0, sysctl_vmm_destroy, "A", NULL); | NULL, 0, sysctl_vmm_destroy, "A", NULL); | ||||
static struct cdevsw vmmdevsw = { | static struct cdevsw vmmdevsw = { | ||||
.d_name = "vmmdev", | .d_name = "vmmdev", | ||||
.d_version = D_VERSION, | .d_version = D_VERSION, | ||||
.d_ioctl = vmmdev_ioctl, | .d_ioctl = vmmdev_ioctl, | ||||
.d_mmap_single = vmmdev_mmap_single, | .d_mmap_single = vmmdev_mmap_single, | ||||
.d_read = vmmdev_rw, | .d_read = vmmdev_rw, | ||||
.d_write = vmmdev_rw, | .d_write = vmmdev_rw, | ||||
}; | }; | ||||
static int | static int | ||||
sysctl_vmm_create(SYSCTL_HANDLER_ARGS) | sysctl_vmm_create(SYSCTL_HANDLER_ARGS) | ||||
{ | { | ||||
int error; | |||||
struct vm *vm; | struct vm *vm; | ||||
struct cdev *cdev; | struct cdev *cdev; | ||||
struct vmmdev_softc *sc, *sc2; | struct vmmdev_softc *sc, *sc2; | ||||
char buf[VM_MAX_NAMELEN]; | char *buf; | ||||
int error, buflen; | |||||
error = vmm_priv_check(req->td->td_ucred); | error = vmm_priv_check(req->td->td_ucred); | ||||
if (error) | if (error) | ||||
return (error); | return (error); | ||||
strlcpy(buf, "beavis", sizeof(buf)); | buflen = VM_MAX_NAMELEN + 1; | ||||
error = sysctl_handle_string(oidp, buf, sizeof(buf), req); | buf = malloc(buflen, M_VMMDEV, M_WAITOK | M_ZERO); | ||||
strlcpy(buf, "beavis", buflen); | |||||
error = sysctl_handle_string(oidp, buf, buflen, req); | |||||
Done Inline ActionsSame here. markj: Same here. | |||||
if (error != 0 || req->newptr == NULL) | if (error != 0 || req->newptr == NULL) | ||||
return (error); | goto out; | ||||
mtx_lock(&vmmdev_mtx); | mtx_lock(&vmmdev_mtx); | ||||
sc = vmmdev_lookup(buf); | sc = vmmdev_lookup(buf); | ||||
mtx_unlock(&vmmdev_mtx); | mtx_unlock(&vmmdev_mtx); | ||||
if (sc != NULL) | if (sc != NULL) { | ||||
return (EEXIST); | error = EEXIST; | ||||
goto out; | |||||
} | |||||
error = vm_create(buf, &vm); | error = vm_create(buf, &vm); | ||||
if (error != 0) | if (error != 0) | ||||
return (error); | goto out; | ||||
sc = malloc(sizeof(struct vmmdev_softc), M_VMMDEV, M_WAITOK | M_ZERO); | sc = malloc(sizeof(struct vmmdev_softc), M_VMMDEV, M_WAITOK | M_ZERO); | ||||
sc->vm = vm; | sc->vm = vm; | ||||
SLIST_INIT(&sc->devmem); | SLIST_INIT(&sc->devmem); | ||||
/* | /* | ||||
* Lookup the name again just in case somebody sneaked in when we | * Lookup the name again just in case somebody sneaked in when we | ||||
* dropped the lock. | * dropped the lock. | ||||
*/ | */ | ||||
mtx_lock(&vmmdev_mtx); | mtx_lock(&vmmdev_mtx); | ||||
sc2 = vmmdev_lookup(buf); | sc2 = vmmdev_lookup(buf); | ||||
if (sc2 == NULL) { | if (sc2 == NULL) { | ||||
SLIST_INSERT_HEAD(&head, sc, link); | SLIST_INSERT_HEAD(&head, sc, link); | ||||
sc->flags |= VSC_LINKED; | sc->flags |= VSC_LINKED; | ||||
} | } | ||||
mtx_unlock(&vmmdev_mtx); | mtx_unlock(&vmmdev_mtx); | ||||
if (sc2 != NULL) { | if (sc2 != NULL) { | ||||
vmmdev_destroy(sc); | vmmdev_destroy(sc); | ||||
return (EEXIST); | error = EEXIST; | ||||
goto out; | |||||
} | } | ||||
error = make_dev_p(MAKEDEV_CHECKNAME, &cdev, &vmmdevsw, NULL, | error = make_dev_p(MAKEDEV_CHECKNAME, &cdev, &vmmdevsw, NULL, | ||||
UID_ROOT, GID_WHEEL, 0600, "vmm/%s", buf); | UID_ROOT, GID_WHEEL, 0600, "vmm/%s", buf); | ||||
if (error != 0) { | if (error != 0) { | ||||
vmmdev_destroy(sc); | vmmdev_destroy(sc); | ||||
return (error); | goto out; | ||||
} | } | ||||
mtx_lock(&vmmdev_mtx); | mtx_lock(&vmmdev_mtx); | ||||
sc->cdev = cdev; | sc->cdev = cdev; | ||||
sc->cdev->si_drv1 = sc; | sc->cdev->si_drv1 = sc; | ||||
mtx_unlock(&vmmdev_mtx); | mtx_unlock(&vmmdev_mtx); | ||||
error = 0; | |||||
markjUnsubmitted Not Done Inline ActionsIt is redundant to set error here. markj: It is redundant to set `error` here. | |||||
return (0); | out: | ||||
free(buf, M_VMMDEV); | |||||
return (error); | |||||
} | } | ||||
SYSCTL_PROC(_hw_vmm, OID_AUTO, create, | SYSCTL_PROC(_hw_vmm, OID_AUTO, create, | ||||
CTLTYPE_STRING | CTLFLAG_RW | CTLFLAG_PRISON, | CTLTYPE_STRING | CTLFLAG_RW | CTLFLAG_PRISON, | ||||
NULL, 0, sysctl_vmm_create, "A", NULL); | NULL, 0, sysctl_vmm_create, "A", NULL); | ||||
void | void | ||||
vmmdev_init(void) | vmmdev_init(void) | ||||
{ | { | ||||
▲ Show 20 Lines • Show All 112 Lines • Show Last 20 Lines |
It might be nicer to use sizeof() instead of the constants as then there'd be less to change for an MFC if you left the memseg size as SPECNAMELEN in an MFC. In this case dsc->name is always a valid C string, so it would just be 'sizeof(mseg->name)' instead of the constant.