2015-04-10 11:41:16

by Kweh, Hock Leong

[permalink] [raw]
Subject: [PATCH v3 0/3] Enable a capsule loader interface for user to update

From: "Kweh, Hock Leong" <[email protected]>

Hi Guys,

This patchset is created on top of "efi: Capsule update support" patch:
http://permalink.gmane.org/gmane.linux.kernel.efi/4837

It expose a sysfs loader interface for user to upload the capsule binary
and calling efi_capsule_update() API to pass the binary to EFI firmware.

Thanks.

---
changelog v3:
* changes base on the design discussion in the mailing list:
https://lkml.org/lkml/2015/2/24/307
* 1st patch introduce a new API request_firmware_direct_full_path() in
firmware_class for developer to pass in full path to the firmware file
* 2nd patch fix a bug for commit "firmware_loader: handle timeout via
wait_for_completion_interruptible_timeout()"
* 3rd patch introduce the capsule loader interface kernel module


Kweh, Hock Leong (3):
firmware loader: introduce new API -
request_firmware_direct_full_path()
firmware_loader: fix positive return value being treat as error
return
efi: an sysfs interface for user to update efi firmware

drivers/base/firmware_class.c | 48 +++++++-
drivers/firmware/efi/Kconfig | 12 ++
drivers/firmware/efi/Makefile | 1
drivers/firmware/efi/efi-capsule-loader.c | 169 +++++++++++++++++++++++++++++
include/linux/firmware.h | 9 ++
5 files changed, 234 insertions(+), 5 deletions(-)
create mode 100644 drivers/firmware/efi/efi-capsule-loader.c

--
1.7.9.5


2015-04-10 14:00:28

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] Enable a capsule loader interface for user to update

On Sat, Apr 11, 2015 at 03:40:41AM +0800, Kweh, Hock Leong wrote:
> From: "Kweh, Hock Leong" <[email protected]>
>
> Hi Guys,
>
> This patchset is created on top of "efi: Capsule update support" patch:
> http://permalink.gmane.org/gmane.linux.kernel.efi/4837
>
> It expose a sysfs loader interface for user to upload the capsule binary
> and calling efi_capsule_update() API to pass the binary to EFI firmware.

What I'm missing from those 0/n mails is why we need this? What is
the problem you're trying to solve and why is Peter's userspace-only
solution not enough.

Now I have a fairly good idea why we might/could/would need the kernel
interface but I don't believe the every reader has followed the whole
discussion.

So please start with the Why. Try to sell it to me as best as you can.
Details will be discussed later anyway.

Thanks.

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

2015-04-10 11:41:30

by Kweh, Hock Leong

[permalink] [raw]
Subject: [PATCH v3 1/3] firmware loader: introduce new API - request_firmware_direct_full_path()

From: "Kweh, Hock Leong" <[email protected]>

Introduce this new API for loading firmware from a specific location
instead of /lib/firmware/ by providing a full path to the firmware
file.

Cc: Matt Fleming <[email protected]>
Signed-off-by: Kweh, Hock Leong <[email protected]>
---
drivers/base/firmware_class.c | 46 ++++++++++++++++++++++++++++++++++++-----
include/linux/firmware.h | 9 ++++++++
2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 6c5c9ed..e03235d 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -111,6 +111,7 @@ static inline long firmware_loading_timeout(void)
#define FW_OPT_FALLBACK 0
#endif
#define FW_OPT_NO_WARN (1U << 3)
+#define FW_OPT_FULL_PATH (1U << 4)

struct firmware_cache {
/* firmware_buf instance will be added into the below list */
@@ -318,20 +319,29 @@ fail:
}

static int fw_get_filesystem_firmware(struct device *device,
- struct firmware_buf *buf)
+ struct firmware_buf *buf,
+ unsigned int opt_flags)
{
int i;
int rc = -ENOENT;
char *path = __getname();
+ int path_array_size = 1;
+ static const char * const root_path[] = {"/"};
+ char **temp_path = (char **)root_path;

- for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
+ if (!(opt_flags & FW_OPT_FULL_PATH)) {
+ temp_path = (char **)fw_path;
+ path_array_size = ARRAY_SIZE(fw_path);
+ }
+
+ for (i = 0; i < path_array_size; i++) {
struct file *file;

/* skip the unset customized path */
- if (!fw_path[i][0])
+ if (!temp_path[i][0])
continue;

- snprintf(path, PATH_MAX, "%s/%s", fw_path[i], buf->fw_id);
+ snprintf(path, PATH_MAX, "%s/%s", temp_path[i], buf->fw_id);

file = filp_open(path, O_RDONLY, 0);
if (IS_ERR(file))
@@ -1118,7 +1128,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
}
}

