[RESEND PATCH v1 0/8] Extend FW framework for user FW uploads

[RESEND PATCH v1 0/8] Extend FW framework for user FW uploads

[Russ Weight]

Extend the firmware loader subsystem to support a persistent sysfs
interface that userspace may use to initiate a firmware update. For
example, FPGA based PCIe cards automatically load firmware and FPGA images
from local FLASH when the card boots. The images in FLASH may be updated
with new images that are uploaded by the user.

A device driver may call firmware_upload_register() to expose persistent
"loading" and "data" sysfs files at /sys/class/firmare/<NAME>/*. These
files are used in the same way as the fallback sysfs "loading" and "data"
files. However, when 0 is written to "loading" to complete the write of
firmware data, the data is also transferred to the lower-level driver
using pre-registered call-back functions. The data transfer is done in
the context of a kernel worker thread.

Additional sysfs nodes are added in the same location as "loading" and
"data" to monitor the transfer of the image data to the device using
callback functions provided by the lower-level device driver and to allow
the data transfer to be cancelled.

Example usage:

$ pwd
/sys/class/firmware/n3000bmc-sec-update.8
$ ls
cancel  device  loading  remaining_size  subsystem
data    error   power    status          uevent
$ echo 1 > loading
$ cat /tmp/firmware.bin > data
$ echo 0 > loading
$ while :; do cat status; cat remaining_size ; sleep 3; done
preparing
44590080
<--snip-->
transferring
44459008
transferring
44311552
<--snip-->
transferring
173056
<--snip-->
programming
0
<--snip-->
idle
0
^C
$ cat error

The first two patches in this set make minor changes to enable the
fw_priv data structure and the sysfs interfaces to be used multiple times
during the existence of the device driver instance. The third patch is
mostly a reorganization of existing code in preparation for sharing common
code with the firmware-upload support. The fourth and fifth patches provide
the code for user-initiated firmware uploads. The final 3 patches extend
selftest support to test firmware-upload functionality.


Changelog RFC -> v1:
  - Renamed files fw_sysfs.c and fw_sysfs.h to sysfs.c and sysfs.h
  - Moved "MODULE_IMPORT_NS(FIRMWARE_LOADER_PRIVATE);" from sysfs.c to
    sysfs.h to address an error identified by the kernel test robot
    <lkp@intel.com>
  - renamed fw_upload_register() and fw_upload_unregister() to
    firmware_upload_register() and fw_upload_unregister().
  - Moved ifdef'd section of code out of firmware_loading_store() in sysfs.c
    into a new function, fw_upload_start(), in sysfs_upload.c.
  - Changed #defines to enums for error codes and progress states
  - Added additional kernel-doc supported symbols into the documentation.
    Some rewording in documentation as well.
  - Added module reference counting for the parent module in the
    firmware_upload_register() and firmware_upload_unregister() functions
    to fix problems found when testing with test_firmware module.
  - Removed unnecessary module reference counting for THIS_MODULE.
  - Added a new patch to modify the test_firmware module to support
    testing of the firmware upload mechanism.
  - Added a new patch to modify the test_firmware module to support
    error injection for firmware upload.
  - Added a new patch to extend the existing firmware selftests to cover
    firmware upload.

Russ Weight (8):
  firmware_loader: Clear data and size in fw_free_paged_buf
  firmware_loader: Check fw_state_is_done in loading_store
  firmware_loader: Split sysfs support from fallback
  firmware_loader: Add firmware-upload support
  firmware_loader: Add sysfs nodes to monitor fw_upload
  test_firmware: Add test support for firmware upload
  test_firmware: Error injection for firmware upload
  selftests: firmware: Add firmware upload selftests

 .../ABI/testing/sysfs-class-firmware          |  77 ++++
 .../driver-api/firmware/fw_upload.rst         | 117 +++++
 Documentation/driver-api/firmware/index.rst   |   1 +
 drivers/base/firmware_loader/Kconfig          |  18 +
 drivers/base/firmware_loader/Makefile         |   2 +
 drivers/base/firmware_loader/fallback.c       | 430 -----------------
 drivers/base/firmware_loader/fallback.h       |  46 +-
 drivers/base/firmware_loader/firmware.h       |  11 +
 drivers/base/firmware_loader/main.c           |  18 +-
 drivers/base/firmware_loader/sysfs.c          | 435 ++++++++++++++++++
 drivers/base/firmware_loader/sysfs.h          | 100 ++++
 drivers/base/firmware_loader/sysfs_upload.c   | 396 ++++++++++++++++
 drivers/base/firmware_loader/sysfs_upload.h   |  47 ++
 include/linux/firmware.h                      |  82 ++++
 lib/test_firmware.c                           | 378 +++++++++++++++
 tools/testing/selftests/firmware/Makefile     |   2 +-
 tools/testing/selftests/firmware/config       |   1 +
 tools/testing/selftests/firmware/fw_lib.sh    |   7 +
 .../selftests/firmware/fw_run_tests.sh        |   4 +
 tools/testing/selftests/firmware/fw_upload.sh | 214 +++++++++
 20 files changed, 1900 insertions(+), 486 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-firmware
 create mode 100644 Documentation/driver-api/firmware/fw_upload.rst
 create mode 100644 drivers/base/firmware_loader/sysfs.c
 create mode 100644 drivers/base/firmware_loader/sysfs.h
 create mode 100644 drivers/base/firmware_loader/sysfs_upload.c
 create mode 100644 drivers/base/firmware_loader/sysfs_upload.h
 create mode 100755 tools/testing/selftests/firmware/fw_upload.sh

-- 
2.25.1

[RESEND PATCH v1 1/8] firmware_loader: Clear data and size in fw_free_paged_buf

[Russ Weight]

The fw_free_paged_buf() function resets the paged buffer information in
the fw_priv data structure. Additionally, clear the data and size members
of fw_priv in order to facilitate the reuse of fw_priv. This is being
done in preparation for enabling userspace to initiate multiple firmware
uploads using this sysfs interface.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
v1:
  - No change from RFC patch
---
 drivers/base/firmware_loader/main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 94d1789a233e..2cc11d93753a 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -253,6 +253,8 @@ void fw_free_paged_buf(struct fw_priv *fw_priv)
 	fw_priv->pages = NULL;
 	fw_priv->page_array_size = 0;
 	fw_priv->nr_pages = 0;
+	fw_priv->data = NULL;
+	fw_priv->size = 0;
 }
 
 int fw_grow_paged_buf(struct fw_priv *fw_priv, int pages_needed)
-- 
2.25.1

[RESEND PATCH v1 2/8] firmware_loader: Check fw_state_is_done in loading_store

[Russ Weight]

Add the fw_state_is_done() function and exit early from
firmware_loading_store() if the state is already "done". This is being done
in preparation for supporting persistent sysfs nodes to allow userspace to
upload firmware to a device, potentially reusing the sysfs loading and data
files multiple times.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
v1:
  - No change from RFC patch
---
 drivers/base/firmware_loader/fallback.c | 2 +-
 drivers/base/firmware_loader/firmware.h | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 4afb0e9312c0..d82e055a4297 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -250,7 +250,7 @@ static ssize_t firmware_loading_store(struct device *dev,
 
 	mutex_lock(&fw_lock);
 	fw_priv = fw_sysfs->fw_priv;
-	if (fw_state_is_aborted(fw_priv))
+	if (fw_state_is_aborted(fw_priv) || fw_state_is_done(fw_priv))
 		goto out;
 
 	switch (loading) {
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 2889f446ad41..58768d16f8df 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -149,6 +149,11 @@ static inline void fw_state_done(struct fw_priv *fw_priv)
 	__fw_state_set(fw_priv, FW_STATUS_DONE);
 }
 
+static inline bool fw_state_is_done(struct fw_priv *fw_priv)
+{
+	return __fw_state_check(fw_priv, FW_STATUS_DONE);
+}
+
 int assign_fw(struct firmware *fw, struct device *device);
 
 #ifdef CONFIG_FW_LOADER
-- 
2.25.1

[RESEND PATCH v1 3/8] firmware_loader: Split sysfs support from fallback

[Russ Weight]

In preparation for sharing the "loading" and "data" sysfs nodes with the
new firmware upload support, split out sysfs functionality from fallback.c
and fallback.h into sysfs.c and sysfs.h. This includes the firmware
class driver code that is associated with the sysfs files and the
fw_fallback_config support for the timeout sysfs node.

CONFIG_FW_LOADER_SYSFS is created and is selected by
CONFIG_FW_LOADER_USER_HELPER in order to include sysfs.o in
firmware_class-objs.

This is mostly just a code reorganization. There are a few symbols that
change in scope, and these can be identified by looking at the header
file changes. A few white-space warnings from checkpatch are also
addressed in this patch.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
v1:
  - Renamed files fw_sysfs.c and fw_sysfs.h to sysfs.c and sysfs.h
  - Moved "MODULE_IMPORT_NS(FIRMWARE_LOADER_PRIVATE);" from sysfs.c to
    sysfs.h to address an error identified by the kernel test robot
    <lkp@intel.com>
---
 drivers/base/firmware_loader/Kconfig    |   4 +
 drivers/base/firmware_loader/Makefile   |   1 +
 drivers/base/firmware_loader/fallback.c | 430 ------------------------
 drivers/base/firmware_loader/fallback.h |  46 +--
 drivers/base/firmware_loader/sysfs.c    | 411 ++++++++++++++++++++++
 drivers/base/firmware_loader/sysfs.h    |  96 ++++++
 6 files changed, 513 insertions(+), 475 deletions(-)
 create mode 100644 drivers/base/firmware_loader/sysfs.c
 create mode 100644 drivers/base/firmware_loader/sysfs.h

diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
index 38f3b66bf52b..9e03178eee00 100644
--- a/drivers/base/firmware_loader/Kconfig
+++ b/drivers/base/firmware_loader/Kconfig
@@ -29,6 +29,9 @@ if FW_LOADER
 config FW_LOADER_PAGED_BUF
 	bool
 
+config FW_LOADER_SYSFS
+	bool
+
 config EXTRA_FIRMWARE
 	string "Build named firmware blobs into the kernel binary"
 	help
@@ -72,6 +75,7 @@ config EXTRA_FIRMWARE_DIR
 
 config FW_LOADER_USER_HELPER
 	bool "Enable the firmware sysfs fallback mechanism"
+	select FW_LOADER_SYSFS
 	select FW_LOADER_PAGED_BUF
 	help
 	  This option enables a sysfs loading facility to enable firmware
diff --git a/drivers/base/firmware_loader/Makefile b/drivers/base/firmware_loader/Makefile
index e87843408fe6..aab213f82288 100644
--- a/drivers/base/firmware_loader/Makefile
+++ b/drivers/base/firmware_loader/Makefile
@@ -6,5 +6,6 @@ obj-$(CONFIG_FW_LOADER)	+= firmware_class.o
 firmware_class-objs := main.o
 firmware_class-$(CONFIG_FW_LOADER_USER_HELPER) += fallback.o
 firmware_class-$(CONFIG_EFI_EMBEDDED_FIRMWARE) += fallback_platform.o
+firmware_class-$(CONFIG_FW_LOADER_SYSFS) += sysfs.o
 
 obj-y += builtin/
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index d82e055a4297..bf68e3947814 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -3,12 +3,9 @@
 #include <linux/types.h>
 #include <linux/kconfig.h>
 #include <linux/list.h>
-#include <linux/slab.h>
 #include <linux/security.h>
-#include <linux/highmem.h>
 #include <linux/umh.h>
 #include <linux/sysctl.h>
-#include <linux/vmalloc.h>
 #include <linux/module.h>
 
 #include "fallback.h"
@@ -18,22 +15,6 @@
  * firmware fallback mechanism
  */
 
-MODULE_IMPORT_NS(FIRMWARE_LOADER_PRIVATE);
-
-extern struct firmware_fallback_config fw_fallback_config;
-
-/* These getters are vetted to use int properly */
-static inline int __firmware_loading_timeout(void)
-{
-	return fw_fallback_config.loading_timeout;
-}
-
-/* These setters are vetted to use int properly */
-static void __fw_fallback_set_timeout(int timeout)
-{
-	fw_fallback_config.loading_timeout = timeout;
-}
-
 /*
  * use small loading timeout for caching devices' firmware because all these
  * firmware images have been loaded successfully at lease once, also system is
@@ -58,52 +39,11 @@ static long firmware_loading_timeout(void)
 		__firmware_loading_timeout() * HZ : MAX_JIFFY_OFFSET;
 }
 
-static inline bool fw_sysfs_done(struct fw_priv *fw_priv)
-{
-	return __fw_state_check(fw_priv, FW_STATUS_DONE);
-}
-
-static inline bool fw_sysfs_loading(struct fw_priv *fw_priv)
-{
-	return __fw_state_check(fw_priv, FW_STATUS_LOADING);
-}
-
 static inline int fw_sysfs_wait_timeout(struct fw_priv *fw_priv,  long timeout)
 {
 	return __fw_state_wait_common(fw_priv, timeout);
 }
 
-struct fw_sysfs {
-	bool nowait;
-	struct device dev;
-	struct fw_priv *fw_priv;
-	struct firmware *fw;
-};
-
-static struct fw_sysfs *to_fw_sysfs(struct device *dev)
-{
-	return container_of(dev, struct fw_sysfs, dev);
-}
-
-static void __fw_load_abort(struct fw_priv *fw_priv)
-{
-	/*
-	 * There is a small window in which user can write to 'loading'
-	 * between loading done/aborted and disappearance of 'loading'
-	 */
-	if (fw_state_is_aborted(fw_priv) || fw_sysfs_done(fw_priv))
-		return;
-
-	fw_state_aborted(fw_priv);
-}
-
-static void fw_load_abort(struct fw_sysfs *fw_sysfs)
-{
-	struct fw_priv *fw_priv = fw_sysfs->fw_priv;
-
-	__fw_load_abort(fw_priv);
-}
-
 static LIST_HEAD(pending_fw_head);
 
 void kill_pending_fw_fallback_reqs(bool only_kill_custom)
@@ -120,376 +60,6 @@ void kill_pending_fw_fallback_reqs(bool only_kill_custom)
 	mutex_unlock(&fw_lock);
 }
 
