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

[Andy Lutomirski]

On Wed, Apr 29, 2015 at 4:12 PM, James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
> From: James Bottomley <JBottomley@xxxxxxxx>
>
> 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@xxxxxxxx>
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/