1 files changed, 272 insertions, 153 deletions
diff --git a/sys/kern/subr_firmware.c b/sys/kern/subr_firmware.c
index 29964c4..388a331 100644
--- a/sys/kern/subr_firmware.c
+++ b/sys/kern/subr_firmware.c
@@ -42,40 +42,144 @@ __FBSDID("$FreeBSD$");
 #include <sys/proc.h>
 #include <sys/module.h>
 
+/*
+ * Loadable firmware support. See sys/sys/firmware.h and firmware(9)
+ * form more details on the subsystem.
+ *
+ * 'struct firmware' is the user-visible part of the firmware table.
+ * Additional internal information is stored in a 'struct priv_fw'
+ * (currently a static array). A slot is in use if FW_INUSE is true:
+ */
+
+#define FW_INUSE(p)	((p)->file != NULL || (p)->fw.name != NULL)
+
+/*
+ * fw.name != NULL when an image is registered; file != NULL for
+ * autoloaded images whose handling has not been completed.
+ *
+ * The state of a slot evolves as follows:
+ *	firmware_register	-->  fw.name = image_name
+ *	(autoloaded image)	-->  file = module reference
+ *	firmware_unregister	-->  fw.name = NULL
+ *	(unloadentry complete)	-->  file = NULL
+ *
+ * In order for the above to work, the 'file' field must remain
+ * unchanged in firmware_unregister().
+ *
+ * Images residing in the same module are linked to each other
+ * through the 'parent' argument of firmware_register().
+ * One image (typically, one with the same name as the module to let
+ * the autoloading mechanism work) is considered the parent image for
+ * all other images in the same module. Children affect the refcount
+ * on the parent image preventing improper unloading of the image itself.
+ */
+
+struct priv_fw {
+	int		refcnt;		/* reference count */
+
+	/*
+	 * parent entry, see above. Set on firmware_register(),
+	 * cleared on firmware_unregister().
+	 */
+	struct priv_fw	*parent;
+
+	int 		flags;	/* record FIRMWARE_UNLOAD requests */
+#define FW_UNLOAD	0x100
+
+	/*
+	 * 'file' is private info managed by the autoload/unload code.
+	 * Set at the end of firmware_get(), cleared only in the
+	 * firmware_task, so the latter can depend on its value even
+	 * while the lock is not held.
+	 */
+	linker_file_t   file;	/* module file, if autoloaded */
+
+	/*
+	 * 'fw' is the externally visible image information.
+	 * We do not make it the first field in priv_fw, to avoid the
+	 * temptation of casting pointers to each other.
+	 * Use PRIV_FW(fw) to get a pointer to the cointainer of fw.
+	 * Beware, PRIV_FW does not work for a NULL pointer.
+	 */
+	struct firmware	fw;	/* externally visible information */
+};
+
+/*
+ * PRIV_FW returns the pointer to the container of struct firmware *x.
+ * Cast to intptr_t to override the 'const' attribute of x
+ */
+#define PRIV_FW(x)	((struct priv_fw *)		\
+	((intptr_t)(x) - offsetof(struct priv_fw, fw)) )
+
+/*
+ * At the moment we use a static array as backing store for the registry.
+ * Should we move to a dynamic structure, keep in mind that we cannot
+ * reallocate the array because pointers are held externally.
+ * A list may work, though.
+ */
 #define	FIRMWARE_MAX	30
-static struct firmware firmware_table[FIRMWARE_MAX];
+static struct priv_fw firmware_table[FIRMWARE_MAX];
+
+/*
+ * module release are handled in a separate task as they might sleep.
+ */
 struct task firmware_task;
+
+/*
+ * This mutex protects accesses to the firmware table.
+ */
 struct mtx firmware_mtx;
 MTX_SYSINIT(firmware, &firmware_mtx, "firmware table", MTX_DEF);
 
 /*
+ * Helper function to lookup a name.
+ * As a side effect, it sets the pointer to a free slot, if any.
+ * This way we can concentrate most of the registry scanning in
+ * this function, which makes it easier to replace the registry
+ * with some other data structure.
+ */
+static struct priv_fw *
+lookup(const char *name, struct priv_fw **empty_slot)
+{
+	struct priv_fw *fp = NULL;
+	struct priv_fw *dummy;
+	int i;
+
+	if (empty_slot == NULL)
+		empty_slot = &dummy;
+	*empty_slot = NULL;
+	for (i = 0; i < FIRMWARE_MAX; i++) {
+		fp = &firmware_table[i];
+		if (fp->fw.name != NULL && strcasecmp(name, fp->fw.name) == 0)
+			break;
+		else if (!FW_INUSE(fp))
+			*empty_slot = fp;
+	}
+	return (i < FIRMWARE_MAX ) ? fp : NULL;
+}
+
+/*
  * Register a firmware image with the specified name.  The
  * image name must not already be registered.  If this is a
  * subimage then parent refers to a previously registered
  * image that this should be associated with.
  */