-static ssize_t timeout_show(struct class *class, struct class_attribute *attr,
-			    char *buf)
-{
-	return sysfs_emit(buf, "%d\n", __firmware_loading_timeout());
-}
-
-/**
- * timeout_store() - set number of seconds to wait for firmware
- * @class: device class pointer
- * @attr: device attribute pointer
- * @buf: buffer to scan for timeout value
- * @count: number of bytes in @buf
- *
- *	Sets the number of seconds to wait for the firmware.  Once
- *	this expires an error will be returned to the driver and no
- *	firmware will be provided.
- *
- *	Note: zero means 'wait forever'.
- **/
-static ssize_t timeout_store(struct class *class, struct class_attribute *attr,
-			     const char *buf, size_t count)
-{
-	int tmp_loading_timeout = simple_strtol(buf, NULL, 10);
-
-	if (tmp_loading_timeout < 0)
-		tmp_loading_timeout = 0;
-
-	__fw_fallback_set_timeout(tmp_loading_timeout);
-
-	return count;
-}
-static CLASS_ATTR_RW(timeout);
-
-static struct attribute *firmware_class_attrs[] = {
-	&class_attr_timeout.attr,
-	NULL,
-};
-ATTRIBUTE_GROUPS(firmware_class);
-
-static void fw_dev_release(struct device *dev)
-{
-	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
-
-	kfree(fw_sysfs);
-}
-
-static int do_firmware_uevent(struct fw_sysfs *fw_sysfs, struct kobj_uevent_env *env)
-{
-	if (add_uevent_var(env, "FIRMWARE=%s", fw_sysfs->fw_priv->fw_name))
-		return -ENOMEM;
-	if (add_uevent_var(env, "TIMEOUT=%i", __firmware_loading_timeout()))
-		return -ENOMEM;
-	if (add_uevent_var(env, "ASYNC=%d", fw_sysfs->nowait))
-		return -ENOMEM;
-
-	return 0;
-}
-
-static int firmware_uevent(struct device *dev, struct kobj_uevent_env *env)
-{
-	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
-	int err = 0;
-
-	mutex_lock(&fw_lock);
-	if (fw_sysfs->fw_priv)
-		err = do_firmware_uevent(fw_sysfs, env);
-	mutex_unlock(&fw_lock);
-	return err;
-}
-
-static struct class firmware_class = {
-	.name		= "firmware",
-	.class_groups	= firmware_class_groups,
-	.dev_uevent	= firmware_uevent,
-	.dev_release	= fw_dev_release,
-};
-
-int register_sysfs_loader(void)
-{
-	int ret = class_register(&firmware_class);
-
-	if (ret != 0)
-		return ret;
-	return register_firmware_config_sysctl();
-}
-
-void unregister_sysfs_loader(void)
-{
-	unregister_firmware_config_sysctl();
-	class_unregister(&firmware_class);
-}
-
-static ssize_t firmware_loading_show(struct device *dev,
-				     struct device_attribute *attr, char *buf)
-{
-	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
-	int loading = 0;
-
-	mutex_lock(&fw_lock);
-	if (fw_sysfs->fw_priv)
-		loading = fw_sysfs_loading(fw_sysfs->fw_priv);
-	mutex_unlock(&fw_lock);
-
-	return sysfs_emit(buf, "%d\n", loading);
-}
-
-/**
- * firmware_loading_store() - set value in the 'loading' control file
- * @dev: device pointer
- * @attr: device attribute pointer
- * @buf: buffer to scan for loading control value
- * @count: number of bytes in @buf
- *
- *	The relevant values are:
- *
- *	 1: Start a load, discarding any previous partial load.
- *	 0: Conclude the load and hand the data to the driver code.
- *	-1: Conclude the load with an error and discard any written data.
- **/
-static ssize_t firmware_loading_store(struct device *dev,
-				      struct device_attribute *attr,
-				      const char *buf, size_t count)
-{
-	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
-	struct fw_priv *fw_priv;
-	ssize_t written = count;
-	int loading = simple_strtol(buf, NULL, 10);
-
-	mutex_lock(&fw_lock);
-	fw_priv = fw_sysfs->fw_priv;
-	if (fw_state_is_aborted(fw_priv) || fw_state_is_done(fw_priv))
-		goto out;
-
-	switch (loading) {
-	case 1:
-		/* discarding any previous partial load */
-		if (!fw_sysfs_done(fw_priv)) {
-			fw_free_paged_buf(fw_priv);
-			fw_state_start(fw_priv);
-		}
-		break;
-	case 0:
-		if (fw_sysfs_loading(fw_priv)) {
-			int rc;
-
-			/*
-			 * Several loading requests may be pending on
-			 * one same firmware buf, so let all requests
-			 * see the mapped 'buf->data' once the loading
-			 * is completed.
-			 * */
-			rc = fw_map_paged_buf(fw_priv);
-			if (rc)
-				dev_err(dev, "%s: map pages failed\n",
-					__func__);
-			else
-				rc = security_kernel_post_load_data(fw_priv->data,
-						fw_priv->size,
-						LOADING_FIRMWARE, "blob");
-
-			/*
-			 * Same logic as fw_load_abort, only the DONE bit
-			 * is ignored and we set ABORT only on failure.
-			 */
-			if (rc) {
-				fw_state_aborted(fw_priv);
-				written = rc;
-			} else {
-				fw_state_done(fw_priv);
-			}
-			break;
-		}
-		fallthrough;
-	default:
-		dev_err(dev, "%s: unexpected value (%d)\n", __func__, loading);
-		fallthrough;
-	case -1:
-		fw_load_abort(fw_sysfs);
-		break;
-	}
-out:
-	mutex_unlock(&fw_lock);
-	return written;
-}
-
-static DEVICE_ATTR(loading, 0644, firmware_loading_show, firmware_loading_store);
-
-static void firmware_rw_data(struct fw_priv *fw_priv, char *buffer,
-			   loff_t offset, size_t count, bool read)
-{
-	if (read)
-		memcpy(buffer, fw_priv->data + offset, count);
-	else
-		memcpy(fw_priv->data + offset, buffer, count);
-}
-
-static void firmware_rw(struct fw_priv *fw_priv, char *buffer,
-			loff_t offset, size_t count, bool read)
-{
-	while (count) {
-		void *page_data;
-		int page_nr = offset >> PAGE_SHIFT;
-		int page_ofs = offset & (PAGE_SIZE-1);
-		int page_cnt = min_t(size_t, PAGE_SIZE - page_ofs, count);
-
-		page_data = kmap(fw_priv->pages[page_nr]);
-
-		if (read)
-			memcpy(buffer, page_data + page_ofs, page_cnt);
-		else
-			memcpy(page_data + page_ofs, buffer, page_cnt);
-
-		kunmap(fw_priv->pages[page_nr]);
-		buffer += page_cnt;
-		offset += page_cnt;
-		count -= page_cnt;
-	}
-}
-
-static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj,
-				  struct bin_attribute *bin_attr,
-				  char *buffer, loff_t offset, size_t count)
-{
-	struct device *dev = kobj_to_dev(kobj);
-	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
-	struct fw_priv *fw_priv;
-	ssize_t ret_count;
-
-	mutex_lock(&fw_lock);
-	fw_priv = fw_sysfs->fw_priv;
-	if (!fw_priv || fw_sysfs_done(fw_priv)) {
-		ret_count = -ENODEV;
-		goto out;
-	}
-	if (offset > fw_priv->size) {
-		ret_count = 0;
-		goto out;
-	}
-	if (count > fw_priv->size - offset)
-		count = fw_priv->size - offset;
-
-	ret_count = count;
-
-	if (fw_priv->data)
-		firmware_rw_data(fw_priv, buffer, offset, count, true);
-	else
-		firmware_rw(fw_priv, buffer, offset, count, true);
-
-out:
-	mutex_unlock(&fw_lock);
-	return ret_count;
-}
-
-static int fw_realloc_pages(struct fw_sysfs *fw_sysfs, int min_size)
-{
-	int err;
-
-	err = fw_grow_paged_buf(fw_sysfs->fw_priv,
-				PAGE_ALIGN(min_size) >> PAGE_SHIFT);
-	if (err)
-		fw_load_abort(fw_sysfs);
-	return err;
-}
-
-/**
- * firmware_data_write() - write method for firmware
- * @filp: open sysfs file
- * @kobj: kobject for the device
- * @bin_attr: bin_attr structure
- * @buffer: buffer being written
- * @offset: buffer offset for write in total data store area
- * @count: buffer size
- *
- *	Data written to the 'data' attribute will be later handed to
- *	the driver as a firmware image.
- **/
-static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj,
-				   struct bin_attribute *bin_attr,
-				   char *buffer, loff_t offset, size_t count)
-{
-	struct device *dev = kobj_to_dev(kobj);
-	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
-	struct fw_priv *fw_priv;
-	ssize_t retval;
-
-	if (!capable(CAP_SYS_RAWIO))
-		return -EPERM;
-
-	mutex_lock(&fw_lock);
-	fw_priv = fw_sysfs->fw_priv;
-	if (!fw_priv || fw_sysfs_done(fw_priv)) {
-		retval = -ENODEV;
-		goto out;
-	}
-
-	if (fw_priv->data) {
-		if (offset + count > fw_priv->allocated_size) {
-			retval = -ENOMEM;
-			goto out;
-		}
-		firmware_rw_data(fw_priv, buffer, offset, count, false);
-		retval = count;
-	} else {
-		retval = fw_realloc_pages(fw_sysfs, offset + count);
-		if (retval)
-			goto out;
-
-		retval = count;
-		firmware_rw(fw_priv, buffer, offset, count, false);
-	}
-
-	fw_priv->size = max_t(size_t, offset + count, fw_priv->size);
-out:
-	mutex_unlock(&fw_lock);
-	return retval;
-}
-
-static struct bin_attribute firmware_attr_data = {
-	.attr = { .name = "data", .mode = 0644 },
-	.size = 0,
-	.read = firmware_data_read,
-	.write = firmware_data_write,
-};
-
-static struct attribute *fw_dev_attrs[] = {
-	&dev_attr_loading.attr,
-	NULL
-};
-
-static struct bin_attribute *fw_dev_bin_attrs[] = {
-	&firmware_attr_data,
-	NULL
-};
-
-static const struct attribute_group fw_dev_attr_group = {
-	.attrs = fw_dev_attrs,
-	.bin_attrs = fw_dev_bin_attrs,
-};
-
-static const struct attribute_group *fw_dev_attr_groups[] = {
-	&fw_dev_attr_group,
-	NULL
-};
-
-static struct fw_sysfs *
-fw_create_instance(struct firmware *firmware, const char *fw_name,
-		   struct device *device, u32 opt_flags)
-{
-	struct fw_sysfs *fw_sysfs;
-	struct device *f_dev;
-
-	fw_sysfs = kzalloc(sizeof(*fw_sysfs), GFP_KERNEL);
-	if (!fw_sysfs) {
-		fw_sysfs = ERR_PTR(-ENOMEM);
-		goto exit;
-	}
-
-	fw_sysfs->nowait = !!(opt_flags & FW_OPT_NOWAIT);
-	fw_sysfs->fw = firmware;
-	f_dev = &fw_sysfs->dev;
-
-	device_initialize(f_dev);
-	dev_set_name(f_dev, "%s", fw_name);
-	f_dev->parent = device;
-	f_dev->class = &firmware_class;
-	f_dev->groups = fw_dev_attr_groups;
-exit:
-	return fw_sysfs;
-}
-
 /**
  * fw_load_sysfs_fallback() - load a firmware via the sysfs fallback mechanism
  * @fw_sysfs: firmware sysfs information for the firmware to load
diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h
index 9f3055d3b4ca..144148595660 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -6,29 +6,7 @@
 #include <linux/device.h>
 
 #include "firmware.h"
-
-/**
- * struct firmware_fallback_config - firmware fallback configuration settings
- *
- * Helps describe and fine tune the fallback mechanism.
- *
- * @force_sysfs_fallback: force the sysfs fallback mechanism to be used
- * 	as if one had enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.
- * 	Useful to help debug a CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y
- * 	functionality on a kernel where that config entry has been disabled.
- * @ignore_sysfs_fallback: force to disable the sysfs fallback mechanism.
- * 	This emulates the behaviour as if we had set the kernel
- * 	config CONFIG_FW_LOADER_USER_HELPER=n.
- * @old_timeout: for internal use
- * @loading_timeout: the timeout to wait for the fallback mechanism before
- * 	giving up, in seconds.
- */
-struct firmware_fallback_config {
-	unsigned int force_sysfs_fallback;
-	unsigned int ignore_sysfs_fallback;
-	int old_timeout;
-	int loading_timeout;
-};
+#include "sysfs.h"
 
 #ifdef CONFIG_FW_LOADER_USER_HELPER
 int firmware_fallback_sysfs(struct firmware *fw, const char *name,
@@ -40,19 +18,6 @@ void kill_pending_fw_fallback_reqs(bool only_kill_custom);
 void fw_fallback_set_cache_timeout(void);
 void fw_fallback_set_default_timeout(void);
 
-int register_sysfs_loader(void);
-void unregister_sysfs_loader(void);
-#ifdef CONFIG_SYSCTL
-extern int register_firmware_config_sysctl(void);
-extern void unregister_firmware_config_sysctl(void);
-#else
-static inline int register_firmware_config_sysctl(void)
-{
-	return 0;
-}
-static inline void unregister_firmware_config_sysctl(void) { }
-#endif /* CONFIG_SYSCTL */
-
 #else /* CONFIG_FW_LOADER_USER_HELPER */
 static inline int firmware_fallback_sysfs(struct firmware *fw, const char *name,
 					  struct device *device,
@@ -66,15 +31,6 @@ static inline int firmware_fallback_sysfs(struct firmware *fw, const char *name,
 static inline void kill_pending_fw_fallback_reqs(bool only_kill_custom) { }
 static inline void fw_fallback_set_cache_timeout(void) { }
 static inline void fw_fallback_set_default_timeout(void) { }
-
-static inline int register_sysfs_loader(void)
-{
-	return 0;
-}
-
-static inline void unregister_sysfs_loader(void)
-{
-}
 #endif /* CONFIG_FW_LOADER_USER_HELPER */
 
 #ifdef CONFIG_EFI_EMBEDDED_FIRMWARE
diff --git a/drivers/base/firmware_loader/sysfs.c b/drivers/base/firmware_loader/sysfs.c
new file mode 100644
index 000000000000..49aeff45b123
--- /dev/null
+++ b/drivers/base/firmware_loader/sysfs.c
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/highmem.h>
+#include <linux/module.h>
+#include <linux/security.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include "firmware.h"
+#include "sysfs.h"
+
+/*
+ * sysfs support for firmware loader
+ */
+
+static inline bool fw_sysfs_done(struct fw_priv *fw_priv)
+{
+	return __fw_state_check(fw_priv, FW_STATUS_DONE);
+}
+
+static inline bool fw_sysfs_loading(struct fw_priv *fw_priv)
+{
+	return __fw_state_check(fw_priv, FW_STATUS_LOADING);
+}
+
+void __fw_load_abort(struct fw_priv *fw_priv)
+{
+	/*
+	 * There is a small window in which user can write to 'loading'
+	 * between loading done/aborted and disappearance of 'loading'
+	 */
+	if (fw_state_is_aborted(fw_priv) || fw_sysfs_done(fw_priv))
+		return;
+
+	fw_state_aborted(fw_priv);
+}
+
+static ssize_t timeout_show(struct class *class, struct class_attribute *attr,
+			    char *buf)
+{
+	return sysfs_emit(buf, "%d\n", __firmware_loading_timeout());
+}
+
+/**
+ * timeout_store() - set number of seconds to wait for firmware
+ * @class: device class pointer
+ * @attr: device attribute pointer
+ * @buf: buffer to scan for timeout value
+ * @count: number of bytes in @buf
+ *
+ *	Sets the number of seconds to wait for the firmware.  Once
+ *	this expires an error will be returned to the driver and no
+ *	firmware will be provided.
+ *
+ *	Note: zero means 'wait forever'.
+ **/
+static ssize_t timeout_store(struct class *class, struct class_attribute *attr,
+			     const char *buf, size_t count)
+{
+	int tmp_loading_timeout = simple_strtol(buf, NULL, 10);
+
+	if (tmp_loading_timeout < 0)
+		tmp_loading_timeout = 0;
+
+	__fw_fallback_set_timeout(tmp_loading_timeout);
+
+	return count;
+}
+static CLASS_ATTR_RW(timeout);
+
+static struct attribute *firmware_class_attrs[] = {
+	&class_attr_timeout.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(firmware_class);
+
+static void fw_dev_release(struct device *dev)
+{
+	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
+
+	kfree(fw_sysfs);
+}
+
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+static int do_firmware_uevent(struct fw_sysfs *fw_sysfs, struct kobj_uevent_env *env)
+{
+	if (add_uevent_var(env, "FIRMWARE=%s", fw_sysfs->fw_priv->fw_name))
+		return -ENOMEM;
+	if (add_uevent_var(env, "TIMEOUT=%i", __firmware_loading_timeout()))
+		return -ENOMEM;
+	if (add_uevent_var(env, "ASYNC=%d", fw_sysfs->nowait))
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int firmware_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
+	int err = 0;
+
+	mutex_lock(&fw_lock);
+	if (fw_sysfs->fw_priv)
+		err = do_firmware_uevent(fw_sysfs, env);
+	mutex_unlock(&fw_lock);
+	return err;
+}
+#endif /* CONFIG_FW_LOADER_USER_HELPER */
+
+static struct class firmware_class = {
+	.name		= "firmware",
+	.class_groups	= firmware_class_groups,
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+	.dev_uevent	= firmware_uevent,
+#endif
+	.dev_release	= fw_dev_release,
+};
+
+int register_sysfs_loader(void)
+{
+	int ret = class_register(&firmware_class);
+
+	if (ret != 0)
+		return ret;
+	return register_firmware_config_sysctl();
+}
+
+void unregister_sysfs_loader(void)
+{
+	unregister_firmware_config_sysctl();
+	class_unregister(&firmware_class);
+}
+
+static ssize_t firmware_loading_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
+	int loading = 0;
+
+	mutex_lock(&fw_lock);
+	if (fw_sysfs->fw_priv)
+		loading = fw_sysfs_loading(fw_sysfs->fw_priv);
+	mutex_unlock(&fw_lock);
+
+	return sysfs_emit(buf, "%d\n", loading);
+}
+
+/**
+ * firmware_loading_store() - set value in the 'loading' control file
+ * @dev: device pointer
+ * @attr: device attribute pointer
+ * @buf: buffer to scan for loading control value
+ * @count: number of bytes in @buf
+ *
+ *	The relevant values are:
+ *
+ *	 1: Start a load, discarding any previous partial load.
+ *	 0: Conclude the load and hand the data to the driver code.
+ *	-1: Conclude the load with an error and discard any written data.
+ **/
+static ssize_t firmware_loading_store(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
+	struct fw_priv *fw_priv;
+	ssize_t written = count;
+	int loading = simple_strtol(buf, NULL, 10);
+
+	mutex_lock(&fw_lock);
+	fw_priv = fw_sysfs->fw_priv;
+	if (fw_state_is_aborted(fw_priv) || fw_state_is_done(fw_priv))
+		goto out;
+
+	switch (loading) {
+	case 1:
+		/* discarding any previous partial load */
+		if (!fw_sysfs_done(fw_priv)) {
+			fw_free_paged_buf(fw_priv);
+			fw_state_start(fw_priv);
+		}
+		break;
+	case 0:
+		if (fw_sysfs_loading(fw_priv)) {
+			int rc;
+
+			/*
+			 * Several loading requests may be pending on
+			 * one same firmware buf, so let all requests
+			 * see the mapped 'buf->data' once the loading
+			 * is completed.
+			 */
+			rc = fw_map_paged_buf(fw_priv);
+			if (rc)
+				dev_err(dev, "%s: map pages failed\n",
+					__func__);
+			else
+				rc = security_kernel_post_load_data(fw_priv->data,
+								    fw_priv->size,
+								    LOADING_FIRMWARE,
+								    "blob");
+
+			/*
+			 * Same logic as fw_load_abort, only the DONE bit
+			 * is ignored and we set ABORT only on failure.
+			 */
+			if (rc) {
+				fw_state_aborted(fw_priv);
+				written = rc;
+			} else {
+				fw_state_done(fw_priv);
+			}
+			break;
+		}
+		fallthrough;
+	default:
+		dev_err(dev, "%s: unexpected value (%d)\n", __func__, loading);
+		fallthrough;
+	case -1:
+		fw_load_abort(fw_sysfs);
+		break;
+	}
+out:
+	mutex_unlock(&fw_lock);
+	return written;
+}
+
+static DEVICE_ATTR(loading, 0644, firmware_loading_show, firmware_loading_store);
+
+static void firmware_rw_data(struct fw_priv *fw_priv, char *buffer,
+			     loff_t offset, size_t count, bool read)
+{
+	if (read)
+		memcpy(buffer, fw_priv->data + offset, count);
+	else
+		memcpy(fw_priv->data + offset, buffer, count);
+}
+
+static void firmware_rw(struct fw_priv *fw_priv, char *buffer,
+			loff_t offset, size_t count, bool read)
+{
+	while (count) {
+		void *page_data;
+		int page_nr = offset >> PAGE_SHIFT;
+		int page_ofs = offset & (PAGE_SIZE - 1);
+		int page_cnt = min_t(size_t, PAGE_SIZE - page_ofs, count);
+
+		page_data = kmap(fw_priv->pages[page_nr]);
+
+		if (read)
+			memcpy(buffer, page_data + page_ofs, page_cnt);
+		else
+			memcpy(page_data + page_ofs, buffer, page_cnt);
+
+		kunmap(fw_priv->pages[page_nr]);
+		buffer += page_cnt;
+		offset += page_cnt;
+		count -= page_cnt;
+	}
+}
+
+static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj,
+				  struct bin_attribute *bin_attr,
+				  char *buffer, loff_t offset, size_t count)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
+	struct fw_priv *fw_priv;
+	ssize_t ret_count;
+
+	mutex_lock(&fw_lock);
+	fw_priv = fw_sysfs->fw_priv;
+	if (!fw_priv || fw_sysfs_done(fw_priv)) {
+		ret_count = -ENODEV;
+		goto out;
+	}
+	if (offset > fw_priv->size) {
+		ret_count = 0;
+		goto out;
+	}
+	if (count > fw_priv->size - offset)
+		count = fw_priv->size - offset;
+
+	ret_count = count;
+
+	if (fw_priv->data)
+		firmware_rw_data(fw_priv, buffer, offset, count, true);
+	else
+		firmware_rw(fw_priv, buffer, offset, count, true);
+
+out:
+	mutex_unlock(&fw_lock);
+	return ret_count;
+}
+
+static int fw_realloc_pages(struct fw_sysfs *fw_sysfs, int min_size)
+{
+	int err;
+
+	err = fw_grow_paged_buf(fw_sysfs->fw_priv,
+				PAGE_ALIGN(min_size) >> PAGE_SHIFT);
+	if (err)
+		fw_load_abort(fw_sysfs);
+	return err;
+}
+
+/**
+ * firmware_data_write() - write method for firmware
+ * @filp: open sysfs file
+ * @kobj: kobject for the device
+ * @bin_attr: bin_attr structure
+ * @buffer: buffer being written
+ * @offset: buffer offset for write in total data store area
+ * @count: buffer size
+ *
+ *	Data written to the 'data' attribute will be later handed to
+ *	the driver as a firmware image.
+ **/
+static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj,
+				   struct bin_attribute *bin_attr,
+				   char *buffer, loff_t offset, size_t count)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
+	struct fw_priv *fw_priv;
+	ssize_t retval;
+
+	if (!capable(CAP_SYS_RAWIO))
+		return -EPERM;
+
+	mutex_lock(&fw_lock);
+	fw_priv = fw_sysfs->fw_priv;
+	if (!fw_priv || fw_sysfs_done(fw_priv)) {
+		retval = -ENODEV;
+		goto out;
+	}
+
+	if (fw_priv->data) {
+		if (offset + count > fw_priv->allocated_size) {
+			retval = -ENOMEM;
+			goto out;
+		}
+		firmware_rw_data(fw_priv, buffer, offset, count, false);
+		retval = count;
+	} else {
+		retval = fw_realloc_pages(fw_sysfs, offset + count);
+		if (retval)
+			goto out;
+
+		retval = count;
+		firmware_rw(fw_priv, buffer, offset, count, false);
+	}
+
+	fw_priv->size = max_t(size_t, offset + count, fw_priv->size);
+out:
+	mutex_unlock(&fw_lock);
+	return retval;
+}
+
+static struct bin_attribute firmware_attr_data = {
+	.attr = { .name = "data", .mode = 0644 },
+	.size = 0,
+	.read = firmware_data_read,
+	.write = firmware_data_write,
+};
+
+static struct attribute *fw_dev_attrs[] = {
+	&dev_attr_loading.attr,
+	NULL
+};
+
+static struct bin_attribute *fw_dev_bin_attrs[] = {
+	&firmware_attr_data,
+	NULL
+};
+
+static const struct attribute_group fw_dev_attr_group = {
+	.attrs = fw_dev_attrs,
+	.bin_attrs = fw_dev_bin_attrs,
+};
+
+static const struct attribute_group *fw_dev_attr_groups[] = {
+	&fw_dev_attr_group,
+	NULL
+};
+
+struct fw_sysfs *
+fw_create_instance(struct firmware *firmware, const char *fw_name,
+		   struct device *device, u32 opt_flags)
+{
+	struct fw_sysfs *fw_sysfs;
+	struct device *f_dev;
+
+	fw_sysfs = kzalloc(sizeof(*fw_sysfs), GFP_KERNEL);
+	if (!fw_sysfs) {
+		fw_sysfs = ERR_PTR(-ENOMEM);
+		goto exit;
+	}
+
+	fw_sysfs->nowait = !!(opt_flags & FW_OPT_NOWAIT);
+	fw_sysfs->fw = firmware;
+	f_dev = &fw_sysfs->dev;
+
+	device_initialize(f_dev);
+	dev_set_name(f_dev, "%s", fw_name);
+	f_dev->parent = device;
+	f_dev->class = &firmware_class;
+	f_dev->groups = fw_dev_attr_groups;
+exit:
+	return fw_sysfs;
+}
diff --git a/drivers/base/firmware_loader/sysfs.h b/drivers/base/firmware_loader/sysfs.h
new file mode 100644
index 000000000000..5e2aff7bf6e7
--- /dev/null
+++ b/drivers/base/firmware_loader/sysfs.h
@@ -0,0 +1,96 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __FIRMWARE_SYSFS_H
+#define __FIRMWARE_SYSFS_H
+
+#include <linux/device.h>
+
+MODULE_IMPORT_NS(FIRMWARE_LOADER_PRIVATE);
+
+extern struct firmware_fallback_config fw_fallback_config;
+
+#ifdef CONFIG_FW_LOADER_USER_HELPER
+/**
+ * struct firmware_fallback_config - firmware fallback configuration settings
+ *
+ * Helps describe and fine tune the fallback mechanism.
+ *
+ * @force_sysfs_fallback: force the sysfs fallback mechanism to be used
+ *	as if one had enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.
+ *	Useful to help debug a CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y
+ *	functionality on a kernel where that config entry has been disabled.
+ * @ignore_sysfs_fallback: force to disable the sysfs fallback mechanism.
+ *	This emulates the behaviour as if we had set the kernel
+ *	config CONFIG_FW_LOADER_USER_HELPER=n.
+ * @old_timeout: for internal use
+ * @loading_timeout: the timeout to wait for the fallback mechanism before
+ *	giving up, in seconds.
+ */
+struct firmware_fallback_config {
+	unsigned int force_sysfs_fallback;
+	unsigned int ignore_sysfs_fallback;
+	int old_timeout;
+	int loading_timeout;
+};
+
+int register_sysfs_loader(void);
+void unregister_sysfs_loader(void);
+#ifdef CONFIG_SYSCTL
+int register_firmware_config_sysctl(void);
+void unregister_firmware_config_sysctl(void);
+#else
+static inline int register_firmware_config_sysctl(void)
+{
+	return 0;
+}
+
+static inline void unregister_firmware_config_sysctl(void) { }
+#endif /* CONFIG_SYSCTL */
+#else /* CONFIG_FW_LOADER_USER_HELPER */
+static inline int register_sysfs_loader(void)
+{
+	return 0;
+}
+
+static inline void unregister_sysfs_loader(void)
+{
+}
+#endif /* CONFIG_FW_LOADER_USER_HELPER */
+
+struct fw_sysfs {
+	bool nowait;
+	struct device dev;
+	struct fw_priv *fw_priv;
+	struct firmware *fw;
+};
+
+static inline struct fw_sysfs *to_fw_sysfs(struct device *dev)
+{
+	return container_of(dev, struct fw_sysfs, dev);
+}
+
+/* These getters are vetted to use int properly */
+static inline int __firmware_loading_timeout(void)
+{
+	return fw_fallback_config.loading_timeout;
+}
+
+/* These setters are vetted to use int properly */
+static inline void __fw_fallback_set_timeout(int timeout)
+{
+	fw_fallback_config.loading_timeout = timeout;
+}
+
+void __fw_load_abort(struct fw_priv *fw_priv);
+
+static inline void fw_load_abort(struct fw_sysfs *fw_sysfs)
+{
+	struct fw_priv *fw_priv = fw_sysfs->fw_priv;
+
+	__fw_load_abort(fw_priv);
+}
+
+struct fw_sysfs *
+fw_create_instance(struct firmware *firmware, const char *fw_name,
+		   struct device *device, u32 opt_flags);
+
+#endif /* __FIRMWARE_SYSFS_H */
-- 
2.25.1

[RESEND PATCH v1 4/8] firmware_loader: Add firmware-upload support

[Russ Weight]

Extend the firmware subsystem to support a persistent sysfs interface that
userspace may use to initiate a firmware update. For example, FPGA based
PCIe cards load firmware and FPGA images from local FLASH when the card
boots. The images in FLASH may be updated with new images provided by the
user at his/her convenience.

A device driver may call firmware_upload_register() to expose persistent
"loading" and "data" sysfs files. These files are used in the same way as
the fallback sysfs "loading" and "data" files. When 0 is written to
"loading" to complete the write of firmware data, the data is transferred
to the lower-level driver using pre-registered call-back functions. The
data transfer is done in the context of a kernel worker thread.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
v1:
  - renamed fw_upload_register() and fw_upload_unregister() to
    firmware_upload_register() and fw_upload_unregister().
  - Moved ifdef'd section of code out of firmware_loading_store() in sysfs.c
    into a new function, fw_upload_start(), in sysfs_upload.c.
  - Changed #defines to enums for error codes and progress states
  - Added additional kernel-doc supported symbols into the documentation.
    Some rewording in documentation as well.
  - Added module reference counting for the parent module in the
    firmware_upload_register() and firmware_upload_unregister() functions
    to fix problems found when testing with test_firmware module.
  - Removed unnecessary module reference counting for THIS_MODULE. This
    module holds a reference count for the parent module, and the parent
    module has symbol dependencies on this module.
   
