[PATCH 0/2] platform/x86: dell_rbu: make usable

[PATCH 0/2] platform/x86: dell_rbu: make usable

[Rosen Penev]

First commit updates the API to one not disabled by default on many distros.

Second commit fixes upload of firmware more than 4MB in size, which is most
modern ones,

Rosen Penev (2):
  platform/x86: dell_rbu: Use the firmware upload API
  platform/x86: dell_rbu: Allocate large monolithic images contiguously

 Documentation/admin-guide/dell_rbu.rst |  25 ++--
 drivers/platform/x86/dell/Kconfig      |   2 +-
 drivers/platform/x86/dell/dell_rbu.c   | 161 ++++++++++++++++++-------
 3 files changed, 131 insertions(+), 57 deletions(-)

-- 
2.56.1

[PATCH 1/2] platform/x86: dell_rbu: Use the firmware upload API

[Rosen Penev]

dell_rbu receives the BIOS image through the legacy firmware sysfs
fallback: writing "init" to image_type calls request_firmware_nowait()
with FW_ACTION_NOUEVENT, and userspace feeds the image through
/sys/class/firmware/dell_rbu/{loading,data}. This is why DELL_RBU
selects FW_LOADER_USER_HELPER, an option most distributions disable.
On such kernels the driver cannot be enabled at all.

The firmware upload API (FW_UPLOAD) was added for this use case and
provides the same /sys/class/firmware/<name>/{loading,data} interface
without the user helper. Register a firmware upload device at module init
and move the work done in callbackfn_rbu() into the prepare() op,
which receives the whole image. The BIOS picks the image up from memory
on the next reboot, so write(), poll_complete() and cancel() have
nothing to do.

The upload entries now persist for the lifetime of the module, so
the "init" image_type workaround for recreating them and the
entry_created tracking are no longer needed. "init" is still accepted
and frees any downloaded image, as documented. Allocation or
packetizing failures are now reported via the error attribute instead
of being silently dropped.

The upload device is registered from module init and unregistered from
module exit, so pass a NULL module to firmware_upload_register():
otherwise the reference it takes would pin dell_rbu and make it
impossible to unload. firmware_upload_unregister() cancels and flushes
any in-flight upload before returning.

The userspace sequence documented in dell_rbu.rst (echo 1 > loading,
cp image > data, echo 0 > loading) is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 Documentation/admin-guide/dell_rbu.rst |  25 +++---
 drivers/platform/x86/dell/Kconfig      |   2 +-
 drivers/platform/x86/dell/dell_rbu.c   | 107 ++++++++++++++++---------
 3 files changed, 81 insertions(+), 53 deletions(-)

diff --git a/Documentation/admin-guide/dell_rbu.rst b/Documentation/admin-guide/dell_rbu.rst
index 2196caf1b939..d33927b0d52d 100644
--- a/Documentation/admin-guide/dell_rbu.rst
+++ b/Documentation/admin-guide/dell_rbu.rst
@@ -47,6 +47,10 @@ The driver load creates the following directories under the /sys file system::
 
 	/sys/class/firmware/dell_rbu/loading
 	/sys/class/firmware/dell_rbu/data
+	/sys/class/firmware/dell_rbu/status
+	/sys/class/firmware/dell_rbu/error
+	/sys/class/firmware/dell_rbu/remaining_size
+	/sys/class/firmware/dell_rbu/cancel
 	/sys/devices/platform/dell_rbu/image_type
 	/sys/devices/platform/dell_rbu/data
 	/sys/devices/platform/dell_rbu/packet_size
@@ -95,26 +99,21 @@ Do the steps below to download the BIOS image.
 2) cp bios_image.hdr /sys/class/firmware/dell_rbu/data
 3) echo 0 > /sys/class/firmware/dell_rbu/loading
 
-The /sys/class/firmware/dell_rbu/ entries will remain till the following is
-done.
-
-::
+The /sys/class/firmware/dell_rbu/ entries are provided by the firmware upload
+framework (see Documentation/driver-api/firmware/fw_upload.rst) and remain
+for as long as the driver is loaded. Once step 3 is done,
+/sys/class/firmware/dell_rbu/status returns to "idle" and
+/sys/class/firmware/dell_rbu/error is empty if the image was accepted.
+A download in progress can be aborted with::
 
 	echo -1 > /sys/class/firmware/dell_rbu/loading
 
-Until this step is completed the driver cannot be unloaded.
-
 Also echoing either mono, packet or init in to image_type will free up the
 memory allocated by the driver.
 
-If a user by accident executes steps 1 and 3 above without executing step 2;
-it will make the /sys/class/firmware/dell_rbu/ entries disappear.
-
-The entries can be recreated by doing the following::
-
-	echo init > /sys/devices/platform/dell_rbu/image_type
-
 .. note:: echoing init in image_type does not change its original value.
