Re: [RFC 3/3] efi: add capsule update capability via sysfs

Andy Lutomirski
2015-04-29T23:25:51Z

On Wed, Apr 29, 2015 at 4:12 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> From: James Bottomley <JBottomley@Odin.com>
>
> The firmware update should be applied simply by doing
>
> cat fw_file > /sys/firmware/capsule/update
>
> With a properly formatted fw_file. Any error will be returned on close of
> stdout. util-linux returns errors correctly from closing stdout, but firmware
> shippers should check whatever utilities package they use correctly captures
> the error return on close.

s/util-linux/coreutils/

This still makes my API sense itch. It's kind of an abuse of open/write/close.

>
> Signed-off-by: James Bottomley <JBottomley@Odin.com>
> ---
> drivers/firmware/efi/Makefile | 2 +-
> drivers/firmware/efi/capsule.c | 78 ++++++++++++++++++++++++++++++++++++++++++
> drivers/firmware/efi/capsule.h | 2 ++
> drivers/firmware/efi/efi.c | 8 +++++
> 4 files changed, 89 insertions(+), 1 deletion(-)
> create mode 100644 drivers/firmware/efi/capsule.c
> create mode 100644 drivers/firmware/efi/capsule.h
>
> diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
> index d8be608..698846e 100644
> --- a/drivers/firmware/efi/Makefile
> +++ b/drivers/firmware/efi/Makefile
> @@ -1,7 +1,7 @@
> #
> # Makefile for linux kernel
> #
> -obj-$(CONFIG_EFI) += efi.o vars.o reboot.o
> +obj-$(CONFIG_EFI) += efi.o vars.o reboot.o capsule.o
> obj-$(CONFIG_EFI_VARS) += efivars.o
> obj-$(CONFIG_EFI_VARS_PSTORE) += efi-pstore.o
> obj-$(CONFIG_UEFI_CPER) += cper.o
> diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
> new file mode 100644
> index 0000000..1fd78e7
> --- /dev/null
> +++ b/drivers/firmware/efi/capsule.c
> @@ -0,0 +1,78 @@
> +#include <linux/efi.h>
> +#include <linux/slab.h>
> +#include <linux/transaction_helper.h>
> +
> +#include "capsule.h"
> +
> +static struct kset *capsule_kset;
> +static struct transaction_buf *capsule_buf;
> +
> +static int capsule_data_write(struct file *file, struct kobject *kobj,
> + struct bin_attribute *attr,
> + char *buffer, loff_t offset, size_t count)
> +{
> + if (!capsule_buf) {
> + capsule_buf = kmalloc(sizeof(*capsule_buf), GFP_KERNEL);
> + if (!capsule_buf)
> + return -ENOMEM;
> + transaction_init(capsule_buf);
> + }
> +
> + return transaction_write(capsule_buf, buffer, offset, count);
> +}

This seems unlikely to have good effects if two struct files are active at once.

Also, I think you crash if you open and close without calling write,
and I don't know what whether there can be spurious flushes (fsync?).

--Andy