---
 .../ABI/testing/sysfs-class-firmware          |  32 ++
 .../driver-api/firmware/fw_upload.rst         | 100 +++++++
 Documentation/driver-api/firmware/index.rst   |   1 +
 drivers/base/firmware_loader/Kconfig          |  14 +
 drivers/base/firmware_loader/Makefile         |   1 +
 drivers/base/firmware_loader/firmware.h       |   6 +
 drivers/base/firmware_loader/main.c           |  16 +-
 drivers/base/firmware_loader/sysfs.c          |  19 +-
 drivers/base/firmware_loader/sysfs.h          |   4 +
 drivers/base/firmware_loader/sysfs_upload.c   | 275 ++++++++++++++++++
 drivers/base/firmware_loader/sysfs_upload.h   |  42 +++
 include/linux/firmware.h                      |  82 ++++++
 12 files changed, 580 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-firmware
 create mode 100644 Documentation/driver-api/firmware/fw_upload.rst
 create mode 100644 drivers/base/firmware_loader/sysfs_upload.c
 create mode 100644 drivers/base/firmware_loader/sysfs_upload.h

diff --git a/Documentation/ABI/testing/sysfs-class-firmware b/Documentation/ABI/testing/sysfs-class-firmware
new file mode 100644
index 000000000000..a2e518f0bf8a
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-firmware
@@ -0,0 +1,32 @@
+What: 		/sys/class/firmware/.../data
+Date:		Mar 2022
+KernelVersion:	5.18
+Contact:	Russ Weight <russell.h.weight@intel.com>
+Description:	The data sysfs file is used for firmware-fallback and for
+		firmware uploads. Cat a firmware image to this sysfs file
+		after you echo 1 to the loading sysfs file. When the firmware
+		image write is complete, echo 0 to the loading sysfs file. This
+		sequence will signal the completion of the firmware write and
+		signal the lower-level driver that the firmware data is
+		available.
+
+What: 		/sys/class/firmware/.../loading
+Date:		Mar 2022
+KernelVersion:	5.18
+Contact:	Russ Weight <russell.h.weight@intel.com>
+Description:	The loading sysfs file is used for both firmware-fallback and
+		for firmware uploads. Echo 1 onto the loading file to indicate
+		you are writing a firmware file to the data sysfs node. Echo
+		-1 onto this file to abort the data write or echo 0 onto this
+		file to indicate that the write is complete. For firmware
+		uploads, the zero value also triggers the transfer of the
+		firmware data to the lower-level device driver.
+
+What: 		/sys/class/firmware/.../timeout
+Date:		Mar 2022
+KernelVersion:	5.18
+Contact:	Russ Weight <russell.h.weight@intel.com>
+Description:	This file supports the timeout mechanism for firmware
+		fallback.  This file has no affect on firmware uploads. For
+		more information on timeouts please see the documentation
+		for firmware fallback.
diff --git a/Documentation/driver-api/firmware/fw_upload.rst b/Documentation/driver-api/firmware/fw_upload.rst
new file mode 100644
index 000000000000..2aaac9c70e41
--- /dev/null
+++ b/Documentation/driver-api/firmware/fw_upload.rst
@@ -0,0 +1,100 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===================
+Firmware Upload API
+===================
+
+A device driver that registers with the firmware loader will expose
+persistent sysfs nodes to enable users to initiate firmware updates for
+that device.  It is the responsibility of the device driver and/or the
+device itself to perform any validation on the data received. Firmware
+upload uses the same *loading* and *data* sysfs files described in the
+documentation for firmware fallback.
+
+Register for firmware upload
+============================
+
+A device driver registers for firmware upload by calling
+firmware_upload_register(). Among the parameter list is a name to
+identify the device under /sys/class/firmware. A user may initiate a
+firmware upload by echoing a 1 to the *loading* sysfs file for the target
+device. Next, the user writes the firmware image to the *data* sysfs
+file. After writing the firmware data, the user echos 0 to the *loading*
+sysfs file to signal completion. Echoing 0 to *loading* also triggers the
+transfer of the firmware to the lower-lever device driver in the context
+of a kernel worker thread.
+
+To use the firmware upload API, write a driver that implements a set of
+ops.  The probe function calls firmware_upload_register() and the remove
+function calls firmware_upload_unregister() such as::
+
+	static const struct fw_upload_ops m10bmc_ops = {
+		.prepare = m10bmc_sec_prepare,
+		.write = m10bmc_sec_write,
+		.poll_complete = m10bmc_sec_poll_complete,
+		.cancel = m10bmc_sec_cancel,
+		.cleanup = m10bmc_sec_cleanup,
+	};
+
+	static int m10bmc_sec_probe(struct platform_device *pdev)
+	{
+		const char *fw_name, *truncate;
+		struct m10bmc_sec *sec;
+		struct fw_upload *fwl;
+		unsigned int len;
+
+		sec = devm_kzalloc(&pdev->dev, sizeof(*sec), GFP_KERNEL);
+		if (!sec)
+			return -ENOMEM;
+
+		sec->dev = &pdev->dev;
+		sec->m10bmc = dev_get_drvdata(pdev->dev.parent);
+		dev_set_drvdata(&pdev->dev, sec);
+
+		fw_name = dev_name(sec->dev);
+		truncate = strstr(fw_name, ".auto");
+		len = (truncate) ? truncate - fw_name : strlen(fw_name);
+		sec->fw_name = kmemdup_nul(fw_name, len, GFP_KERNEL);
+
+		fwl = firmware_upload_register(sec->dev, sec->fw_name, &m10bmc_ops, sec);
+		if (IS_ERR(fwl)) {
+			dev_err(sec->dev, "Firmware Upload driver failed to start\n");
+			kfree(sec->fw_name);
+			return PTR_ERR(fwl);
+		}
+
+		sec->fwl = fwl;
+		return 0;
+	}
+
+	static int m10bmc_sec_remove(struct platform_device *pdev)
+	{
+		struct m10bmc_sec *sec = dev_get_drvdata(&pdev->dev);
+
+		firmware_upload_unregister(sec->fwl);
+		kfree(sec->fw_name);
+		return 0;
+	}
+
+firmware_upload_register
+------------------------
+.. kernel-doc:: drivers/base/firmware_loader/sysfs_upload.c
+   :identifiers: firmware_upload_register
+
+firmware_upload_unregister
+--------------------------
+.. kernel-doc:: drivers/base/firmware_loader/sysfs_upload.c
+   :identifiers: firmware_upload_unregister
+
+Firmware Upload Ops
+-------------------
+.. kernel-doc:: include/linux/firmware.h
+   :identifiers: fw_upload_ops
+
+Firmware Upload Error Codes
+---------------------------
+The following error codes may be returned by the driver ops in case of
+failure:
+
+.. kernel-doc:: include/linux/firmware.h
+   :identifiers: fw_upload_err
diff --git a/Documentation/driver-api/firmware/index.rst b/Documentation/driver-api/firmware/index.rst
index 57415d657173..9d2c19dc8e36 100644
--- a/Documentation/driver-api/firmware/index.rst
+++ b/Documentation/driver-api/firmware/index.rst
@@ -8,6 +8,7 @@ Linux Firmware API
    core
    efi/index
    request_firmware
+   fw_upload
    other_interfaces
 
 .. only::  subproject and html
diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
index 9e03178eee00..adf2b182d74d 100644
--- a/drivers/base/firmware_loader/Kconfig
+++ b/drivers/base/firmware_loader/Kconfig
@@ -190,5 +190,19 @@ config FW_CACHE
 
 	  If unsure, say Y.
 
+config FW_UPLOAD
+	bool "Enable users to initiate firmware updates using sysfs"
+	select FW_LOADER_SYSFS
+	select FW_LOADER_PAGED_BUF
+	help
+	  Enabling this option will allow device drivers to expose a persistent
+	  sysfs interface that allows firmware updates to be initiated from
+	  userspace. For example, FPGA based PCIe cards load firmware and FPGA
+	  images from local FLASH when the card boots. The images in FLASH may
+	  be updated with new images provided by the user. Enable this device
+	  to support cards that rely on user-initiated updates for firmware files.
+
+	  If unsure, say N.
+
 endif # FW_LOADER
 endmenu
diff --git a/drivers/base/firmware_loader/Makefile b/drivers/base/firmware_loader/Makefile
index aab213f82288..60d19f9e0ddc 100644
--- a/drivers/base/firmware_loader/Makefile
+++ b/drivers/base/firmware_loader/Makefile
@@ -7,5 +7,6 @@ firmware_class-objs := main.o
 firmware_class-$(CONFIG_FW_LOADER_USER_HELPER) += fallback.o
 firmware_class-$(CONFIG_EFI_EMBEDDED_FIRMWARE) += fallback_platform.o
 firmware_class-$(CONFIG_FW_LOADER_SYSFS) += sysfs.o
+firmware_class-$(CONFIG_FW_UPLOAD) += sysfs_upload.o
 
 obj-y += builtin/
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 58768d16f8df..4019f9423de8 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -87,6 +87,7 @@ struct fw_priv {
 };
 
 extern struct mutex fw_lock;
+extern struct firmware_cache fw_cache;
 
 static inline bool __fw_state_check(struct fw_priv *fw_priv,
 				    enum fw_status status)
@@ -154,7 +155,12 @@ static inline bool fw_state_is_done(struct fw_priv *fw_priv)
 	return __fw_state_check(fw_priv, FW_STATUS_DONE);
 }
 
+int alloc_lookup_fw_priv(const char *fw_name, struct firmware_cache *fwc,
+			 struct fw_priv **fw_priv, void *dbuf, size_t size,
+			 size_t offset, u32 opt_flags);
 int assign_fw(struct firmware *fw, struct device *device);
+void free_fw_priv(struct fw_priv *fw_priv);
+void fw_state_init(struct fw_priv *fw_priv);
 
 #ifdef CONFIG_FW_LOADER
 bool firmware_is_builtin(const struct firmware *fw);
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 2cc11d93753a..874a5ef31c56 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -91,9 +91,9 @@ static inline struct fw_priv *to_fw_priv(struct kref *ref)
  * guarding for corner cases a global lock should be OK */
 DEFINE_MUTEX(fw_lock);
 
-static struct firmware_cache fw_cache;
+struct firmware_cache fw_cache;
 
-static void fw_state_init(struct fw_priv *fw_priv)
+void fw_state_init(struct fw_priv *fw_priv)
 {
 	struct fw_state *fw_st = &fw_priv->fw_st;
 
@@ -163,13 +163,9 @@ static struct fw_priv *__lookup_fw_priv(const char *fw_name)
 }
 
 /* Returns 1 for batching firmware requests with the same name */
-static int alloc_lookup_fw_priv(const char *fw_name,
-				struct firmware_cache *fwc,
-				struct fw_priv **fw_priv,
-				void *dbuf,
-				size_t size,
-				size_t offset,
-				u32 opt_flags)
+int alloc_lookup_fw_priv(const char *fw_name, struct firmware_cache *fwc,
+			 struct fw_priv **fw_priv, void *dbuf, size_t size,
+			 size_t offset, u32 opt_flags)
 {
 	struct fw_priv *tmp;
 
@@ -224,7 +220,7 @@ static void __free_fw_priv(struct kref *ref)
 	kfree(fw_priv);
 }
 
-static void free_fw_priv(struct fw_priv *fw_priv)
+void free_fw_priv(struct fw_priv *fw_priv)
 {
 	struct firmware_cache *fwc = fw_priv->fwc;
 	spin_lock(&fwc->lock);
diff --git a/drivers/base/firmware_loader/sysfs.c b/drivers/base/firmware_loader/sysfs.c
index 49aeff45b123..4beb0685d90a 100644
--- a/drivers/base/firmware_loader/sysfs.c
+++ b/drivers/base/firmware_loader/sysfs.c
@@ -6,8 +6,8 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
-#include "firmware.h"
 #include "sysfs.h"
+#include "sysfs_upload.h"
 
 /*
  * sysfs support for firmware loader
@@ -78,6 +78,10 @@ static void fw_dev_release(struct device *dev)
 {
 	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
 
+	if (fw_sysfs->fw_upload_priv) {
+		free_fw_priv(fw_sysfs->fw_priv);
+		kfree(fw_sysfs->fw_upload_priv);
+	}
 	kfree(fw_sysfs);
 }
 
@@ -209,6 +213,14 @@ static ssize_t firmware_loading_store(struct device *dev,
 				written = rc;
 			} else {
 				fw_state_done(fw_priv);
+
+				/*
+				 * If this is a user-initiated firmware upload
+				 * then start the upload in a worker thread now.
+				 */
+				rc = fw_upload_start(fw_sysfs);
+				if (rc)
+					written = rc;
 			}
 			break;
 		}
@@ -218,6 +230,9 @@ static ssize_t firmware_loading_store(struct device *dev,
 		fallthrough;
 	case -1:
 		fw_load_abort(fw_sysfs);
+		if (fw_sysfs->fw_upload_priv)
+			fw_state_init(fw_sysfs->fw_priv);
+
 		break;
 	}
 out:
@@ -225,7 +240,7 @@ static ssize_t firmware_loading_store(struct device *dev,
 	return written;
 }
 
-static DEVICE_ATTR(loading, 0644, firmware_loading_show, firmware_loading_store);
+DEVICE_ATTR(loading, 0644, firmware_loading_show, firmware_loading_store);
 
 static void firmware_rw_data(struct fw_priv *fw_priv, char *buffer,
 			     loff_t offset, size_t count, bool read)
diff --git a/drivers/base/firmware_loader/sysfs.h b/drivers/base/firmware_loader/sysfs.h
index 5e2aff7bf6e7..01aeb3f692cb 100644
--- a/drivers/base/firmware_loader/sysfs.h
+++ b/drivers/base/firmware_loader/sysfs.h
@@ -4,9 +4,12 @@
 
 #include <linux/device.h>
 
+#include "firmware.h"
+
 MODULE_IMPORT_NS(FIRMWARE_LOADER_PRIVATE);
 
 extern struct firmware_fallback_config fw_fallback_config;