+   It is kept for compatibility; the /sys/class/firmware/dell_rbu/ entries
+   no longer need to be recreated.
 
 Also the driver provides /sys/devices/platform/dell_rbu/data readonly file to
 read back the image downloaded.
diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
index f6acf1cc2a2f..1672ed04a36a 100644
--- a/drivers/platform/x86/dell/Kconfig
+++ b/drivers/platform/x86/dell/Kconfig
@@ -89,7 +89,7 @@ config DELL_RBU
 	default m
 	depends on X86
 	select FW_LOADER
-	select FW_LOADER_USER_HELPER
+	select FW_UPLOAD
 	help
 	 Say m if you want to have the option of updating the BIOS for your
 	 DELL system. Note you need a Dell OpenManage or Dell Update package (DUP)
diff --git a/drivers/platform/x86/dell/dell_rbu.c b/drivers/platform/x86/dell/dell_rbu.c
index 768b15f406d3..a121f59a215d 100644
--- a/drivers/platform/x86/dell/dell_rbu.c
+++ b/drivers/platform/x86/dell/dell_rbu.c
@@ -60,12 +60,11 @@ static struct _rbu_data {
 	unsigned long num_packets;
 	unsigned long packetsize;
 	unsigned long imagesize;
-	int entry_created;
 } rbu_data;
 
 static char image_type[MAX_IMAGE_LENGTH + 1] = "mono";
 module_param_string(image_type, image_type, sizeof (image_type), 0);
-MODULE_PARM_DESC(image_type, "BIOS image type. choose- mono or packet or init");
+MODULE_PARM_DESC(image_type, "BIOS image type. choose- mono or packet");
 
 static unsigned long allocation_floor = 0x100000;
 module_param(allocation_floor, ulong, 0644);
