Enhancements and fixes for the spigen(4) driver...

- Resources used by spigen_mmap_single() are now tracked using
  devfs_set_cdevpriv() rather than in the softc.

- Since resources are now tracked per-open-fd, there is no need to try to
  impose any exclusive-open logic, so flags related to that are removed.

- Flags used to track open status to prevent detach() when the device is
  open are replaced with calls to device_busy()/device_unbusy().  That
  extends the protection up the hierarchy so that the spibus and hardware
  controller drivers also can't be detached while the device is open/in use.

- Arbitrary limits on the maximum size of a transfer are removed, along with
  the sysctl variables that allowed the limits to be changed.  There is just
  no reason to limit the size of a spi transfer to the machine's page size.
  Or to any other arbitrary value, really.

- Most of the locking is removed.  It was mostly protecting access to flags
  and fields in the softc that no longer exist.  The locking that remains is
  just to prevent concurrent calls to device_[un]busy().

- The code was calling malloc() with M_WAITOK while holding a mutex in
  several places.  Since most of the locking is gone, that's fixed.
This commit is contained in:
ian 2018-07-11 17:54:41 +00:00
parent 76b31e9732
commit 25b17c09d6

View File

@ -41,7 +41,6 @@ __FBSDID("$FreeBSD$");
#include <sys/proc.h>
#include <sys/rwlock.h>
#include <sys/spigenio.h>
#include <sys/sysctl.h>
#include <sys/types.h>
#include <vm/vm.h>
@ -55,13 +54,16 @@ __FBSDID("$FreeBSD$");
#ifdef FDT
#include <dev/ofw/ofw_bus_subr.h>
static struct ofw_compat_data compat_data[] = {
{"freebsd,spigen", true},
{NULL, false}
};
#endif
#include "spibus_if.h"
#define SPIGEN_OPEN (1 << 0)
#define SPIGEN_MMAP_BUSY (1 << 1)
struct spigen_softc {
device_t sc_dev;
struct cdev *sc_cdev;
@ -69,13 +71,12 @@ struct spigen_softc {
struct cdev *sc_adev; /* alias device */
#endif
struct mtx sc_mtx;
uint32_t sc_command_length_max; /* cannot change while mmapped */
uint32_t sc_data_length_max; /* cannot change while mmapped */
vm_object_t sc_mmap_buffer; /* command, then data */
vm_offset_t sc_mmap_kvaddr;
size_t sc_mmap_buffer_size;
int sc_debug;
int sc_flags;
};
struct spigen_mmap {
vm_object_t bufobj;
vm_offset_t kvaddr;
size_t bufsize;
};
static int
@ -92,7 +93,7 @@ spigen_probe(device_t dev)
#ifdef FDT
if (ofw_bus_status_okay(dev) &&
ofw_bus_is_compatible(dev, "freebsd,spigen"))
ofw_bus_search_compatible(dev, compat_data)->ocd_data)
rv = BUS_PROBE_DEFAULT;
#endif
@ -115,76 +116,6 @@ static struct cdevsw spigen_cdevsw = {
.d_close = spigen_close
};
static int
spigen_command_length_max_proc(SYSCTL_HANDLER_ARGS)
{
struct spigen_softc *sc = (struct spigen_softc *)arg1;
uint32_t command_length_max;
int error;
mtx_lock(&sc->sc_mtx);
command_length_max = sc->sc_command_length_max;
mtx_unlock(&sc->sc_mtx);
error = sysctl_handle_int(oidp, &command_length_max,
sizeof(command_length_max), req);
if (error == 0 && req->newptr != NULL) {
mtx_lock(&sc->sc_mtx);
if (sc->sc_mmap_buffer != NULL)
error = EBUSY;
else
sc->sc_command_length_max = command_length_max;
mtx_unlock(&sc->sc_mtx);
}
return (error);
}
static int
spigen_data_length_max_proc(SYSCTL_HANDLER_ARGS)
{
struct spigen_softc *sc = (struct spigen_softc *)arg1;
uint32_t data_length_max;
int error;
mtx_lock(&sc->sc_mtx);
data_length_max = sc->sc_data_length_max;
mtx_unlock(&sc->sc_mtx);
error = sysctl_handle_int(oidp, &data_length_max,
sizeof(data_length_max), req);
if (error == 0 && req->newptr != NULL) {
mtx_lock(&sc->sc_mtx);
if (sc->sc_mmap_buffer != NULL)
error = EBUSY;
else
sc->sc_data_length_max = data_length_max;
mtx_unlock(&sc->sc_mtx);
}
return (error);
}
static void
spigen_sysctl_init(struct spigen_softc *sc)
{
struct sysctl_ctx_list *ctx;
struct sysctl_oid *tree_node;
struct sysctl_oid_list *tree;
/*
* Add system sysctl tree/handlers.
*/
ctx = device_get_sysctl_ctx(sc->sc_dev);
tree_node = device_get_sysctl_tree(sc->sc_dev);
tree = SYSCTL_CHILDREN(tree_node);
SYSCTL_ADD_PROC(ctx, tree, OID_AUTO, "command_length_max",
CTLFLAG_MPSAFE | CTLFLAG_RW | CTLTYPE_UINT, sc, sizeof(*sc),
spigen_command_length_max_proc, "IU", "SPI command header portion (octets)");
SYSCTL_ADD_PROC(ctx, tree, OID_AUTO, "data_length_max",
CTLFLAG_MPSAFE | CTLFLAG_RW | CTLTYPE_UINT, sc, sizeof(*sc),
spigen_data_length_max_proc, "IU", "SPI data trailer portion (octets)");
SYSCTL_ADD_INT(ctx, tree, OID_AUTO, "data", CTLFLAG_RW,
&sc->sc_debug, 0, "debug flags");
}
static int
spigen_attach(device_t dev)
{
@ -198,8 +129,6 @@ spigen_attach(device_t dev)
sc = device_get_softc(dev);
sc->sc_dev = dev;
sc->sc_command_length_max = PAGE_SIZE;
sc->sc_data_length_max = PAGE_SIZE;
mtx_init(&sc->sc_mtx, device_get_nameunit(dev), NULL, MTX_DEF);
@ -230,30 +159,23 @@ spigen_attach(device_t dev)
}
#endif
spigen_sysctl_init(sc);
return (0);
}
static int
spigen_open(struct cdev *cdev, int oflags, int devtype, struct thread *td)
{
int error;
device_t dev;
struct spigen_softc *sc;
error = 0;
dev = cdev->si_drv1;
sc = device_get_softc(dev);
mtx_lock(&sc->sc_mtx);
if (sc->sc_flags & SPIGEN_OPEN)
error = EBUSY;
else
sc->sc_flags |= SPIGEN_OPEN;
device_busy(sc->sc_dev);
mtx_unlock(&sc->sc_mtx);
return (error);
return (0);
}
static int
@ -261,23 +183,16 @@ spigen_transfer(struct cdev *cdev, struct spigen_transfer *st)
{
struct spi_command transfer = SPI_COMMAND_INITIALIZER;
device_t dev = cdev->si_drv1;
struct spigen_softc *sc = device_get_softc(dev);
int error = 0;
mtx_lock(&sc->sc_mtx);
if (st->st_command.iov_len == 0)
error = EINVAL;
else if (st->st_command.iov_len > sc->sc_command_length_max ||
st->st_data.iov_len > sc->sc_data_length_max)
error = ENOMEM;
mtx_unlock(&sc->sc_mtx);
if (error)
return (error);
#if 0
device_printf(dev, "cmd %p %u data %p %u\n", st->st_command.iov_base,
st->st_command.iov_len, st->st_data.iov_base, st->st_data.iov_len);
#endif
if (st->st_command.iov_len == 0)
return (EINVAL);
transfer.tx_cmd = transfer.rx_cmd = malloc(st->st_command.iov_len,
M_DEVBUF, M_WAITOK);
if (st->st_data.iov_len > 0) {
@ -313,37 +228,22 @@ spigen_transfer_mmapped(struct cdev *cdev, struct spigen_transfer_mmapped *stm)
{
struct spi_command transfer = SPI_COMMAND_INITIALIZER;
device_t dev = cdev->si_drv1;
struct spigen_softc *sc = device_get_softc(dev);
int error = 0;
struct spigen_mmap *mmap;
int error;
mtx_lock(&sc->sc_mtx);
if (sc->sc_flags & SPIGEN_MMAP_BUSY)
error = EBUSY;
else if (stm->stm_command_length > sc->sc_command_length_max ||
stm->stm_data_length > sc->sc_data_length_max)
error = E2BIG;
else if (sc->sc_mmap_buffer == NULL)
error = EINVAL;
else if (sc->sc_mmap_buffer_size <
stm->stm_command_length + stm->stm_data_length)
error = ENOMEM;
if (error == 0)
sc->sc_flags |= SPIGEN_MMAP_BUSY;
mtx_unlock(&sc->sc_mtx);
if (error)
if ((error = devfs_get_cdevpriv((void **)&mmap)) != 0)
return (error);
transfer.tx_cmd = transfer.rx_cmd = (void *)sc->sc_mmap_kvaddr;
if (mmap->bufsize < stm->stm_command_length + stm->stm_data_length)
return (E2BIG);
transfer.tx_cmd = transfer.rx_cmd = (void *)((uintptr_t)mmap->kvaddr);
transfer.tx_cmd_sz = transfer.rx_cmd_sz = stm->stm_command_length;
transfer.tx_data = transfer.rx_data =
(void *)(sc->sc_mmap_kvaddr + stm->stm_command_length);
(void *)((uintptr_t)mmap->kvaddr + stm->stm_command_length);
transfer.tx_data_sz = transfer.rx_data_sz = stm->stm_data_length;
error = SPIBUS_TRANSFER(device_get_parent(dev), dev, &transfer);
mtx_lock(&sc->sc_mtx);
KASSERT((sc->sc_flags & SPIGEN_MMAP_BUSY), ("mmap no longer marked busy"));
sc->sc_flags &= ~(SPIGEN_MMAP_BUSY);
mtx_unlock(&sc->sc_mtx);
return (error);
}
@ -380,14 +280,26 @@ spigen_ioctl(struct cdev *cdev, u_long cmd, caddr_t data, int fflag,
return (error);
}
static void
spigen_mmap_cleanup(void *arg)
{
struct spigen_mmap *mmap = arg;
if (mmap->kvaddr != 0)
pmap_qremove(mmap->kvaddr, mmap->bufsize / PAGE_SIZE);
if (mmap->bufobj != NULL)
vm_object_deallocate(mmap->bufobj);
free(mmap, M_DEVBUF);
}
static int
spigen_mmap_single(struct cdev *cdev, vm_ooffset_t *offset,
vm_size_t size, struct vm_object **object, int nprot)
{
device_t dev = cdev->si_drv1;
struct spigen_softc *sc = device_get_softc(dev);
struct spigen_mmap *mmap;
vm_page_t *m;
size_t n, pages;
int error;
if (size == 0 ||
(nprot & (PROT_EXEC | PROT_READ | PROT_WRITE))
@ -396,34 +308,38 @@ spigen_mmap_single(struct cdev *cdev, vm_ooffset_t *offset,
size = roundup2(size, PAGE_SIZE);
pages = size / PAGE_SIZE;
mtx_lock(&sc->sc_mtx);
if (sc->sc_mmap_buffer != NULL) {
mtx_unlock(&sc->sc_mtx);
if (devfs_get_cdevpriv((void **)&mmap) == 0)
return (EBUSY);
} else if (size > sc->sc_command_length_max + sc->sc_data_length_max) {
mtx_unlock(&sc->sc_mtx);
return (E2BIG);
mmap = malloc(sizeof(*mmap), M_DEVBUF, M_ZERO | M_WAITOK);
if ((mmap->kvaddr = kva_alloc(size)) == 0) {
spigen_mmap_cleanup(mmap);
return (ENOMEM);
}
sc->sc_mmap_buffer_size = size;
*offset = 0;
sc->sc_mmap_buffer = *object = vm_pager_allocate(OBJT_PHYS, 0, size,
nprot, *offset, curthread->td_ucred);
mmap->bufsize = size;
mmap->bufobj = vm_pager_allocate(OBJT_PHYS, 0, size, nprot, 0,
curthread->td_ucred);
m = malloc(sizeof(*m) * pages, M_TEMP, M_WAITOK);
VM_OBJECT_WLOCK(*object);
vm_object_reference_locked(*object); // kernel and userland both
VM_OBJECT_WLOCK(mmap->bufobj);
vm_object_reference_locked(mmap->bufobj); // kernel and userland both
for (n = 0; n < pages; n++) {
m[n] = vm_page_grab(*object, n,
m[n] = vm_page_grab(mmap->bufobj, n,
VM_ALLOC_NOBUSY | VM_ALLOC_ZERO | VM_ALLOC_WIRED);
m[n]->valid = VM_PAGE_BITS_ALL;
}
VM_OBJECT_WUNLOCK(*object);
sc->sc_mmap_kvaddr = kva_alloc(size);
pmap_qenter(sc->sc_mmap_kvaddr, m, pages);
VM_OBJECT_WUNLOCK(mmap->bufobj);
pmap_qenter(mmap->kvaddr, m, pages);
free(m, M_TEMP);
mtx_unlock(&sc->sc_mtx);
if (*object == NULL)
return (EINVAL);
if ((error = devfs_set_cdevpriv(mmap, spigen_mmap_cleanup)) != 0) {
/* Two threads were racing through this code; we lost. */
spigen_mmap_cleanup(mmap);
return (error);
}
*offset = 0;
*object = mmap->bufobj;
return (0);
}
@ -434,16 +350,7 @@ spigen_close(struct cdev *cdev, int fflag, int devtype, struct thread *td)
struct spigen_softc *sc = device_get_softc(dev);
mtx_lock(&sc->sc_mtx);
if (sc->sc_mmap_buffer != NULL) {
pmap_qremove(sc->sc_mmap_kvaddr,
sc->sc_mmap_buffer_size / PAGE_SIZE);
kva_free(sc->sc_mmap_kvaddr, sc->sc_mmap_buffer_size);
sc->sc_mmap_kvaddr = 0;
vm_object_deallocate(sc->sc_mmap_buffer);
sc->sc_mmap_buffer = NULL;
sc->sc_mmap_buffer_size = 0;
}
sc->sc_flags &= ~(SPIGEN_OPEN);
device_unbusy(sc->sc_dev);
mtx_unlock(&sc->sc_mtx);
return (0);
}
@ -455,15 +362,6 @@ spigen_detach(device_t dev)
sc = device_get_softc(dev);
mtx_lock(&sc->sc_mtx);
if (sc->sc_flags & SPIGEN_OPEN) {
mtx_unlock(&sc->sc_mtx);
return (EBUSY);
}
mtx_unlock(&sc->sc_mtx);
mtx_destroy(&sc->sc_mtx);
#ifdef SPIGEN_LEGACY_CDEVNAME
if (sc->sc_adev)
destroy_dev(sc->sc_adev);
@ -472,6 +370,8 @@ spigen_detach(device_t dev)
if (sc->sc_cdev)
destroy_dev(sc->sc_cdev);
mtx_destroy(&sc->sc_mtx);
return (0);
}
@ -494,3 +394,6 @@ static driver_t spigen_driver = {
DRIVER_MODULE(spigen, spibus, spigen_driver, spigen_devclass, 0, 0);
MODULE_DEPEND(spigen, spibus, 1, 1, 1);
#ifdef FDT
SIMPLEBUS_PNP_INFO(compat_data);
#endif