+extern struct device_attribute dev_attr_loading;
 
 #ifdef CONFIG_FW_LOADER_USER_HELPER
 /**
@@ -61,6 +64,7 @@ struct fw_sysfs {
 	struct device dev;
 	struct fw_priv *fw_priv;
 	struct firmware *fw;
+	void *fw_upload_priv;
 };
 
 static inline struct fw_sysfs *to_fw_sysfs(struct device *dev)
diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c
new file mode 100644
index 000000000000..4c24f50d57e0
--- /dev/null
+++ b/drivers/base/firmware_loader/sysfs_upload.c
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/firmware.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#include "sysfs.h"
+#include "sysfs_upload.h"
+
+/*
+ * Support for user-space to initiate a firmware upload to a device.
+ */
+
+static void fw_upload_update_progress(struct fw_upload_priv *fwlp,
+				      enum fw_upload_prog new_progress)
+{
+	mutex_lock(&fwlp->lock);
+	fwlp->progress = new_progress;
+	mutex_unlock(&fwlp->lock);
+}
+
+static void fw_upload_set_error(struct fw_upload_priv *fwlp,
+				enum fw_upload_err err_code)
+{
+	mutex_lock(&fwlp->lock);
+	fwlp->err_progress = fwlp->progress;
+	fwlp->err_code = err_code;
+	mutex_unlock(&fwlp->lock);
+}
+
+static void fw_upload_prog_complete(struct fw_upload_priv *fwlp)
+{
+	mutex_lock(&fwlp->lock);
+	fwlp->progress = FW_UPLOAD_PROG_IDLE;
+	mutex_unlock(&fwlp->lock);
+}
+
+static void fw_upload_main(struct work_struct *work)
+{
+	struct fw_upload_priv *fwlp;
+	struct fw_sysfs *fw_sysfs;
+	u32 written = 0, offset = 0;
+	enum fw_upload_err ret;
+	struct device *fw_dev;
+	struct fw_upload *fwl;
+
+	fwlp = container_of(work, struct fw_upload_priv, work);
+	fwl = fwlp->fw_upload;
+	fw_sysfs = (struct fw_sysfs *)fwl->priv;
+	fw_dev = &fw_sysfs->dev;
+
+	fw_upload_update_progress(fwlp, FW_UPLOAD_PROG_PREPARING);
+	ret = fwlp->ops->prepare(fwl, fwlp->data, fwlp->remaining_size);
+	if (ret != FW_UPLOAD_ERR_NONE) {
+		fw_upload_set_error(fwlp, ret);
+		goto putdev_exit;
+	}
+
+	fw_upload_update_progress(fwlp, FW_UPLOAD_PROG_TRANSFERRING);
+	while (fwlp->remaining_size) {
+		ret = fwlp->ops->write(fwl, fwlp->data, offset,
+					fwlp->remaining_size, &written);
+		if (ret != FW_UPLOAD_ERR_NONE || !written) {
+			if (ret == FW_UPLOAD_ERR_NONE) {
+				dev_warn(fw_dev, "write-op wrote zero data\n");
+				ret = FW_UPLOAD_ERR_RW_ERROR;
+			}
+			fw_upload_set_error(fwlp, ret);
+			goto done;
+		}
+
+		fwlp->remaining_size -= written;
+		offset += written;
+	}
+
+	fw_upload_update_progress(fwlp, FW_UPLOAD_PROG_PROGRAMMING);
+	ret = fwlp->ops->poll_complete(fwl);
+	if (ret != FW_UPLOAD_ERR_NONE)
+		fw_upload_set_error(fwlp, ret);
+
+done:
+	if (fwlp->ops->cleanup)
+		fwlp->ops->cleanup(fwl);
+
+putdev_exit:
+	put_device(fw_dev->parent);
+
+	/*
+	 * Note: fwlp->remaining_size is left unmodified here to provide
+	 * additional information on errors. It will be reinitialized when
+	 * the next firmeware upload begins.
+	 */
+	mutex_lock(&fw_lock);
+	fw_free_paged_buf(fw_sysfs->fw_priv);
+	fw_state_init(fw_sysfs->fw_priv);
+	mutex_unlock(&fw_lock);
+	fwlp->data = NULL;
+	fw_upload_prog_complete(fwlp);
+}
+
+/*
+ * Start a worker thread to upload data to the parent driver.
+ * Must be called with fw_lock held.
+ */
+int fw_upload_start(struct fw_sysfs *fw_sysfs)
+{
+	struct fw_priv *fw_priv = fw_sysfs->fw_priv;
+	struct device *fw_dev = &fw_sysfs->dev;
+	struct fw_upload_priv *fwlp;
+
+	if (!fw_sysfs->fw_upload_priv)
+		return 0;
+
+	if (!fw_priv->size) {
+		fw_free_paged_buf(fw_priv);
+		fw_state_init(fw_sysfs->fw_priv);
+		return 0;
+	}
+
+	fwlp = fw_sysfs->fw_upload_priv;
+	mutex_lock(&fwlp->lock);
+
+	/* Do not interfere with an on-going fw_upload */
+	if (fwlp->progress != FW_UPLOAD_PROG_IDLE) {
+		mutex_unlock(&fwlp->lock);
+		return -EBUSY;
+	}
+
+	get_device(fw_dev->parent); /* released in fw_upload_main */
+
+	fwlp->progress = FW_UPLOAD_PROG_RECEIVING;
+	fwlp->err_code = 0;
+	fwlp->remaining_size = fw_priv->size;
+	fwlp->data = fw_priv->data;
+
+	pr_debug("%s: fw-%s fw_priv=%p data=%p size=%u\n",
+		 __func__, fw_priv->fw_name,
+		 fw_priv, fw_priv->data,
+		 (unsigned int)fw_priv->size);
+
+	queue_work(system_long_wq, &fwlp->work);
+	mutex_unlock(&fwlp->lock);
+
+	return 0;
+}
+
+/**
+ * firmware_upload_register() - register for the firmware upload sysfs API
+ * @parent: parent device instantiating firmware upload
+ * @name: firmware name to be associated with this device
+ * @ops: pointer to structure of firmware upload ops
+ * @dd_handle: pointer to parent driver private data
+ *
+ *	@name must be unique among all users of firmware upload. The firmware
+ *	sysfs files for this device will be found at /sys/class/firmware/@name.
+ *
+ *	Return: struct fw_upload pointer or ERR_PTR()
+ *
+ **/
+struct fw_upload *
+firmware_upload_register(struct module *module, struct device *parent,
+			 const char *name, const struct fw_upload_ops *ops,
+			 void *dd_handle)
+{
+	u32 opt_flags = FW_OPT_NOCACHE;
+	struct fw_upload *fw_upload;
+	struct fw_upload_priv *fw_upload_priv;
+	struct fw_sysfs *fw_sysfs;
+	struct fw_priv *fw_priv;
+	struct device *fw_dev;
+	int ret;
+
+	if (!name || name[0] == '\0')
+		return ERR_PTR(-EINVAL);
+
+	if (!ops || !ops->cancel || !ops->prepare ||
+	    !ops->write || !ops->poll_complete) {
+		dev_err(parent, "Attempt to register without all required ops\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (!try_module_get(module))
+		return ERR_PTR(-EFAULT);
+
+	fw_upload = kzalloc(sizeof(*fw_upload), GFP_KERNEL);
+	if (!fw_upload) {
+		ret = -ENOMEM;
+		goto exit_module_put;
+	}
+
+	fw_upload_priv = kzalloc(sizeof(*fw_upload_priv), GFP_KERNEL);
+	if (!fw_upload_priv) {
+		ret = -ENOMEM;
+		goto free_fw_upload;
+	}
+
+	fw_upload_priv->fw_upload = fw_upload;
+	fw_upload_priv->ops = ops;
+	mutex_init(&fw_upload_priv->lock);
+	fw_upload_priv->module = module;
+	fw_upload_priv->name = name;
+	fw_upload_priv->err_code = 0;
+	fw_upload_priv->progress = FW_UPLOAD_PROG_IDLE;
+	INIT_WORK(&fw_upload_priv->work, fw_upload_main);
+	fw_upload->dd_handle = dd_handle;
+
+	fw_sysfs = fw_create_instance(NULL, name, parent, opt_flags);
+	if (IS_ERR(fw_sysfs)) {
+		ret = PTR_ERR(fw_sysfs);
+		goto free_fw_upload_priv;
+	}
+	fw_upload->priv = fw_sysfs;
+	fw_sysfs->fw_upload_priv = fw_upload_priv;
+	fw_dev = &fw_sysfs->dev;
+
+	ret = alloc_lookup_fw_priv(name, &fw_cache, &fw_priv,  NULL, 0, 0,
+				   FW_OPT_NOCACHE);
+	if (ret != 0) {
+		if (ret > 0)
+			ret = -EINVAL;
+		goto free_fw_sysfs;
+	}
+	fw_sysfs->fw_priv = fw_priv;
+
+	ret = device_add(fw_dev);
+	if (ret) {
+		dev_err(fw_dev, "%s: device_register failed\n", __func__);
+		put_device(fw_dev);
+		goto exit_module_put;
+	}
+
+	return fw_upload;
+
+free_fw_sysfs:
+	kfree(fw_sysfs);
+
+free_fw_upload_priv:
+	kfree(fw_upload_priv);
+
+free_fw_upload:
+	kfree(fw_upload);
+
+exit_module_put:
+	module_put(module);
+
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(firmware_upload_register);
+
+/**
+ * firmware_upload_unregister() - Unregister firmware upload interface
+ * @fw_upload: pointer to struct fw_upload
+ **/
+void firmware_upload_unregister(struct fw_upload *fw_upload)
+{
+	struct fw_sysfs *fw_sysfs = fw_upload->priv;
+	struct fw_upload_priv *fw_upload_priv = fw_sysfs->fw_upload_priv;
+
+	mutex_lock(&fw_upload_priv->lock);
+	if (fw_upload_priv->progress == FW_UPLOAD_PROG_IDLE) {
+		mutex_unlock(&fw_upload_priv->lock);
+		goto unregister;
+	}
+
+	fw_upload_priv->ops->cancel(fw_upload);
+	mutex_unlock(&fw_upload_priv->lock);
+
+	/* Ensure lower-level device-driver is finished */
+	flush_work(&fw_upload_priv->work);
+
+unregister:
+	device_unregister(&fw_sysfs->dev);
+	module_put(fw_upload_priv->module);
+}
+EXPORT_SYMBOL_GPL(firmware_upload_unregister);
diff --git a/drivers/base/firmware_loader/sysfs_upload.h b/drivers/base/firmware_loader/sysfs_upload.h
new file mode 100644
index 000000000000..ca923265c62c
--- /dev/null
+++ b/drivers/base/firmware_loader/sysfs_upload.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __FIRMWARE_UPLOAD_H
+#define __FIRMWARE_UPLOAD_H
+
+#include <linux/device.h>
+
+/* Update progress codes */
+enum fw_upload_prog {
+	FW_UPLOAD_PROG_IDLE,
+	FW_UPLOAD_PROG_RECEIVING,
+	FW_UPLOAD_PROG_PREPARING,
+	FW_UPLOAD_PROG_TRANSFERRING,
+	FW_UPLOAD_PROG_PROGRAMMING,
+	FW_UPLOAD_PROG_MAX
+};
+
+struct fw_upload_priv {
+	struct fw_upload *fw_upload;
+	struct module *module;
+	const char *name;
+	const struct fw_upload_ops *ops;
+	struct mutex lock;		  /* protect data structure contents */
+	struct work_struct work;
+	const u8 *data;			  /* pointer to update data */
+	u32 remaining_size;		  /* size remaining to transfer */
+	enum fw_upload_prog progress;
+	enum fw_upload_prog err_progress; /* progress at time of failure */
+	enum fw_upload_err err_code;	  /* security manager error code */
+};
+
+#ifdef CONFIG_FW_UPLOAD
+int fw_upload_start(struct fw_sysfs *fw_sysfs);
+umode_t fw_upload_is_visible(struct kobject *kobj, struct attribute *attr, int n);
+#else
+static inline int fw_upload_start(struct fw_sysfs *fw_sysfs)
+{
+	return 0;
+}
+#endif
+
+#endif /* __FIRMWARE_UPLOAD_H */
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index ec2ccfebef65..de7fea3bca51 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -17,6 +17,64 @@ struct firmware {
 	void *priv;
 };
 
+/**
+ * enum fw_upload_err - firmware upload error codes
+ * @FW_UPLOAD_ERR_NONE: returned to indicate success
+ * @FW_UPLOAD_ERR_HW_ERROR: error signalled by hardware, see kernel log
+ * @FW_UPLOAD_ERR_TIMEOUT: SW timed out on handshake with HW/firmware
+ * @FW_UPLOAD_ERR_CANCELED: upload was cancelled by the user
+ * @FW_UPLOAD_ERR_BUSY: there is an upload operation already in progress
+ * @FW_UPLOAD_ERR_INVALID_SIZE: invalid firmware image size
+ * @FW_UPLOAD_ERR_RW_ERROR: read or write to HW failed, see kernel log
+ * @FW_UPLOAD_ERR_WEAROUT: FLASH device is approaching wear-out, wait & retry
+ * @FW_UPLOAD_ERR_MAX: Maximum error code marker
+ */
+enum fw_upload_err {
+	FW_UPLOAD_ERR_NONE,
+	FW_UPLOAD_ERR_HW_ERROR,
+	FW_UPLOAD_ERR_TIMEOUT,
+	FW_UPLOAD_ERR_CANCELED,
+	FW_UPLOAD_ERR_BUSY,
+	FW_UPLOAD_ERR_INVALID_SIZE,
+	FW_UPLOAD_ERR_RW_ERROR,
+	FW_UPLOAD_ERR_WEAROUT,
+	FW_UPLOAD_ERR_MAX
+};
+
+struct fw_upload {
+	void *dd_handle; /* reference to parent driver */
+	void *priv;	 /* firmware loader private fields */
+};
+
+/**
+ * struct fw_upload_ops - device specific operations to support firmware upload
+ * @prepare:		  Required: Prepare secure update
+ * @write:		  Required: The write() op receives the remaining
+ *			  size to be written and must return the actual
+ *			  size written or a negative error code. The write()
+ *			  op will be called repeatedly until all data is
+ *			  written.
+ * @poll_complete:	  Required: Check for the completion of the
+ *			  HW authentication/programming process.
+ * @cancel:		  Required: Request cancellation of update. This op
+ *			  is called from the context of a different kernel
+ *			  thread, so race conditions need to be considered.
+ * @cleanup:		  Optional: Complements the prepare()
+ *			  function and is called at the completion
+ *			  of the update, on success or failure, if the
+ *			  prepare function succeeded.
+ */
+struct fw_upload_ops {
+	enum fw_upload_err (*prepare)(struct fw_upload *fw_upload,
+				      const u8 *data, u32 size);
+	enum fw_upload_err (*write)(struct fw_upload *fw_upload,
+				    const u8 *data, u32 offset,
+				    u32 size, u32 *written);
+	enum fw_upload_err (*poll_complete)(struct fw_upload *fw_upload);
+	void (*cancel)(struct fw_upload *fw_upload);
+	void (*cleanup)(struct fw_upload *fw_upload);
+};
+
 struct module;
 struct device;
 
@@ -112,6 +170,30 @@ static inline int request_partial_firmware_into_buf
 
 #endif
 
+#ifdef CONFIG_FW_UPLOAD
+
+struct fw_upload *
+firmware_upload_register(struct module *module, struct device *parent,
+			 const char *name, const struct fw_upload_ops *ops,
+			 void *dd_handle);
+void firmware_upload_unregister(struct fw_upload *fw_upload);
+
+#else
+
+static inline struct fw_upload *
+firmware_upload_register(struct module *module, struct device *parent,
+			 const char *name, const struct fw_upload_ops *ops,
+			 void *dd_handle)
+{
+		return ERR_PTR(-EINVAL);
+}
+
+static inline void firmware_upload_unregister(struct fw_upload *fw_upload)
+{
+}
+
+#endif
+
 int firmware_request_cache(struct device *device, const char *name);
 
 #endif
-- 
2.25.1

[RESEND PATCH v1 5/8] firmware_loader: Add sysfs nodes to monitor fw_upload

[Russ Weight]

Add additional sysfs nodes to monitor the transfer of firmware upload data
to the target device:

cancel: Write 1 to cancel the data transfer
error: Display error status for a failed firmware upload
remaining_size: Display the remaining amount of data to be transferred
status: Display the progress of the firmware upload

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
v1:
  - Adapted to enums and filename changes. Otherwise no changes.
---
 .../ABI/testing/sysfs-class-firmware          |  45 +++++++
 .../driver-api/firmware/fw_upload.rst         |  19 ++-
 drivers/base/firmware_loader/sysfs.c          |   9 ++
 drivers/base/firmware_loader/sysfs_upload.c   | 121 ++++++++++++++++++
 drivers/base/firmware_loader/sysfs_upload.h   |   5 +
 5 files changed, 198 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-class-firmware b/Documentation/ABI/testing/sysfs-class-firmware
index a2e518f0bf8a..5653cb2d6e23 100644
--- a/Documentation/ABI/testing/sysfs-class-firmware
+++ b/Documentation/ABI/testing/sysfs-class-firmware
@@ -10,6 +10,30 @@ Description:	The data sysfs file is used for firmware-fallback and for
 		signal the lower-level driver that the firmware data is
 		available.
 
+What: 		/sys/class/firmware/.../cancel
+Date:		Mar 2022
+KernelVersion:	5.18
+Contact:	Russ Weight <russell.h.weight@intel.com>
+Description:	Write-only. For firmware uploads, write a "1" to this file to
+		request that the transfer of firmware data to the lower-level
+		device be canceled. This request will be rejected (EBUSY) if
+		the update cannot be canceled (e.g. a FLASH write is in
+		progress) or (ENODEV) if there is no firmware update in progress.
+
+What: 		/sys/class/firmware/.../error
+Date:		Mar 2022
+KernelVersion:	5.18
+Contact:	Russ Weight <russell.h.weight@intel.com>
+Description:	Read-only. Returns a string describing a failed firmware
+		upload. This string will be in the form of <STATUS>:<ERROR>,
+		where <STATUS> will be one of the status strings described
+		for the status sysfs file and <ERROR> will be one of the
+		following: "hw-error", "timeout", "user-abort", "device-busy",
+		"invalid-file-size", "read-write-error", "flash-wearout". The
+		error sysfs file is only meaningful when the current firmware
+		upload status is "idle". If this file is read while a firmware
+		transfer is in progress, then the read will fail with EBUSY.
+
 What: 		/sys/class/firmware/.../loading
 Date:		Mar 2022
 KernelVersion:	5.18
@@ -22,6 +46,27 @@ Description:	The loading sysfs file is used for both firmware-fallback and
 		uploads, the zero value also triggers the transfer of the
 		firmware data to the lower-level device driver.
 
+What: 		/sys/class/firmware/.../remaining_size
+Date:		Mar 2022
+KernelVersion:	5.18
+Contact:	Russ Weight <russell.h.weight@intel.com>
+Description:	Read-only. For firmware upload, this file contains the size
+		of the firmware data that remains to be transferred to the
+		lower-level device driver. The size value is initialized to
+		the full size of the firmware image that was previously
+		written to the data sysfs file. This value is periodically
+		updated during the "transferring" phase of the firmware
+		upload.
+		Format: "%u".
+
+What: 		/sys/class/firmware/.../status
+Date:		Mar 2022
+KernelVersion:	5.18
+Contact:	Russ Weight <russell.h.weight@intel.com>
+Description:	Read-only. Returns a string describing the current status of
+		a firmware upload. The string will be one of the following:
+		idle, "receiving", "preparing", "transferring", "programming".
+
 What: 		/sys/class/firmware/.../timeout
 Date:		Mar 2022
 KernelVersion:	5.18
diff --git a/Documentation/driver-api/firmware/fw_upload.rst b/Documentation/driver-api/firmware/fw_upload.rst
index 2aaac9c70e41..00c8e19d99b7 100644
--- a/Documentation/driver-api/firmware/fw_upload.rst
+++ b/Documentation/driver-api/firmware/fw_upload.rst
@@ -9,7 +9,8 @@ persistent sysfs nodes to enable users to initiate firmware updates for
 that device.  It is the responsibility of the device driver and/or the
 device itself to perform any validation on the data received. Firmware
 upload uses the same *loading* and *data* sysfs files described in the
-documentation for firmware fallback.
+documentation for firmware fallback. It also adds additional sysfs files
+to provide status on the transfer of the firmware image to the device.
 
 Register for firmware upload
 ============================
@@ -98,3 +99,19 @@ failure:
 
 .. kernel-doc:: include/linux/firmware.h
    :identifiers: fw_upload_err
+
+Sysfs Attributes
+================
+
+In addition to the *loading* and *data* sysfs files, there are additional
+sysfs files to monitor the status of the data transfer to the target
+device and to determine the final pass/fail status of the transfer.
+Depending on the device and the size of the firmware image, a firmware
+update could take milliseconds or minutes.
+
+The additional sysfs files are:
+
+* status - provides an indication of the progress of a firmware update
+* error - provides error information for a failed firmware update
+* remaining_size - tracks the data transfer portion of an update
+* cancel - echo 1 to this file to cancel the update
diff --git a/drivers/base/firmware_loader/sysfs.c b/drivers/base/firmware_loader/sysfs.c
index 4beb0685d90a..dbfb61e144be 100644
--- a/drivers/base/firmware_loader/sysfs.c
+++ b/drivers/base/firmware_loader/sysfs.c
@@ -381,6 +381,12 @@ static struct bin_attribute firmware_attr_data = {
 
 static struct attribute *fw_dev_attrs[] = {
 	&dev_attr_loading.attr,
+#ifdef CONFIG_FW_UPLOAD
+	&dev_attr_cancel.attr,
+	&dev_attr_status.attr,
+	&dev_attr_error.attr,
+	&dev_attr_remaining_size.attr,
+#endif
 	NULL
 };
 
@@ -392,6 +398,9 @@ static struct bin_attribute *fw_dev_bin_attrs[] = {
 static const struct attribute_group fw_dev_attr_group = {
 	.attrs = fw_dev_attrs,
 	.bin_attrs = fw_dev_bin_attrs,
+#ifdef CONFIG_FW_UPLOAD
+	.is_visible = fw_upload_is_visible,
+#endif
 };
 
 static const struct attribute_group *fw_dev_attr_groups[] = {
diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c
index 4c24f50d57e0..0b308fff65a9 100644
--- a/drivers/base/firmware_loader/sysfs_upload.c
+++ b/drivers/base/firmware_loader/sysfs_upload.c
@@ -11,6 +11,127 @@
  * Support for user-space to initiate a firmware upload to a device.
  */
 
+static const char * const fw_upload_prog_str[] = {
+	[FW_UPLOAD_PROG_IDLE]	      = "idle",
+	[FW_UPLOAD_PROG_RECEIVING]    = "receiving",
+	[FW_UPLOAD_PROG_PREPARING]    = "preparing",
+	[FW_UPLOAD_PROG_TRANSFERRING] = "transferring",
+	[FW_UPLOAD_PROG_PROGRAMMING]  = "programming"
+};
+
+static const char * const fw_upload_err_str[] = {
+	[FW_UPLOAD_ERR_NONE]	     = "none",
+	[FW_UPLOAD_ERR_HW_ERROR]     = "hw-error",
+	[FW_UPLOAD_ERR_TIMEOUT]	     = "timeout",
+	[FW_UPLOAD_ERR_CANCELED]     = "user-abort",
+	[FW_UPLOAD_ERR_BUSY]	     = "device-busy",
+	[FW_UPLOAD_ERR_INVALID_SIZE] = "invalid-file-size",
+	[FW_UPLOAD_ERR_RW_ERROR]     = "read-write-error",
+	[FW_UPLOAD_ERR_WEAROUT]	     = "flash-wearout",
+};
+
+static const char *fw_upload_progress(struct device *dev,
+				      enum fw_upload_prog prog)
+{
+	const char *status = "unknown-status";
+
+	if (prog < FW_UPLOAD_PROG_MAX)
+		status = fw_upload_prog_str[prog];
+	else
+		dev_err(dev, "Invalid status during secure update: %d\n", prog);
+
+	return status;
+}
+
+static const char *fw_upload_error(struct device *dev,
+				   enum fw_upload_err err_code)
+{
+	const char *error = "unknown-error";
+
+	if (err_code < FW_UPLOAD_ERR_MAX)
+		error = fw_upload_err_str[err_code];
+	else
+		dev_err(dev, "Invalid error code during secure update: %d\n",
+			err_code);
+
+	return error;
+}
+
+static ssize_t
+status_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct fw_upload_priv *fwlp = to_fw_sysfs(dev)->fw_upload_priv;
+
+	return sysfs_emit(buf, "%s\n", fw_upload_progress(dev, fwlp->progress));
+}
+DEVICE_ATTR_RO(status);
+
+static ssize_t
+error_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct fw_upload_priv *fwlp = to_fw_sysfs(dev)->fw_upload_priv;
+	int ret;
+
+	mutex_lock(&fwlp->lock);
+
+	if (fwlp->progress != FW_UPLOAD_PROG_IDLE)
+		ret = -EBUSY;
+	else if (!fwlp->err_code)
+		ret = 0;
+	else
+		ret = sysfs_emit(buf, "%s:%s\n",
+				 fw_upload_progress(dev, fwlp->err_progress),
+				 fw_upload_error(dev, fwlp->err_code));
+
+	mutex_unlock(&fwlp->lock);
+
+	return ret;
+}
+DEVICE_ATTR_RO(error);
+
+static ssize_t cancel_store(struct device *dev, struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	struct fw_upload_priv *fwlp = to_fw_sysfs(dev)->fw_upload_priv;
+	int ret = count;
+	bool cancel;
+
+	if (kstrtobool(buf, &cancel) || !cancel)
+		return -EINVAL;
+
+	mutex_lock(&fwlp->lock);
+	if (fwlp->progress == FW_UPLOAD_PROG_IDLE)
+		ret = -ENODEV;
+
+	fwlp->ops->cancel(fwlp->fw_upload);
+	mutex_unlock(&fwlp->lock);
+
+	return ret;
+}
+DEVICE_ATTR_WO(cancel);
+
+static ssize_t remaining_size_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct fw_upload_priv *fwlp = to_fw_sysfs(dev)->fw_upload_priv;
+
+	return sysfs_emit(buf, "%u\n", fwlp->remaining_size);
+}
+DEVICE_ATTR_RO(remaining_size);
+
+umode_t
+fw_upload_is_visible(struct kobject *kobj, struct attribute *attr, int n)
+{
+	static struct fw_sysfs *fw_sysfs;
+
+	fw_sysfs = to_fw_sysfs(kobj_to_dev(kobj));
+
+	if (fw_sysfs->fw_upload_priv || attr == &dev_attr_loading.attr)
+		return attr->mode;
+
+	return 0;
+}
+
 static void fw_upload_update_progress(struct fw_upload_priv *fwlp,
 				      enum fw_upload_prog new_progress)
 {
diff --git a/drivers/base/firmware_loader/sysfs_upload.h b/drivers/base/firmware_loader/sysfs_upload.h
index ca923265c62c..9afb1cfbc6f6 100644
--- a/drivers/base/firmware_loader/sysfs_upload.h
+++ b/drivers/base/firmware_loader/sysfs_upload.h
@@ -30,6 +30,11 @@ struct fw_upload_priv {
 };
 
 #ifdef CONFIG_FW_UPLOAD
+extern struct device_attribute dev_attr_status;
+extern struct device_attribute dev_attr_error;
+extern struct device_attribute dev_attr_cancel;
+extern struct device_attribute dev_attr_remaining_size;
+
 int fw_upload_start(struct fw_sysfs *fw_sysfs);
 umode_t fw_upload_is_visible(struct kobject *kobj, struct attribute *attr, int n);
 #else
-- 
2.25.1

[RESEND PATCH v1 6/8] test_firmware: Add test support for firmware upload

[Russ Weight]

Add support for testing the firmware upload driver. There are four sysfs
nodes added:

upload_register: write-only
  Write the name of the firmware device node to be created

upload_unregister: write-only
  Write the name of the firmware device node to be destroyed

config_upload_name: read/write
  Set the name to be used by upload_read

upload_read: read-only
  Read back the data associated with the firmware device node named
  in config_upload_name

You can create multiple, concurrent firmware device nodes for firmware
upload testing. Read firmware back and validate it using config_upload_name
and upload_red.

Example:
    $ cd /sys/devices/virtual/misc/test_firmware
    $ echo -n fw1 > upload_register
    $ ls fw1
    cancel  data  device  error  loading  power  remaining_size  status
    subsystem  uevent
    $ dd if=/dev/urandom of=/tmp/random-firmware.bin bs=512 count=4
    4+0 records in
    4+0 records out
    2048 bytes (2.0 kB, 2.0 KiB) copied, 0.000131959 s, 15.5 MB/s
    $ echo 1 > fw1/loading
    $ cat /tmp/random-firmware.bin > fw1/data
    $ echo 0 > fw1/loading
    $ cat fw1/status
    idle
    $ cat fw1/error
    $ echo -n fw1 > config_upload_name
    $ cmp /tmp/random-firmware.bin upload_read
    $ echo $?
    0
    $ echo -n fw1 > upload_unregister

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
 lib/test_firmware.c | 261 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 261 insertions(+)

diff --git a/lib/test_firmware.c b/lib/test_firmware.c
index 1bccd6cd5f48..d6fe00c4972f 100644
--- a/lib/test_firmware.c
+++ b/lib/test_firmware.c
@@ -31,9 +31,12 @@ MODULE_IMPORT_NS(TEST_FIRMWARE);
 #define TEST_FIRMWARE_NAME	"test-firmware.bin"
 #define TEST_FIRMWARE_NUM_REQS	4
 #define TEST_FIRMWARE_BUF_SIZE	SZ_1K
+#define TEST_UPLOAD_MAX_SIZE	SZ_2K
+#define TEST_UPLOAD_BLK_SIZE	37	/* Avoid powers of two in testing */
 
 static DEFINE_MUTEX(test_fw_mutex);
 static const struct firmware *test_firmware;
+static LIST_HEAD(test_upload_list);
 
 struct test_batched_req {
 	u8 idx;
@@ -63,6 +66,7 @@ struct test_batched_req {
  * @reqs: stores all requests information
  * @read_fw_idx: index of thread from which we want to read firmware results
  *	from through the read_fw trigger.
+ * @upload_name: firmware name to be used with upload_read sysfs node
  * @test_result: a test may use this to collect the result from the call
  *	of the request_firmware*() calls used in their tests. In order of
  *	priority we always keep first any setup error. If no setup errors were
@@ -101,6 +105,7 @@ struct test_config {
 	bool send_uevent;
 	u8 num_requests;
 	u8 read_fw_idx;
+	char *upload_name;
 
 	/*
 	 * These below don't belong her but we'll move them once we create
@@ -112,8 +117,28 @@ struct test_config {
 			    struct device *device);
 };
 
+struct test_firmware_upload {
+	char *name;
+	struct list_head node;
+	char *buf;
+	size_t size;
+	bool cancel_request;
+	struct fw_upload *fwl;
+};
+
 static struct test_config *test_fw_config;
 
+static struct test_firmware_upload *upload_lookup_name(const char *name)
+{
+	struct test_firmware_upload *tst;
+
+	list_for_each_entry(tst, &test_upload_list, node)
+		if (strncmp(name, tst->name, strlen(tst->name)) == 0)
+			return tst;
+
+	return NULL;
+}
+
 static ssize_t test_fw_misc_read(struct file *f, char __user *buf,
 				 size_t size, loff_t *offset)
 {
@@ -198,6 +223,7 @@ static int __test_firmware_config_init(void)
 	test_fw_config->req_firmware = request_firmware;
 	test_fw_config->test_result = 0;
 	test_fw_config->reqs = NULL;
+	test_fw_config->upload_name = NULL;
 
 	return 0;
 
@@ -277,6 +303,13 @@ static ssize_t config_show(struct device *dev,
 			test_fw_config->sync_direct ? "true" : "false");
 	len += scnprintf(buf + len, PAGE_SIZE - len,
 			"read_fw_idx:\t%u\n", test_fw_config->read_fw_idx);
+	if (test_fw_config->upload_name)
+		len += scnprintf(buf + len, PAGE_SIZE - len,
+				"upload_name:\t%s\n",
+				test_fw_config->upload_name);
+	else
+		len += scnprintf(buf + len, PAGE_SIZE - len,
+				"upload_name:\tEMTPY\n");
 
 	mutex_unlock(&test_fw_mutex);
 
@@ -392,6 +425,32 @@ static ssize_t config_name_show(struct device *dev,
 }
 static DEVICE_ATTR_RW(config_name);
 
+static ssize_t config_upload_name_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t count)
+{
+	struct test_firmware_upload *tst;
+	int ret = count;
+
+	mutex_lock(&test_fw_mutex);
+	tst = upload_lookup_name(buf);
+	if (tst)
+		test_fw_config->upload_name = tst->name;
+	else
+		ret = -EINVAL;
+	mutex_unlock(&test_fw_mutex);
+
+	return ret;
+}
+
+static ssize_t config_upload_name_show(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buf)
+{
+	return config_test_show_str(buf, test_fw_config->upload_name);
+}
+static DEVICE_ATTR_RW(config_upload_name);
+
 static ssize_t config_num_requests_store(struct device *dev,
 					 struct device_attribute *attr,
 					 const char *buf, size_t count)
@@ -989,6 +1048,167 @@ ssize_t trigger_batched_requests_async_store(struct device *dev,
 }
 static DEVICE_ATTR_WO(trigger_batched_requests_async);
 
+static void upload_release(struct test_firmware_upload *tst)
+{
+	firmware_upload_unregister(tst->fwl);
+	kfree(tst->buf);
+	kfree(tst->name);
+	kfree(tst);
+}
+
+static void upload_release_all(void)
+{
+	struct test_firmware_upload *tst, *tmp;
+
+	list_for_each_entry_safe(tst, tmp, &test_upload_list, node) {
+		list_del(&tst->node);
+		upload_release(tst);
+	}
+	test_fw_config->upload_name = NULL;
+}
+
+static enum fw_upload_err test_fw_upload_prepare(struct fw_upload *fwl,
+						 const u8 *data, u32 size)
+{
+	struct test_firmware_upload *tst = fwl->dd_handle;
+
+	tst->cancel_request = false;
+
+	if (!size || size > TEST_UPLOAD_MAX_SIZE)
+		return FW_UPLOAD_ERR_INVALID_SIZE;
+
+	memset(tst->buf, 0, TEST_UPLOAD_MAX_SIZE);
+	tst->size = size;
+
+	return FW_UPLOAD_ERR_NONE;
+}
+
+static enum fw_upload_err test_fw_upload_write(struct fw_upload *fwl,
+					       const u8 *data, u32 offset,
+					       u32 size, u32 *written)
+{
+	struct test_firmware_upload *tst = fwl->dd_handle;
+	u32 blk_size;
+
+	if (tst->cancel_request)
+		return FW_UPLOAD_ERR_CANCELED;
+
+	blk_size = min_t(u32, TEST_UPLOAD_BLK_SIZE, size);
+	memcpy(tst->buf + offset, data + offset, blk_size);
+
+	*written = blk_size;
+	return FW_UPLOAD_ERR_NONE;
+}
+
+static enum fw_upload_err test_fw_upload_complete(struct fw_upload *fwl)
+{
+	struct test_firmware_upload *tst = fwl->dd_handle;
+
+	if (tst->cancel_request)
+		return FW_UPLOAD_ERR_CANCELED;
+
+	return FW_UPLOAD_ERR_NONE;
+}
+
+static void test_fw_upload_cancel(struct fw_upload *fwl)
+{
+	struct test_firmware_upload *tst = fwl->dd_handle;
+
+	tst->cancel_request = true;
+}
+
+static const struct fw_upload_ops upload_test_ops = {
+	.prepare = test_fw_upload_prepare,
+	.write = test_fw_upload_write,
+	.poll_complete = test_fw_upload_complete,
+	.cancel = test_fw_upload_cancel,
+};
+
+static ssize_t upload_register_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	struct test_firmware_upload *tst;
+	struct fw_upload *fwl;
+	char *name;
+	int ret;
+
+	name = kstrndup(buf, count, GFP_KERNEL);
+	if (!name)
+		return -ENOMEM;
+
+	mutex_lock(&test_fw_mutex);
+	tst = upload_lookup_name(name);
+	if (tst) {
+		ret = -EEXIST;
+		goto free_name;
+	}
+
+	tst = kzalloc(sizeof(*tst), GFP_KERNEL);
+	if (!tst) {
+		ret = -ENOMEM;
+		goto free_name;
+	}
+
+	tst->name = name;
+	tst->buf = kzalloc(TEST_UPLOAD_MAX_SIZE, GFP_KERNEL);
+	if (!tst->buf) {
+		ret = -ENOMEM;
+		goto free_tst;
+	}
+
+	fwl = firmware_upload_register(THIS_MODULE, dev, tst->name,
+				       &upload_test_ops, tst);
+	if (IS_ERR(fwl)) {
+		ret = PTR_ERR(fwl);
+		goto free_buf;
+	}
+
+	tst->fwl = fwl;
+	list_add_tail(&tst->node, &test_upload_list);
+	mutex_unlock(&test_fw_mutex);
+	return count;
+
+free_buf:
+	kfree(tst->buf);
+
+free_tst:
+	kfree(tst);
+
+free_name:
+	mutex_unlock(&test_fw_mutex);
+	kfree(name);
+
+	return ret;
+}
+static DEVICE_ATTR_WO(upload_register);
+
+static ssize_t upload_unregister_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t count)
+{
+	struct test_firmware_upload *tst;
+	int ret = count;
+
+	mutex_lock(&test_fw_mutex);
+	tst = upload_lookup_name(buf);
+	if (!tst) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (test_fw_config->upload_name == tst->name)
+		test_fw_config->upload_name = NULL;
+
+	list_del(&tst->node);
+	upload_release(tst);
+
+out:
+	mutex_unlock(&test_fw_mutex);
+	return ret;
+}
+static DEVICE_ATTR_WO(upload_unregister);
+
 static ssize_t test_result_show(struct device *dev,
 				struct device_attribute *attr,
 				char *buf)
@@ -1051,6 +1271,42 @@ static ssize_t read_firmware_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(read_firmware);
 
+static ssize_t upload_read_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct test_firmware_upload *tst;
+	int ret = -EINVAL;
+
+	if (!test_fw_config->upload_name) {
+		pr_err("Set config_upload_name before using upload_read\n");
+		return -EINVAL;
+	}
+
+	mutex_lock(&test_fw_mutex);
+	list_for_each_entry(tst, &test_upload_list, node)
+		if (tst->name == test_fw_config->upload_name)
+			break;
+
+	if (tst->name != test_fw_config->upload_name) {
+		pr_err("Firmware name not found: %s\n",
+		       test_fw_config->upload_name);
+		goto out;
+	}
+
+	if (tst->size > PAGE_SIZE) {
+		pr_err("Testing interface must use PAGE_SIZE firmware for now\n");
+		goto out;
+	}
+
+	memcpy(buf, tst->buf, tst->size);
+	ret = tst->size;
+out:
+	mutex_unlock(&test_fw_mutex);
+	return ret;
+}
+static DEVICE_ATTR_RO(upload_read);
+
 #define TEST_FW_DEV_ATTR(name)          &dev_attr_##name.attr
 
 static struct attribute *test_dev_attrs[] = {
@@ -1066,6 +1322,7 @@ static struct attribute *test_dev_attrs[] = {
 	TEST_FW_DEV_ATTR(config_sync_direct),
 	TEST_FW_DEV_ATTR(config_send_uevent),
 	TEST_FW_DEV_ATTR(config_read_fw_idx),
+	TEST_FW_DEV_ATTR(config_upload_name),
 
 	/* These don't use the config at all - they could be ported! */
 	TEST_FW_DEV_ATTR(trigger_request),
@@ -1082,6 +1339,9 @@ static struct attribute *test_dev_attrs[] = {
 	TEST_FW_DEV_ATTR(release_all_firmware),
 	TEST_FW_DEV_ATTR(test_result),
 	TEST_FW_DEV_ATTR(read_firmware),
+	TEST_FW_DEV_ATTR(upload_read),
+	TEST_FW_DEV_ATTR(upload_register),
+	TEST_FW_DEV_ATTR(upload_unregister),
 	NULL,
 };
 
@@ -1128,6 +1388,7 @@ static void __exit test_firmware_exit(void)
 	mutex_lock(&test_fw_mutex);
 	release_firmware(test_firmware);
 	misc_deregister(&test_fw_misc_device);
+	upload_release_all();
 	__test_firmware_config_free();
 	kfree(test_fw_config);
 	mutex_unlock(&test_fw_mutex);
-- 
2.25.1

[RESEND PATCH v1 7/8] test_firmware: Error injection for firmware upload

[Russ Weight]

Add error injection capability to the test_firmware module specifically
for firmware upload testing. Error injection instructions are transferred
as the first part of the firmware payload. The format of an error
injection string is similar to the error strings that may be read from
the error sysfs node.

To inject the error "programming:hw-error", one would use the error
injection string "inject:programming:hw-error" as the firmware payload:

$ echo 1 > loading
$ echo inject:programming:hw-error > data
$ echo 0 > loading
$ cat status
idle
$ cat error
programming:hw-error

The first part of the error string is the progress state of the upload at
the time of the error. The progress state would be one of the following:
"preparing", "transferring", or "programming". The second part of the
error string is one of the following: "hw-error", "timeout", "device-busy",
"invalid-file-size", "read-write-error", "flash-wearout", and "user-abort".

Note that all of the error strings except "user-abort" will fail without
delay. The "user-abort" error will cause the firmware upload to stall at
the requested progress state for up to 5 minutes to allow you to echo 1
to the cancel sysfs node. It is this cancellation that causes the
'user-abort" error. If the upload is not cancelled within the 5 minute
time period, then the upload will complete without an error.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
 lib/test_firmware.c | 127 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 122 insertions(+), 5 deletions(-)

diff --git a/lib/test_firmware.c b/lib/test_firmware.c
index d6fe00c4972f..e9f9deeb3a2f 100644
--- a/lib/test_firmware.c
+++ b/lib/test_firmware.c
@@ -117,12 +117,18 @@ struct test_config {
 			    struct device *device);
 };
 
+struct upload_inject_err {
+	const char *prog;
+	enum fw_upload_err err_code;
+};
+
 struct test_firmware_upload {
 	char *name;
 	struct list_head node;
 	char *buf;
 	size_t size;
 	bool cancel_request;
+	struct upload_inject_err inject;
 	struct fw_upload *fwl;
 };
 
@@ -1067,20 +1073,105 @@ static void upload_release_all(void)
 	test_fw_config->upload_name = NULL;
 }
 
+/*
+ * This table is replicated from .../firmware_loader/sysfs_upload.c
+ * and needs to be kept in sync.
+ */
+static const char * const fw_upload_err_str[] = {
+	[FW_UPLOAD_ERR_NONE]	     = "none",
+	[FW_UPLOAD_ERR_HW_ERROR]     = "hw-error",
+	[FW_UPLOAD_ERR_TIMEOUT]	     = "timeout",
+	[FW_UPLOAD_ERR_CANCELED]     = "user-abort",
+	[FW_UPLOAD_ERR_BUSY]	     = "device-busy",
+	[FW_UPLOAD_ERR_INVALID_SIZE] = "invalid-file-size",
+	[FW_UPLOAD_ERR_RW_ERROR]     = "read-write-error",
+	[FW_UPLOAD_ERR_WEAROUT]	     = "flash-wearout",
+};
+
+static void upload_err_inject_error(struct test_firmware_upload *tst,
+				    const u8 *p, const char *prog)
+{
+	enum fw_upload_err err;
+
+	for (err = FW_UPLOAD_ERR_NONE + 1; err < FW_UPLOAD_ERR_MAX; err++) {
+		if (strncmp(p, fw_upload_err_str[err],
+			    strlen(fw_upload_err_str[err])) == 0) {
+			tst->inject.prog = prog;
+			tst->inject.err_code = err;
+			return;
+		}
+	}
+}
+
+static void upload_err_inject_prog(struct test_firmware_upload *tst,
+				   const u8 *p)
+{
+	static const char * const progs[] = {
+		"preparing:", "transferring:", "programming:"
+	};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(progs); i++) {
+		if (strncmp(p, progs[i], strlen(progs[i])) == 0) {
+			upload_err_inject_error(tst, p + strlen(progs[i]),
+						progs[i]);
+			return;
+		}
+	}
+}
+
+#define FIVE_MINUTES_MS	(5 * 60 * 1000)
+static enum fw_upload_err
+fw_upload_wait_on_cancel(struct test_firmware_upload *tst)
+{
+	int ms_delay;
+
+	for (ms_delay = 0; ms_delay < FIVE_MINUTES_MS; ms_delay += 100) {
+		msleep(100);
+		if (tst->cancel_request)
+			return FW_UPLOAD_ERR_CANCELED;
+	}
+	return FW_UPLOAD_ERR_NONE;
+}
+
 static enum fw_upload_err test_fw_upload_prepare(struct fw_upload *fwl,
 						 const u8 *data, u32 size)
 {
 	struct test_firmware_upload *tst = fwl->dd_handle;
+	enum fw_upload_err ret = FW_UPLOAD_ERR_NONE;
+	const char *progress = "preparing:";
 
 	tst->cancel_request = false;
 
-	if (!size || size > TEST_UPLOAD_MAX_SIZE)
-		return FW_UPLOAD_ERR_INVALID_SIZE;
+	if (!size || size > TEST_UPLOAD_MAX_SIZE) {
+		ret = FW_UPLOAD_ERR_INVALID_SIZE;
+		goto err_out;
+	}
+
+	if (strncmp(data, "inject:", strlen("inject:")) == 0)
+		upload_err_inject_prog(tst, data + strlen("inject:"));
 
 	memset(tst->buf, 0, TEST_UPLOAD_MAX_SIZE);
 	tst->size = size;
 
-	return FW_UPLOAD_ERR_NONE;
+	if (tst->inject.err_code == FW_UPLOAD_ERR_NONE ||
+	    strncmp(tst->inject.prog, progress, strlen(progress)) != 0)
+		return FW_UPLOAD_ERR_NONE;
+
+	if (tst->inject.err_code == FW_UPLOAD_ERR_CANCELED)
+		ret = fw_upload_wait_on_cancel(tst);
+	else
+		ret = tst->inject.err_code;
+
+err_out:
+	/*
+	 * The cleanup op only executes if the prepare op succeeds.
+	 * If the prepare op fails, it must do it's own clean-up.
+	 */
+	tst->inject.err_code = FW_UPLOAD_ERR_NONE;
+	tst->inject.prog = NULL;
+
+	return ret;
 }
 
 static enum fw_upload_err test_fw_upload_write(struct fw_upload *fwl,
@@ -1088,6 +1179,7 @@ static enum fw_upload_err test_fw_upload_write(struct fw_upload *fwl,
 					       u32 size, u32 *written)
 {
 	struct test_firmware_upload *tst = fwl->dd_handle;
+	const char *progress = "transferring:";
 	u32 blk_size;
 
 	if (tst->cancel_request)
@@ -1097,17 +1189,33 @@ static enum fw_upload_err test_fw_upload_write(struct fw_upload *fwl,
 	memcpy(tst->buf + offset, data + offset, blk_size);
 
 	*written = blk_size;
-	return FW_UPLOAD_ERR_NONE;
+
+	if (tst->inject.err_code == FW_UPLOAD_ERR_NONE ||
+	    strncmp(tst->inject.prog, progress, strlen(progress)) != 0)
+		return FW_UPLOAD_ERR_NONE;
+
+	if (tst->inject.err_code == FW_UPLOAD_ERR_CANCELED)
+		return fw_upload_wait_on_cancel(tst);
+
+	return tst->inject.err_code;
 }
 
 static enum fw_upload_err test_fw_upload_complete(struct fw_upload *fwl)
 {
 	struct test_firmware_upload *tst = fwl->dd_handle;
+	const char *progress = "programming:";
 
 	if (tst->cancel_request)
 		return FW_UPLOAD_ERR_CANCELED;
 
-	return FW_UPLOAD_ERR_NONE;
+	if (tst->inject.err_code == FW_UPLOAD_ERR_NONE ||
+	    strncmp(tst->inject.prog, progress, strlen(progress)) != 0)
+		return FW_UPLOAD_ERR_NONE;
+
+	if (tst->inject.err_code == FW_UPLOAD_ERR_CANCELED)
+		return fw_upload_wait_on_cancel(tst);
+
+	return tst->inject.err_code;
 }
 
 static void test_fw_upload_cancel(struct fw_upload *fwl)
@@ -1117,11 +1225,20 @@ static void test_fw_upload_cancel(struct fw_upload *fwl)
 	tst->cancel_request = true;
 }
 
+static void test_fw_cleanup(struct fw_upload *fwl)
+{
+	struct test_firmware_upload *tst = fwl->dd_handle;
+
+	tst->inject.err_code = FW_UPLOAD_ERR_NONE;
+	tst->inject.prog = NULL;
+}
+
 static const struct fw_upload_ops upload_test_ops = {
 	.prepare = test_fw_upload_prepare,
 	.write = test_fw_upload_write,
 	.poll_complete = test_fw_upload_complete,
 	.cancel = test_fw_upload_cancel,
+	.cleanup = test_fw_cleanup
 };
 
 static ssize_t upload_register_store(struct device *dev,
-- 
2.25.1

[RESEND PATCH v1 8/8] selftests: firmware: Add firmware upload selftests

[Russ Weight]

Add selftests to verify the firmware upload mechanism. These test
include simple firmware uploads as well as upload cancellation and
error injection. The test creates three firmware devices and verifies
that they all work correctly and independently.

Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
 tools/testing/selftests/firmware/Makefile     |   2 +-
 tools/testing/selftests/firmware/config       |   1 +
 tools/testing/selftests/firmware/fw_lib.sh    |   7 +
 .../selftests/firmware/fw_run_tests.sh        |   4 +
 tools/testing/selftests/firmware/fw_upload.sh | 214 ++++++++++++++++++
 5 files changed, 227 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/firmware/fw_upload.sh

diff --git a/tools/testing/selftests/firmware/Makefile b/tools/testing/selftests/firmware/Makefile
index 40211cd8f0e6..7992969deaa2 100644
--- a/tools/testing/selftests/firmware/Makefile
+++ b/tools/testing/selftests/firmware/Makefile
@@ -4,7 +4,7 @@ CFLAGS = -Wall \
          -O2
 
 TEST_PROGS := fw_run_tests.sh
-TEST_FILES := fw_fallback.sh fw_filesystem.sh fw_lib.sh
+TEST_FILES := fw_fallback.sh fw_filesystem.sh fw_upload.sh fw_lib.sh
 TEST_GEN_FILES := fw_namespace
 
 include ../lib.mk
diff --git a/tools/testing/selftests/firmware/config b/tools/testing/selftests/firmware/config
index bf634dda0720..6e402519b117 100644
--- a/tools/testing/selftests/firmware/config
+++ b/tools/testing/selftests/firmware/config
@@ -3,3 +3,4 @@ CONFIG_FW_LOADER=y
 CONFIG_FW_LOADER_USER_HELPER=y
 CONFIG_IKCONFIG=y
 CONFIG_IKCONFIG_PROC=y
+CONFIG_FW_UPLOAD=y
diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh
index 5b8c0fedee76..fe8d34dbe7ca 100755
--- a/tools/testing/selftests/firmware/fw_lib.sh
+++ b/tools/testing/selftests/firmware/fw_lib.sh
@@ -63,6 +63,7 @@ check_setup()
 	HAS_FW_LOADER_USER_HELPER="$(kconfig_has CONFIG_FW_LOADER_USER_HELPER=y)"
 	HAS_FW_LOADER_USER_HELPER_FALLBACK="$(kconfig_has CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y)"
 	HAS_FW_LOADER_COMPRESS="$(kconfig_has CONFIG_FW_LOADER_COMPRESS=y)"
+	HAS_FW_UPLOAD="$(kconfig_has CONFIG_FW_UPLOAD=y)"
 	PROC_FW_IGNORE_SYSFS_FALLBACK="0"
 	PROC_FW_FORCE_SYSFS_FALLBACK="0"
 
@@ -113,6 +114,12 @@ verify_reqs()
 			exit 0
 		fi
 	fi
+	if [ "$TEST_REQS_FW_UPLOAD" = "yes" ]; then
+		if [ ! "$HAS_FW_UPLOAD" = "yes" ]; then
+			echo "firmware upload disabled so ignoring test"
+			exit 0
+		fi
+	fi
 }
 
 setup_tmp_file()
diff --git a/tools/testing/selftests/firmware/fw_run_tests.sh b/tools/testing/selftests/firmware/fw_run_tests.sh
index 777377078d5e..f6d95a2d5124 100755
--- a/tools/testing/selftests/firmware/fw_run_tests.sh
+++ b/tools/testing/selftests/firmware/fw_run_tests.sh
@@ -22,6 +22,10 @@ run_tests()
 	proc_set_force_sysfs_fallback $1
 	proc_set_ignore_sysfs_fallback $2
 	$TEST_DIR/fw_fallback.sh
+
+	proc_set_force_sysfs_fallback $1
+	proc_set_ignore_sysfs_fallback $2
+	$TEST_DIR/fw_upload.sh
 }
 
 run_test_config_0001()
diff --git a/tools/testing/selftests/firmware/fw_upload.sh b/tools/testing/selftests/firmware/fw_upload.sh
new file mode 100755
index 000000000000..c7a6f06c9adb
--- /dev/null
+++ b/tools/testing/selftests/firmware/fw_upload.sh
@@ -0,0 +1,214 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# This validates the user-initiated fw upload mechanism of the firmware
+# loader. It verifies that one or more firmware devices can be created
+# for a device driver. It also verifies the data transfer, the
+# cancellation support, and the error flows.
+set -e
+
+TEST_REQS_FW_UPLOAD="yes"
+TEST_DIR=$(dirname $0)
+
+progress_states="preparing transferring  programming"
+errors="hw-error
+	timeout
+	device-busy
+	invalid-file-size
+	read-write-error
+	flash-wearout"
+error_abort="user-abort"
+fwname1=fw1
+fwname2=fw2
+fwname3=fw3
+
+source $TEST_DIR/fw_lib.sh
+
+check_mods
+check_setup
+verify_reqs
+
+trap "upload_finish" EXIT
+
+upload_finish() {
+	local fwdevs="$fwname1 $fwname2 $fwname3"
+
+	for name in $fwdevs; do
+		if [ -e "$DIR/$name" ]; then
+			echo -n "$name" > "$DIR"/upload_unregister
+		fi
+	done
+}
+
+upload_fw() {
+	local name="$1"
+	local file="$2"
+
+	echo 1 > "$DIR"/"$name"/loading
+	cat "$file" > "$DIR"/"$name"/data
+	echo 0 > "$DIR"/"$name"/loading
+}
+
+verify_fw() {
+	local name="$1"
+	local file="$2"
+
+	echo -n "$name" > "$DIR"/config_upload_name
+	if ! cmp "$file" "$DIR"/upload_read > /dev/null 2>&1; then
+		echo "$0: firmware compare for $name did not match" >&2
+		exit 1
+	fi
+
+	echo "$0: firmware upload for $name works" >&2
+	return 0
+}
+
+inject_error() {
+	local name="$1"
+	local status="$2"
+	local error="$3"
+
+	echo 1 > "$DIR"/"$name"/loading
+	echo -n "inject":"$status":"$error" > "$DIR"/"$name"/data
+	echo 0 > "$DIR"/"$name"/loading
+}
+
+await_status() {
+	local name="$1"
+	local expected="$2"
+	local status
+	local i
+
+	let i=0
+	while [ $i -lt 50 ]; do
+		status=$(cat "$DIR"/"$name"/status)
+		if [ "$status" = "$expected" ]; then
+			return 0;
+		fi
+		sleep 1e-03
+		let i=$i+1
+	done
+
+	echo "$0: Invalid status: Expected $expected, Actual $status" >&2
+	return 1;
+}
+
+await_idle() {
+	local name="$1"
+
+	await_status "$name" "idle"
+	return $?
+}
+
+expect_error() {
+	local name="$1"
+	local expected="$2"
+	local error=$(cat "$DIR"/"$name"/error)
+
+	if [ "$error" != "$expected" ]; then
+		echo "Invalid error: Expected $expected, Actual $error" >&2
+		return 1
+	fi
+
+	return 0
+}
+
+random_firmware() {
+	local bs="$1"
+	local count="$2"
+	local file=$(mktemp -p /tmp uploadfwXXX.bin)
+
+	dd if=/dev/urandom of="$file" bs="$bs" count="$count" > /dev/null 2>&1
+	echo "$file"
+}
+
+test_upload_cancel() {
+	local name="$1"
+	local status
+
+	for status in $progress_states; do
+		inject_error $name $status $error_abort
+		if ! await_status $name $status; then
+			exit 1
+		fi
+
+		echo 1 > "$DIR"/"$name"/cancel
+
+		if ! await_idle $name; then
+			exit 1
+		fi
+
+		if ! expect_error $name "$status":"$error_abort"; then
+			exit 1
+		fi
+	done
+
+	echo "$0: firmware upload cancellation works"
+	return 0
+}
+
+test_error_handling() {
+	local name=$1
+	local status
+	local error
+
+	for status in $progress_states; do
+		for error in $errors; do
+			inject_error $name $status $error
+
+			if ! await_idle $name; then
+				exit 1
+			fi
+
+			if ! expect_error $name "$status":"$error"; then
+				exit 1
+			fi
+
+		done
+	done
+	echo "$0: firmware upload error handling works"
+}
+
+test_fw_too_big() {
+	local name=$1
+	local fw_too_big=`random_firmware 512 5`
+	local expected="preparing:invalid-file-size"
+
+	upload_fw $name $fw_too_big
+	rm -f $fw_too_big
+
+	if ! await_idle $name; then
+		exit 1
+	fi
+
+	if ! expect_error $name $expected; then
+		exit 1
+	fi
+
+	echo "$0: oversized firmware error handling works"
+}
+
+echo -n "$fwname1" > "$DIR"/upload_register
+echo -n "$fwname2" > "$DIR"/upload_register
+echo -n "$fwname3" > "$DIR"/upload_register
+
+test_upload_cancel $fwname1
+test_error_handling $fwname1
+test_fw_too_big $fwname1
+
+fw_file1=`random_firmware 512 4`
+fw_file2=`random_firmware 512 3`
+fw_file3=`random_firmware 512 2`
+
+upload_fw $fwname1 $fw_file1
+upload_fw $fwname2 $fw_file2
+upload_fw $fwname3 $fw_file3
+
+verify_fw ${fwname1} ${fw_file1}
+verify_fw ${fwname2} ${fw_file2}
+verify_fw ${fwname3} ${fw_file3}
+
+echo -n "$fwname1" > "$DIR"/upload_unregister
+echo -n "$fwname2" > "$DIR"/upload_unregister
+echo -n "$fwname3" > "$DIR"/upload_unregister
+
+exit 0
-- 
2.25.1

Re: [RESEND PATCH v1 1/8] firmware_loader: Clear data and size in fw_free_paged_buf

[Tom Rix]

On 3/23/22 4:33 PM, Russ Weight wrote:
> The fw_free_paged_buf() function resets the paged buffer information in
> the fw_priv data structure. Additionally, clear the data and size members
> of fw_priv in order to facilitate the reuse of fw_priv. This is being
> done in preparation for enabling userspace to initiate multiple firmware
> uploads using this sysfs interface.
>
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
> v1:
>    - No change from RFC patch
> ---
>   drivers/base/firmware_loader/main.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
> index 94d1789a233e..2cc11d93753a 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -253,6 +253,8 @@ void fw_free_paged_buf(struct fw_priv *fw_priv)

Why isn't a vfree needed or realloc done?

Tom

>   	fw_priv->pages = NULL;
>   	fw_priv->page_array_size = 0;
>   	fw_priv->nr_pages = 0;
> +	fw_priv->data = NULL;
> +	fw_priv->size = 0;
>   }
>   
>   int fw_grow_paged_buf(struct fw_priv *fw_priv, int pages_needed)

Re: [RESEND PATCH v1 1/8] firmware_loader: Clear data and size in fw_free_paged_buf

[Russ Weight]

Hi Tom,

On 3/28/22 06:27, Tom Rix wrote:
>
> On 3/23/22 4:33 PM, Russ Weight wrote:
>> The fw_free_paged_buf() function resets the paged buffer information in
>> the fw_priv data structure. Additionally, clear the data and size members
>> of fw_priv in order to facilitate the reuse of fw_priv. This is being
>> done in preparation for enabling userspace to initiate multiple firmware
>> uploads using this sysfs interface.
>>
>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
>> ---
>> v1:
>>    - No change from RFC patch
>> ---
>>   drivers/base/firmware_loader/main.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
>> index 94d1789a233e..2cc11d93753a 100644
>> --- a/drivers/base/firmware_loader/main.c
>> +++ b/drivers/base/firmware_loader/main.c
>> @@ -253,6 +253,8 @@ void fw_free_paged_buf(struct fw_priv *fw_priv)
>
> Why isn't a vfree needed or realloc done?

The free and realloc support was present prior to my changes. The page
buffer support was designed such that if a firmware write was cancelled, the
existing fw_priv structure could be re-used for another write in the context
of the same firmware upload. However, there was no prior case for completing
a write and then reusing the fw_priv structure for subsequent firmware writes;
fw_priv previously had a one-time use. The changes I have made are to enable
the re-use of the fw_priv structure.

Initially, fw_priv->data is NULL. The "realloc" functionality happens during
the write of the data binary attribute here:

https://github.com/torvalds/linux/blob/ae085d7f9365de7da27ab5c0d16b12d51ea7fca9/drivers/base/firmware_loader/fallback.c#L426

The fw_priv->data pointer remains NULL until all data is written and the
user writes '0' to the loading attribute. The fw_priv->data pointer is set in
fw_map_paged_buf() which is called here:

https://github.com/torvalds/linux/blob/ae085d7f9365de7da27ab5c0d16b12d51ea7fca9/drivers/base/firmware_loader/fallback.c#L274

In the unmodified code, the fw_priv->data pointer is never cleared. My changes
reset the pointer to NULL after the memory is released so that the fw_priv can
be resused.

The new firmware-upload happens in the context of a kernel worker thread and the work
function is fw_upload_main(). At the end of fw_upload_main(), fw_free_paged_buf()
is called to do the free. This is the function that is being modified by the lines
below. This function calls "__free_page(fw_priv->pages[i])" in a loop to free the
memory pages. It also calls "vunmap(fw_priv->data)" to free the virtual mapping.
You can see the unmodified implementation of this function here:

https://github.com/torvalds/linux/blob/ae085d7f9365de7da27ab5c0d16b12d51ea7fca9/drivers/base/firmware_loader/main.c#L241

- Russ

>
> Tom
>
>>       fw_priv->pages = NULL;
>>       fw_priv->page_array_size = 0;
>>       fw_priv->nr_pages = 0;
>> +    fw_priv->data = NULL;
>> +    fw_priv->size = 0;
>>   }
>>     int fw_grow_paged_buf(struct fw_priv *fw_priv, int pages_needed)
>

Re: [RESEND PATCH v1 1/8] firmware_loader: Clear data and size in fw_free_paged_buf

[Tom Rix]

On 3/28/22 11:09 AM, Russ Weight wrote:
> Hi Tom,
>
> On 3/28/22 06:27, Tom Rix wrote:
>> On 3/23/22 4:33 PM, Russ Weight wrote:
>>> The fw_free_paged_buf() function resets the paged buffer information in
>>> the fw_priv data structure. Additionally, clear the data and size members
>>> of fw_priv in order to facilitate the reuse of fw_priv. This is being
>>> done in preparation for enabling userspace to initiate multiple firmware
>>> uploads using this sysfs interface.
>>>
>>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
>>> ---
>>> v1:
>>>     - No change from RFC patch
>>> ---
>>>    drivers/base/firmware_loader/main.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
>>> index 94d1789a233e..2cc11d93753a 100644
>>> --- a/drivers/base/firmware_loader/main.c
>>> +++ b/drivers/base/firmware_loader/main.c
>>> @@ -253,6 +253,8 @@ void fw_free_paged_buf(struct fw_priv *fw_priv)
>> Why isn't a vfree needed or realloc done?

I am looking at the use of this function in __free_fw_priv

       if (fw_is_paged_buf(fw_priv))
           fw_free_paged_buf(fw_priv);
       else if (!fw_priv->allocated_size)
           vfree(fw_priv->data);

Where it seems like there is another way to set data, so it needs 
another way to unset.

The vfree here looks suspect because the pointer comes in from 
request_firmware_info_buf with a hope that it was allocated by vmalloc.

Tom

> The free and realloc support was present prior to my changes. The page
> buffer support was designed such that if a firmware write was cancelled, the
> existing fw_priv structure could be re-used for another write in the context
> of the same firmware upload. However, there was no prior case for completing
> a write and then reusing the fw_priv structure for subsequent firmware writes;
> fw_priv previously had a one-time use. The changes I have made are to enable
> the re-use of the fw_priv structure.
>
> Initially, fw_priv->data is NULL. The "realloc" functionality happens during
> the write of the data binary attribute here:
>
> https://github.com/torvalds/linux/blob/ae085d7f9365de7da27ab5c0d16b12d51ea7fca9/drivers/base/firmware_loader/fallback.c#L426
>
> The fw_priv->data pointer remains NULL until all data is written and the
> user writes '0' to the loading attribute. The fw_priv->data pointer is set in
> fw_map_paged_buf() which is called here:
>
> https://github.com/torvalds/linux/blob/ae085d7f9365de7da27ab5c0d16b12d51ea7fca9/drivers/base/firmware_loader/fallback.c#L274
>
> In the unmodified code, the fw_priv->data pointer is never cleared. My changes
> reset the pointer to NULL after the memory is released so that the fw_priv can
> be resused.
>
> The new firmware-upload happens in the context of a kernel worker thread and the work
> function is fw_upload_main(). At the end of fw_upload_main(), fw_free_paged_buf()
> is called to do the free. This is the function that is being modified by the lines
> below. This function calls "__free_page(fw_priv->pages[i])" in a loop to free the
> memory pages. It also calls "vunmap(fw_priv->data)" to free the virtual mapping.
> You can see the unmodified implementation of this function here:
>
> https://github.com/torvalds/linux/blob/ae085d7f9365de7da27ab5c0d16b12d51ea7fca9/drivers/base/firmware_loader/main.c#L241
>
> - Russ
>
>> Tom
>>
>>>        fw_priv->pages = NULL;
>>>        fw_priv->page_array_size = 0;
>>>        fw_priv->nr_pages = 0;
>>> +    fw_priv->data = NULL;
>>> +    fw_priv->size = 0;
>>>    }
>>>      int fw_grow_paged_buf(struct fw_priv *fw_priv, int pages_needed)

Re: [RESEND PATCH v1 1/8] firmware_loader: Clear data and size in fw_free_paged_buf

[Russ Weight]

On 3/28/22 11:52, Tom Rix wrote:
>
> On 3/28/22 11:09 AM, Russ Weight wrote:
>> Hi Tom,
>>
>> On 3/28/22 06:27, Tom Rix wrote:
>>> On 3/23/22 4:33 PM, Russ Weight wrote:
>>>> The fw_free_paged_buf() function resets the paged buffer information in
>>>> the fw_priv data structure. Additionally, clear the data and size members
>>>> of fw_priv in order to facilitate the reuse of fw_priv. This is being
>>>> done in preparation for enabling userspace to initiate multiple firmware
>>>> uploads using this sysfs interface.
>>>>
>>>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
>>>> ---
>>>> v1:
>>>>     - No change from RFC patch
>>>> ---
>>>>    drivers/base/firmware_loader/main.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
>>>> index 94d1789a233e..2cc11d93753a 100644
>>>> --- a/drivers/base/firmware_loader/main.c
>>>> +++ b/drivers/base/firmware_loader/main.c
>>>> @@ -253,6 +253,8 @@ void fw_free_paged_buf(struct fw_priv *fw_priv)
>>> Why isn't a vfree needed or realloc done?
>
> I am looking at the use of this function in __free_fw_priv
>
>       if (fw_is_paged_buf(fw_priv))
>           fw_free_paged_buf(fw_priv);
>       else if (!fw_priv->allocated_size)
>           vfree(fw_priv->data);
>
> Where it seems like there is another way to set data, so it needs another way to unset.
>
> The vfree here looks suspect because the pointer comes in from request_firmware_info_buf with a hope that it was allocated by vmalloc.

There are places in the code where, if fw_priv->data is set, it is assumed that it
was allocated by the caller. In the sysfs-upload path, vmalloc is never used - only
paged buffers.

As I have revisited the code to answer your questions, I see that in the firmware
fallback path, after it checks for (!fw_priv->data), it sets "fw_priv->is_paged_buf = true".
I realize now that I should be setting "fw_priv->is_paged_buf = true" in firmware_upload_register(). I'll make that change and rebase on 5.18-rc1 when it is
available.

Thanks for the comments!

- Russ
>
> Tom
>
>> The free and realloc support was present prior to my changes. The page
>> buffer support was designed such that if a firmware write was cancelled, the
>> existing fw_priv structure could be re-used for another write in the context
>> of the same firmware upload. However, there was no prior case for completing
>> a write and then reusing the fw_priv structure for subsequent firmware writes;
>> fw_priv previously had a one-time use. The changes I have made are to enable
>> the re-use of the fw_priv structure.
>>
>> Initially, fw_priv->data is NULL. The "realloc" functionality happens during
>> the write of the data binary attribute here:
>>
>> https://github.com/torvalds/linux/blob/ae085d7f9365de7da27ab5c0d16b12d51ea7fca9/drivers/base/firmware_loader/fallback.c#L426
>>
>> The fw_priv->data pointer remains NULL until all data is written and the
>> user writes '0' to the loading attribute. The fw_priv->data pointer is set in
>> fw_map_paged_buf() which is called here:
>>
>> https://github.com/torvalds/linux/blob/ae085d7f9365de7da27ab5c0d16b12d51ea7fca9/drivers/base/firmware_loader/fallback.c#L274
>>
>> In the unmodified code, the fw_priv->data pointer is never cleared. My changes
>> reset the pointer to NULL after the memory is released so that the fw_priv can
>> be resused.
>>
>> The new firmware-upload happens in the context of a kernel worker thread and the work
>> function is fw_upload_main(). At the end of fw_upload_main(), fw_free_paged_buf()
>> is called to do the free. This is the function that is being modified by the lines
>> below. This function calls "__free_page(fw_priv->pages[i])" in a loop to free the
>> memory pages. It also calls "vunmap(fw_priv->data)" to free the virtual mapping.
>> You can see the unmodified implementation of this function here:
>>
>> https://github.com/torvalds/linux/blob/ae085d7f9365de7da27ab5c0d16b12d51ea7fca9/drivers/base/firmware_loader/main.c#L241
>>
>> - Russ
>>
>>> Tom
>>>
>>>>        fw_priv->pages = NULL;
>>>>        fw_priv->page_array_size = 0;
>>>>        fw_priv->nr_pages = 0;
>>>> +    fw_priv->data = NULL;
>>>> +    fw_priv->size = 0;
>>>>    }
>>>>      int fw_grow_paged_buf(struct fw_priv *fw_priv, int pages_needed)
>

