RE: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface

["Kweh, Hock Leong" <hock.leong.kweh-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 5650 bytes --]

> -----Original Message-----
> From: Kweh, Hock Leong
> Sent: Tuesday, February 24, 2015 6:54 PM
> 
> In callbackfn_efi_capsule, you call request_firmware_nowait.  When that
> callback is invoked, I think that the /sys/class/firmware/efi-capsule-file
> directory doesn't exist at all.
> If the callback takes longer than it takes your script to make it through a full
> iteration, then it will try uploading the second capsule before the firmware
> class directory is there, so it will fail.
> 
> But I just realized that your script has a loop below to handle that.
> It's this:
> 
>                  oldtime=$(date +%S)
>                  oldtime=$(((time + 2) % 60))
>                  until [ -f /sys/class/firmware/efi-capsule-file/loading ]
>                  do
>                          newtime=$(date +%S)
>                          if [ $newtime -eq $oldtime ]
>                          then
>                                  break
>                          fi
>                  done
> 
> Aside from the fact that this loop itself is racy (it may loop forever if
> something goes wrong in the kernel, since $newtime -eq $oldtime may
> never happen), it should help, if you're lucky.  But there's another bug.
> 
> 
> Here's the race:
> 
> User:
> echo 1 > /sys/class/firmware/efi-capsule-file/loading
> cat capsule1 > /sys/class/firmware/efi-capsule-file/data
> echo 0 > /sys/class/firmware/efi-capsule-file/loading
> Kernel: Be a little slow here due to preemption or whatever.
> 
> User:
> -f /sys/class/firmware/efi-capsule-file/loading returns true capsules_loaded
> == 0 Assume failure, incorrectly
> 
> Kernel: catch up and increment capsules_loaded.
> 
> If these patches get applied, then I think that the protocol needs to be
> documented in Documentation/ABI.  It should say something like:
> 
> To upload an EFI capsule, do this:
> 
> Write 1 to /sys/class/firmware/efi-capsule-file/loading
> Write the capsule to /sys/class/firmware/efi-capsule-file/data
> Write 0 to /sys/class/firmware/efi-capsule-file/loading
> 
> Make sure that /sys/class/firmware/efi-capsule-file disappears and comes
> back, perhaps by cd-ing there and waiting for all the files in the directory to
> go away.
> 
> Then, and only then, read capsules_loaded to detect success.
> 
> 
> Once you've written that doc, please seriously consider whether this
> interface is justifiable.  I think it sucks.
> 
> --Andy

Hi All,

After some internal discussion and re-design prototyping & testing on
this efi capsule interface kernel module, I would like to start a discussion
here on the new idea and wish to get input for the implementation and
eventually upstream.

This new idea will expose 2 read-only file notes from the efi capsule kernel
module - "capsule_ticket" and "capsule_report". The "capsule_ticket" will
let user to obtain a NON-ZERO number and then perform a mutex lock. The
obtained number will be used later for "capsule_report" status checking.
Unlock mutex is done after "echo 0 > loading" at the end of callback function.

So the process steps basically look like this:
1.) cat capsule_ticket    =======> acquire a number and lock mutex then
                                                                     expose firmware_class user helper
                                                                     interface as well as start timer for timeout
                                                                     counting
2.) repeat step 1 if obtained a "0" number
3.) echo 1 > loading
4.) cat bin > data
5.) echo 0 > loading        =======> stop the timeout counting  then unlock
                                                                      mutex at the end of callback routine 
6.) cat capsule_report   =======> grep the number acquired from beginning
                                                                      for checking succeeded/failed

The ticket numbering is starting from 1 to 999 and then rolls over from
1 to 999 again. The "capsule_report" will show the whole list of the acquired
entries and will be limited to 128 entries which is capped within one page buffer.

The format of this "capsule_report" output will look like:
"Capsule $num uploads successful/failed/aborted"

There is a 2mins time out (internal) for each of the number acquired.
After that, the interface will be closed/disappeared.

I have attached a SAMPLE script and kernel module code in case you are not
able to understand my description above.

I believe this would really take care of the multi-capsule update concurrently
issue and also asynchronous status reporting issue. Besides, this will indirectly
take care the module unload issue with "rmmod" and not "rmmod -f".
What do you guys think?