- ret = fw_get_filesystem_firmware(device, fw->priv);
+ ret = fw_get_filesystem_firmware(device, fw->priv, opt_flags);
if (ret) {
if (!(opt_flags & FW_OPT_NO_WARN))
dev_warn(device,
@@ -1205,6 +1215,32 @@ int request_firmware_direct(const struct firmware **firmware_p,
EXPORT_SYMBOL_GPL(request_firmware_direct);

/**
+ * request_firmware_direct_full_path: - load firmware directly from exact
+ * full path
+ * @firmware_p: pointer to firmware image
+ * @name: full path to the firmware file with file name
+ * @device: device for which firmware is being loaded
+ *
+ * This function works like request_firmware_direct(), but this doesn't
+ * search the /lib/firmware/ for the firmware file. It support exact full
+ * path to the firmware file for loading.
+ **/
+int request_firmware_direct_full_path(const struct firmware **firmware_p,
+ const char *name, struct device *device)
+{
+ int ret;
+
+ __module_get(THIS_MODULE);
+ ret = _request_firmware(firmware_p, name, device,
+ FW_OPT_UEVENT | FW_OPT_NO_WARN |
+ FW_OPT_FULL_PATH);
+ module_put(THIS_MODULE);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(request_firmware_direct_full_path);
+
+/**
* release_firmware: - release the resource associated with a firmware image
* @fw: firmware resource to release
**/
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index 5c41c5e..b7c6435 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -47,6 +47,8 @@ int request_firmware_nowait(
void (*cont)(const struct firmware *fw, void *context));
int request_firmware_direct(const struct firmware **fw, const char *name,
struct device *device);
+int request_firmware_direct_full_path(const struct firmware **fw,
+ const char *name, struct device *device);

void release_firmware(const struct firmware *fw);
#else
@@ -75,5 +77,12 @@ static inline int request_firmware_direct(const struct firmware **fw,
return -EINVAL;
}

+static inline int request_firmware_direct_full_path(const struct firmware **fw,
+ const char *name,
+ struct device *device)
+{
+ return -EINVAL;
+}
+
#endif
#endif
--
1.7.9.5

2015-04-10 11:41:50

by Kweh, Hock Leong

[permalink] [raw]
Subject: [PATCH v3 2/3] firmware_loader: fix positive return value being treat as error return

From: "Kweh, Hock Leong" <[email protected]>

Due to wait_for_completion_interruptible_timeout() will return
its remaining timeout jiffies value if timeout does not happen,
the error check code "if (ret)" will treat that as error return.
So, fixing the issue by re-assigning back a 'Zero' to the return
value when the wait is completed without timeout.

Signed-off-by: Kweh, Hock Leong <[email protected]>
---
drivers/base/firmware_class.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index e03235d..b38975d 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -923,6 +923,8 @@ static int _request_firmware_load(struct firmware_priv *fw_priv,
mutex_lock(&fw_lock);
fw_load_abort(fw_priv);
mutex_unlock(&fw_lock);
+ } else {
+ retval = 0;
}

if (is_fw_load_aborted(buf))
--
1.7.9.5

2015-04-10 11:41:58

by Kweh, Hock Leong

[permalink] [raw]
Subject: [PATCH v3 3/3] efi: an sysfs interface for user to update efi firmware

From: "Kweh, Hock Leong" <[email protected]>

Introducing a kernel module to expose capsule loader interface
for user to upload capsule binaries. This module leverage the
request_firmware_direct_full_path() to obtain the binary at a
specific path input by user.

Example method to load the capsule binary:
echo -n "/path/to/capsule/binary" > /sys/devices/platform/efi_capsule_loader/capsule_loader

Cc: Matt Fleming <[email protected]>
Signed-off-by: Kweh, Hock Leong <[email protected]>
---
drivers/firmware/efi/Kconfig | 12 ++
drivers/firmware/efi/Makefile | 1 +
drivers/firmware/efi/efi-capsule-loader.c | 169 +++++++++++++++++++++++++++++
3 files changed, 182 insertions(+)
create mode 100644 drivers/firmware/efi/efi-capsule-loader.c

diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index f712d47..3e84ec0 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -60,6 +60,18 @@ config EFI_RUNTIME_WRAPPERS
config EFI_ARMSTUB
bool

+config EFI_CAPSULE_LOADER
+ tristate "EFI capsule loader"
+ depends on EFI
+ select FW_LOADER
+ help
+ This option exposes a loader interface for user to load EFI
+ capsule binary and update the EFI firmware through 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..5ab031a 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_LOADER) += efi-capsule-loader.o
diff --git a/drivers/firmware/efi/efi-capsule-loader.c b/drivers/firmware/efi/efi-capsule-loader.c
new file mode 100644
index 0000000..84b979b
--- /dev/null
+++ b/drivers/firmware/efi/efi-capsule-loader.c
@@ -0,0 +1,169 @@
+/*
+ * EFI capsule loader 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/highmem.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/efi.h>
+#include <linux/firmware.h>
+
+#define DEV_NAME "efi_capsule_loader"
+
+static DEFINE_MUTEX(capsule_loader_lock);
+static struct platform_device *efi_capsule_pdev;
+
+/*
+ * 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;
+
+ if (!count) {
+ pr_err("%s: Received zero binary size\n", __func__);
+ return -ENOENT;
+ }
+
+ 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;
+}
+
+static ssize_t capsule_loader_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret = -EBUSY;
+ const struct firmware *fw;
+
+ pr_debug("%s: Received path = %s\n", __func__, buf);
+ if (mutex_trylock(&capsule_loader_lock)) {
+ ret = request_firmware_direct_full_path(&fw, buf,
+ &efi_capsule_pdev->dev);
+ if (ret) {
+ pr_err("%s: request_firmware_direct_full_path() %s\n",
+ __func__,
+ "failed");
+ mutex_unlock(&capsule_loader_lock);
+ return ret;
+ }
+
+ ret = efi_capsule_store(fw);
+ if (ret)
+ pr_err("%s: %s, return error code = %d\n",
+ __func__,
+ "Failed to store capsule binary",
+ ret);
+ release_firmware(fw);
+ mutex_unlock(&capsule_loader_lock);
+ }
+
+ return ret ? ret : count;
+}
+static DEVICE_ATTR_WO(capsule_loader);
+
+static int __init efi_capsule_loader_init(void)
+{
+ int ret = 0;
+
+ 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);
+ }
+
+ /*
+ * create this file node for user to pass in the full path of the
+ * capsule binary
+ */
+ ret = device_create_file(&efi_capsule_pdev->dev,
+ &dev_attr_capsule_loader);
+ if (ret) {
+ pr_err("%s: device_create_file() failed\n", __func__);
+ platform_device_unregister(efi_capsule_pdev);
+ }
+
+ return ret;
+}
+module_init(efi_capsule_loader_init);
+
+/*
+ * To remove this kernel module, just perform:
+ * rmmod efi_capsule_loader.ko
+ */
+static void __exit efi_capsule_loader_exit(void)
+{
+ platform_device_unregister(efi_capsule_pdev);
+}
+module_exit(efi_capsule_loader_exit);
+
+MODULE_DESCRIPTION("EFI capsule firmware binary loader");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5

2015-04-12 08:38:28

by Kweh, Hock Leong

[permalink] [raw]
Subject: RE: [PATCH v3 0/3] Enable a capsule loader interface for user to update

> -----Original Message-----
> From: Borislav Petkov [mailto:[email protected]]
> Sent: Friday, April 10, 2015 9:58 PM
>
> On Sat, Apr 11, 2015 at 03:40:41AM +0800, Kweh, Hock Leong wrote:
> > From: "Kweh, Hock Leong" <[email protected]>
> >
> > Hi Guys,
> >
> > This patchset is created on top of "efi: Capsule update support" patch:
> > http://permalink.gmane.org/gmane.linux.kernel.efi/4837
> >
> > It expose a sysfs loader interface for user to upload the capsule binary
> > and calling efi_capsule_update() API to pass the binary to EFI firmware.
>
> What I'm missing from those 0/n mails is why we need this? What is
> the problem you're trying to solve and why is Peter's userspace-only
> solution not enough.
>
> Now I have a fairly good idea why we might/could/would need the kernel
> interface but I don't believe the every reader has followed the whole
> discussion.
>
> So please start with the Why. Try to sell it to me as best as you can.
> Details will be discussed later anyway.
>
> Thanks.
>

Hi Borislav,

I will make the summary, for more detail, you can refer to the threads here:
- https://lkml.org/lkml/2015/3/10/418
- https://lkml.org/lkml/2015/3/10/262

Summary:
- Kernel interface could provide more flexibility than just firmware updates.
- For embedded small foot print devices, not all vendors are affordable to include all userland tools to their product.
- Not all platform support ESRT.


Regards,
Wilson

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?