Re: [RESEND PATCH v1 2/8] firmware_loader: Check fw_state_is_done in loading_store

[Tom Rix]

On 3/23/22 4:33 PM, Russ Weight wrote:
> Add the fw_state_is_done() function and exit early from
> firmware_loading_store() if the state is already "done". This is being done
> in preparation for supporting persistent sysfs nodes to allow userspace to
> upload firmware to a device, potentially reusing the sysfs loading and data
> files multiple times.
>
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
> v1:
>    - No change from RFC patch
> ---
>   drivers/base/firmware_loader/fallback.c | 2 +-
>   drivers/base/firmware_loader/firmware.h | 5 +++++
>   2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index 4afb0e9312c0..d82e055a4297 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -250,7 +250,7 @@ static ssize_t firmware_loading_store(struct device *dev,
>   
>   	mutex_lock(&fw_lock);
>   	fw_priv = fw_sysfs->fw_priv;
> -	if (fw_state_is_aborted(fw_priv))
> +	if (fw_state_is_aborted(fw_priv) || fw_state_is_done(fw_priv))
>   		goto out;
>   
>   	switch (loading) {
> diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
> index 2889f446ad41..58768d16f8df 100644
> --- a/drivers/base/firmware_loader/firmware.h
> +++ b/drivers/base/firmware_loader/firmware.h
> @@ -149,6 +149,11 @@ static inline void fw_state_done(struct fw_priv *fw_priv)
>   	__fw_state_set(fw_priv, FW_STATUS_DONE);
>   }
>   
> +static inline bool fw_state_is_done(struct fw_priv *fw_priv)
> +{
> +	return __fw_state_check(fw_priv, FW_STATUS_DONE);

This looks like the fw_sysfs_done() in fallback.c

IMO this one and similar wrappers should move to firmware.h and use the 
*_is_* naming.

Tom

> +}
> +
>   int assign_fw(struct firmware *fw, struct device *device);
>   
>   #ifdef CONFIG_FW_LOADER

Re: [RESEND PATCH v1 3/8] firmware_loader: Split sysfs support from fallback

[Tom Rix]

On 3/23/22 4:33 PM, Russ Weight wrote:
> In preparation for sharing the "loading" and "data" sysfs nodes with the
> new firmware upload support, split out sysfs functionality from fallback.c
> and fallback.h into sysfs.c and sysfs.h. This includes the firmware
> class driver code that is associated with the sysfs files and the
> fw_fallback_config support for the timeout sysfs node.
>
> CONFIG_FW_LOADER_SYSFS is created and is selected by
> CONFIG_FW_LOADER_USER_HELPER in order to include sysfs.o in
> firmware_class-objs.
>
> This is mostly just a code reorganization. There are a few symbols that
> change in scope, and these can be identified by looking at the header
> file changes. A few white-space warnings from checkpatch are also
> addressed in this patch.
>
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> ---
> v1:
>    - Renamed files fw_sysfs.c and fw_sysfs.h to sysfs.c and sysfs.h
>    - Moved "MODULE_IMPORT_NS(FIRMWARE_LOADER_PRIVATE);" from sysfs.c to
>      sysfs.h to address an error identified by the kernel test robot
>      <lkp@intel.com>
> ---
>   drivers/base/firmware_loader/Kconfig    |   4 +
>   drivers/base/firmware_loader/Makefile   |   1 +
>   drivers/base/firmware_loader/fallback.c | 430 ------------------------
>   drivers/base/firmware_loader/fallback.h |  46 +--
>   drivers/base/firmware_loader/sysfs.c    | 411 ++++++++++++++++++++++
>   drivers/base/firmware_loader/sysfs.h    |  96 ++++++
>   6 files changed, 513 insertions(+), 475 deletions(-)
>   create mode 100644 drivers/base/firmware_loader/sysfs.c
>   create mode 100644 drivers/base/firmware_loader/sysfs.h
>
> diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
> index 38f3b66bf52b..9e03178eee00 100644
> --- a/drivers/base/firmware_loader/Kconfig
> +++ b/drivers/base/firmware_loader/Kconfig
> @@ -29,6 +29,9 @@ if FW_LOADER
>   config FW_LOADER_PAGED_BUF
>   	bool
>   
> +config FW_LOADER_SYSFS
> +	bool
> +
>   config EXTRA_FIRMWARE
>   	string "Build named firmware blobs into the kernel binary"
>   	help
> @@ -72,6 +75,7 @@ config EXTRA_FIRMWARE_DIR
>   
>   config FW_LOADER_USER_HELPER
>   	bool "Enable the firmware sysfs fallback mechanism"
> +	select FW_LOADER_SYSFS

Is this code reordering necessary ?

This config is not removed or renamed later and has the same configs are 
the later FW_UPLOAD.

Maybe leave fallback.c as-is and rename FW_LOADER_USER_HELPER to 
FW_LOADER_SYSFS because the name is more descriptive.

The 'sorry we suck' help message replaced with a shorter message to 
indicate this is now a more capable config.

The later FW_UPLOAD would have a 'depends on FW_LOADER_SYSFS'

If you end up needing to do the reorder, move it to patch 1 because 
bisecting-wise it should not depend on improvements in the current 1,2 
patches.

Tom

>   	select FW_LOADER_PAGED_BUF
>   	help
>   	  This option enables a sysfs loading facility to enable firmware
> diff --git a/drivers/base/firmware_loader/Makefile b/drivers/base/firmware_loader/Makefile
> index e87843408fe6..aab213f82288 100644
> --- a/drivers/base/firmware_loader/Makefile
> +++ b/drivers/base/firmware_loader/Makefile
> @@ -6,5 +6,6 @@ obj-$(CONFIG_FW_LOADER)	+= firmware_class.o
>   firmware_class-objs := main.o
>   firmware_class-$(CONFIG_FW_LOADER_USER_HELPER) += fallback.o
>   firmware_class-$(CONFIG_EFI_EMBEDDED_FIRMWARE) += fallback_platform.o
> +firmware_class-$(CONFIG_FW_LOADER_SYSFS) += sysfs.o
>   
>   obj-y += builtin/
> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index d82e055a4297..bf68e3947814 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -3,12 +3,9 @@
>   #include <linux/types.h>
>   #include <linux/kconfig.h>
>   #include <linux/list.h>
> -#include <linux/slab.h>
>   #include <linux/security.h>
> -#include <linux/highmem.h>
>   #include <linux/umh.h>
>   #include <linux/sysctl.h>
> -#include <linux/vmalloc.h>
>   #include <linux/module.h>
>   
>   #include "fallback.h"
> @@ -18,22 +15,6 @@
>    * firmware fallback mechanism
>    */
>   
> -MODULE_IMPORT_NS(FIRMWARE_LOADER_PRIVATE);
> -
> -extern struct firmware_fallback_config fw_fallback_config;
> -
> -/* These getters are vetted to use int properly */
> -static inline int __firmware_loading_timeout(void)
> -{
> -	return fw_fallback_config.loading_timeout;
> -}
> -
> -/* These setters are vetted to use int properly */
> -static void __fw_fallback_set_timeout(int timeout)
> -{
> -	fw_fallback_config.loading_timeout = timeout;
> -}
> -
>   /*
>    * use small loading timeout for caching devices' firmware because all these
>    * firmware images have been loaded successfully at lease once, also system is
> @@ -58,52 +39,11 @@ static long firmware_loading_timeout(void)
>   		__firmware_loading_timeout() * HZ : MAX_JIFFY_OFFSET;
>   }
>   
> -static inline bool fw_sysfs_done(struct fw_priv *fw_priv)
> -{
> -	return __fw_state_check(fw_priv, FW_STATUS_DONE);
> -}
> -
> -static inline bool fw_sysfs_loading(struct fw_priv *fw_priv)
> -{
> -	return __fw_state_check(fw_priv, FW_STATUS_LOADING);
> -}
> -
>   static inline int fw_sysfs_wait_timeout(struct fw_priv *fw_priv,  long timeout)
>   {
>   	return __fw_state_wait_common(fw_priv, timeout);
>   }
>   
> -struct fw_sysfs {
> -	bool nowait;
> -	struct device dev;
> -	struct fw_priv *fw_priv;
> -	struct firmware *fw;
> -};
> -
> -static struct fw_sysfs *to_fw_sysfs(struct device *dev)
> -{
> -	return container_of(dev, struct fw_sysfs, dev);
> -}
> -
> -static void __fw_load_abort(struct fw_priv *fw_priv)
> -{
> -	/*
> -	 * There is a small window in which user can write to 'loading'
> -	 * between loading done/aborted and disappearance of 'loading'
> -	 */
> -	if (fw_state_is_aborted(fw_priv) || fw_sysfs_done(fw_priv))
> -		return;
> -
> -	fw_state_aborted(fw_priv);
> -}
> -
> -static void fw_load_abort(struct fw_sysfs *fw_sysfs)
> -{
> -	struct fw_priv *fw_priv = fw_sysfs->fw_priv;
> -
> -	__fw_load_abort(fw_priv);
> -}
> -
>   static LIST_HEAD(pending_fw_head);
>   
>   void kill_pending_fw_fallback_reqs(bool only_kill_custom)
> @@ -120,376 +60,6 @@ void kill_pending_fw_fallback_reqs(bool only_kill_custom)
>   	mutex_unlock(&fw_lock);
>   }
>   
> -static ssize_t timeout_show(struct class *class, struct class_attribute *attr,
> -			    char *buf)
> -{
> -	return sysfs_emit(buf, "%d\n", __firmware_loading_timeout());
> -}
> -
> -/**
> - * timeout_store() - set number of seconds to wait for firmware
> - * @class: device class pointer
> - * @attr: device attribute pointer
> - * @buf: buffer to scan for timeout value
> - * @count: number of bytes in @buf
> - *
> - *	Sets the number of seconds to wait for the firmware.  Once
> - *	this expires an error will be returned to the driver and no
> - *	firmware will be provided.
> - *
> - *	Note: zero means 'wait forever'.
> - **/
> -static ssize_t timeout_store(struct class *class, struct class_attribute *attr,
> -			     const char *buf, size_t count)
> -{
> -	int tmp_loading_timeout = simple_strtol(buf, NULL, 10);
> -
> -	if (tmp_loading_timeout < 0)
> -		tmp_loading_timeout = 0;
> -
> -	__fw_fallback_set_timeout(tmp_loading_timeout);
> -
> -	return count;
> -}
> -static CLASS_ATTR_RW(timeout);
> -
> -static struct attribute *firmware_class_attrs[] = {
> -	&class_attr_timeout.attr,
> -	NULL,
> -};
> -ATTRIBUTE_GROUPS(firmware_class);
> -
> -static void fw_dev_release(struct device *dev)
> -{
> -	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
> -
> -	kfree(fw_sysfs);
> -}
> -
> -static int do_firmware_uevent(struct fw_sysfs *fw_sysfs, struct kobj_uevent_env *env)
> -{
> -	if (add_uevent_var(env, "FIRMWARE=%s", fw_sysfs->fw_priv->fw_name))
> -		return -ENOMEM;
> -	if (add_uevent_var(env, "TIMEOUT=%i", __firmware_loading_timeout()))
> -		return -ENOMEM;
> -	if (add_uevent_var(env, "ASYNC=%d", fw_sysfs->nowait))
> -		return -ENOMEM;
> -
> -	return 0;
> -}
> -
> -static int firmware_uevent(struct device *dev, struct kobj_uevent_env *env)
> -{
> -	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
> -	int err = 0;
> -
> -	mutex_lock(&fw_lock);
> -	if (fw_sysfs->fw_priv)
> -		err = do_firmware_uevent(fw_sysfs, env);
> -	mutex_unlock(&fw_lock);
> -	return err;
> -}
> -
> -static struct class firmware_class = {
> -	.name		= "firmware",
> -	.class_groups	= firmware_class_groups,
> -	.dev_uevent	= firmware_uevent,
> -	.dev_release	= fw_dev_release,
> -};
> -
> -int register_sysfs_loader(void)
> -{
> -	int ret = class_register(&firmware_class);
> -
> -	if (ret != 0)
> -		return ret;
> -	return register_firmware_config_sysctl();
> -}
> -
> -void unregister_sysfs_loader(void)
> -{
> -	unregister_firmware_config_sysctl();
> -	class_unregister(&firmware_class);
> -}
> -
> -static ssize_t firmware_loading_show(struct device *dev,
> -				     struct device_attribute *attr, char *buf)
> -{
> -	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
> -	int loading = 0;
> -
> -	mutex_lock(&fw_lock);
> -	if (fw_sysfs->fw_priv)
> -		loading = fw_sysfs_loading(fw_sysfs->fw_priv);
> -	mutex_unlock(&fw_lock);
> -
> -	return sysfs_emit(buf, "%d\n", loading);
> -}
> -
> -/**
> - * firmware_loading_store() - set value in the 'loading' control file
> - * @dev: device pointer
> - * @attr: device attribute pointer
> - * @buf: buffer to scan for loading control value
> - * @count: number of bytes in @buf
> - *
> - *	The relevant values are:
> - *
> - *	 1: Start a load, discarding any previous partial load.
> - *	 0: Conclude the load and hand the data to the driver code.
> - *	-1: Conclude the load with an error and discard any written data.
> - **/
> -static ssize_t firmware_loading_store(struct device *dev,
> -				      struct device_attribute *attr,
> -				      const char *buf, size_t count)
> -{
> -	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
> -	struct fw_priv *fw_priv;
> -	ssize_t written = count;
> -	int loading = simple_strtol(buf, NULL, 10);
> -
> -	mutex_lock(&fw_lock);
> -	fw_priv = fw_sysfs->fw_priv;
> -	if (fw_state_is_aborted(fw_priv) || fw_state_is_done(fw_priv))
> -		goto out;
> -
> -	switch (loading) {
> -	case 1:
> -		/* discarding any previous partial load */
> -		if (!fw_sysfs_done(fw_priv)) {
> -			fw_free_paged_buf(fw_priv);
> -			fw_state_start(fw_priv);
> -		}
> -		break;
> -	case 0:
> -		if (fw_sysfs_loading(fw_priv)) {
> -			int rc;
> -
> -			/*
> -			 * Several loading requests may be pending on
> -			 * one same firmware buf, so let all requests
> -			 * see the mapped 'buf->data' once the loading
> -			 * is completed.
> -			 * */
> -			rc = fw_map_paged_buf(fw_priv);
> -			if (rc)
> -				dev_err(dev, "%s: map pages failed\n",
> -					__func__);
> -			else
> -				rc = security_kernel_post_load_data(fw_priv->data,
> -						fw_priv->size,
> -						LOADING_FIRMWARE, "blob");
> -
> -			/*
> -			 * Same logic as fw_load_abort, only the DONE bit
> -			 * is ignored and we set ABORT only on failure.
> -			 */
> -			if (rc) {
> -				fw_state_aborted(fw_priv);
> -				written = rc;
> -			} else {
> -				fw_state_done(fw_priv);
> -			}
> -			break;
> -		}
> -		fallthrough;
> -	default:
> -		dev_err(dev, "%s: unexpected value (%d)\n", __func__, loading);
> -		fallthrough;
> -	case -1:
> -		fw_load_abort(fw_sysfs);
> -		break;
> -	}
> -out:
> -	mutex_unlock(&fw_lock);
> -	return written;
> -}
> -
> -static DEVICE_ATTR(loading, 0644, firmware_loading_show, firmware_loading_store);
> -
> -static void firmware_rw_data(struct fw_priv *fw_priv, char *buffer,
> -			   loff_t offset, size_t count, bool read)
> -{
> -	if (read)
> -		memcpy(buffer, fw_priv->data + offset, count);
> -	else
> -		memcpy(fw_priv->data + offset, buffer, count);
> -}
> -
> -static void firmware_rw(struct fw_priv *fw_priv, char *buffer,
> -			loff_t offset, size_t count, bool read)
> -{
> -	while (count) {
> -		void *page_data;
> -		int page_nr = offset >> PAGE_SHIFT;
> -		int page_ofs = offset & (PAGE_SIZE-1);
> -		int page_cnt = min_t(size_t, PAGE_SIZE - page_ofs, count);
> -
> -		page_data = kmap(fw_priv->pages[page_nr]);
> -
> -		if (read)
> -			memcpy(buffer, page_data + page_ofs, page_cnt);
> -		else
> -			memcpy(page_data + page_ofs, buffer, page_cnt);
> -
> -		kunmap(fw_priv->pages[page_nr]);
> -		buffer += page_cnt;
> -		offset += page_cnt;
> -		count -= page_cnt;
> -	}
> -}
> -
> -static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj,
> -				  struct bin_attribute *bin_attr,
> -				  char *buffer, loff_t offset, size_t count)
> -{
> -	struct device *dev = kobj_to_dev(kobj);
> -	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
> -	struct fw_priv *fw_priv;
> -	ssize_t ret_count;
> -
> -	mutex_lock(&fw_lock);
> -	fw_priv = fw_sysfs->fw_priv;
> -	if (!fw_priv || fw_sysfs_done(fw_priv)) {
> -		ret_count = -ENODEV;
> -		goto out;
> -	}
> -	if (offset > fw_priv->size) {
> -		ret_count = 0;
> -		goto out;
> -	}
> -	if (count > fw_priv->size - offset)
> -		count = fw_priv->size - offset;
> -
> -	ret_count = count;
> -
> -	if (fw_priv->data)
> -		firmware_rw_data(fw_priv, buffer, offset, count, true);
> -	else
> -		firmware_rw(fw_priv, buffer, offset, count, true);
> -
> -out:
> -	mutex_unlock(&fw_lock);
> -	return ret_count;
> -}
> -
> -static int fw_realloc_pages(struct fw_sysfs *fw_sysfs, int min_size)
> -{
> -	int err;
> -
> -	err = fw_grow_paged_buf(fw_sysfs->fw_priv,
> -				PAGE_ALIGN(min_size) >> PAGE_SHIFT);
> -	if (err)
> -		fw_load_abort(fw_sysfs);
> -	return err;
> -}
> -
> -/**
> - * firmware_data_write() - write method for firmware
> - * @filp: open sysfs file
> - * @kobj: kobject for the device
> - * @bin_attr: bin_attr structure
> - * @buffer: buffer being written
> - * @offset: buffer offset for write in total data store area
> - * @count: buffer size
> - *
> - *	Data written to the 'data' attribute will be later handed to
> - *	the driver as a firmware image.
> - **/
> -static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj,
> -				   struct bin_attribute *bin_attr,
> -				   char *buffer, loff_t offset, size_t count)
> -{
> -	struct device *dev = kobj_to_dev(kobj);
> -	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
> -	struct fw_priv *fw_priv;
> -	ssize_t retval;
> -
> -	if (!capable(CAP_SYS_RAWIO))
> -		return -EPERM;
> -
> -	mutex_lock(&fw_lock);
> -	fw_priv = fw_sysfs->fw_priv;
> -	if (!fw_priv || fw_sysfs_done(fw_priv)) {
> -		retval = -ENODEV;
> -		goto out;
> -	}
> -
> -	if (fw_priv->data) {
> -		if (offset + count > fw_priv->allocated_size) {
> -			retval = -ENOMEM;
> -			goto out;
> -		}
> -		firmware_rw_data(fw_priv, buffer, offset, count, false);
> -		retval = count;
> -	} else {
> -		retval = fw_realloc_pages(fw_sysfs, offset + count);
> -		if (retval)
> -			goto out;
> -
> -		retval = count;
> -		firmware_rw(fw_priv, buffer, offset, count, false);
> -	}
> -
> -	fw_priv->size = max_t(size_t, offset + count, fw_priv->size);
> -out:
> -	mutex_unlock(&fw_lock);
> -	return retval;
> -}
> -
> -static struct bin_attribute firmware_attr_data = {
> -	.attr = { .name = "data", .mode = 0644 },
> -	.size = 0,
> -	.read = firmware_data_read,
> -	.write = firmware_data_write,
> -};
> -
> -static struct attribute *fw_dev_attrs[] = {
> -	&dev_attr_loading.attr,
> -	NULL
> -};
> -
> -static struct bin_attribute *fw_dev_bin_attrs[] = {
> -	&firmware_attr_data,
> -	NULL
> -};
> -
> -static const struct attribute_group fw_dev_attr_group = {
> -	.attrs = fw_dev_attrs,
> -	.bin_attrs = fw_dev_bin_attrs,
> -};
> -
> -static const struct attribute_group *fw_dev_attr_groups[] = {
> -	&fw_dev_attr_group,
> -	NULL
> -};
> -
> -static struct fw_sysfs *
> -fw_create_instance(struct firmware *firmware, const char *fw_name,
> -		   struct device *device, u32 opt_flags)
> -{
> -	struct fw_sysfs *fw_sysfs;
> -	struct device *f_dev;
> -
> -	fw_sysfs = kzalloc(sizeof(*fw_sysfs), GFP_KERNEL);
> -	if (!fw_sysfs) {
> -		fw_sysfs = ERR_PTR(-ENOMEM);
> -		goto exit;
> -	}
> -
> -	fw_sysfs->nowait = !!(opt_flags & FW_OPT_NOWAIT);
> -	fw_sysfs->fw = firmware;
> -	f_dev = &fw_sysfs->dev;
> -
> -	device_initialize(f_dev);
> -	dev_set_name(f_dev, "%s", fw_name);
> -	f_dev->parent = device;
> -	f_dev->class = &firmware_class;
> -	f_dev->groups = fw_dev_attr_groups;
> -exit:
> -	return fw_sysfs;
> -}
> -
>   /**
>    * fw_load_sysfs_fallback() - load a firmware via the sysfs fallback mechanism
>    * @fw_sysfs: firmware sysfs information for the firmware to load
> diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h
> index 9f3055d3b4ca..144148595660 100644
> --- a/drivers/base/firmware_loader/fallback.h
> +++ b/drivers/base/firmware_loader/fallback.h
> @@ -6,29 +6,7 @@
>   #include <linux/device.h>
>   
>   #include "firmware.h"
> -
> -/**
> - * struct firmware_fallback_config - firmware fallback configuration settings
> - *
> - * Helps describe and fine tune the fallback mechanism.
> - *
> - * @force_sysfs_fallback: force the sysfs fallback mechanism to be used
> - * 	as if one had enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.
> - * 	Useful to help debug a CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y
> - * 	functionality on a kernel where that config entry has been disabled.
> - * @ignore_sysfs_fallback: force to disable the sysfs fallback mechanism.
> - * 	This emulates the behaviour as if we had set the kernel
> - * 	config CONFIG_FW_LOADER_USER_HELPER=n.
> - * @old_timeout: for internal use
> - * @loading_timeout: the timeout to wait for the fallback mechanism before
> - * 	giving up, in seconds.
> - */
> -struct firmware_fallback_config {
> -	unsigned int force_sysfs_fallback;
> -	unsigned int ignore_sysfs_fallback;
> -	int old_timeout;
> -	int loading_timeout;
> -};
> +#include "sysfs.h"
>   
>   #ifdef CONFIG_FW_LOADER_USER_HELPER
>   int firmware_fallback_sysfs(struct firmware *fw, const char *name,
> @@ -40,19 +18,6 @@ void kill_pending_fw_fallback_reqs(bool only_kill_custom);
>   void fw_fallback_set_cache_timeout(void);
>   void fw_fallback_set_default_timeout(void);
>   
> -int register_sysfs_loader(void);
> -void unregister_sysfs_loader(void);
> -#ifdef CONFIG_SYSCTL
> -extern int register_firmware_config_sysctl(void);
> -extern void unregister_firmware_config_sysctl(void);
> -#else
> -static inline int register_firmware_config_sysctl(void)
> -{
> -	return 0;
> -}
> -static inline void unregister_firmware_config_sysctl(void) { }
> -#endif /* CONFIG_SYSCTL */
> -
>   #else /* CONFIG_FW_LOADER_USER_HELPER */
>   static inline int firmware_fallback_sysfs(struct firmware *fw, const char *name,
>   					  struct device *device,
> @@ -66,15 +31,6 @@ static inline int firmware_fallback_sysfs(struct firmware *fw, const char *name,
>   static inline void kill_pending_fw_fallback_reqs(bool only_kill_custom) { }
>   static inline void fw_fallback_set_cache_timeout(void) { }
>   static inline void fw_fallback_set_default_timeout(void) { }
> -
> -static inline int register_sysfs_loader(void)
> -{
> -	return 0;
> -}
> -
> -static inline void unregister_sysfs_loader(void)
> -{
> -}
>   #endif /* CONFIG_FW_LOADER_USER_HELPER */
>   
>   #ifdef CONFIG_EFI_EMBEDDED_FIRMWARE
> diff --git a/drivers/base/firmware_loader/sysfs.c b/drivers/base/firmware_loader/sysfs.c
> new file mode 100644
> index 000000000000..49aeff45b123
> --- /dev/null
> +++ b/drivers/base/firmware_loader/sysfs.c
> @@ -0,0 +1,411 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/highmem.h>
> +#include <linux/module.h>
> +#include <linux/security.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#include "firmware.h"
> +#include "sysfs.h"
> +
> +/*
> + * sysfs support for firmware loader
> + */
> +
> +static inline bool fw_sysfs_done(struct fw_priv *fw_priv)
> +{
> +	return __fw_state_check(fw_priv, FW_STATUS_DONE);
> +}
> +
> +static inline bool fw_sysfs_loading(struct fw_priv *fw_priv)
> +{
> +	return __fw_state_check(fw_priv, FW_STATUS_LOADING);
> +}
> +
> +void __fw_load_abort(struct fw_priv *fw_priv)
> +{
> +	/*
> +	 * There is a small window in which user can write to 'loading'
> +	 * between loading done/aborted and disappearance of 'loading'
> +	 */
> +	if (fw_state_is_aborted(fw_priv) || fw_sysfs_done(fw_priv))
> +		return;
> +
> +	fw_state_aborted(fw_priv);
> +}
> +
> +static ssize_t timeout_show(struct class *class, struct class_attribute *attr,
> +			    char *buf)
> +{
> +	return sysfs_emit(buf, "%d\n", __firmware_loading_timeout());
> +}
> +
> +/**
> + * timeout_store() - set number of seconds to wait for firmware
> + * @class: device class pointer
> + * @attr: device attribute pointer
> + * @buf: buffer to scan for timeout value
> + * @count: number of bytes in @buf
> + *
> + *	Sets the number of seconds to wait for the firmware.  Once
> + *	this expires an error will be returned to the driver and no
> + *	firmware will be provided.
> + *
> + *	Note: zero means 'wait forever'.
> + **/
> +static ssize_t timeout_store(struct class *class, struct class_attribute *attr,
> +			     const char *buf, size_t count)
> +{
> +	int tmp_loading_timeout = simple_strtol(buf, NULL, 10);
> +
> +	if (tmp_loading_timeout < 0)
> +		tmp_loading_timeout = 0;
> +
> +	__fw_fallback_set_timeout(tmp_loading_timeout);
> +
> +	return count;
> +}
> +static CLASS_ATTR_RW(timeout);
> +
> +static struct attribute *firmware_class_attrs[] = {
> +	&class_attr_timeout.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(firmware_class);
> +
> +static void fw_dev_release(struct device *dev)
> +{
> +	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
> +
> +	kfree(fw_sysfs);
> +}
> +
> +#ifdef CONFIG_FW_LOADER_USER_HELPER
> +static int do_firmware_uevent(struct fw_sysfs *fw_sysfs, struct kobj_uevent_env *env)
> +{
> +	if (add_uevent_var(env, "FIRMWARE=%s", fw_sysfs->fw_priv->fw_name))
> +		return -ENOMEM;
> +	if (add_uevent_var(env, "TIMEOUT=%i", __firmware_loading_timeout()))
> +		return -ENOMEM;
> +	if (add_uevent_var(env, "ASYNC=%d", fw_sysfs->nowait))
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +static int firmware_uevent(struct device *dev, struct kobj_uevent_env *env)
> +{
> +	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
> +	int err = 0;
> +
> +	mutex_lock(&fw_lock);
> +	if (fw_sysfs->fw_priv)
> +		err = do_firmware_uevent(fw_sysfs, env);
> +	mutex_unlock(&fw_lock);
> +	return err;
> +}
> +#endif /* CONFIG_FW_LOADER_USER_HELPER */
> +
> +static struct class firmware_class = {
> +	.name		= "firmware",
> +	.class_groups	= firmware_class_groups,
> +#ifdef CONFIG_FW_LOADER_USER_HELPER
> +	.dev_uevent	= firmware_uevent,
> +#endif
> +	.dev_release	= fw_dev_release,
> +};
> +
> +int register_sysfs_loader(void)
> +{
> +	int ret = class_register(&firmware_class);
> +
> +	if (ret != 0)
> +		return ret;
> +	return register_firmware_config_sysctl();
> +}
> +
> +void unregister_sysfs_loader(void)
> +{
> +	unregister_firmware_config_sysctl();
> +	class_unregister(&firmware_class);
> +}
> +
> +static ssize_t firmware_loading_show(struct device *dev,
> +				     struct device_attribute *attr, char *buf)
> +{
> +	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
> +	int loading = 0;
> +
> +	mutex_lock(&fw_lock);
> +	if (fw_sysfs->fw_priv)
> +		loading = fw_sysfs_loading(fw_sysfs->fw_priv);
> +	mutex_unlock(&fw_lock);
> +
> +	return sysfs_emit(buf, "%d\n", loading);
> +}
> +
> +/**
> + * firmware_loading_store() - set value in the 'loading' control file
> + * @dev: device pointer
> + * @attr: device attribute pointer
> + * @buf: buffer to scan for loading control value
> + * @count: number of bytes in @buf
> + *
> + *	The relevant values are:
> + *
> + *	 1: Start a load, discarding any previous partial load.
> + *	 0: Conclude the load and hand the data to the driver code.
> + *	-1: Conclude the load with an error and discard any written data.
> + **/
> +static ssize_t firmware_loading_store(struct device *dev,
> +				      struct device_attribute *attr,
> +				      const char *buf, size_t count)
> +{
> +	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
> +	struct fw_priv *fw_priv;
> +	ssize_t written = count;
> +	int loading = simple_strtol(buf, NULL, 10);
> +
> +	mutex_lock(&fw_lock);
> +	fw_priv = fw_sysfs->fw_priv;
> +	if (fw_state_is_aborted(fw_priv) || fw_state_is_done(fw_priv))
> +		goto out;
> +
> +	switch (loading) {
> +	case 1:
> +		/* discarding any previous partial load */
> +		if (!fw_sysfs_done(fw_priv)) {
> +			fw_free_paged_buf(fw_priv);
> +			fw_state_start(fw_priv);
> +		}
> +		break;
> +	case 0:
> +		if (fw_sysfs_loading(fw_priv)) {
> +			int rc;
> +
> +			/*
> +			 * Several loading requests may be pending on
> +			 * one same firmware buf, so let all requests
> +			 * see the mapped 'buf->data' once the loading
> +			 * is completed.
> +			 */
> +			rc = fw_map_paged_buf(fw_priv);
> +			if (rc)
> +				dev_err(dev, "%s: map pages failed\n",
> +					__func__);
> +			else
> +				rc = security_kernel_post_load_data(fw_priv->data,
> +								    fw_priv->size,
> +								    LOADING_FIRMWARE,
> +								    "blob");
> +
> +			/*
> +			 * Same logic as fw_load_abort, only the DONE bit
> +			 * is ignored and we set ABORT only on failure.
> +			 */
> +			if (rc) {
> +				fw_state_aborted(fw_priv);
> +				written = rc;
> +			} else {
> +				fw_state_done(fw_priv);
> +			}
> +			break;
> +		}
> +		fallthrough;
> +	default:
> +		dev_err(dev, "%s: unexpected value (%d)\n", __func__, loading);
> +		fallthrough;
> +	case -1:
> +		fw_load_abort(fw_sysfs);
> +		break;
> +	}
> +out:
> +	mutex_unlock(&fw_lock);
> +	return written;
> +}
> +
> +static DEVICE_ATTR(loading, 0644, firmware_loading_show, firmware_loading_store);
> +
> +static void firmware_rw_data(struct fw_priv *fw_priv, char *buffer,
> +			     loff_t offset, size_t count, bool read)
> +{
> +	if (read)
> +		memcpy(buffer, fw_priv->data + offset, count);
> +	else
> +		memcpy(fw_priv->data + offset, buffer, count);
> +}
> +
> +static void firmware_rw(struct fw_priv *fw_priv, char *buffer,
> +			loff_t offset, size_t count, bool read)
> +{
> +	while (count) {
> +		void *page_data;
> +		int page_nr = offset >> PAGE_SHIFT;
> +		int page_ofs = offset & (PAGE_SIZE - 1);
> +		int page_cnt = min_t(size_t, PAGE_SIZE - page_ofs, count);
> +
> +		page_data = kmap(fw_priv->pages[page_nr]);
> +
> +		if (read)
> +			memcpy(buffer, page_data + page_ofs, page_cnt);
> +		else
> +			memcpy(page_data + page_ofs, buffer, page_cnt);
> +
> +		kunmap(fw_priv->pages[page_nr]);
> +		buffer += page_cnt;
> +		offset += page_cnt;
> +		count -= page_cnt;
> +	}
> +}
> +
> +static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj,
> +				  struct bin_attribute *bin_attr,
> +				  char *buffer, loff_t offset, size_t count)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
> +	struct fw_priv *fw_priv;
> +	ssize_t ret_count;
> +
> +	mutex_lock(&fw_lock);
> +	fw_priv = fw_sysfs->fw_priv;
> +	if (!fw_priv || fw_sysfs_done(fw_priv)) {
> +		ret_count = -ENODEV;
> +		goto out;
> +	}
> +	if (offset > fw_priv->size) {
> +		ret_count = 0;
> +		goto out;
> +	}
> +	if (count > fw_priv->size - offset)
> +		count = fw_priv->size - offset;
> +
> +	ret_count = count;
> +
> +	if (fw_priv->data)
> +		firmware_rw_data(fw_priv, buffer, offset, count, true);
> +	else
> +		firmware_rw(fw_priv, buffer, offset, count, true);
> +
> +out:
> +	mutex_unlock(&fw_lock);
> +	return ret_count;
> +}
> +
> +static int fw_realloc_pages(struct fw_sysfs *fw_sysfs, int min_size)
> +{
> +	int err;
> +
> +	err = fw_grow_paged_buf(fw_sysfs->fw_priv,
> +				PAGE_ALIGN(min_size) >> PAGE_SHIFT);
> +	if (err)
> +		fw_load_abort(fw_sysfs);
> +	return err;
> +}
> +
> +/**
> + * firmware_data_write() - write method for firmware
> + * @filp: open sysfs file
> + * @kobj: kobject for the device
> + * @bin_attr: bin_attr structure
> + * @buffer: buffer being written
> + * @offset: buffer offset for write in total data store area
> + * @count: buffer size
> + *
> + *	Data written to the 'data' attribute will be later handed to
> + *	the driver as a firmware image.
> + **/
> +static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj,
> +				   struct bin_attribute *bin_attr,
> +				   char *buffer, loff_t offset, size_t count)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
> +	struct fw_priv *fw_priv;
> +	ssize_t retval;
> +
> +	if (!capable(CAP_SYS_RAWIO))
> +		return -EPERM;
> +
> +	mutex_lock(&fw_lock);
> +	fw_priv = fw_sysfs->fw_priv;
> +	if (!fw_priv || fw_sysfs_done(fw_priv)) {
> +		retval = -ENODEV;
> +		goto out;
> +	}
> +
> +	if (fw_priv->data) {
> +		if (offset + count > fw_priv->allocated_size) {
> +			retval = -ENOMEM;
> +			goto out;
> +		}
> +		firmware_rw_data(fw_priv, buffer, offset, count, false);
> +		retval = count;
> +	} else {
> +		retval = fw_realloc_pages(fw_sysfs, offset + count);
> +		if (retval)
> +			goto out;
> +
> +		retval = count;
> +		firmware_rw(fw_priv, buffer, offset, count, false);
> +	}
> +
> +	fw_priv->size = max_t(size_t, offset + count, fw_priv->size);
> +out:
> +	mutex_unlock(&fw_lock);
> +	return retval;
> +}
> +
> +static struct bin_attribute firmware_attr_data = {
> +	.attr = { .name = "data", .mode = 0644 },
> +	.size = 0,
> +	.read = firmware_data_read,
> +	.write = firmware_data_write,
> +};
> +
> +static struct attribute *fw_dev_attrs[] = {
> +	&dev_attr_loading.attr,
> +	NULL
> +};
> +
> +static struct bin_attribute *fw_dev_bin_attrs[] = {
> +	&firmware_attr_data,
> +	NULL
> +};
> +
> +static const struct attribute_group fw_dev_attr_group = {
> +	.attrs = fw_dev_attrs,
> +	.bin_attrs = fw_dev_bin_attrs,
> +};
> +
> +static const struct attribute_group *fw_dev_attr_groups[] = {
> +	&fw_dev_attr_group,
> +	NULL
> +};
> +
> +struct fw_sysfs *
> +fw_create_instance(struct firmware *firmware, const char *fw_name,
> +		   struct device *device, u32 opt_flags)
> +{
> +	struct fw_sysfs *fw_sysfs;
> +	struct device *f_dev;
> +
> +	fw_sysfs = kzalloc(sizeof(*fw_sysfs), GFP_KERNEL);
> +	if (!fw_sysfs) {
> +		fw_sysfs = ERR_PTR(-ENOMEM);
> +		goto exit;
> +	}
> +
> +	fw_sysfs->nowait = !!(opt_flags & FW_OPT_NOWAIT);
> +	fw_sysfs->fw = firmware;
> +	f_dev = &fw_sysfs->dev;
> +
> +	device_initialize(f_dev);
> +	dev_set_name(f_dev, "%s", fw_name);
> +	f_dev->parent = device;
> +	f_dev->class = &firmware_class;
> +	f_dev->groups = fw_dev_attr_groups;
> +exit:
> +	return fw_sysfs;
> +}
> diff --git a/drivers/base/firmware_loader/sysfs.h b/drivers/base/firmware_loader/sysfs.h
> new file mode 100644
> index 000000000000..5e2aff7bf6e7
> --- /dev/null
> +++ b/drivers/base/firmware_loader/sysfs.h
> @@ -0,0 +1,96 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __FIRMWARE_SYSFS_H
> +#define __FIRMWARE_SYSFS_H
> +
> +#include <linux/device.h>
> +
> +MODULE_IMPORT_NS(FIRMWARE_LOADER_PRIVATE);
> +
> +extern struct firmware_fallback_config fw_fallback_config;
> +
> +#ifdef CONFIG_FW_LOADER_USER_HELPER
> +/**
> + * struct firmware_fallback_config - firmware fallback configuration settings
> + *
> + * Helps describe and fine tune the fallback mechanism.
> + *
> + * @force_sysfs_fallback: force the sysfs fallback mechanism to be used
> + *	as if one had enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.
> + *	Useful to help debug a CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y
> + *	functionality on a kernel where that config entry has been disabled.
> + * @ignore_sysfs_fallback: force to disable the sysfs fallback mechanism.
> + *	This emulates the behaviour as if we had set the kernel
> + *	config CONFIG_FW_LOADER_USER_HELPER=n.
> + * @old_timeout: for internal use
> + * @loading_timeout: the timeout to wait for the fallback mechanism before
> + *	giving up, in seconds.
> + */
> +struct firmware_fallback_config {
> +	unsigned int force_sysfs_fallback;
> +	unsigned int ignore_sysfs_fallback;
> +	int old_timeout;
> +	int loading_timeout;
> +};
> +
> +int register_sysfs_loader(void);
> +void unregister_sysfs_loader(void);
> +#ifdef CONFIG_SYSCTL
> +int register_firmware_config_sysctl(void);
> +void unregister_firmware_config_sysctl(void);
> +#else
> +static inline int register_firmware_config_sysctl(void)
> +{
> +	return 0;
> +}
> +
> +static inline void unregister_firmware_config_sysctl(void) { }
> +#endif /* CONFIG_SYSCTL */
> +#else /* CONFIG_FW_LOADER_USER_HELPER */
> +static inline int register_sysfs_loader(void)
> +{
> +	return 0;
> +}
> +
> +static inline void unregister_sysfs_loader(void)
> +{
> +}
> +#endif /* CONFIG_FW_LOADER_USER_HELPER */
> +
> +struct fw_sysfs {
> +	bool nowait;
> +	struct device dev;
> +	struct fw_priv *fw_priv;
> +	struct firmware *fw;
> +};
> +
> +static inline struct fw_sysfs *to_fw_sysfs(struct device *dev)
> +{
> +	return container_of(dev, struct fw_sysfs, dev);
> +}
> +
> +/* These getters are vetted to use int properly */
> +static inline int __firmware_loading_timeout(void)
> +{
> +	return fw_fallback_config.loading_timeout;
> +}
> +
> +/* These setters are vetted to use int properly */
> +static inline void __fw_fallback_set_timeout(int timeout)
> +{
> +	fw_fallback_config.loading_timeout = timeout;
> +}
> +
> +void __fw_load_abort(struct fw_priv *fw_priv);
> +
> +static inline void fw_load_abort(struct fw_sysfs *fw_sysfs)
> +{
> +	struct fw_priv *fw_priv = fw_sysfs->fw_priv;
> +
> +	__fw_load_abort(fw_priv);
> +}
> +
> +struct fw_sysfs *
> +fw_create_instance(struct firmware *firmware, const char *fw_name,
> +		   struct device *device, u32 opt_flags);
> +
> +#endif /* __FIRMWARE_SYSFS_H */

Re: [RESEND PATCH v1 2/8] firmware_loader: Check fw_state_is_done in loading_store

[Russ Weight]

On 4/2/22 07:47, Tom Rix wrote:
>
> On 3/23/22 4:33 PM, Russ Weight wrote:
>> Add the fw_state_is_done() function and exit early from
>> firmware_loading_store() if the state is already "done". This is being done
>> in preparation for supporting persistent sysfs nodes to allow userspace to
>> upload firmware to a device, potentially reusing the sysfs loading and data
>> files multiple times.
>>
>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
>> ---
>> v1:
>>    - No change from RFC patch
>> ---
>>   drivers/base/firmware_loader/fallback.c | 2 +-
>>   drivers/base/firmware_loader/firmware.h | 5 +++++
>>   2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
>> index 4afb0e9312c0..d82e055a4297 100644
>> --- a/drivers/base/firmware_loader/fallback.c
>> +++ b/drivers/base/firmware_loader/fallback.c
>> @@ -250,7 +250,7 @@ static ssize_t firmware_loading_store(struct device *dev,
>>         mutex_lock(&fw_lock);
>>       fw_priv = fw_sysfs->fw_priv;
>> -    if (fw_state_is_aborted(fw_priv))
>> +    if (fw_state_is_aborted(fw_priv) || fw_state_is_done(fw_priv))
>>           goto out;
>>         switch (loading) {
>> diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
>> index 2889f446ad41..58768d16f8df 100644
>> --- a/drivers/base/firmware_loader/firmware.h
>> +++ b/drivers/base/firmware_loader/firmware.h
>> @@ -149,6 +149,11 @@ static inline void fw_state_done(struct fw_priv *fw_priv)
>>       __fw_state_set(fw_priv, FW_STATUS_DONE);
>>   }
>>   +static inline bool fw_state_is_done(struct fw_priv *fw_priv)
>> +{
>> +    return __fw_state_check(fw_priv, FW_STATUS_DONE);
>
> This looks like the fw_sysfs_done() in fallback.c
>
> IMO this one and similar wrappers should move to firmware.h and use the *_is_* naming.

Thanks for catching that Tom. Yes, the new fw_state_is_done() function is exactly
the same as fw_sysfs_done(). I'll follow your suggestion and move fw_sysfs_done()
and fw_sysfs_loading() to firmware.h as fw_state_is_done() and fw_state_is_loading().

- Russ
>
> Tom
>
>> +}
>> +
>>   int assign_fw(struct firmware *fw, struct device *device);
>>     #ifdef CONFIG_FW_LOADER
>

Re: [RESEND PATCH v1 3/8] firmware_loader: Split sysfs support from fallback

[Russ Weight]

On 4/2/22 08:15, Tom Rix wrote:
>
> On 3/23/22 4:33 PM, Russ Weight wrote:
>> In preparation for sharing the "loading" and "data" sysfs nodes with the
>> new firmware upload support, split out sysfs functionality from fallback.c
>> and fallback.h into sysfs.c and sysfs.h. This includes the firmware
>> class driver code that is associated with the sysfs files and the
>> fw_fallback_config support for the timeout sysfs node.
>>
>> CONFIG_FW_LOADER_SYSFS is created and is selected by
>> CONFIG_FW_LOADER_USER_HELPER in order to include sysfs.o in
>> firmware_class-objs.
>>
>> This is mostly just a code reorganization. There are a few symbols that
>> change in scope, and these can be identified by looking at the header
>> file changes. A few white-space warnings from checkpatch are also
>> addressed in this patch.
>>
>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
>> ---
>> v1:
>>    - Renamed files fw_sysfs.c and fw_sysfs.h to sysfs.c and sysfs.h
>>    - Moved "MODULE_IMPORT_NS(FIRMWARE_LOADER_PRIVATE);" from sysfs.c to
>>      sysfs.h to address an error identified by the kernel test robot
>>      <lkp@intel.com>
>> ---
>>   drivers/base/firmware_loader/Kconfig    |   4 +
>>   drivers/base/firmware_loader/Makefile   |   1 +
>>   drivers/base/firmware_loader/fallback.c | 430 ------------------------
>>   drivers/base/firmware_loader/fallback.h |  46 +--
>>   drivers/base/firmware_loader/sysfs.c    | 411 ++++++++++++++++++++++
>>   drivers/base/firmware_loader/sysfs.h    |  96 ++++++
>>   6 files changed, 513 insertions(+), 475 deletions(-)
>>   create mode 100644 drivers/base/firmware_loader/sysfs.c
>>   create mode 100644 drivers/base/firmware_loader/sysfs.h
>>
>> diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
>> index 38f3b66bf52b..9e03178eee00 100644
>> --- a/drivers/base/firmware_loader/Kconfig
>> +++ b/drivers/base/firmware_loader/Kconfig
>> @@ -29,6 +29,9 @@ if FW_LOADER
>>   config FW_LOADER_PAGED_BUF
>>       bool
>>   +config FW_LOADER_SYSFS
>> +    bool
>> +
>>   config EXTRA_FIRMWARE
>>       string "Build named firmware blobs into the kernel binary"
>>       help
>> @@ -72,6 +75,7 @@ config EXTRA_FIRMWARE_DIR
>>     config FW_LOADER_USER_HELPER
>>       bool "Enable the firmware sysfs fallback mechanism"
>> +    select FW_LOADER_SYSFS
>
> Is this code reordering necessary ?
>
> This config is not removed or renamed later and has the same configs are the later FW_UPLOAD.

FW_UPLOAD and FW_LOADER_USER_HELPER both select FW_LOADER_SYSFS and
FW_LOADER_PAGED_BUF, but they aren't exactly the same. With the split,
you can have fallback.c without compiling sysfs_upload.c and you can
have sysfs_upload.c without compiling fallback.c.

>
> Maybe leave fallback.c as-is and rename FW_LOADER_USER_HELPER to FW_LOADER_SYSFS because the name is more descriptive.
>
> The 'sorry we suck' help message replaced with a shorter message to indicate this is now a more capable config.
>
> The later FW_UPLOAD would have a 'depends on FW_LOADER_SYSFS'

I could, of course, leave fallback.c as is and add to it, but then you
couldn't compile in the firmware-upload support without also including
the firmware-fallback support, and vice versa. That seems like a
disadvantage to me. Isn't it better to allow a user to select these
features separately but have them share code if they are both selected?

>
> If you end up needing to do the reorder, move it to patch 1 because bisecting-wise it should not depend on improvements in the current 1,2 patches.

Patches 1 & 2 are standalone patches. They could be accepted
independently without implementing firmware-upload. They don't depend
on the code reorganization, and in my opinion, they are easier to
understand when viewed before the code reorganization.

What do other's think? Should I keep all the code in fallback.c? Or is
it better to split out the code? If I keep the split, should I move
patches 1 & 2 to after the split?

- Russ

>
> Tom
>
>>       select FW_LOADER_PAGED_BUF
>>       help
>>         This option enables a sysfs loading facility to enable firmware