----------------------------------------------------------------------------------------------

There is another idea during internal discussion. The 2nd idea would require
some changes to drivers/base/firmware_class.c user helper interface for the
mutex locking as well as status return. Mutex lock will be performed while
doing the 'echo 1 > loading' session and status return will be performed after
the 'echo 0 > loading'. The mutex unlock will be done at 'echo 0 > loading' too
or may be at the status reading session for avoid asynchronous reporting.

This will likely changed the original firmware class user helper interface design
behavior. But this approach could save the 2 file notes from the 1st approach.


Which of the approach do you guys prefer? Thanks.


Regards,
Wilson


[-- Attachment #2: newcapsule.txt --]
[-- Type: text/plain, Size: 947 bytes --]

#!/bin/sh

for arg in "$@"
do
	if [ -f $arg ]
	then
		retry=0
		until [ ! $num = '0' ]
		do
			sleep 1
			num=$(cat /sys/devices/platform/efi_capsule_user_helper/capsule_ticket)
			if [ $retry -le 130 ]
			then
				retry=$((retry + 1))
			else
				echo "Failed to acquire capsule ticket"
				exit 1
			fi
		done

		oldtime=$(date +%S)
		oldtime=$(((time + 2) % 60))
		until [ -f /sys/class/firmware/efi-capsule-file/loading ]
		do
			newtime=$(date +%S)
			if [ $newtime -eq $oldtime ]
			then
				echo "Failed to expose user helper interface"
				exit 1
			fi
		done

		echo 1 > /sys/class/firmware/efi-capsule-file/loading
		cat $arg > /sys/class/firmware/efi-capsule-file/data
		echo 0 > /sys/class/firmware/efi-capsule-file/loading

		until [ -n "$result" ]
		do
			result=$(cat /sys/devices/platform/efi_capsule_user_helper/capsule_report | grep $num)
		done
		echo $result " (" $arg ")"
	else
		echo "File $arg not found !!"
	fi
done
exit 0

[-- Attachment #3: 0002-efi-Capsule-update-with-user-helper-interface.patch --]
[-- Type: application/octet-stream, Size: 12646 bytes --]

From 7395877e6895231255baab6838d8b92e44f0342b Mon Sep 17 00:00:00 2001
Message-Id: <7395877e6895231255baab6838d8b92e44f0342b.1424244086.git.hock.leong.kweh@intel.com>
In-Reply-To: <90701184a919bb58c8c71cd5d9d2e28cc722e5d1.1424244086.git.hock.leong.kweh@intel.com>
References: <90701184a919bb58c8c71cd5d9d2e28cc722e5d1.1424244086.git.hock.leong.kweh@intel.com>
From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
Date: Tue, 2 Sep 2014 22:10:42 +0800
Subject: [PATCH 2/2] efi: Capsule update with user helper interface

Introducing a kernel module to expose user helper interface for
user to upload capsule binaries. This module leverage the
request_firmware_nowait() to expose an interface to user.

Example steps to load the capsule binary:
1.) num=$(cat /sys/devices/platform/efi_capsule_user_helper/capsule_ticket)
2.) continue to acquire the number if the number is '0'
3.) echo 1 > /sys/class/firmware/efi-capsule-file/loading
4.) cat capsule.bin > /sys/class/firmware/efi-capsule-file/data
5.) echo 0 > /sys/class/firmware/efi-capsule-file/loading
6.) cat /sys/devices/platform/efi_capsule_user_helper/capsule_report | grep $num

Whereas, this module does not allow the capsule binaries to be
obtained from the request_firmware library search path. If any
capsule binary loaded from firmware seach path, the module will
abort the capsule storing function.

Besides the request_firmware user helper interface, this module
also exposes two read only file notes to interact with user.

- 'capsule_ticket' allows user to acquire a number and enabling
firmware_class user helper interface for uploading the capsule
binary. The ticket number is starting from 001 to 999. If
obtained a number of '0' means someone is performing the loading.
Please wait until the loading is done and obtained the number in
the correct range then perform to the next steps. A 2 minutes
time out is implemented for each loading activity.

- 'capsule_report' allows user to verify the status of the
capsule loading. User can verify by using the number acquired at
the 1st step with 'capsule_ticket'.