-struct firmware *
+const struct firmware *
 firmware_register(const char *imagename, const void *data, size_t datasize,
-    unsigned int version, struct firmware *parent)
+    unsigned int version, const struct firmware *parent)
 {
-	struct firmware *frp = NULL;
-	int i;
+	struct priv_fw *match, *frp;
 
 	mtx_lock(&firmware_mtx);
-	for (i = 0; i < FIRMWARE_MAX; i++) {
-		struct firmware *fp = &firmware_table[i];
-
-		if (fp->name == NULL) {
-			if (frp == NULL)
-				frp = fp;
-			continue;
-		}
-		if (strcasecmp(imagename, fp->name) == 0) {
-			mtx_unlock(&firmware_mtx);
-			printf("%s: image %s already registered!\n",
-				__func__, imagename);
-			return NULL;
-		}
+	/*
+	 * Do a lookup to make sure the name is unique or find a free slot.
+	 */
+	match = lookup(imagename, &frp);
+	if (match != NULL) {
+		mtx_unlock(&firmware_mtx);
+		printf("%s: image %s already registered!\n",
+			__func__, imagename);
+		return NULL;
 	}
 	if (frp == NULL) {
 		mtx_unlock(&firmware_mtx);
@@ -83,53 +187,20 @@ firmware_register(const char *imagename, const void *data, size_t datasize,
 		    __func__, imagename);
 		return NULL;
 	}
-	frp->name = imagename;
-	frp->data = data;
-	frp->datasize = datasize;
-	frp->version = version;
-	frp->refcnt = 0;
-	frp->flags = 0;
-	if (parent != NULL)
-		parent->refcnt++;
-	frp->parent = parent;
-	frp->file = NULL;
+	bzero(frp, sizeof(frp));	/* start from a clean record */
+	frp->fw.name = imagename;
+	frp->fw.data = data;
+	frp->fw.datasize = datasize;
+	frp->fw.version = version;
+	if (parent != NULL) {
+		frp->parent = PRIV_FW(parent);
+		frp->parent->refcnt++;
+	}
 	mtx_unlock(&firmware_mtx);
-
 	if (bootverbose)
 		printf("firmware: '%s' version %u: %zu bytes loaded at %p\n",
 		    imagename, version, datasize, data);
-
-	return frp;
-}
-
-static void
-clearentry(struct firmware *fp)
-{
-	KASSERT(fp->refcnt == 0, ("image %s refcnt %u", fp->name, fp->refcnt));
-	fp->name = NULL;
-	fp->file = NULL;
-	fp->data = NULL;
-	fp->datasize = 0;
-	fp->version = 0;
-	fp->flags = 0;
-	if (fp->parent != NULL) {	/* release parent reference */
-		fp->parent->refcnt--;
-		fp->parent = NULL;
-	}
-}
-
-static struct firmware *
-lookup(const char *name)
-{
-	struct firmware *fp;
-	int i;
-
-	for (i = 0; i < FIRMWARE_MAX; i++) {
-		fp = &firmware_table[i];
-		if (fp->name != NULL && strcasecmp(name, fp->name) == 0)
-			return fp;
-	}
-	return NULL;
+	return &frp->fw;
 }
 
 /*
@@ -140,62 +211,61 @@ lookup(const char *name)
 int
 firmware_unregister(const char *imagename)
 {
-	struct firmware *fp;
-	int refcnt = 0;
+	struct priv_fw *fp;
+	int err;
 
 	mtx_lock(&firmware_mtx);
-	/*
-	 * NB: it is ok for the lookup to fail; this can happen
-	 * when a module is unloaded on last reference and the
-	 * module unload handler unregister's each of it's
-	 * firmware images.
-	 */
-	fp = lookup(imagename);
-	if (fp != NULL) {
-		refcnt = fp->refcnt;
-		if (refcnt == 0)
-			clearentry(fp);
+	fp = lookup(imagename, NULL);
+	if (fp == NULL) {
+		/*
+		 * It is ok for the lookup to fail; this can happen
+		 * when a module is unloaded on last reference and the
+		 * module unload handler unregister's each of it's
+		 * firmware images.
+		 */
+		err = 0;
+	} else if (fp->refcnt != 0) {	/* cannot unregister */
+		err = EBUSY;
+	}  else {
+		linker_file_t   x = fp->file;	/* save value */
+
+		if (fp->parent != NULL)	/* release parent reference */
+			fp->parent->refcnt--;
+		/*
+		 * Clear the whole entry with bzero to make sure we
+		 * do not forget anything. Then restore 'file' which is
+		 * non-null for autoloaded images.
+		 */
+		bzero(fp, sizeof(struct priv_fw));
+		fp->file = x;
+		err = 0;
 	}
 	mtx_unlock(&firmware_mtx);
-	return (refcnt != 0 ? EBUSY : 0);
+	return err;
 }
 
 /*
  * Lookup and potentially load the specified firmware image.
- * If the firmware is not found in the registry attempt to
- * load a kernel module with the image name.  If the firmware
- * is located a reference is returned.  The caller must release
- * this reference for the image to be eligible for removal/unload.
+ * If the firmware is not found in the registry, try to load a kernel
+ * module named as the image name.
+ * If the firmware is located, a reference is returned. The caller must
+ * release this reference for the image to be eligible for removal/unload.
  */
-struct firmware *
+const struct firmware *
 firmware_get(const char *imagename)
 {
 	struct thread *td;
-	struct firmware *fp;
+	struct priv_fw *fp;
 	linker_file_t result;
-	int requested_load = 0;
 
-again:
 	mtx_lock(&firmware_mtx);
-	fp = lookup(imagename);
-	if (fp != NULL) {
-		if (requested_load)
-			fp->file = result;
-		fp->refcnt++;
-		mtx_unlock(&firmware_mtx);
-		return fp;
-	}
+	fp = lookup(imagename, NULL);
+	if (fp != NULL)
+		goto found;
 	/*
-	 * Image not present, try to load the module holding it
-	 * or if we already tried give up.
+	 * Image not present, try to load the module holding it.
 	 */
 	mtx_unlock(&firmware_mtx);
-	if (requested_load) {
-		printf("%s: failed to load firmware image %s\n",
-		    __func__, imagename);
-		(void) linker_release_module(imagename, NULL, NULL);
-		return NULL;
-	}
 	td = curthread;
 	if (priv_check(td, PRIV_FIRMWARE_LOAD) != 0 ||
 	    securelevel_gt(td->td_ucred, 0) != 0) {
@@ -204,69 +274,105 @@ again:
 		return NULL;
 	}
 	(void) linker_reference_module(imagename, NULL, &result);
-	requested_load = 1;
-	goto again;		/* sort of an Algol-style for loop */
-}
-
-static void
-unloadentry(void *unused1, int unused2)
-{
-	struct firmware *fp;
-	linker_file_t file;
-	int i, err;
-
+	/*
+	 * After loading the module, see if the image is registered now.
+	 */
 	mtx_lock(&firmware_mtx);
-	for (;;) {
-		/* Look for an unwanted entry that we explicitly loaded. */
-		for (i = 0; i < FIRMWARE_MAX; i++) {
-			fp = &firmware_table[i];
-			if (fp->name != NULL && fp->file != NULL &&
-			    fp->refcnt == 0 &&
-			    (fp->flags & FIRMWAREFLAG_KEEPKLDREF) == 0)
-				break;
-			fp = NULL;
-		}
-		if (fp == NULL)
-			break;
-		file = fp->file;
-		/* No longer explicitly loaded. */
-		fp->file = NULL;
+	fp = lookup(imagename, NULL);
+	if (fp == NULL) {
 		mtx_unlock(&firmware_mtx);
-
-		err = linker_release_module(NULL, NULL, file);
-
-		mtx_lock(&firmware_mtx);
-		if (err) {
-			/*
-			 * If linker_release_module() failed then we still
-			 * hold a reference on the module so it should not be
-			 * possible for it to go away or be re-registered.
-			 */
-			KASSERT(fp->file == NULL,
-			    ("firmware entry reused while referenced!"));
-			fp->file = file;
-		}
+		printf("%s: failed to load firmware image %s\n",
+			__func__, imagename);
+		(void) linker_release_module(imagename, NULL, NULL);
+		return NULL;
 	}
+	fp->file = result;	/* record the module identity */
+
+found:				/* common exit point on success */
+	fp->refcnt++;
 	mtx_unlock(&firmware_mtx);
+	return &fp->fw;
 }
 
 /*
- * Release a reference to a firmware image returned by
- * firmware_get.  The reference is released and if this is
- * the last reference to the firmware image the associated
- * module may be released/unloaded.
+ * Release a reference to a firmware image returned by firmware_get.
+ * The caller may specify, with the FIRMWARE_UNLOAD flag, its desire
+ * to release the resource, but the flag is only advisory.
+ *
+ * If this is the last reference to the firmware image, and this is an
+ * autoloaded module, wake up the firmware_task to figure out what to do
+ * with the associated module.
  */
 void
-firmware_put(struct firmware *fp, int flags)
+firmware_put(const struct firmware *p, int flags)
 {
+	struct priv_fw *fp = PRIV_FW(p);
+
 	mtx_lock(&firmware_mtx);
 	fp->refcnt--;
 	if (fp->refcnt == 0) {
-		if ((flags & FIRMWARE_UNLOAD) == 0)
-			fp->flags |= FIRMWAREFLAG_KEEPKLDREF;
+		if (flags & FIRMWARE_UNLOAD)
+			fp->flags |= FW_UNLOAD;
+		if (fp->file)
+			taskqueue_enqueue(taskqueue_thread, &firmware_task);
+	}
+	mtx_unlock(&firmware_mtx);
+}
+
+/*
+ * The body of the task in charge of unloading autoloaded modules
+ * that are not needed anymore.
+ * Images can be cross-linked so we may need to make multiple passes,
+ * but the time we spend in the loop is bounded because we clear entries
+ * as we touch them.
+ */
+static void
+unloadentry(void *unused1, int unused2)
+{
+	int limit = FIRMWARE_MAX;
+	int i;	/* current cycle */
+
+	mtx_lock(&firmware_mtx);
+	/*
+	 * Scan the table. limit is set to make sure we make another
+	 * full sweep after matching an entry that requires unloading.
+	 */
+	for (i = 0; i < limit; i++) {
+		struct priv_fw *fp;
+		int err;
+
+		fp = &firmware_table[i % FIRMWARE_MAX];
+		if (fp->fw.name == NULL || fp->file == NULL ||
+		    fp->refcnt != 0 || (fp->flags & FW_UNLOAD) == 0)
+			continue;
+
+		/*
+		 * Found an entry. Now:
+		 * 1. bump up limit to make sure we make another full round;
+		 * 2. clear FW_UNLOAD so we don't try this entry again.
+		 * 3. release the lock while trying to unload the module.
+		 * 'file' remains set so that the entry cannot be reused
+		 * in the meantime (it also means that fp->file will
+		 * not change while we release the lock).
+		 */
+		limit = i + FIRMWARE_MAX;	/* make another full round */
+		fp->flags &= ~FW_UNLOAD;	/* do not try again */
+
+		mtx_unlock(&firmware_mtx);
+		err = linker_release_module(NULL, NULL, fp->file);
+		mtx_lock(&firmware_mtx);
+
+		/*
+		 * We rely on the module to call firmware_unregister()
+		 * on unload to actually release the entry.
+		 * If err = 0 we can drop our reference as the system
+		 * accepted it. Otherwise unloading failed (e.g. the
+		 * module itself gave an error) so our reference is
+		 * still valid.
+		 */
+		if (err == 0)
+			fp->file = NULL; 
 	}
-	if (fp->file)
-		taskqueue_enqueue(taskqueue_thread, &firmware_task);
 	mtx_unlock(&firmware_mtx);
 }
 
@@ -276,21 +382,34 @@ firmware_put(struct firmware *fp, int flags)
 static int
 firmware_modevent(module_t mod, int type, void *unused)
 {
-	struct firmware *fp;
-	int i;
+	struct priv_fw *fp;
+	int i, err = EINVAL;
 
 	switch (type) {
 	case MOD_LOAD:
 		TASK_INIT(&firmware_task, 0, unloadentry, NULL);
 		return 0;
+
 	case MOD_UNLOAD:
+		/* request all autoloaded modules to be released */
+		mtx_lock(&firmware_mtx);
 		for (i = 0; i < FIRMWARE_MAX; i++) {
 			fp = &firmware_table[i];
-			fp->flags &= ~FIRMWAREFLAG_KEEPKLDREF;
+			fp->flags |= FW_UNLOAD;;
 		}
+		mtx_unlock(&firmware_mtx);
 		taskqueue_enqueue(taskqueue_thread, &firmware_task);
 		taskqueue_drain(taskqueue_thread, &firmware_task);
-		return 0;
+		for (i = 0; i < FIRMWARE_MAX; i++) {
+			fp = &firmware_table[i];
+			if (fp->fw.name != NULL) {
+				printf("%s: image %p ref %d still active slot %d\n",
+					__func__, fp->fw.name,
+					fp->refcnt,  i);
+				err = EINVAL;
+			}
+		}
+		return err;
 	}
 	return EINVAL;
 }