Mailing List Archive

[PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware
From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>

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 <matt.fleming@intel.com>
Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Tue, Apr 14, 2015 at 05:44:56PM +0800, Kweh, Hock Leong wrote:
> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
>
> 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

Ick, why not just have the firmware file location present, and copy it
to the sysfs file directly from userspace, instead of this two-step
process?

> +/*
> + * To remove this kernel module, just perform:
> + * rmmod efi_capsule_loader.ko

This comment is not needed.


> + */
> +static void __exit efi_capsule_loader_exit(void)
> +{
> + platform_device_unregister(efi_capsule_pdev);

This is not a platform device, don't abuse that interface please.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Tue, Apr 14, 2015 at 10:09 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Apr 14, 2015 at 05:44:56PM +0800, Kweh, Hock Leong wrote:
>> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
>>
>> 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
>
> Ick, why not just have the firmware file location present, and copy it
> to the sysfs file directly from userspace, instead of this two-step
> process?

Because it's not at all obvious how error handling should work in that case.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
> -----Original Message-----
> From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, April 14, 2015 10:09 PM
>
> On Tue, Apr 14, 2015 at 05:44:56PM +0800, Kweh, Hock Leong wrote:
> > From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> >
> > 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
>
> Ick, why not just have the firmware file location present, and copy it
> to the sysfs file directly from userspace, instead of this two-step
> process?

Err .... I may not catch your meaning correctly. Are you trying to say
that you would prefer the user to perform:

cat file.bin > /sys/.../capsule_loader

instead of

echo -n "/path/to/binary" > /sys/..../capsule_laoder


The reason we stick with the firmware_class is because we don't
want to replicate a code which already mature and has open API
for developer to use.

>
> > +/*
> > + * To remove this kernel module, just perform:
> > + * rmmod efi_capsule_loader.ko
>
> This comment is not needed.

Okay, I will remove this.

>
>
> > + */
> > +static void __exit efi_capsule_loader_exit(void)
> > +{
> > + platform_device_unregister(efi_capsule_pdev);
>
> This is not a platform device, don't abuse that interface please.
>
> greg k-h

Okay, so you would recommend to use device_register() for this case?
Or you would think that this is more suitable to use class_register()?


Thanks & Regards,
Wilson

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Wed, Apr 15, 2015 at 11:32:29AM +0000, Kweh, Hock Leong wrote:
> > -----Original Message-----
> > From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> > Sent: Tuesday, April 14, 2015 10:09 PM
> >
> > On Tue, Apr 14, 2015 at 05:44:56PM +0800, Kweh, Hock Leong wrote:
> > > From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> > >
> > > 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
> >
> > Ick, why not just have the firmware file location present, and copy it
> > to the sysfs file directly from userspace, instead of this two-step
> > process?
>
> Err .... I may not catch your meaning correctly. Are you trying to say
> that you would prefer the user to perform:
>
> cat file.bin > /sys/.../capsule_loader
>
> instead of
>
> echo -n "/path/to/binary" > /sys/..../capsule_laoder

Yes. What's the namespace of your /path/to/binary/ and how do you know
the kernel has the same one when it does the firmware load call? By
just copying the data with 'cat', you don't have to worry about
namespace issues at all.

> The reason we stick with the firmware_class is because we don't
> want to replicate a code which already mature and has open API
> for developer to use.

That's fine, but adding a new api to the firmware interface seems odd to
me, just because you don't like using /lib/ or any of the other
"standard" locations for firmware blobs. And note, that path is
configurable.

> > > + */
> > > +static void __exit efi_capsule_loader_exit(void)
> > > +{
> > > + platform_device_unregister(efi_capsule_pdev);
> >
> > This is not a platform device, don't abuse that interface please.
> >
> > greg k-h
>
> Okay, so you would recommend to use device_register() for this case?
> Or you would think that this is more suitable to use class_register()?

A class isn't needed, you just want a device right? So just use a
device, but not a platform device, as that isn't what you have here.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Tue, Apr 14, 2015 at 11:52:48AM -0400, Andy Lutomirski wrote:
> On Tue, Apr 14, 2015 at 10:09 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Tue, Apr 14, 2015 at 05:44:56PM +0800, Kweh, Hock Leong wrote:
> >> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> >>
> >> 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
> >
> > Ick, why not just have the firmware file location present, and copy it
> > to the sysfs file directly from userspace, instead of this two-step
> > process?
>
> Because it's not at all obvious how error handling should work in that case.

I don't understand how the error handling is any different. The kernel
ends up copying the data in through the firmware interface both ways, we
just aren't creating yet-another-firmware-path in the system.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Apr 15, 2015 6:20 AM, "Greg Kroah-Hartman"
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Apr 14, 2015 at 11:52:48AM -0400, Andy Lutomirski wrote:
> > On Tue, Apr 14, 2015 at 10:09 AM, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > > On Tue, Apr 14, 2015 at 05:44:56PM +0800, Kweh, Hock Leong wrote:
> > >> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> > >>
> > >> 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
> > >
> > > Ick, why not just have the firmware file location present, and copy it
> > > to the sysfs file directly from userspace, instead of this two-step
> > > process?
> >
> > Because it's not at all obvious how error handling should work in that case.
>
> I don't understand how the error handling is any different. The kernel
> ends up copying the data in through the firmware interface both ways, we
> just aren't creating yet-another-firmware-path in the system.

If I run uefi-update-capsule foo.bin, I want it to complain if the
UEFI call fails. In contrast, if I boot and my ath10k firmware is
bad, there's no explicit user interaction that should fail; instead I
have no network device and I get to read the logs and figure out why.
IOW bad volatile device firmware is just like a bad device driver,
whereas bad UEFI capsules are problems that should be reported to
whatever tried to send them to UEFI.

--Andy

>
> thanks,
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Wed, Apr 15, 2015 at 8:45 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Apr 15, 2015 6:20 AM, "Greg Kroah-Hartman"
> <gregkh@linuxfoundation.org> wrote:
>>
>> On Tue, Apr 14, 2015 at 11:52:48AM -0400, Andy Lutomirski wrote:
>> > On Tue, Apr 14, 2015 at 10:09 AM, Greg Kroah-Hartman
>> > <gregkh@linuxfoundation.org> wrote:
>> > > On Tue, Apr 14, 2015 at 05:44:56PM +0800, Kweh, Hock Leong wrote:
>> > >> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
>> > >>
>> > >> 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
>> > >
>> > > Ick, why not just have the firmware file location present, and copy it
>> > > to the sysfs file directly from userspace, instead of this two-step
>> > > process?
>> >
>> > Because it's not at all obvious how error handling should work in that case.
>>
>> I don't understand how the error handling is any different. The kernel
>> ends up copying the data in through the firmware interface both ways, we
>> just aren't creating yet-another-firmware-path in the system.
>
> If I run uefi-update-capsule foo.bin, I want it to complain if the
> UEFI call fails. In contrast, if I boot and my ath10k firmware is
> bad, there's no explicit user interaction that should fail; instead I
> have no network device and I get to read the logs and figure out why.
> IOW bad volatile device firmware is just like a bad device driver,
> whereas bad UEFI capsules are problems that should be reported to
> whatever tried to send them to UEFI.
>
> --Andy
>
There are several types of errors that can be returned by
UpdateCapsule(), allowing
us to distinguish between capsules that are not supported by the
platform, capsules
that must be updated at boot time, and capsule updates that failed due
to a device error.
I think we really do want this type of information returned to capsule loader.

Roy

>>
>> thanks,
>>
>> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
> -----Original Message-----
> From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> Sent: Wednesday, April 15, 2015 9:19 PM
>
> On Wed, Apr 15, 2015 at 11:32:29AM +0000, Kweh, Hock Leong wrote:
> > > -----Original Message-----
> > > From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> > > Sent: Tuesday, April 14, 2015 10:09 PM
> > >
> > > On Tue, Apr 14, 2015 at 05:44:56PM +0800, Kweh, Hock Leong wrote:
> > > > From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> > > >
> > > > 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
> > >
> > > Ick, why not just have the firmware file location present, and copy it
> > > to the sysfs file directly from userspace, instead of this two-step
> > > process?
> >
> > Err .... I may not catch your meaning correctly. Are you trying to say
> > that you would prefer the user to perform:
> >
> > cat file.bin > /sys/.../capsule_loader
> >
> > instead of
> >
> > echo -n "/path/to/binary" > /sys/..../capsule_laoder
>
> Yes. What's the namespace of your /path/to/binary/ and how do you know
> the kernel has the same one when it does the firmware load call? By
> just copying the data with 'cat', you don't have to worry about
> namespace issues at all.

Hi Greg,

Let me double confirm that I understand your concern correctly. You are
trying to tell that some others module may use a 'same' namespace to
request the firmware but never release it. Then when our module trying
to request the firmware by passing in the 'same' namespace, I will get the
previous data instead of the current binary data from the path I want.

Hmm .... I believe this concern also apply to all the current request_firmware
APIs right? And I believe the coincidence to have 'same' file name namespace
would be higher than full path namespace.

I may not able to control other module developers on the namespace naming.
But I could ensure each firmware request will be released before the next
request firmware happen in this module. This module will return error if it
does not receive a real efi capsule binary.

Hmm ......

>
> > The reason we stick with the firmware_class is because we don't
> > want to replicate a code which already mature and has open API
> > for developer to use.
>
> That's fine, but adding a new api to the firmware interface seems odd to
> me, just because you don't like using /lib/ or any of the other
> "standard" locations for firmware blobs. And note, that path is
> configurable.

If I am not mistaken, I believe you are referring the module param path.
Yes, I do aware of it. If I need a more flexibility method to alter the custom
path, the current design would require system reboot or module re-insmod.
So, leveraging the request_firmware_direct_full_path() could make things
a little bit easier from this point of view.

Besides, this new API actually helps to overcome the confusedness of having
2 or more same file name binaries at the firmware search paths (fw_path_para;
/lib/firmware/update/; /lib/firmware/). Due to user have the possibility to
configure kernel command param / module param for this fw_path_para,
it may have chances to point to a path that have same file name the /lib/firmware/
also have. With this API, it can make it specific to the path that developer wants.

>
> > > > + */
> > > > +static void __exit efi_capsule_loader_exit(void)
> > > > +{
> > > > + platform_device_unregister(efi_capsule_pdev);
> > >
> > > This is not a platform device, don't abuse that interface please.
> > >
> > > greg k-h
> >
> > Okay, so you would recommend to use device_register() for this case?
> > Or you would think that this is more suitable to use class_register()?
>
> A class isn't needed, you just want a device right? So just use a
> device, but not a platform device, as that isn't what you have here.
>
> thanks,
>
> greg k-h

Okay, will do this. Thanks.


Regards,
Wilson

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Thu, Apr 16, 2015 at 09:42:31AM +0000, Kweh, Hock Leong wrote:
> > -----Original Message-----
> > From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> > Sent: Wednesday, April 15, 2015 9:19 PM
> >
> > On Wed, Apr 15, 2015 at 11:32:29AM +0000, Kweh, Hock Leong wrote:
> > > > -----Original Message-----
> > > > From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> > > > Sent: Tuesday, April 14, 2015 10:09 PM
> > > >
> > > > On Tue, Apr 14, 2015 at 05:44:56PM +0800, Kweh, Hock Leong wrote:
> > > > > From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> > > > >
> > > > > 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
> > > >
> > > > Ick, why not just have the firmware file location present, and copy it
> > > > to the sysfs file directly from userspace, instead of this two-step
> > > > process?
> > >
> > > Err .... I may not catch your meaning correctly. Are you trying to say
> > > that you would prefer the user to perform:
> > >
> > > cat file.bin > /sys/.../capsule_loader
> > >
> > > instead of
> > >
> > > echo -n "/path/to/binary" > /sys/..../capsule_laoder
> >
> > Yes. What's the namespace of your /path/to/binary/ and how do you know
> > the kernel has the same one when it does the firmware load call? By
> > just copying the data with 'cat', you don't have to worry about
> > namespace issues at all.
>
> Hi Greg,
>
> Let me double confirm that I understand your concern correctly. You are
> trying to tell that some others module may use a 'same' namespace to
> request the firmware but never release it. Then when our module trying
> to request the firmware by passing in the 'same' namespace, I will get the
> previous data instead of the current binary data from the path I want.

Yes.

> Hmm .... I believe this concern also apply to all the current request_firmware
> APIs right? And I believe the coincidence to have 'same' file name namespace
> would be higher than full path namespace.

Not really, the kernel namespace is what matters at that point in time.

And maybe it does matter, I haven't thought through all of the issues.
But passing a path from userspace, to the kernel, to have the kernel
turn around again and use that path is full of nasty consequences at
times due to namespaces, let's avoid all of that please.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Wed, Apr 15, 2015 at 05:19:09PM -0700, Roy Franz wrote:
> On Wed, Apr 15, 2015 at 8:45 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> > On Apr 15, 2015 6:20 AM, "Greg Kroah-Hartman"
> > <gregkh@linuxfoundation.org> wrote:
> >>
> >> On Tue, Apr 14, 2015 at 11:52:48AM -0400, Andy Lutomirski wrote:
> >> > On Tue, Apr 14, 2015 at 10:09 AM, Greg Kroah-Hartman
> >> > <gregkh@linuxfoundation.org> wrote:
> >> > > On Tue, Apr 14, 2015 at 05:44:56PM +0800, Kweh, Hock Leong wrote:
> >> > >> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> >> > >>
> >> > >> 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
> >> > >
> >> > > Ick, why not just have the firmware file location present, and copy it
> >> > > to the sysfs file directly from userspace, instead of this two-step
> >> > > process?
> >> >
> >> > Because it's not at all obvious how error handling should work in that case.
> >>
> >> I don't understand how the error handling is any different. The kernel
> >> ends up copying the data in through the firmware interface both ways, we
> >> just aren't creating yet-another-firmware-path in the system.
> >
> > If I run uefi-update-capsule foo.bin, I want it to complain if the
> > UEFI call fails. In contrast, if I boot and my ath10k firmware is
> > bad, there's no explicit user interaction that should fail; instead I
> > have no network device and I get to read the logs and figure out why.
> > IOW bad volatile device firmware is just like a bad device driver,
> > whereas bad UEFI capsules are problems that should be reported to
> > whatever tried to send them to UEFI.
> >
> > --Andy
> >
> There are several types of errors that can be returned by
> UpdateCapsule(), allowing
> us to distinguish between capsules that are not supported by the
> platform, capsules
> that must be updated at boot time, and capsule updates that failed due
> to a device error.
> I think we really do want this type of information returned to capsule loader.

Ok, all of that sounds like you really want a character device, with an
ioctl, to do this. Just use a misc device and your infrastructure will
be handled for you (sysfs, character device, etc.) and away you go.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Fri, 17 Apr, at 03:49:24PM, Greg KH wrote:
>
> Not really, the kernel namespace is what matters at that point in time.
>
> And maybe it does matter, I haven't thought through all of the issues.
> But passing a path from userspace, to the kernel, to have the kernel
> turn around again and use that path is full of nasty consequences at
> times due to namespaces, let's avoid all of that please.

Oh crap. The namespace issue is a good point and not something I'd
thought of at all.

At this point, I think we've really run out of objections to Andy's
suggestion of implementing this as a misc device, right?

The concern I had about userspace tooling can partly be addressed by
including the source in tools/ in the kernel tree. So provided we do
that, I'm happy enough to see this implemented as a misc device because
the other options we've explored just haven't panned out.

Objections?

--
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
> -----Original Message-----
> From: Matt Fleming [mailto:matt@codeblueprint.co.uk]
> Sent: Friday, April 17, 2015 10:37 PM
>
> On Fri, 17 Apr, at 03:49:24PM, Greg KH wrote:
> >
> > Not really, the kernel namespace is what matters at that point in time.
> >
> > And maybe it does matter, I haven't thought through all of the issues.
> > But passing a path from userspace, to the kernel, to have the kernel
> > turn around again and use that path is full of nasty consequences at
> > times due to namespaces, let's avoid all of that please.
>
> Oh crap. The namespace issue is a good point and not something I'd
> thought of at all.
>
> At this point, I think we've really run out of objections to Andy's
> suggestion of implementing this as a misc device, right?
>
> The concern I had about userspace tooling can partly be addressed by
> including the source in tools/ in the kernel tree. So provided we do
> that, I'm happy enough to see this implemented as a misc device because
> the other options we've explored just haven't panned out.
>
> Objections?
>
> --
> Matt Fleming, Intel Open Source Technology Center

Hi Greg, Matt & Andy,

Wait .... wait a minute. I am lost. I think I have missed something.
Let me summarize the message I got from the email threads.
=========================================================
Greg:- Recommends to use "cat file.bin > /sys/.../capsule_loader" due to
there is concern on kernel namespace with this submitted design which using
the request firmware API.

Andy:- Prefer to have an interface that could return the error code and
suggested char device where the ioctl can meet the purpose.

Matt:- Prefer to use kernel interface only as embedded world may not want
to include userland tool.
==========================================================

I think I have missed somewhere why not using "cat file.bin > /sys/.../loader"
and now change to misc device. Is it because the ioctl could return a custom
error code (for example: platform not supported, capsule header error)
where the "cat file.bin > /sys/.../loader" interface cannot do? Hmm ......

Regarding the 'reboot require' status, is it critical to have a 1 to 1 status match
with the capsule upload binary? Is it okay to have one sysfs file note to tell the
overall status (for example: 10 capsule binaries uploaded but one require
reboot, so the status shows reboot require is yes)? I am not here trying to argue
anything. I am just trying to find out what kind of info is needed but the sysfs
could not provide.

Please imagine if your whole Linux system (kernel + rootfs) has to fit into 6MB
space and you don't even have the gcc compiler included into the package.
I believe in this environment, kernel interface + shell command is the only
interaction that user could work with.

Btw, just to make sure I get it correctly, is misc device refer to the device
that created by misc_register() from drivers/char/misc.c and not asked to
put this kernel module under drivers/misc/* location, right?

And Matt mentioned including the source into tools/* in kernel tree. I have
a question: Is this tool can be compiled during kernel compilation and
eventually auto included into the rootfs package? Sorry, I am new to OS
creation and maybe this is stupid question.


Thanks & Regards,
Wilson

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Mon, Apr 20, 2015 at 03:28:32AM +0000, Kweh, Hock Leong wrote:
> Regarding the 'reboot require' status, is it critical to have a 1 to 1 status match
> with the capsule upload binary? Is it okay to have one sysfs file note to tell the
> overall status (for example: 10 capsule binaries uploaded but one require
> reboot, so the status shows reboot require is yes)? I am not here trying to argue
> anything. I am just trying to find out what kind of info is needed but the sysfs
> could not provide.
>
> Please imagine if your whole Linux system (kernel + rootfs) has to fit into 6MB
> space and you don't even have the gcc compiler included into the package.
> I believe in this environment, kernel interface + shell command is the only
> interaction that user could work with.

Why would you have to have gcc on such a system? Why is that a
requirement for having an ioctl/char interface?

And if you only have 6Mb of space, you don't have UEFI, sorry, there's
no way that firmware can get that small.

> Btw, just to make sure I get it correctly, is misc device refer to the device
> that created by misc_register() from drivers/char/misc.c and not asked to
> put this kernel module under drivers/misc/* location, right?

Yes, use misc_register()

> And Matt mentioned including the source into tools/* in kernel tree. I have
> a question: Is this tool can be compiled during kernel compilation and
> eventually auto included into the rootfs package? Sorry, I am new to OS
> creation and maybe this is stupid question.

If you ask to build it as part of the configuration, yes, it can be
built. See how the tools are build as part of the kernel tree for more
information about this.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Fri, 2015-04-17 at 15:49 +0200, Greg Kroah-Hartman wrote:
> On Thu, Apr 16, 2015 at 09:42:31AM +0000, Kweh, Hock Leong wrote:
> > > -----Original Message-----
> > > From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> > > Sent: Wednesday, April 15, 2015 9:19 PM
> > >
> > > On Wed, Apr 15, 2015 at 11:32:29AM +0000, Kweh, Hock Leong wrote:
> > > > > -----Original Message-----
> > > > > From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> > > > > Sent: Tuesday, April 14, 2015 10:09 PM
> > > > >
> > > > > On Tue, Apr 14, 2015 at 05:44:56PM +0800, Kweh, Hock Leong wrote:
> > > > > > From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
> > > > > >
> > > > > > 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
> > > > >
> > > > > Ick, why not just have the firmware file location present, and copy it
> > > > > to the sysfs file directly from userspace, instead of this two-step
> > > > > process?
> > > >
> > > > Err .... I may not catch your meaning correctly. Are you trying to say
> > > > that you would prefer the user to perform:
> > > >
> > > > cat file.bin > /sys/.../capsule_loader
> > > >
> > > > instead of
> > > >
> > > > echo -n "/path/to/binary" > /sys/..../capsule_laoder
> > >
> > > Yes. What's the namespace of your /path/to/binary/ and how do you know
> > > the kernel has the same one when it does the firmware load call? By
> > > just copying the data with 'cat', you don't have to worry about
> > > namespace issues at all.
> >
> > Hi Greg,
> >
> > Let me double confirm that I understand your concern correctly. You are
> > trying to tell that some others module may use a 'same' namespace to
> > request the firmware but never release it. Then when our module trying
> > to request the firmware by passing in the 'same' namespace, I will get the
> > previous data instead of the current binary data from the path I want.
>
> Yes.
>
> > Hmm .... I believe this concern also apply to all the current request_firmware
> > APIs right? And I believe the coincidence to have 'same' file name namespace
> > would be higher than full path namespace.
>
> Not really, the kernel namespace is what matters at that point in time.
>
> And maybe it does matter, I haven't thought through all of the issues.
> But passing a path from userspace, to the kernel, to have the kernel
> turn around again and use that path is full of nasty consequences at
> times due to namespaces, let's avoid all of that please.

So just to clarify this, namespaces are designed not to cause a problem
here, provided the operation is handled correctly (this is key; it is
easy do design operations which will screw up no end if done wrongly).

The file name to object translation is handled by the mount name space,
which is the operative one of the process doing the echo. For a
longstanding object (i.e. one which will exist beyond the call to the
system of the current process) you need either to convert to the actual
underlying object (usually a file descriptor) which has an existence
independent of the namespace (and perform all the necessary security
validations before returning control back to userspace, so they occur
within all the namespace constraints of the calling process), or store
sufficient information to redo whatever operation you need to within the
namespace (the former is by far preferred for long lived operations).

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
> -----Original Message-----
> From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> Sent: Monday, April 20, 2015 10:43 PM
>
> On Mon, Apr 20, 2015 at 03:28:32AM +0000, Kweh, Hock Leong wrote:
> > Regarding the 'reboot require' status, is it critical to have a 1 to 1 status
> match
> > with the capsule upload binary? Is it okay to have one sysfs file note to tell
> the
> > overall status (for example: 10 capsule binaries uploaded but one require
> > reboot, so the status shows reboot require is yes)? I am not here trying to
> argue
> > anything. I am just trying to find out what kind of info is needed but the
> sysfs
> > could not provide.
> >
> > Please imagine if your whole Linux system (kernel + rootfs) has to fit into
> 6MB
> > space and you don't even have the gcc compiler included into the package.
> > I believe in this environment, kernel interface + shell command is the only
> > interaction that user could work with.
>
> Why would you have to have gcc on such a system? Why is that a
> requirement for having an ioctl/char interface?

This is my logic:
- Besides writing a C program (for example), I am not aware any shell script
could perform an ioctl function call. This led me to if I don't have an execution
binary then I need a compiler to compile the source to execution binary.

- For embedded product as mentioned above, not all vendors willing to carry
the userland tool when they are struggling to fit into small memory space.
Yet, you may say this tool would not eat up a lot of space compare to others.
But when the source of this tool being upstream-ed to the tools/ kernel tree,
we cannot stop people to contribute and make the tool more features support,
eventually the embedded product may need to drop the tool.

>
> And if you only have 6Mb of space, you don't have UEFI, sorry, there's
> no way that firmware can get that small.

Actually there is. Quark is one of the examples. The kernel + rootfs take
up 6MB and the UEFI consume only 2MB, so total size 8MB in the spi chip.
If you have an Intel Galileo board, you don't need any mass storage (SD & USB),
you are able to boot to Linux console.


Thanks & Regards,
Wilson
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Tue, Apr 21, 2015 at 03:23:55AM +0000, Kweh, Hock Leong wrote:
> > -----Original Message-----
> > From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> > Sent: Monday, April 20, 2015 10:43 PM
> >
> > On Mon, Apr 20, 2015 at 03:28:32AM +0000, Kweh, Hock Leong wrote:
> > > Regarding the 'reboot require' status, is it critical to have a 1 to 1 status
> > match
> > > with the capsule upload binary? Is it okay to have one sysfs file note to tell
> > the
> > > overall status (for example: 10 capsule binaries uploaded but one require
> > > reboot, so the status shows reboot require is yes)? I am not here trying to
> > argue
> > > anything. I am just trying to find out what kind of info is needed but the
> > sysfs
> > > could not provide.
> > >
> > > Please imagine if your whole Linux system (kernel + rootfs) has to fit into
> > 6MB
> > > space and you don't even have the gcc compiler included into the package.
> > > I believe in this environment, kernel interface + shell command is the only
> > > interaction that user could work with.
> >
> > Why would you have to have gcc on such a system? Why is that a
> > requirement for having an ioctl/char interface?
>
> This is my logic:
> - Besides writing a C program (for example), I am not aware any shell script
> could perform an ioctl function call. This led me to if I don't have an execution
> binary then I need a compiler to compile the source to execution binary.

You would have built it on a separate machine, like the one you used to
build your kernel and other binary packages you are running.

> - For embedded product as mentioned above, not all vendors willing to carry
> the userland tool when they are struggling to fit into small memory space.
> Yet, you may say this tool would not eat up a lot of space compare to others.
> But when the source of this tool being upstream-ed to the tools/ kernel tree,
> we cannot stop people to contribute and make the tool more features support,
> eventually the embedded product may need to drop the tool.

So because someday in the future someone might make the code that is
open source too big for us to use, we are going to reject the idea
today? That really doesn't make any sense.

> > And if you only have 6Mb of space, you don't have UEFI, sorry, there's
> > no way that firmware can get that small.
>
> Actually there is. Quark is one of the examples. The kernel + rootfs take
> up 6MB and the UEFI consume only 2MB, so total size 8MB in the spi chip.
> If you have an Intel Galileo board, you don't need any mass storage (SD & USB),
> you are able to boot to Linux console.

Does Galieleo support this UEFI feature? If so, great, how big is a
binary that can read a file and write the data using an ioctl?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Tue, 2015-04-21 at 09:56 +0200, Greg Kroah-Hartman wrote:
> On Tue, Apr 21, 2015 at 03:23:55AM +0000, Kweh, Hock Leong wrote:
> > > -----Original Message-----
> > > From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> > > Sent: Monday, April 20, 2015 10:43 PM
> > >
> > > On Mon, Apr 20, 2015 at 03:28:32AM +0000, Kweh, Hock Leong wrote:
> > > > Regarding the 'reboot require' status, is it critical to have a 1 to 1 status
> > > match
> > > > with the capsule upload binary? Is it okay to have one sysfs file note to tell
> > > the
> > > > overall status (for example: 10 capsule binaries uploaded but one require
> > > > reboot, so the status shows reboot require is yes)? I am not here trying to
> > > argue
> > > > anything. I am just trying to find out what kind of info is needed but the
> > > sysfs
> > > > could not provide.
> > > >
> > > > Please imagine if your whole Linux system (kernel + rootfs) has to fit into
> > > 6MB
> > > > space and you don't even have the gcc compiler included into the package.
> > > > I believe in this environment, kernel interface + shell command is the only
> > > > interaction that user could work with.
> > >
> > > Why would you have to have gcc on such a system? Why is that a
> > > requirement for having an ioctl/char interface?
> >
> > This is my logic:
> > - Besides writing a C program (for example), I am not aware any shell script
> > could perform an ioctl function call. This led me to if I don't have an execution
> > binary then I need a compiler to compile the source to execution binary.
>
> You would have built it on a separate machine, like the one you used to
> build your kernel and other binary packages you are running.
>
> > - For embedded product as mentioned above, not all vendors willing to carry
> > the userland tool when they are struggling to fit into small memory space.
> > Yet, you may say this tool would not eat up a lot of space compare to others.
> > But when the source of this tool being upstream-ed to the tools/ kernel tree,
> > we cannot stop people to contribute and make the tool more features support,
> > eventually the embedded product may need to drop the tool.
>
> So because someday in the future someone might make the code that is
> open source too big for us to use, we are going to reject the idea
> today? That really doesn't make any sense.

I think we can all agree that interfaces that don't require special
purpose tools are easier to use. That doesn't mean that every interface
has to not use them, because that would be impossible. However it does
mean that if we can get away without using them, we should.

> > > And if you only have 6Mb of space, you don't have UEFI, sorry, there's
> > > no way that firmware can get that small.
> >
> > Actually there is. Quark is one of the examples. The kernel + rootfs take
> > up 6MB and the UEFI consume only 2MB, so total size 8MB in the spi chip.
> > If you have an Intel Galileo board, you don't need any mass storage (SD & USB),
> > you are able to boot to Linux console.
>
> Does Galieleo support this UEFI feature? If so, great, how big is a
> binary that can read a file and write the data using an ioctl?

The capsule file is usually the same size as the NVRAM for an embedded
system (on Galileo Gen I, this is 8MB) it usually includes not only the
UEFI but also the OS payload. I actually load UEFI only capsules on
Galileo and they're around 2MB.

There have been a lot of red herrings in this thread (like namespace
confusion and ideas about how big UEFI FW can be), but the problem
summary is:

We need a way of updating FW including payload via the UEFI
capsule mechanism. Since the payload is usually as big as the
NVRAM and the NVRAM contains the OS, we can't place the payload
at any OS location. Unlike usual firmware, the capsule update
is fire and forget (once the update is done we don't need the
capsule file anymore).

So what we need is a way to load the capsule data from arbitrary storage
and then trigger the update.

Andy, just on the misc device idea, what about triggering the capsule
update from close()? In theory close returns an error code (not sure if
most tools actually check this, though). That means we can do the write
in chunks but pass it in atomically on the close and cat will work
(provided it checks the return code of close).

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Tue, Apr 21, 2015 at 6:21 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> Andy, just on the misc device idea, what about triggering the capsule
> update from close()? In theory close returns an error code (not sure if
> most tools actually check this, though). That means we can do the write
> in chunks but pass it in atomically on the close and cat will work
> (provided it checks the return code of close).

I thought about this but IIRC cat doesn't check the return value from close.

--Andy

>
> James
>
>



--
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Tue, 2015-04-21 at 18:58 -0700, Andy Lutomirski wrote:
> On Tue, Apr 21, 2015 at 6:21 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > Andy, just on the misc device idea, what about triggering the capsule
> > update from close()? In theory close returns an error code (not sure if
> > most tools actually check this, though). That means we can do the write
> > in chunks but pass it in atomically on the close and cat will work
> > (provided it checks the return code of close).
>
> I thought about this but IIRC cat doesn't check the return value from close.

It does in my copy (coreutils-8.23) :

if (!STREQ (infile, "-") && close (input_desc) < 0)
{
error (0, errno, "%s", infile);
ok = false;
}
[...]
if (have_read_stdin && close (STDIN_FILENO) < 0)
error (EXIT_FAILURE, errno, _("closing standard input"));


James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Tue, Apr 21, 2015 at 7:20 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Tue, 2015-04-21 at 18:58 -0700, Andy Lutomirski wrote:
>> On Tue, Apr 21, 2015 at 6:21 PM, James Bottomley
>> <James.Bottomley@hansenpartnership.com> wrote:
>> > Andy, just on the misc device idea, what about triggering the capsule
>> > update from close()? In theory close returns an error code (not sure if
>> > most tools actually check this, though). That means we can do the write
>> > in chunks but pass it in atomically on the close and cat will work
>> > (provided it checks the return code of close).
>>
>> I thought about this but IIRC cat doesn't check the return value from close.
>
> It does in my copy (coreutils-8.23) :
>
> if (!STREQ (infile, "-") && close (input_desc) < 0)
> {
> error (0, errno, "%s", infile);
> ok = false;
> }
> [...]
> if (have_read_stdin && close (STDIN_FILENO) < 0)
> error (EXIT_FAILURE, errno, _("closing standard input"));
>

True, but it's stdout that we care about, not stdin :(

--Andy

>
> James
>
>



--
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Tue, 2015-04-21 at 20:24 -0700, Andy Lutomirski wrote:
> On Tue, Apr 21, 2015 at 7:20 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Tue, 2015-04-21 at 18:58 -0700, Andy Lutomirski wrote:
> >> On Tue, Apr 21, 2015 at 6:21 PM, James Bottomley
> >> <James.Bottomley@hansenpartnership.com> wrote:
> >> > Andy, just on the misc device idea, what about triggering the capsule
> >> > update from close()? In theory close returns an error code (not sure if
> >> > most tools actually check this, though). That means we can do the write
> >> > in chunks but pass it in atomically on the close and cat will work
> >> > (provided it checks the return code of close).
> >>
> >> I thought about this but IIRC cat doesn't check the return value from close.
> >
> > It does in my copy (coreutils-8.23) :
> >
> > if (!STREQ (infile, "-") && close (input_desc) < 0)
> > {
> > error (0, errno, "%s", infile);
> > ok = false;
> > }
> > [...]
> > if (have_read_stdin && close (STDIN_FILENO) < 0)
> > error (EXIT_FAILURE, errno, _("closing standard input"));
> >
>
> True, but it's stdout that we care about, not stdin :(

Gosh you're determined to force me to wade through this source code,
aren't you? That's handled in lib/closeout.c:

/* Close standard output. On error, issue a diagnostic and _exit
with status 'exit_failure'.

...


The point is that, admittedly much to my surprise, it all looks to be
handled by cat ... so we could proceed to have the transaction completed
in close in a misc device (or a sysfs file).

Unless there are any other rabbits you'd like to pull out of the hat?

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Tue, Apr 21, 2015 at 06:58:58PM -0700, Andy Lutomirski wrote:
> On Tue, Apr 21, 2015 at 6:21 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > Andy, just on the misc device idea, what about triggering the capsule
> > update from close()? In theory close returns an error code (not sure if
> > most tools actually check this, though). That means we can do the write
> > in chunks but pass it in atomically on the close and cat will work
> > (provided it checks the return code of close).
>
> I thought about this but IIRC cat doesn't check the return value from close.

I checked this for the use case we'd talked about before - gnu cat
/does/ check the error code, but it's easy to miss how, because
coreutils code has some good ole' gnu-code complexity. It'll print the
strerror() representation, but it always exits with 1 as the error
code.

Specifically the close on the output is handled by this:
---------------
initialize_main (&argc, &argv);
set_program_name (argv[0]);
setlocale (LC_ALL, "");
bindtextdomain (PACKAGE, LOCALEDIR);
textdomain (PACKAGE);

/* Arrange to close stdout if we exit via the
case_GETOPT_HELP_CHAR or case_GETOPT_VERSION_CHAR code.
Normally STDOUT_FILENO is used rather than stdout, so
close_stdout does nothing. */
atexit (close_stdout);

/* Parse command line options. */

while ((c = getopt_long (argc, argv, "benstuvAET", long_options, NULL))
---------------

Which in turn does:
---------------
void
close_stdout (void)
{
if (close_stream (stdout) != 0
&& !(ignore_EPIPE && errno == EPIPE))
{
char const *write_error = _("write error");
if (file_name)
error (0, errno, "%s: %s", quotearg_colon (file_name),
write_error);
else
error (0, errno, "%s", write_error);

_exit (exit_failure);
}

if (close_stream (stderr) != 0)
_exit (exit_failure);
}
---------------

exit_failure is a global from libcoreutils.a which cat never changes
from the default, so it's always 1.

(And of course error() is coreutils' own implementation rather than
glibc's because hey maybe you're not using glibc, but still, it's
there.)

So it's /annoying/ to propagate the error from there programatically,
but it can work.

--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
On Wed, 2015-04-22 at 09:27 -0400, Peter Jones wrote:
> On Tue, Apr 21, 2015 at 06:58:58PM -0700, Andy Lutomirski wrote:
> > On Tue, Apr 21, 2015 at 6:21 PM, James Bottomley
> > <James.Bottomley@hansenpartnership.com> wrote:
> > > Andy, just on the misc device idea, what about triggering the capsule
> > > update from close()? In theory close returns an error code (not sure if
> > > most tools actually check this, though). That means we can do the write
> > > in chunks but pass it in atomically on the close and cat will work
> > > (provided it checks the return code of close).
> >
> > I thought about this but IIRC cat doesn't check the return value from close.
>
> I checked this for the use case we'd talked about before - gnu cat
> /does/ check the error code, but it's easy to miss how, because
> coreutils code has some good ole' gnu-code complexity. It'll print the
> strerror() representation, but it always exits with 1 as the error
> code.
>
> Specifically the close on the output is handled by this:
> ---------------
> initialize_main (&argc, &argv);
> set_program_name (argv[0]);
> setlocale (LC_ALL, "");
> bindtextdomain (PACKAGE, LOCALEDIR);
> textdomain (PACKAGE);
>
> /* Arrange to close stdout if we exit via the
> case_GETOPT_HELP_CHAR or case_GETOPT_VERSION_CHAR code.
> Normally STDOUT_FILENO is used rather than stdout, so
> close_stdout does nothing. */
> atexit (close_stdout);
>
> /* Parse command line options. */
>
> while ((c = getopt_long (argc, argv, "benstuvAET", long_options, NULL))
> ---------------
>
> Which in turn does:
> ---------------
> void
> close_stdout (void)
> {
> if (close_stream (stdout) != 0
> && !(ignore_EPIPE && errno == EPIPE))
> {
> char const *write_error = _("write error");
> if (file_name)
> error (0, errno, "%s: %s", quotearg_colon (file_name),
> write_error);
> else
> error (0, errno, "%s", write_error);
>
> _exit (exit_failure);
> }
>
> if (close_stream (stderr) != 0)
> _exit (exit_failure);
> }
> ---------------
>
> exit_failure is a global from libcoreutils.a which cat never changes
> from the default, so it's always 1.
>
> (And of course error() is coreutils' own implementation rather than
> glibc's because hey maybe you're not using glibc, but still, it's
> there.)
>
> So it's /annoying/ to propagate the error from there programatically,
> but it can work.

Yes, I think we've all agreed we can do it ... it's now a question of
whether we can stomach the ick factor of actually initiating a
transaction in close ... I'm still feeling queasy.

There are quite a few of these 'transactional blob' problems where we'd
like to use a file/device approach because the data is just passed to
something but have problems because the something wants all or nothing
rather than chunks. I think all of us who work at the coal face on this
are not enthused by an ioctl solution because of the need for
non-standard tools to effect it.

The alternative might be a two file approach (either in sysfs or a mini
custom fs), one for load up data and the other for initiate transaction
with the data errors (like overflow) being returned on the load up file
and the transaction errors being returned on the write that initiates
the transaction.

My architectural sense is that transaction on close, provided we can
make it a more universally accepted idea, has a lot of potential because
it's more intuitive than the two file approach.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware [ In reply to ]
> Yes, I think we've all agreed we can do it ... it's now a question of
> whether we can stomach the ick factor of actually initiating a
> transaction in close ... I'm still feeling queasy.

NFS does transactions - including figuring out if the data will fit - on
file close. It does that for real data so I'd relax. With hard disks we
don't even complete a set of actions on close they just float around for
a bit and get committed (but if there is a media failure you lose if you
didnt commit them first via fsync etc)

> The alternative might be a two file approach (either in sysfs or a mini
> custom fs), one for load up data and the other for initiate transaction
> with the data errors (like overflow) being returned on the load up file
> and the transaction errors being returned on the write that initiates
> the transaction.

The two file problem then turns into the "which transaction of the two
done in parallel" problem.

> My architectural sense is that transaction on close, provided we can
> make it a more universally accepted idea, has a lot of potential because
> it's more intuitive than the two file approach.

Agreed

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

1 2 3  View All