@@ -81,7 +80,7 @@ struct packet_data {
 static struct list_head packet_data_list;
 
 static struct platform_device *rbu_device;
-static int context;
+static struct fw_upload *rbu_fw_upload;
 
 static void init_packet_head(void)
 {
@@ -497,41 +496,72 @@ static ssize_t data_read(struct file *filp, struct kobject *kobj,
 }
 static const BIN_ATTR_RO(data, 0);
 
-static void callbackfn_rbu(const struct firmware *fw, void *context)
+/*
+ * The whole image is handed to prepare() by the firmware upload core, so
+ * all of the work happens here. The BIOS consumes the image from memory on
+ * the next reboot; there is nothing to stream to hardware afterwards.
+ */
+static enum fw_upload_err rbu_fw_prepare(struct fw_upload *fw_upload,
+					 const u8 *data, u32 size)
 {
-	rbu_data.entry_created = 0;
-
-	if (!fw)
-		return;
-
-	if (!fw->size)
-		goto out;
+	enum fw_upload_err ret = FW_UPLOAD_ERR_NONE;
 
 	spin_lock(&rbu_data.lock);
 	if (!strcmp(image_type, "mono")) {
-		if (!img_update_realloc(fw->size))
-			memcpy(rbu_data.image_update_buffer,
-				fw->data, fw->size);
+		if (img_update_realloc(size))
+			ret = FW_UPLOAD_ERR_RW_ERROR;
+		else
+			memcpy(rbu_data.image_update_buffer, data, size);
 	} else if (!strcmp(image_type, "packet")) {
 		/*
 		 * we need to free previous packets if a
 		 * new hunk of packets needs to be downloaded
 		 */
 		packet_empty_list();
-		if (packetize_data(fw->data, fw->size))
+		if (packetize_data(data, size)) {
 			/* Incase something goes wrong when we are
 			 * in middle of packetizing the data, we
 			 * need to free up whatever packets might
 			 * have been created before we quit.
 			 */
 			packet_empty_list();
-	} else
+			ret = FW_UPLOAD_ERR_RW_ERROR;
+		}
+	} else {
 		pr_debug("invalid image type specified\n");
+		ret = FW_UPLOAD_ERR_FW_INVALID;
+	}
 	spin_unlock(&rbu_data.lock);
- out:
-	release_firmware(fw);
+
+	return ret;
+}
+
+static enum fw_upload_err rbu_fw_write(struct fw_upload *fw_upload,
+				       const u8 *data, u32 offset,
+				       u32 size, u32 *written)
+{
+	/* Already consumed in rbu_fw_prepare() */
+	*written = size;
+	return FW_UPLOAD_ERR_NONE;
+}
+
+static enum fw_upload_err rbu_fw_poll_complete(struct fw_upload *fw_upload)
+{
+	return FW_UPLOAD_ERR_NONE;
+}
+
+static void rbu_fw_cancel(struct fw_upload *fw_upload)
+{
+	/* Nothing in flight to abort; the image only lives in memory. */
 }
 
+static const struct fw_upload_ops rbu_fw_ops = {
+	.prepare = rbu_fw_prepare,
+	.write = rbu_fw_write,
+	.poll_complete = rbu_fw_poll_complete,
+	.cancel = rbu_fw_cancel,
+};
+
 static ssize_t image_type_read(struct file *filp, struct kobject *kobj,
 			       const struct bin_attribute *bin_attr,
 			       char *buffer, loff_t pos, size_t count)
@@ -547,7 +577,6 @@ static ssize_t image_type_write(struct file *filp, struct kobject *kobj,
 				char *buffer, loff_t pos, size_t count)
 {
 	int rc = count;
-	int req_firm_rc = 0;
 	int i;
 	spin_lock(&rbu_data.lock);
 	/*
@@ -567,26 +596,10 @@ static ssize_t image_type_write(struct file *filp, struct kobject *kobj,
 		strscpy(image_type, "packet");
 	else if (strstr(buffer, "init")) {
 		/*
-		 * If due to the user error the driver gets in a bad
-		 * state where even though it is loaded , the
-		 * /sys/class/firmware/dell_rbu entries are missing.
-		 * to cover this situation the user can recreate entries
-		 * by writing init to image_type.
+		 * Historically this recreated the /sys/class/firmware/dell_rbu
+		 * entries. They are now persistent, so "init" only frees
+		 * any previously downloaded image, as before.
 		 */
-		if (!rbu_data.entry_created) {
-			spin_unlock(&rbu_data.lock);
-			req_firm_rc = request_firmware_nowait(THIS_MODULE,
-				FW_ACTION_NOUEVENT, "dell_rbu",
-				&rbu_device->dev, GFP_KERNEL, &context,
-				callbackfn_rbu);
-			if (req_firm_rc) {
-				pr_err("request_firmware_nowait failed %d\n", rc);
-				rc = -EIO;
-			} else
-				rbu_data.entry_created = 1;
-
-			spin_lock(&rbu_data.lock);
-		}
 	} else {
 		pr_warn("image_type is invalid\n");
 		spin_unlock(&rbu_data.lock);
@@ -661,9 +674,24 @@ static int __init dcdrbu_init(void)
 	if (rc)
 		goto out_devreg;
 
-	rbu_data.entry_created = 0;
+	/*
+	 * Registered from module init and unregistered from module exit, so
+	 * don't let the upload device pin this module: that would make it
+	 * impossible to unload. firmware_upload_unregister() cancels and
+	 * flushes any in-flight upload before returning.
+	 */
+	rbu_fw_upload = firmware_upload_register(NULL, &rbu_device->dev,
+						 "dell_rbu", &rbu_fw_ops, NULL);
+	if (IS_ERR(rbu_fw_upload)) {
+		rc = PTR_ERR(rbu_fw_upload);
+		pr_err("firmware_upload_register failed %d\n", rc);
+		goto out_group;
+	}
+
 	return 0;
 
+out_group:
+	sysfs_remove_group(&rbu_device->dev.kobj, &rbu_group);
 out_devreg:
 	platform_device_unregister(rbu_device);
 	return rc;
@@ -671,6 +699,7 @@ static int __init dcdrbu_init(void)
 
 static __exit void dcdrbu_exit(void)
 {
+	firmware_upload_unregister(rbu_fw_upload);
 	spin_lock(&rbu_data.lock);
 	packet_empty_list();
 	img_update_free();
-- 
2.56.1

[PATCH 2/2] platform/x86: dell_rbu: Allocate large monolithic images contiguously

[Rosen Penev]

Monolithic RBU images must live in a single physically contiguous block
below 4GB. img_update_realloc() gets that block from __get_free_pages(),
which cannot return more than MAX_PAGE_ORDER (4MB on x86). Current Dell
BIOS packages are larger than that: the Precision M4800 A26 image
(the PFS container extracted from the update executable) is about 16MB,
and SMBIOS structure 0xDE on that system reports no packet-mode support,
so the image cannot be loaded at all.

When the required order exceeds MAX_PAGE_ORDER, fall back to
alloc_contig_pages() with __GFP_DMA32 so the range is still below 4GB,
and free it with free_contig_range(). Smaller images keep using the
buddy allocator as before.

Tested on a Precision M4800 by uploading the 16MB A26 image through
/sys/class/firmware/dell_rbu and reading it back byte-for-byte from
/sys/devices/platform/dell_rbu/data; a 300KB image still uses the
buddy allocator path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/platform/x86/dell/dell_rbu.c | 54 +++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/dell/dell_rbu.c b/drivers/platform/x86/dell/dell_rbu.c
index a121f59a215d..d71fa9584831 100644
--- a/drivers/platform/x86/dell/dell_rbu.c
+++ b/drivers/platform/x86/dell/dell_rbu.c
@@ -41,6 +41,8 @@
 #include <linux/moduleparam.h>
 #include <linux/firmware.h>
 #include <linux/dma-mapping.h>
+#include <linux/gfp.h>
+#include <linux/mm.h>
 #include <asm/set_memory.h>
 
 MODULE_AUTHOR("Abhay Salunke <abhay_salunke@dell.com>");
@@ -55,6 +57,7 @@ static struct _rbu_data {
 	unsigned long image_update_buffer_size;
 	unsigned long bios_image_size;
 	int image_update_ordernum;
+	unsigned long image_update_contig_pages;
 	spinlock_t lock;
 	unsigned long packet_read_count;
 	unsigned long num_packets;
@@ -335,6 +338,37 @@ static void packet_empty_list(void)
 	rbu_data.imagesize = 0;
 }
 
+/*
+ * Monolithic images must sit in one physically contiguous block below 4GB.
+ * Current BIOS images (16MB on some systems) exceed what the buddy allocator
+ * can hand out (MAX_PAGE_ORDER), so fall back to the contiguous range
+ * allocator for those when it is available.
+ */
+#ifdef CONFIG_CONTIG_ALLOC
+static void *img_alloc_contig(unsigned long nr_pages)
+{
+	struct page *page;
+
+	page = alloc_contig_pages(nr_pages, GFP_KERNEL | __GFP_DMA32 | __GFP_NOWARN,
+				  first_online_node, NULL);
+	return page ? page_address(page) : NULL;
+}
+
+static void img_free_contig(void *buf, unsigned long nr_pages)
+{
+	free_contig_range(PHYS_PFN(virt_to_phys(buf)), nr_pages);
+}
+#else
+static void *img_alloc_contig(unsigned long nr_pages)
+{
+	return NULL;
+}
+
+static void img_free_contig(void *buf, unsigned long nr_pages)
+{
+}
+#endif
+
 /*
  * img_update_free: Frees the buffer allocated for storing BIOS image
  * Always called with lock held and returned with lock held
@@ -349,13 +383,18 @@ static void img_update_free(void)
 	 */
 	memset(rbu_data.image_update_buffer, 0,
 		rbu_data.image_update_buffer_size);
-	free_pages((unsigned long) rbu_data.image_update_buffer,
-		rbu_data.image_update_ordernum);
+	if (rbu_data.image_update_contig_pages)
+		img_free_contig(rbu_data.image_update_buffer,
+				rbu_data.image_update_contig_pages);
+	else
+		free_pages((unsigned long) rbu_data.image_update_buffer,
+			rbu_data.image_update_ordernum);
 
 	/*
 	 * Re-initialize the rbu_data variables after a free
 	 */
 	rbu_data.image_update_ordernum = -1;
+	rbu_data.image_update_contig_pages = 0;
 	rbu_data.image_update_buffer = NULL;
 	rbu_data.image_update_buffer_size = 0;
 	rbu_data.bios_image_size = 0;
@@ -375,6 +414,7 @@ static int img_update_realloc(unsigned long size)
 {
 	unsigned char *image_update_buffer = NULL;
 	unsigned long img_buf_phys_addr;
+	unsigned long contig_pages = 0;
 	int ordernum;
 
 	/*
@@ -404,8 +444,13 @@ static int img_update_realloc(unsigned long size)
 	spin_unlock(&rbu_data.lock);
 
 	ordernum = get_order(size);
-	image_update_buffer =
-		(unsigned char *)__get_free_pages(GFP_DMA32, ordernum);
+	if (ordernum <= MAX_PAGE_ORDER) {
+		image_update_buffer =
+			(unsigned char *)__get_free_pages(GFP_DMA32, ordernum);
+	} else {
+		contig_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
+		image_update_buffer = img_alloc_contig(contig_pages);
+	}
 	spin_lock(&rbu_data.lock);
 	if (!image_update_buffer) {
 		pr_debug("Not enough memory for image update: size = %ld\n", size);
@@ -420,6 +465,7 @@ static int img_update_realloc(unsigned long size)
 	rbu_data.image_update_buffer_size = size;
 	rbu_data.bios_image_size = rbu_data.image_update_buffer_size;
 	rbu_data.image_update_ordernum = ordernum;
+	rbu_data.image_update_contig_pages = contig_pages;
 	return 0;
 }
 
-- 
2.56.1