This design cater the preventing of multi capsules concurrent
loading problem as well as the status reporting synchronization.

Cc: Matt Fleming <matt.fleming@intel.com>
Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
---
 drivers/firmware/efi/Kconfig                   |   13 +
 drivers/firmware/efi/Makefile                  |    1 +
 drivers/firmware/efi/efi-capsule-user-helper.c |  316 ++++++++++++++++++++++++
 3 files changed, 330 insertions(+)
 create mode 100644 drivers/firmware/efi/efi-capsule-user-helper.c

diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index f712d47..7dc814e 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -60,6 +60,19 @@ config EFI_RUNTIME_WRAPPERS
 config EFI_ARMSTUB
 	bool
 
+config EFI_CAPSULE_USER_HELPER
+	tristate "EFI capsule user mode helper"
+	depends on EFI
+	select FW_LOADER
+	select FW_LOADER_USER_HELPER
+	help
+	  This option exposes the user mode helper interface for user to load
+	  EFI capsule binary and update the EFI firmware after system reboot.
+	  This feature does not support auto locating capsule binaries at the
+	  firmware lib search path.
+
+	  If unsure, say N.
+
 endmenu
 
 config UEFI_CPER
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 698846e..63f6910 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_UEFI_CPER)			+= cper.o
 obj-$(CONFIG_EFI_RUNTIME_MAP)		+= runtime-map.o
 obj-$(CONFIG_EFI_RUNTIME_WRAPPERS)	+= runtime-wrappers.o
 obj-$(CONFIG_EFI_STUB)			+= libstub/
