* [RFC PATCH 1/5] firmware_loader: Clear data and size in fw_free_paged_buf
2022-02-03 21:30 [RFC PATCH 0/5] Extend FW framework for user FW uploads Russ Weight
@ 2022-02-03 21:30 ` Russ Weight
2022-02-03 21:30 ` [RFC PATCH 2/5] firmware_loader: Check fw_state_is_done in loading_store Russ Weight
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Russ Weight @ 2022-02-03 21:30 UTC (permalink / raw)
To: mcgrof, gregkh, rafael, linux-kernel
Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach,
basheer.ahmed.muddebihal, tianfei.zhang, 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>
---
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
^ permalink raw reply [flat|nested] 14+ messages in thread* [RFC PATCH 2/5] firmware_loader: Check fw_state_is_done in loading_store
2022-02-03 21:30 [RFC PATCH 0/5] Extend FW framework for user FW uploads Russ Weight
2022-02-03 21:30 ` [RFC PATCH 1/5] firmware_loader: Clear data and size in fw_free_paged_buf Russ Weight
@ 2022-02-03 21:30 ` Russ Weight
2022-02-03 21:30 ` [RFC PATCH 3/5] firmware_loader: Split fw_sysfs support from fallback Russ Weight
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Russ Weight @ 2022-02-03 21:30 UTC (permalink / raw)
To: mcgrof, gregkh, rafael, linux-kernel
Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach,
basheer.ahmed.muddebihal, tianfei.zhang, 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>
---
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
^ permalink raw reply [flat|nested] 14+ messages in thread* [RFC PATCH 3/5] firmware_loader: Split fw_sysfs support from fallback
2022-02-03 21:30 [RFC PATCH 0/5] Extend FW framework for user FW uploads Russ Weight
2022-02-03 21:30 ` [RFC PATCH 1/5] firmware_loader: Clear data and size in fw_free_paged_buf Russ Weight
2022-02-03 21:30 ` [RFC PATCH 2/5] firmware_loader: Check fw_state_is_done in loading_store Russ Weight
@ 2022-02-03 21:30 ` Russ Weight
2022-02-03 22:51 ` Luis Chamberlain
2022-02-06 15:43 ` Tom Rix
2022-02-03 21:30 ` [RFC PATCH 4/5] firmware_loader: Add firmware-upload support Russ Weight
` (2 subsequent siblings)
5 siblings, 2 replies; 14+ messages in thread
From: Russ Weight @ 2022-02-03 21:30 UTC (permalink / raw)
To: mcgrof, gregkh, rafael, linux-kernel
Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach,
basheer.ahmed.muddebihal, tianfei.zhang, 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 fw_sysfs.c and fw_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 fw_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>
---
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/fw_sysfs.c | 413 +++++++++++++++++++++++
drivers/base/firmware_loader/fw_sysfs.h | 94 ++++++
6 files changed, 513 insertions(+), 475 deletions(-)
create mode 100644 drivers/base/firmware_loader/fw_sysfs.c
create mode 100644 drivers/base/firmware_loader/fw_sysfs.h
diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
index 5b24f3959255..1bfe18900ed5 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
@@ -70,6 +73,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..787c833d0c6e 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) += fw_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..52adc5a0bf52 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 "fw_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/fw_sysfs.c b/drivers/base/firmware_loader/fw_sysfs.c
new file mode 100644
index 000000000000..70cb1d67ffb2
--- /dev/null
+++ b/drivers/base/firmware_loader/fw_sysfs.c
@@ -0,0 +1,413 @@
+// 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 "fw_sysfs.h"
+
+/*
+ * sysfs support for firmware loader
+ */
+
+MODULE_IMPORT_NS(FIRMWARE_LOADER_PRIVATE);
+
+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/fw_sysfs.h b/drivers/base/firmware_loader/fw_sysfs.h
new file mode 100644
index 000000000000..0ca7f72892b0
--- /dev/null
+++ b/drivers/base/firmware_loader/fw_sysfs.h
@@ -0,0 +1,94 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __FIRMWARE_SYSFS_H
+#define __FIRMWARE_SYSFS_H
+
+#include <linux/device.h>
+
+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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH 3/5] firmware_loader: Split fw_sysfs support from fallback
2022-02-03 21:30 ` [RFC PATCH 3/5] firmware_loader: Split fw_sysfs support from fallback Russ Weight
@ 2022-02-03 22:51 ` Luis Chamberlain
2022-02-11 1:50 ` Russ Weight
2022-02-06 15:43 ` Tom Rix
1 sibling, 1 reply; 14+ messages in thread
From: Luis Chamberlain @ 2022-02-03 22:51 UTC (permalink / raw)
To: Russ Weight
Cc: gregkh, rafael, linux-kernel, trix, lgoncalv, yilun.xu, hao.wu,
matthew.gerlach, basheer.ahmed.muddebihal, tianfei.zhang
On Thu, Feb 03, 2022 at 02:30:50PM -0700, 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 fw_sysfs.c and fw_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 fw_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>
> ---
> 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/fw_sysfs.c | 413 +++++++++++++++++++++++
> drivers/base/firmware_loader/fw_sysfs.h | 94 ++++++
Nit: please just use sysfs.[ch]. The directory already implies its for
firmware.
Luis
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 3/5] firmware_loader: Split fw_sysfs support from fallback
2022-02-03 22:51 ` Luis Chamberlain
@ 2022-02-11 1:50 ` Russ Weight
0 siblings, 0 replies; 14+ messages in thread
From: Russ Weight @ 2022-02-11 1:50 UTC (permalink / raw)
To: Luis Chamberlain
Cc: gregkh, rafael, linux-kernel, trix, lgoncalv, yilun.xu, hao.wu,
matthew.gerlach, basheer.ahmed.muddebihal, tianfei.zhang
On 2/3/22 2:51 PM, Luis Chamberlain wrote:
> On Thu, Feb 03, 2022 at 02:30:50PM -0700, 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 fw_sysfs.c and fw_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 fw_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>
>> ---
>> 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/fw_sysfs.c | 413 +++++++++++++++++++++++
>> drivers/base/firmware_loader/fw_sysfs.h | 94 ++++++
> Nit: please just use sysfs.[ch]. The directory already implies its for
> firmware.
Sure. I'm assuming the same comment would apply to fw_upload.[ch]
int he following patch, so I'll remove fw_ for those as well.
- Russ
>
> Luis
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 3/5] firmware_loader: Split fw_sysfs support from fallback
2022-02-03 21:30 ` [RFC PATCH 3/5] firmware_loader: Split fw_sysfs support from fallback Russ Weight
2022-02-03 22:51 ` Luis Chamberlain
@ 2022-02-06 15:43 ` Tom Rix
2022-02-11 2:24 ` Russ Weight
1 sibling, 1 reply; 14+ messages in thread
From: Tom Rix @ 2022-02-06 15:43 UTC (permalink / raw)
To: Russ Weight, mcgrof, gregkh, rafael, linux-kernel
Cc: lgoncalv, yilun.xu, hao.wu, matthew.gerlach,
basheer.ahmed.muddebihal, tianfei.zhang
On 2/3/22 1:30 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 fw_sysfs.c and fw_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 fw_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>
Consider how bisectable and reviewable this patch is.
I think this patch should be first since reorganizing should not depend
on anything new.
Other users of fallback will want to know if this change is going to
break them, so the reorganization should be as mechanical as possible to
make the review as easy as possible and easy to revert if something goes
wrong. Ex/ the whitespace changes made as a separate patch to the old
file, not embedded in the new file. The new Kconfig added later if needed.
Also consider if this move is needed at all, generalizing in the
existing file is ok.
Tom
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 3/5] firmware_loader: Split fw_sysfs support from fallback
2022-02-06 15:43 ` Tom Rix
@ 2022-02-11 2:24 ` Russ Weight
0 siblings, 0 replies; 14+ messages in thread
From: Russ Weight @ 2022-02-11 2:24 UTC (permalink / raw)
To: Tom Rix, mcgrof, gregkh, rafael, linux-kernel
Cc: lgoncalv, yilun.xu, hao.wu, matthew.gerlach,
basheer.ahmed.muddebihal, tianfei.zhang
On 2/6/22 7:43 AM, Tom Rix wrote:
>
> On 2/3/22 1:30 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 fw_sysfs.c and fw_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 fw_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>
>
> Consider how bisectable and reviewable this patch is.
>
> I think this patch should be first since reorganizing should not depend on anything new.
Hi Tom,
The first two patches are very small and stand on there own. I put them first because they could be taken independent of the rest of the patches.
>
> Other users of fallback will want to know if this change is going to break them, so the reorganization should be as mechanical as possible to make the review as easy as possible and easy to revert if something goes wrong. Ex/ the whitespace changes made as a separate patch to the old file, not embedded in the new file. The new Kconfig added later if needed.
>
> Also consider if this move is needed at all, generalizing in the existing file is ok.
I did give this a lot of consideration. I'm open to reorganizing the code if needed. These are some of the things I considered:
The context of a user-initiated firmware upload is quite different from firmware-fallback. I want to reuse the sysfs support for loading and data, but I don't think firmware upload should share the same config options as firmware-fallback, and incorporating the firmware-upload support into the existing fallback.[ch] files seems pretty ugly. I think the split is the right choice.
The split was largely mechanical, except for the following
(1) Kconfig and Makefile changes - without Makefile changes, the resulting patch wouldn't compile. I suppose I could defer the config change, but it is a small and obvious change.
(2) Data and function scopes: the resulting patch would not compile without these changes.
(3) Whitespace changes: the purpose of these changes was to quiet a handful of errors checkpatch. Moving them to a different patch would defeat the original purpose of make the changes at all. Maybe these changes simply aren't needed?
I could defer the Kconfig change and remove or defer the checkpatch-related white-space changes.
Any other opinions on the code reorganization in this patch?
- Russ
>
> Tom
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH 4/5] firmware_loader: Add firmware-upload support
2022-02-03 21:30 [RFC PATCH 0/5] Extend FW framework for user FW uploads Russ Weight
` (2 preceding siblings ...)
2022-02-03 21:30 ` [RFC PATCH 3/5] firmware_loader: Split fw_sysfs support from fallback Russ Weight
@ 2022-02-03 21:30 ` Russ Weight
2022-02-03 22:59 ` Luis Chamberlain
2022-02-03 21:30 ` [RFC PATCH 5/5] firmware_loader: Add sysfs nodes to monitor fw_upload Russ Weight
2022-02-03 23:00 ` [RFC PATCH 0/5] Extend FW framework for user FW uploads Luis Chamberlain
5 siblings, 1 reply; 14+ messages in thread
From: Russ Weight @ 2022-02-03 21:30 UTC (permalink / raw)
To: mcgrof, gregkh, rafael, linux-kernel
Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach,
basheer.ahmed.muddebihal, tianfei.zhang, 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 fw_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>
---
.../ABI/testing/sysfs-class-firmware | 32 +++
.../driver-api/firmware/fw_upload.rst | 86 +++++++
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/fw_sysfs.c | 50 +++-
drivers/base/firmware_loader/fw_sysfs.h | 4 +
drivers/base/firmware_loader/fw_upload.c | 229 ++++++++++++++++++
drivers/base/firmware_loader/fw_upload.h | 24 ++
drivers/base/firmware_loader/main.c | 16 +-
include/linux/firmware.h | 72 ++++++
12 files changed, 523 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/fw_upload.c
create mode 100644 drivers/base/firmware_loader/fw_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..bf272f627a1f
--- /dev/null
+++ b/Documentation/driver-api/firmware/fw_upload.rst
@@ -0,0 +1,86 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=============
+fw_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 fw_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 fw_upload API, write a driver that implements a set of ops. The
+probe function calls fw_upload_register() and the remove function calls
+fw_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 = fw_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);
+
+ fw_upload_unregister(sec->fwl);
+ kfree(sec->fw_name);
+ return 0;
+ }
+
+fw_upload_register
+------------------
+.. kernel-doc:: drivers/base/firmware_loader/fw_upload.c
+ :functions: fw_upload_register
+
+fw_upload_unregister
+--------------------
+.. kernel-doc:: drivers/base/firmware_loader/fw_upload.c
+ :functions: fw_upload_unregister
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 1bfe18900ed5..cee662f3277b 100644
--- a/drivers/base/firmware_loader/Kconfig
+++ b/drivers/base/firmware_loader/Kconfig
@@ -185,5 +185,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 787c833d0c6e..52ef64bd9357 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) += fw_sysfs.o
+firmware_class-$(CONFIG_FW_UPLOAD) += fw_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/fw_sysfs.c b/drivers/base/firmware_loader/fw_sysfs.c
index 70cb1d67ffb2..9b0cd37c81df 100644
--- a/drivers/base/firmware_loader/fw_sysfs.c
+++ b/drivers/base/firmware_loader/fw_sysfs.c
@@ -6,8 +6,8 @@
#include <linux/slab.h>
#include <linux/types.h>
-#include "firmware.h"
#include "fw_sysfs.h"
+#include "fw_upload.h"
/*
* sysfs support for firmware loader
@@ -80,6 +80,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);
}
@@ -165,6 +169,9 @@ static ssize_t firmware_loading_store(struct device *dev,
const char *buf, size_t count)
{
struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
+#ifdef CONFIG_FW_UPLOAD
+ struct fw_upload_priv *fwlp;
+#endif
struct fw_priv *fw_priv;
ssize_t written = count;
int loading = simple_strtol(buf, NULL, 10);
@@ -211,6 +218,42 @@ static ssize_t firmware_loading_store(struct device *dev,
written = rc;
} else {
fw_state_done(fw_priv);
+
+#ifdef CONFIG_FW_UPLOAD
+ /*
+ * For fw_uploads, start a worker thread to upload
+ * data to the parent driver.
+ */
+ if (!fw_sysfs->fw_upload_priv)
+ break;
+
+ if (!fw_priv->size) {
+ fw_free_paged_buf(fw_priv);
+ fw_state_init(fw_sysfs->fw_priv);
+ break;
+ }
+
+ fwlp = fw_sysfs->fw_upload_priv;
+ mutex_lock(&fwlp->lock);
+
+ /* Do not interfere an on-going fw_upload */
+ if (fwlp->progress != FW_UPLOAD_PROG_IDLE) {
+ mutex_unlock(&fwlp->lock);
+ written = -EBUSY;
+ goto out;
+ }
+
+ 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);
+#endif
}
break;
}
@@ -220,6 +263,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:
@@ -227,7 +273,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/fw_sysfs.h b/drivers/base/firmware_loader/fw_sysfs.h
index 0ca7f72892b0..1aadc12244d7 100644
--- a/drivers/base/firmware_loader/fw_sysfs.h
+++ b/drivers/base/firmware_loader/fw_sysfs.h
@@ -4,7 +4,10 @@
#include <linux/device.h>
+#include "firmware.h"
+
extern struct firmware_fallback_config fw_fallback_config;
+extern struct device_attribute dev_attr_loading;
#ifdef CONFIG_FW_LOADER_USER_HELPER
/**
@@ -59,6 +62,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/fw_upload.c b/drivers/base/firmware_loader/fw_upload.c
new file mode 100644
index 000000000000..bf02c9f09fd2
--- /dev/null
+++ b/drivers/base/firmware_loader/fw_upload.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/firmware.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#include "fw_sysfs.h"
+#include "fw_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,
+ u32 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, u32 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;
+ struct fw_upload *fwl;
+ s32 ret, offset = 0;
+
+ fwlp = container_of(work, struct fw_upload_priv, work);
+ fwl = fwlp->fw_upload;
+ fw_sysfs = (struct fw_sysfs *)fwl->priv;
+
+ get_device(&fw_sysfs->dev);
+ if (!try_module_get(fw_sysfs->dev.parent->driver->owner)) {
+ fw_upload_set_error(fwlp, FW_UPLOAD_ERR_BUSY);
+ goto putdev_exit;
+ }
+
+ fw_upload_update_progress(fwlp, FW_UPLOAD_PROG_PREPARING);
+ ret = fwlp->ops->prepare(fwl, fwlp->data, fwlp->remaining_size);
+ if (ret) {
+ fw_upload_set_error(fwlp, ret);
+ goto modput_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);
+ if (ret <= 0) {
+ if (!ret) {
+ dev_warn(&fw_sysfs->dev,
+ "write-op wrote zero data\n");
+ ret = -FW_UPLOAD_ERR_RW_ERROR;
+ }
+ fw_upload_set_error(fwlp, -ret);
+ goto done;
+ }
+
+ fwlp->remaining_size -= ret;
+ offset += ret;
+ }
+
+ fw_upload_update_progress(fwlp, FW_UPLOAD_PROG_PROGRAMMING);
+ ret = fwlp->ops->poll_complete(fwl);
+ if (ret)
+ fw_upload_set_error(fwlp, ret);
+
+done:
+ if (fwlp->ops->cleanup)
+ fwlp->ops->cleanup(fwl);
+
+modput_exit:
+ module_put(fw_sysfs->dev.parent->driver->owner);
+
+putdev_exit:
+ put_device(&fw_sysfs->dev);
+
+ /*
+ * 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);
+}
+
+/**
+ * fw_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 *
+fw_upload_register(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 *f_dev;
+ int ret;
+
+ __module_get(THIS_MODULE);
+
+ 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);
+ }
+
+ fw_upload = kzalloc(sizeof(*fw_upload), GFP_KERNEL);
+ if (!fw_upload)
+ return ERR_PTR(-ENOMEM);
+
+ 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->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;
+ f_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(f_dev);
+ if (ret) {
+ dev_err(f_dev, "%s: device_register failed\n", __func__);
+ put_device(f_dev);
+ module_put(THIS_MODULE);
+ return ERR_PTR(ret);
+ }
+
+ return fw_upload;
+
+free_fw_sysfs:
+ kfree(fw_sysfs);
+
+free_fw_upload_priv:
+ kfree(fw_upload_priv);
+
+free_fw_upload:
+ kfree(fw_upload);
+
+ module_put(THIS_MODULE);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(fw_upload_register);
+
+/**
+ * fw_upload_unregister() - Unregister firmware upload interface
+ * @fw_upload: pointer to struct fw_upload
+ **/
+void fw_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(THIS_MODULE);
+}
+EXPORT_SYMBOL_GPL(fw_upload_unregister);
diff --git a/drivers/base/firmware_loader/fw_upload.h b/drivers/base/firmware_loader/fw_upload.h
new file mode 100644
index 000000000000..c55ac4047d55
--- /dev/null
+++ b/drivers/base/firmware_loader/fw_upload.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __FIRMWARE_UPLOAD_H
+#define __FIRMWARE_UPLOAD_H
+
+#include <linux/device.h>
+
+struct fw_upload_priv {
+ struct fw_upload *fw_upload;
+ 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 */
+ u32 progress;
+ u32 err_progress; /* progress at time of failure */
+ u32 err_code; /* security manager error code */
+ bool driver_unload;
+};
+
+umode_t
+fw_upload_is_visible(struct kobject *kobj, struct attribute *attr, int n);
+
+#endif /* __FIRMWARE_UPLOAD_H */
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/include/linux/firmware.h b/include/linux/firmware.h
index 3b057dfc8284..9b109f8ff627 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -17,6 +17,56 @@ struct firmware {
void *priv;
};
+/* Update progress codes */
+#define FW_UPLOAD_PROG_IDLE 0
+#define FW_UPLOAD_PROG_RECEIVING 1
+#define FW_UPLOAD_PROG_PREPARING 2
+#define FW_UPLOAD_PROG_TRANSFERRING 3
+#define FW_UPLOAD_PROG_PROGRAMMING 4
+#define FW_UPLOAD_PROG_MAX 5
+
+/* Update error progress codes */
+#define FW_UPLOAD_ERR_HW_ERROR 1
+#define FW_UPLOAD_ERR_TIMEOUT 2
+#define FW_UPLOAD_ERR_CANCELED 3
+#define FW_UPLOAD_ERR_BUSY 4
+#define FW_UPLOAD_ERR_INVALID_SIZE 5
+#define FW_UPLOAD_ERR_RW_ERROR 6
+#define FW_UPLOAD_ERR_WEAROUT 7
+#define FW_UPLOAD_ERR_MAX 8
+
+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 {
+ u32 (*prepare)(struct fw_upload *fw_upload, const u8 *data, u32 size);
+ s32 (*write)(struct fw_upload *fw_upload, const u8 *data,
+ u32 offset, u32 size);
+ u32 (*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 +162,28 @@ static inline int request_partial_firmware_into_buf
#endif
+#ifdef CONFIG_FW_UPLOAD
+
+struct fw_upload *
+fw_upload_register(struct device *parent, const char *name,
+ const struct fw_upload_ops *ops, void *dd_handle);
+void fw_upload_unregister(struct fw_upload *fw_upload);
+
+#else
+
+static inline struct fw_upload *
+fw_upload_register(struct device *parent, const char *name,
+ const struct fw_upload_ops *ops, void *dd_handle)
+{
+ return ERR_PTR(-EINVAL);
+}
+
+static inline void fw_upload_unregister(struct fw_upload *fw_upload)
+{
+}
+
+#endif
+
int firmware_request_cache(struct device *device, const char *name);
#endif
--
2.25.1
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH 4/5] firmware_loader: Add firmware-upload support
2022-02-03 21:30 ` [RFC PATCH 4/5] firmware_loader: Add firmware-upload support Russ Weight
@ 2022-02-03 22:59 ` Luis Chamberlain
2022-02-11 2:29 ` Russ Weight
0 siblings, 1 reply; 14+ messages in thread
From: Luis Chamberlain @ 2022-02-03 22:59 UTC (permalink / raw)
To: Russ Weight
Cc: gregkh, rafael, linux-kernel, trix, lgoncalv, yilun.xu, hao.wu,
matthew.gerlach, basheer.ahmed.muddebihal, tianfei.zhang
On Thu, Feb 03, 2022 at 02:30:51PM -0700, Russ Weight wrote:
> 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 fw_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>
> ---
> .../ABI/testing/sysfs-class-firmware | 32 +++
> .../driver-api/firmware/fw_upload.rst | 86 +++++++
> 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/fw_sysfs.c | 50 +++-
> drivers/base/firmware_loader/fw_sysfs.h | 4 +
> drivers/base/firmware_loader/fw_upload.c | 229 ++++++++++++++++++
> drivers/base/firmware_loader/fw_upload.h | 24 ++
> drivers/base/firmware_loader/main.c | 16 +-
> include/linux/firmware.h | 72 ++++++
> 12 files changed, 523 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/fw_upload.c
> create mode 100644 drivers/base/firmware_loader/fw_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..bf272f627a1f
> --- /dev/null
> +++ b/Documentation/driver-api/firmware/fw_upload.rst
> @@ -0,0 +1,86 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=============
> +fw_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 fw_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 fw_upload API, write a driver that implements a set of ops. The
> +probe function calls fw_upload_register() and the remove function calls
> +fw_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 = fw_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);
> +
> + fw_upload_unregister(sec->fwl);
> + kfree(sec->fw_name);
> + return 0;
> + }
> +
> +fw_upload_register
> +------------------
> +.. kernel-doc:: drivers/base/firmware_loader/fw_upload.c
> + :functions: fw_upload_register
> +
> +fw_upload_unregister
> +--------------------
> +.. kernel-doc:: drivers/base/firmware_loader/fw_upload.c
> + :functions: fw_upload_unregister
> 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 1bfe18900ed5..cee662f3277b 100644
> --- a/drivers/base/firmware_loader/Kconfig
> +++ b/drivers/base/firmware_loader/Kconfig
> @@ -185,5 +185,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 787c833d0c6e..52ef64bd9357 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) += fw_sysfs.o
> +firmware_class-$(CONFIG_FW_UPLOAD) += fw_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/fw_sysfs.c b/drivers/base/firmware_loader/fw_sysfs.c
> index 70cb1d67ffb2..9b0cd37c81df 100644
> --- a/drivers/base/firmware_loader/fw_sysfs.c
> +++ b/drivers/base/firmware_loader/fw_sysfs.c
> @@ -6,8 +6,8 @@
> #include <linux/slab.h>
> #include <linux/types.h>
>
> -#include "firmware.h"
> #include "fw_sysfs.h"
> +#include "fw_upload.h"
>
> /*
> * sysfs support for firmware loader
> @@ -80,6 +80,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);
> }
>
> @@ -165,6 +169,9 @@ static ssize_t firmware_loading_store(struct device *dev,
> const char *buf, size_t count)
> {
> struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
> +#ifdef CONFIG_FW_UPLOAD
> + struct fw_upload_priv *fwlp;
> +#endif
> struct fw_priv *fw_priv;
> ssize_t written = count;
> int loading = simple_strtol(buf, NULL, 10);
> @@ -211,6 +218,42 @@ static ssize_t firmware_loading_store(struct device *dev,
> written = rc;
> } else {
> fw_state_done(fw_priv);
> +
> +#ifdef CONFIG_FW_UPLOAD
> + /*
> + * For fw_uploads, start a worker thread to upload
> + * data to the parent driver.
> + */
> + if (!fw_sysfs->fw_upload_priv)
> + break;
> +
> + if (!fw_priv->size) {
> + fw_free_paged_buf(fw_priv);
> + fw_state_init(fw_sysfs->fw_priv);
> + break;
> + }
> +
> + fwlp = fw_sysfs->fw_upload_priv;
> + mutex_lock(&fwlp->lock);
> +
> + /* Do not interfere an on-going fw_upload */
> + if (fwlp->progress != FW_UPLOAD_PROG_IDLE) {
> + mutex_unlock(&fwlp->lock);
> + written = -EBUSY;
> + goto out;
> + }
> +
> + 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);
> +#endif
If you're going to split this work up it'd be nice to avoid to have
#ifdef stuff here.
> diff --git a/drivers/base/firmware_loader/fw_upload.c b/drivers/base/firmware_loader/fw_upload.c
No need for fw prefix, sysfs-upload.[ch] would make it clearer.
> +EXPORT_SYMBOL_GPL(fw_upload_register);
firmware_upload_register()
> +EXPORT_SYMBOL_GPL(fw_upload_unregister);
firmware_upload_unregister()
> diff --git a/include/linux/firmware.h b/include/linux/firmware.h
> index 3b057dfc8284..9b109f8ff627 100644
> --- a/include/linux/firmware.h
> +++ b/include/linux/firmware.h
> @@ -17,6 +17,56 @@ struct firmware {
> void *priv;
> };
>
> +/* Update progress codes */
> +#define FW_UPLOAD_PROG_IDLE 0
> +#define FW_UPLOAD_PROG_RECEIVING 1
> +#define FW_UPLOAD_PROG_PREPARING 2
> +#define FW_UPLOAD_PROG_TRANSFERRING 3
> +#define FW_UPLOAD_PROG_PROGRAMMING 4
> +#define FW_UPLOAD_PROG_MAX 5
enums with kdoc allows you to nicely document these as well.
> +
> +/* Update error progress codes */
> +#define FW_UPLOAD_ERR_HW_ERROR 1
> +#define FW_UPLOAD_ERR_TIMEOUT 2
> +#define FW_UPLOAD_ERR_CANCELED 3
> +#define FW_UPLOAD_ERR_BUSY 4
> +#define FW_UPLOAD_ERR_INVALID_SIZE 5
> +#define FW_UPLOAD_ERR_RW_ERROR 6
> +#define FW_UPLOAD_ERR_WEAROUT 7
> +#define FW_UPLOAD_ERR_MAX 8
enums with kdoc allows you to nicely document these as well.
Luis
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH 4/5] firmware_loader: Add firmware-upload support
2022-02-03 22:59 ` Luis Chamberlain
@ 2022-02-11 2:29 ` Russ Weight
0 siblings, 0 replies; 14+ messages in thread
From: Russ Weight @ 2022-02-11 2:29 UTC (permalink / raw)
To: Luis Chamberlain
Cc: gregkh, rafael, linux-kernel, trix, lgoncalv, yilun.xu, hao.wu,
matthew.gerlach, basheer.ahmed.muddebihal, tianfei.zhang
On 2/3/22 2:59 PM, Luis Chamberlain wrote:
> On Thu, Feb 03, 2022 at 02:30:51PM -0700, Russ Weight wrote:
>> 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 fw_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>
>> ---
>> .../ABI/testing/sysfs-class-firmware | 32 +++
>> .../driver-api/firmware/fw_upload.rst | 86 +++++++
>> 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/fw_sysfs.c | 50 +++-
>> drivers/base/firmware_loader/fw_sysfs.h | 4 +
>> drivers/base/firmware_loader/fw_upload.c | 229 ++++++++++++++++++
>> drivers/base/firmware_loader/fw_upload.h | 24 ++
>> drivers/base/firmware_loader/main.c | 16 +-
>> include/linux/firmware.h | 72 ++++++
>> 12 files changed, 523 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/fw_upload.c
>> create mode 100644 drivers/base/firmware_loader/fw_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..bf272f627a1f
>> --- /dev/null
>> +++ b/Documentation/driver-api/firmware/fw_upload.rst
>> @@ -0,0 +1,86 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +=============
>> +fw_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 fw_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 fw_upload API, write a driver that implements a set of ops. The
>> +probe function calls fw_upload_register() and the remove function calls
>> +fw_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 = fw_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);
>> +
>> + fw_upload_unregister(sec->fwl);
>> + kfree(sec->fw_name);
>> + return 0;
>> + }
>> +
>> +fw_upload_register
>> +------------------
>> +.. kernel-doc:: drivers/base/firmware_loader/fw_upload.c
>> + :functions: fw_upload_register
>> +
>> +fw_upload_unregister
>> +--------------------
>> +.. kernel-doc:: drivers/base/firmware_loader/fw_upload.c
>> + :functions: fw_upload_unregister
>> 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 1bfe18900ed5..cee662f3277b 100644
>> --- a/drivers/base/firmware_loader/Kconfig
>> +++ b/drivers/base/firmware_loader/Kconfig
>> @@ -185,5 +185,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 787c833d0c6e..52ef64bd9357 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) += fw_sysfs.o
>> +firmware_class-$(CONFIG_FW_UPLOAD) += fw_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/fw_sysfs.c b/drivers/base/firmware_loader/fw_sysfs.c
>> index 70cb1d67ffb2..9b0cd37c81df 100644
>> --- a/drivers/base/firmware_loader/fw_sysfs.c
>> +++ b/drivers/base/firmware_loader/fw_sysfs.c
>> @@ -6,8 +6,8 @@
>> #include <linux/slab.h>
>> #include <linux/types.h>
>>
>> -#include "firmware.h"
>> #include "fw_sysfs.h"
>> +#include "fw_upload.h"
>>
>> /*
>> * sysfs support for firmware loader
>> @@ -80,6 +80,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);
>> }
>>
>> @@ -165,6 +169,9 @@ static ssize_t firmware_loading_store(struct device *dev,
>> const char *buf, size_t count)
>> {
>> struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
>> +#ifdef CONFIG_FW_UPLOAD
>> + struct fw_upload_priv *fwlp;
>> +#endif
>> struct fw_priv *fw_priv;
>> ssize_t written = count;
>> int loading = simple_strtol(buf, NULL, 10);
>> @@ -211,6 +218,42 @@ static ssize_t firmware_loading_store(struct device *dev,
>> written = rc;
>> } else {
>> fw_state_done(fw_priv);
>> +
>> +#ifdef CONFIG_FW_UPLOAD
>> + /*
>> + * For fw_uploads, start a worker thread to upload
>> + * data to the parent driver.
>> + */
>> + if (!fw_sysfs->fw_upload_priv)
>> + break;
>> +
>> + if (!fw_priv->size) {
>> + fw_free_paged_buf(fw_priv);
>> + fw_state_init(fw_sysfs->fw_priv);
>> + break;
>> + }
>> +
>> + fwlp = fw_sysfs->fw_upload_priv;
>> + mutex_lock(&fwlp->lock);
>> +
>> + /* Do not interfere an on-going fw_upload */
>> + if (fwlp->progress != FW_UPLOAD_PROG_IDLE) {
>> + mutex_unlock(&fwlp->lock);
>> + written = -EBUSY;
>> + goto out;
>> + }
>> +
>> + 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);
>> +#endif
> If you're going to split this work up it'd be nice to avoid to have
> #ifdef stuff here.
Yes - I'll encapsulate the code into another function and move it
to the sysfs-upload.c file.
>
>> diff --git a/drivers/base/firmware_loader/fw_upload.c b/drivers/base/firmware_loader/fw_upload.c
> No need for fw prefix, sysfs-upload.[ch] would make it clearer.
OK - I'll switch to sysfs-upload.[ch]
>
>> +EXPORT_SYMBOL_GPL(fw_upload_register);
> firmware_upload_register()
>
>> +EXPORT_SYMBOL_GPL(fw_upload_unregister);
> firmware_upload_unregister()
Sure - I'll change the register/unregister function names.
>
>> diff --git a/include/linux/firmware.h b/include/linux/firmware.h
>> index 3b057dfc8284..9b109f8ff627 100644
>> --- a/include/linux/firmware.h
>> +++ b/include/linux/firmware.h
>> @@ -17,6 +17,56 @@ struct firmware {
>> void *priv;
>> };
>>
>
>> +/* Update progress codes */
>> +#define FW_UPLOAD_PROG_IDLE 0
>> +#define FW_UPLOAD_PROG_RECEIVING 1
>> +#define FW_UPLOAD_PROG_PREPARING 2
>> +#define FW_UPLOAD_PROG_TRANSFERRING 3
>> +#define FW_UPLOAD_PROG_PROGRAMMING 4
>> +#define FW_UPLOAD_PROG_MAX 5
> enums with kdoc allows you to nicely document these as well.
OK. I'll switch to enums. I also realised that the FW_UPLOAD_PROG*
constants can be defined in a smaller scope. I'll move these to
sysfs-upload.h.
>> +
>> +/* Update error progress codes */
>> +#define FW_UPLOAD_ERR_HW_ERROR 1
>> +#define FW_UPLOAD_ERR_TIMEOUT 2
>> +#define FW_UPLOAD_ERR_CANCELED 3
>> +#define FW_UPLOAD_ERR_BUSY 4
>> +#define FW_UPLOAD_ERR_INVALID_SIZE 5
>> +#define FW_UPLOAD_ERR_RW_ERROR 6
>> +#define FW_UPLOAD_ERR_WEAROUT 7
>> +#define FW_UPLOAD_ERR_MAX 8
> enums with kdoc allows you to nicely document these as well.
Thanks,
- Russ
>
> Luis
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH 5/5] firmware_loader: Add sysfs nodes to monitor fw_upload
2022-02-03 21:30 [RFC PATCH 0/5] Extend FW framework for user FW uploads Russ Weight
` (3 preceding siblings ...)
2022-02-03 21:30 ` [RFC PATCH 4/5] firmware_loader: Add firmware-upload support Russ Weight
@ 2022-02-03 21:30 ` Russ Weight
2022-02-03 23:00 ` [RFC PATCH 0/5] Extend FW framework for user FW uploads Luis Chamberlain
5 siblings, 0 replies; 14+ messages in thread
From: Russ Weight @ 2022-02-03 21:30 UTC (permalink / raw)
To: mcgrof, gregkh, rafael, linux-kernel
Cc: trix, lgoncalv, yilun.xu, hao.wu, matthew.gerlach,
basheer.ahmed.muddebihal, tianfei.zhang, 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>
---
.../ABI/testing/sysfs-class-firmware | 45 +++++++
.../driver-api/firmware/fw_upload.rst | 19 ++-
drivers/base/firmware_loader/fw_sysfs.c | 9 ++
drivers/base/firmware_loader/fw_upload.c | 119 ++++++++++++++++++
drivers/base/firmware_loader/fw_upload.h | 5 +
5 files changed, 196 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 bf272f627a1f..28720363ec5c 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
============================
@@ -84,3 +85,19 @@ fw_upload_unregister
--------------------
.. kernel-doc:: drivers/base/firmware_loader/fw_upload.c
:functions: fw_upload_unregister
+
+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/fw_sysfs.c b/drivers/base/firmware_loader/fw_sysfs.c
index 9b0cd37c81df..2e47b5890470 100644
--- a/drivers/base/firmware_loader/fw_sysfs.c
+++ b/drivers/base/firmware_loader/fw_sysfs.c
@@ -414,6 +414,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
};
@@ -425,6 +431,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/fw_upload.c b/drivers/base/firmware_loader/fw_upload.c
index bf02c9f09fd2..5e6417ec67d6 100644
--- a/drivers/base/firmware_loader/fw_upload.c
+++ b/drivers/base/firmware_loader/fw_upload.c
@@ -11,6 +11,125 @@
* 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[] = {
+ [0] = "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, u32 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, u32 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,
u32 new_progress)
{
diff --git a/drivers/base/firmware_loader/fw_upload.h b/drivers/base/firmware_loader/fw_upload.h
index c55ac4047d55..e7f13d41c177 100644
--- a/drivers/base/firmware_loader/fw_upload.h
+++ b/drivers/base/firmware_loader/fw_upload.h
@@ -18,6 +18,11 @@ struct fw_upload_priv {
bool driver_unload;
};
+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;
+
umode_t
fw_upload_is_visible(struct kobject *kobj, struct attribute *attr, int n);
--
2.25.1
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH 0/5] Extend FW framework for user FW uploads
2022-02-03 21:30 [RFC PATCH 0/5] Extend FW framework for user FW uploads Russ Weight
` (4 preceding siblings ...)
2022-02-03 21:30 ` [RFC PATCH 5/5] firmware_loader: Add sysfs nodes to monitor fw_upload Russ Weight
@ 2022-02-03 23:00 ` Luis Chamberlain
2022-02-11 2:30 ` Russ Weight
5 siblings, 1 reply; 14+ messages in thread
From: Luis Chamberlain @ 2022-02-03 23:00 UTC (permalink / raw)
To: Russ Weight
Cc: gregkh, rafael, linux-kernel, trix, lgoncalv, yilun.xu, hao.wu,
matthew.gerlach, basheer.ahmed.muddebihal, tianfei.zhang
On Thu, Feb 03, 2022 at 02:30:47PM -0700, Russ Weight wrote:
> 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 fw_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-->
> <snip>
> transferring
> 173056
> <--snip-->
> programming
> 0
> <--snip-->
> idle
> 0
> ^C
Nice. Please extend lib/test_firmware.c while at it and try to break
and ensure your stuff is not regressed with future changes.
Luis
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH 0/5] Extend FW framework for user FW uploads
2022-02-03 23:00 ` [RFC PATCH 0/5] Extend FW framework for user FW uploads Luis Chamberlain
@ 2022-02-11 2:30 ` Russ Weight
0 siblings, 0 replies; 14+ messages in thread
From: Russ Weight @ 2022-02-11 2:30 UTC (permalink / raw)
To: Luis Chamberlain
Cc: gregkh, rafael, linux-kernel, trix, lgoncalv, yilun.xu, hao.wu,
matthew.gerlach, basheer.ahmed.muddebihal, tianfei.zhang
On 2/3/22 3:00 PM, Luis Chamberlain wrote:
> On Thu, Feb 03, 2022 at 02:30:47PM -0700, Russ Weight wrote:
>> 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 fw_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-->
>> <snip>
>> transferring
>> 173056
>> <--snip-->
>> programming
>> 0
>> <--snip-->
>> idle
>> 0
>> ^C
> Nice. Please extend lib/test_firmware.c while at it and try to break
> and ensure your stuff is not regressed with future changes.
Yes. I'm looking at the test_firmware module and I'll add to it
for the next patch submission.
Thanks,
- Russ
>
> Luis
^ permalink raw reply [flat|nested] 14+ messages in thread