+obj-$(CONFIG_EFI_CAPSULE_USER_HELPER)	+= efi-capsule-user-helper.o
diff --git a/drivers/firmware/efi/efi-capsule-user-helper.c b/drivers/firmware/efi/efi-capsule-user-helper.c
new file mode 100644
index 0000000..05838f5
--- /dev/null
+++ b/drivers/firmware/efi/efi-capsule-user-helper.c
@@ -0,0 +1,316 @@
+/*
+ * EFI capsule user mode helper interface driver.
+ *
+ * Copyright 2015 Intel Corporation
+ *
+ * This file is part of the Linux kernel, and is made available under
+ * the terms of the GNU General Public License version 2.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/highmem.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/reboot.h>
+#include <linux/efi.h>
+#include <linux/firmware.h>
+#include <linux/workqueue.h>
+
+#define CAPSULE_NAME "efi-capsule-file"
+#define DEV_NAME "efi_capsule_user_helper"
+#define STRING_INTEGER_MAX_LENGTH 13
+#define MAX_SYSFS_BUF_LENGTH 4096
+#define CAPSULE_USER_HELPER_TIMEOUT 120
+#define CAPSULE_REPORT_MAX_STR_SIZE 32
+#define CAPSULE_RESULT_PASS "successful"
+#define CAPSULE_RESULT_FAIL "failed    "
+#define CAPSULE_RESULT_EXIT "aborted   "
+
+static DEFINE_MUTEX(user_helper_lock);
+static int capsule_count;
+static struct platform_device *efi_capsule_pdev;
+static struct delayed_work timeout_work;
+static unsigned char *report_buf;
+static int report_buf_head_off, report_buf_write_off;
+
+/*
+ * This function will store the capsule binary and pass it to
+ * efi_capsule_update() API in capsule.c
+ */
+static int efi_capsule_store(const struct firmware *fw)
+{
+	int i;
+	int ret;
+	int count = fw->size;
+	int copy_size = (fw->size > PAGE_SIZE) ? PAGE_SIZE : fw->size;
+	int pages_needed = ALIGN(fw->size, PAGE_SIZE) >> PAGE_SHIFT;
+	struct page **pages;
+	void *page_data;
+	efi_capsule_header_t *capsule = NULL;
+
+	pages = kmalloc_array(pages_needed, sizeof(void *), GFP_KERNEL);
+	if (!pages) {
+		pr_err("%s: kmalloc_array() failed\n", __func__);
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < pages_needed; i++) {
+		pages[i] = alloc_page(GFP_KERNEL);
+		if (!pages[i]) {
+			pr_err("%s: alloc_page() failed\n", __func__);
+			--i;
+			ret = -ENOMEM;
+			goto failed;
+		}
+	}
+
+	for (i = 0; i < pages_needed; i++) {
+		page_data = kmap(pages[i]);
+		memcpy(page_data, fw->data + (i * PAGE_SIZE), copy_size);
+
+		if (i == 0)
+			capsule = (efi_capsule_header_t *)page_data;
+		else
+			kunmap(pages[i]);
+
+		count -= copy_size;
+		if (count < PAGE_SIZE)
+			copy_size = count;
+	}
+
+	ret = efi_capsule_update(capsule, pages);
+	if (ret) {
+		pr_err("%s: efi_capsule_update() failed\n", __func__);
+		--i;
+		goto failed;
+	}
+	kunmap(pages[0]);
+
+	/*
+	 * we cannot free the pages here due to reboot need that data
+	 * maintained.
+	 */
+	return 0;
+
+failed:
+	while (i >= 0)
+		__free_page(pages[i--]);
+	kfree(pages);
+	return ret;
+}
+
+/*
+ * This callback function will be called by request_firmware_nowait() when
+ * user has loaded the capsule binary or aborted user helper interface
+ */
+static void callbackfn_efi_capsule(const struct firmware *fw, void *context)
+{
+	int ret;
+
+	if (fw) {
+		/*
+		 * Binary which is not getting from user helper interface,
+		 * the fw->pages is equal to NULL
+		 */
+		if (!fw->pages) {
+			pr_err("%s: ERROR: Capsule binary '%s' at %s\n",
+			       __func__, CAPSULE_NAME,
+			       "firmware lib search path are not supported");
+			pr_err("user helper interface disabled\n");
+			snprintf(report_buf + report_buf_write_off,
+				 CAPSULE_REPORT_MAX_STR_SIZE,
+				 "Capsule %03d uploads %s\n",
+				 capsule_count,
+				 CAPSULE_RESULT_FAIL);
+		} else {
+			ret = efi_capsule_store(fw);
+			if (!ret) {
+				snprintf(report_buf + report_buf_write_off,
+					 CAPSULE_REPORT_MAX_STR_SIZE,
+					 "Capsule %03d uploads %s\n",
+					 capsule_count,
+					 CAPSULE_RESULT_PASS);
+			} else {
+				pr_err("%s: %s %03d, return error code = %d\n",
+				       __func__,
+				       "Failed to store capsule binary",
+				       capsule_count, ret);
+				snprintf(report_buf + report_buf_write_off,
+					 CAPSULE_REPORT_MAX_STR_SIZE,
+					 "Capsule %03d uploads %s\n",
+					 capsule_count,
+					 CAPSULE_RESULT_FAIL);
+			}
+		}
+		release_firmware(fw);
+	} else {
+		snprintf(report_buf + report_buf_write_off,
+			 CAPSULE_REPORT_MAX_STR_SIZE,
+			 "Capsule %03d uploads %s\n",
+			 capsule_count,
+			 CAPSULE_RESULT_EXIT);
+	}
+
+	report_buf_write_off += CAPSULE_REPORT_MAX_STR_SIZE;
+	report_buf_write_off %= PAGE_SIZE;
+
+	cancel_delayed_work_sync(&timeout_work);
+	mutex_unlock(&user_helper_lock);
+}
+
+static void capsule_user_helper_timeout_work(struct work_struct *work)
+{
+	request_firmware_abort(CAPSULE_NAME);
+}
+
+static ssize_t capsule_ticket_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	int ret;
+
+	if (mutex_trylock(&user_helper_lock)) {
+		/*
+		 * Use request_firmware_nowait() to expose an user helper
+		 * interface for obtaining the capsule binary from user space
+		 */
+		ret = request_firmware_nowait(THIS_MODULE, FW_ACTION_NOHOTPLUG,
+					      CAPSULE_NAME,
+					      &efi_capsule_pdev->dev,
+					      GFP_KERNEL, NULL,
+					      callbackfn_efi_capsule);
+		if (ret) {
+			pr_err("%s: request_firmware_nowait() failed\n",
+			       __func__);
+			mutex_unlock(&user_helper_lock);
+			return snprintf(buf, STRING_INTEGER_MAX_LENGTH, "0\n");
+		}
+
+		queue_delayed_work(system_power_efficient_wq,
+				   &timeout_work,
+				   CAPSULE_USER_HELPER_TIMEOUT * HZ);
+
+		++capsule_count;
+		capsule_count %= 1000 ? : ++capsule_count;
+
+		return snprintf(buf, STRING_INTEGER_MAX_LENGTH, "%03d\n",
+				capsule_count);
+	} else {
+		return snprintf(buf, STRING_INTEGER_MAX_LENGTH, "0\n");
+	}
+}
+
+static DEVICE_ATTR_RO(capsule_ticket);
+
+static ssize_t capsule_report_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	int length = capsule_count * CAPSULE_REPORT_MAX_STR_SIZE;
+
+	length = length > PAGE_SIZE ? PAGE_SIZE : length;
+	memcpy(buf, report_buf + report_buf_head_off, length);
+	return length;
+}
+
+static DEVICE_ATTR_RO(capsule_report);
+
+/* reboot notifier for avoid deadlock with usermode_lock */
+static int capsule_shutdown_notify(struct notifier_block *nb,
+				   unsigned long sys_state,
+				   void *reboot_cmd)
+{
+	request_firmware_abort(CAPSULE_NAME);
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block capsule_shutdown_nb = {
+	.notifier_call = capsule_shutdown_notify,
+	/*
+	 * In order to reboot properly, it is required to ensure the priority
+	 * here is higher than firmware_class fw_shutdown_nb priority
+	 */
+	.priority = 1,
+};
+
+static int __init efi_capsule_user_helper_init(void)
+{
+	int ret;
+
+	efi_capsule_pdev = platform_device_register_simple(DEV_NAME,
+							   PLATFORM_DEVID_NONE,
+							   NULL, 0);
+	if (IS_ERR(efi_capsule_pdev)) {
+		pr_err("%s: platform_device_register_simple() failed\n",
+		       __func__);
+		return PTR_ERR(efi_capsule_pdev);
+	}
+
+	report_buf = devm_kzalloc(&efi_capsule_pdev->dev, PAGE_SIZE,
+				  GFP_KERNEL);
+	if (!report_buf) {
+		ret = -ENOMEM;
+		pr_err("%s: devm_kzalloc() failed\n", __func__);
+		goto out;
+	}
+
+	/*
+	 * create this file node for user to acquire a ticket number and get
+	 * ready to upload capsule binaries
+	 */
+	ret = device_create_file(&efi_capsule_pdev->dev,
+				 &dev_attr_capsule_ticket);
+	if (ret) {
+		pr_err("%s: device_create_file() failed\n", __func__);
+		goto out;
+	}
+
+	/*
+	 * create this file node for user to check the capsule binaries upload
+	 * status
+	 */
+	ret = device_create_file(&efi_capsule_pdev->dev,
+				 &dev_attr_capsule_report);
+	if (ret) {
+		pr_err("%s: device_create_file() failed\n", __func__);
+		goto out;
+	}
+
+	INIT_DELAYED_WORK(&timeout_work, capsule_user_helper_timeout_work);
+	register_reboot_notifier(&capsule_shutdown_nb);
+	return 0;
+
+out:
+	platform_device_unregister(efi_capsule_pdev);
+	return ret;
+}
+module_init(efi_capsule_user_helper_init);
+
+/*
+ * To remove this kernel module, just perform:
+ * rmmod efi_capsule_user_helper.ko
+ *
+ * rmmod -f efi_capsule_user_helper.ko is NOT recommended and NOT supported
+ * by this module design. Any force unload may cause the system crash.
+ */
+static void __exit efi_capsule_user_helper_exit(void)
+{
+	unregister_reboot_notifier(&capsule_shutdown_nb);
+
+	// request_firmware_abort(CAPSULE_NAME);
+	// /*
+	 // * synchronization is needed to make sure request_firmware is fully
+	 // * aborted
+	 // */
+	// while (efi_capsule_pdev->dev.kobj.kref.refcount.counter > 3)
+		// msleep(20); /* avoid busy waiting for cooperative kernel */
+
+	platform_device_unregister(efi_capsule_pdev);
+}
+module_exit(efi_capsule_user_helper_exit);
+
+MODULE_DESCRIPTION("EFI Capsule user helper binary load utility");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5