Add capsule update using error on close semantics
Subsystems (4)
cover Add capsule update using error on close semantics
This is a straw man implementation. The three patches firstly thread the needed ->flush() file op through sysfs and kernfs. The next one extracts transaction buffer handling from firmware_class.c and makes it generic in a lib helper and the third patch adds a bare bones capsule update (I suspect the latter needs more work, since it doesn't implement the scatterlist).
James Bottomley (3):
sysfs,kernfs: add flush operation
firmware_class: split out transaction helpers
efi: add capsule update capability via sysfs
drivers/base/firmware_class.c | 117 ++++--------------------------- drivers/firmware/efi/Makefile | 2 +- drivers/firmware/efi/capsule.c | 78 +++++++++++++++++++++ drivers/firmware/efi/capsule.h | 2 + drivers/firmware/efi/efi.c | 8 +++ fs/kernfs/file.c | 16 +++++ fs/sysfs/file.c | 16 +++++ include/linux/kernfs.h | 2 + include/linux/sysfs.h | 2 + include/linux/transaction_helper.h | 26 +++++++ lib/Makefile | 2 +- lib/transaction_helper.c | 137 +++++++++++++++++++++++++++++++++++++ 12 files changed, 304 insertions(+), 104 deletions(-) create mode 100644 drivers/firmware/efi/capsule.c create mode 100644 drivers/firmware/efi/capsule.h create mode 100644 include/linux/transaction_helper.h create mode 100644 lib/transaction_helper.c
James
RFC 1/3 sysfs,kernfs: add flush operation
From: James Bottomley <[email protected]>
This is necessary to allow sysfs operations to return error in close (we are using close to initiate and return a possible error from a transaction).
--- fs/kernfs/file.c | 16 ++++++++++++++++ fs/sysfs/file.c | 16 ++++++++++++++++ include/linux/kernfs.h | 2 ++ include/linux/sysfs.h | 2 ++ 4 files changed, 36 insertions(+)
fs/kernfs/file.c +16 -0
@@ -802,6 +802,21 @@ static unsigned int kernfs_fop_poll(struct file *filp, poll_table *wait)return DEFAULT_POLLMASK |POLLERR |POLLPRI ; } +static int kernfs_fop_flush (struct file *file,fl_owner_t id) +{ +struct kernfs_open_file *of =kernfs_of (file); +const struct kernfs_ops *ops; +int rc =0 ; + +mutex_lock (&of->mutex); + ops =kernfs_ops (of->kn); +if (ops->flush) + rc = ops->flush (of, id); +mutex_unlock (&of->mutex); + +return rc; +} +static void kernfs_notify_workfn (struct work_struct *work) {struct kernfs_node *kn;
@@ -891,6 +906,7 @@ const struct file_operations kernfs_file_fops = { .open = kernfs_fop_open, .release = kernfs_fop_release, .poll = kernfs_fop_poll, + .flush = kernfs_fop_flush, };/**
fs/sysfs/file.c +16 -0
@@ -162,6 +162,19 @@ static int sysfs_kf_bin_mmap(struct kernfs_open_file *of,return battr->mmap (of->file, kobj, battr, vma); } +static int sysfs_kf_bin_flush (struct kernfs_open_file *of, +fl_owner_t id) +{ +struct bin_attribute *battr = of->kn->priv; +struct kobject *kobj = of->kn->parent->priv; +int rc =0 ; + +if (battr->flush) + rc = battr->flush (of->file, kobj, battr, id); + +return rc; +} +void sysfs_notify (struct kobject *kobj,const char *dir,const char *attr) {struct kernfs_node *kn = kobj->sd, *tmp;
@@ -222,17 +235,20 @@ static const struct kernfs_ops sysfs_bin_kfops_ro = {static const struct kernfs_ops sysfs_bin_kfops_wo = { .write = sysfs_kf_bin_write, + .flush = sysfs_kf_bin_flush, };static const struct kernfs_ops sysfs_bin_kfops_rw = { .read = sysfs_kf_bin_read, .write = sysfs_kf_bin_write, + .flush = sysfs_kf_bin_flush, };static const struct kernfs_ops sysfs_bin_kfops_mmap = { .read = sysfs_kf_bin_read, .write = sysfs_kf_bin_write, .mmap = sysfs_kf_bin_mmap, + .flush = sysfs_kf_bin_flush, }; int sysfs_add_file_mode_ns(struct kernfs_node *parent,
include/linux/kernfs.h +2 -0
@@ -11,6 +11,7 @@#include <linux/err.h> #include <linux/list.h> #include <linux/mutex.h> +#include <linux/fs.h> #include <linux/idr.h> #include <linux/lockdep.h> #include <linux/rbtree.h>
@@ -225,6 +226,7 @@ struct kernfs_ops { loff_t off);int (*mmap)(struct kernfs_open_file *of,struct vm_area_struct *vma); +int (*flush) (struct kernfs_open_file *of,fl_owner_t id);#ifdef CONFIG_DEBUG_LOCK_ALLOC struct lock_class_key lockdep_key;
include/linux/sysfs.h +2 -0
@@ -152,6 +152,8 @@ struct bin_attribute { char *, loff_t, size_t);int (*mmap)(struct file *,struct kobject *,struct bin_attribute *attr,struct vm_area_struct *vma); +int (*flush)(struct file *,struct kobject *, +struct bin_attribute *attr,fl_owner_t id); };/**
Signature · 1 line
2.1.4
Greg Kroah-Hartman
maintainerRe: [RFC 1/3] sysfs,kernfs: add flush operation
Quoted 6 lines · James Bottomley
From: James Bottomley <[email protected]>
This is necessary to allow sysfs operations to return error in close (we are using close to initiate and return a possible error from a transaction).
Signed-off-by James Bottomley <[email protected]>
Are there any side-affects now that we have a flush() call for binary files that the vfs will act differently from not defining one at all?
if not, no objection to me for this change.
thanks,
greg k-h
Re: [RFC 1/3] sysfs,kernfs: add flush operation
Quoted 10 lines · Greg Kroah-Hartman
Quoted 6 lines · James Bottomley
From: James Bottomley <[email protected]>
This is necessary to allow sysfs operations to return error in close (we are using close to initiate and return a possible error from a transaction).
Signed-off-by James Bottomley <[email protected]>Are there any side-affects now that we have a
flush()call for binary files that the vfs will act differently from not defining one at all?
See answer to Andy, but basically, no, VFS flush is only used to tidy up before close.
To be honest, it might be nice if fsync *would* issue a transaction because I think having a multi-transaction API with write/fsync/write is not such a bad thing. It turns out that fsync and fdatasync do have a VFS op, but it's ->fsync() not ->flush(), so we have the option to wire this up if anyone finds a use case.
Quoted 1 line
if not, no objection to me for this change.
Thanks,
James
Quoted 8 lines
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
RFC 2/3 firmware_class: split out transaction helpers
In reply to James BottomleyFrom: James Bottomley <[email protected]>
The firmware class contains code to manage an arbitrary sized buffer for discrete read and write operations. We need precisely this ability to update firmware capsule files (and likely for other transactions as well), so split out the capability into a library helper
--- drivers/base/firmware_class.c | 117 ++++--------------------------- include/linux/transaction_helper.h | 26 +++++++ lib/Makefile | 2 +- lib/transaction_helper.c | 137 +++++++++++++++++++++++++++++++++++++ 4 files changed, 179 insertions(+), 103 deletions(-) create mode 100644 include/linux/transaction_helper.h create mode 100644 lib/transaction_helper.c
drivers/base/firmware_class.c +15 -102
@@ -29,6 +29,7 @@#include <linux/syscore_ops.h> #include <linux/reboot.h> #include <linux/security.h> +#include <linux/transaction_helper.h> #include <generated/utsrelease.h>
@@ -144,10 +145,8 @@ struct firmware_buf {size_t size;#ifdef CONFIG_FW_LOADER_USER_HELPER bool is_paged_buf; +struct transaction_buf *tb;bool need_uevent; -struct page **pages; -int nr_pages; -int page_array_size;struct list_head pending_list;#endif char fw_id[];
@@ -248,13 +247,9 @@ static void __fw_free_buf(struct kref *ref)spin_unlock (&fwc->lock);#ifdef CONFIG_FW_LOADER_USER_HELPER -if (buf->is_paged_buf) { -int i; -vunmap (buf->data); -for (i =0 ; i < buf->nr_pages; i++) -__free_page (buf->pages[i]); -kfree (buf->pages); - } else +if (buf->is_paged_buf) +transaction_free (buf->tb); + else#endif vfree (buf->data);kfree (buf);
@@ -374,7 +369,7 @@ static void fw_set_page_data(struct firmware_buf *buf, struct firmware *fw) { fw->priv = buf;#ifdef CONFIG_FW_LOADER_USER_HELPER - fw->pages = buf->pages; + fw->pages = buf->tb->pages;#endif fw->size = buf->size; fw->data = buf->data;
@@ -591,7 +586,7 @@ static int fw_map_pages_buf(struct firmware_buf *buf)return 0 ;vunmap (buf->data); - buf->data =vmap (buf->pages, buf->nr_pages,0 ,PAGE_KERNEL_RO ); + buf->data =transaction_map (buf->tb,PAGE_KERNEL_RO );if (!buf->data)return -ENOMEM ;return 0 ;
@@ -618,7 +613,6 @@ static ssize_t firmware_loading_store(struct device *dev,struct firmware_buf *fw_buf;ssize_t written = count;int loading =simple_strtol (buf,NULL ,10 ); -int i;mutex_lock (&fw_lock); fw_buf = fw_priv->buf;
@@ -629,12 +623,8 @@ static ssize_t firmware_loading_store(struct device *dev,case 1 :/* discarding any previous partial load */ if (!test_bit (FW_STATUS_DONE , &fw_buf->status)) { -for (i =0 ; i < fw_buf->nr_pages; i++) -__free_page (fw_buf->pages[i]); -kfree (fw_buf->pages); - fw_buf->pages =NULL ; - fw_buf->page_array_size =0 ; - fw_buf->nr_pages =0 ; +transaction_free (fw_buf->tb); +transaction_init (fw_buf->tb);set_bit (FW_STATUS_LOADING , &fw_buf->status); }break ;
@@ -701,74 +691,14 @@ static ssize_t firmware_data_read(struct file *filp, struct kobject *kobj, ret_count = -ENODEV ; goto out; } -if (offset > buf->size) { - ret_count =0 ; - goto out; - } -if (count > buf->size - offset) - count = buf->size - offset; - - ret_count = count; - -while (count) { -void *page_data; -int page_nr = offset >>PAGE_SHIFT ; -int page_ofs = offset & (PAGE_SIZE -1 ); -int page_cnt =min_t (size_t,PAGE_SIZE - page_ofs, count); - - page_data =kmap (buf->pages[page_nr]); - -memcpy (buffer, page_data + page_ofs, page_cnt); -kunmap (buf->pages[page_nr]); - buffer += page_cnt; - offset += page_cnt; - count -= page_cnt; - } + ret_count =transaction_read (buf->tb, buffer, offset, count); + out:mutex_unlock (&fw_lock);return ret_count; } -static int fw_realloc_buffer (struct firmware_priv *fw_priv,int min_size) -{ -struct firmware_buf *buf = fw_priv->buf; -int pages_needed =PAGE_ALIGN (min_size) >>PAGE_SHIFT ; - -/* If the array of pages is too small, grow it... */ -if (buf->page_array_size < pages_needed) { -int new_array_size =max (pages_needed, - buf->page_array_size *2 ); -struct page **new_pages; - - new_pages =kmalloc (new_array_size *sizeof (void *), -GFP_KERNEL ); -if (!new_pages) { -fw_load_abort (fw_priv); -return -ENOMEM ; - } -memcpy (new_pages, buf->pages, - buf->page_array_size *sizeof (void *)); -memset (&new_pages[buf->page_array_size],0 ,sizeof (void *) * - (new_array_size - buf->page_array_size)); -kfree (buf->pages); - buf->pages = new_pages; - buf->page_array_size = new_array_size; - } - -while (buf->nr_pages < pages_needed) { - buf->pages[buf->nr_pages] = -alloc_page (GFP_KERNEL | __GFP_HIGHMEM); - -if (!buf->pages[buf->nr_pages]) { -fw_load_abort (fw_priv); -return -ENOMEM ; - } - buf->nr_pages++; - } -return 0 ; -} -/** * firmware_data_write - write method for firmware * @filp: open sysfs file
@@ -800,29 +730,12 @@ static ssize_t firmware_data_write(struct file *filp, struct kobject *kobj, goto out; } - retval =fw_realloc_buffer (fw_priv, offset + count); -if (retval) - goto out; - - retval = count; - -while (count) { -void *page_data; -int page_nr = offset >>PAGE_SHIFT ; -int page_ofs = offset & (PAGE_SIZE -1 ); -int page_cnt =min_t (size_t,PAGE_SIZE - page_ofs, count); - - page_data =kmap (buf->pages[page_nr]); - -memcpy (page_data + page_ofs, buffer, page_cnt); + retval =transaction_write (buf->tb, buffer, offset, count); +if (retval <0 ) +fw_load_abort (fw_priv); -kunmap (buf->pages[page_nr]); - buffer += page_cnt; - offset += page_cnt; - count -= page_cnt; - } + buf->size = buf->tb->size; - buf->size =max_t (size_t, offset, buf->size); out:mutex_unlock (&fw_lock);return retval;
include/linux/transaction_helper.h new file +26 -0
@@ -0,0 +1,26 @@ +/* +* transaction_helper.h - headers and defines for lib/transaction_helper.c +*/ + +#ifndef _TRANSACTION_HELPER_H_ +#define _TRANSACTION_HELPER_H_ + +#include <linux/vmalloc.h> /* pgprot_t */ + +struct transaction_buf { +void *vaddr; +size_t size; +struct page **pages; +int nr_pages; +int page_array_size; +}; + +void transaction_free (struct transaction_buf *buf); +void *transaction_map (struct transaction_buf *buf,pgprot_t prot); +void transaction_init (struct transaction_buf *buf); +ssize_t transaction_write (struct transaction_buf *buf,char *data, +loff_t offset,size_t count); +ssize_t transaction_read (struct transaction_buf *buf,char *data,loff_t offset, +size_t count); + +#endif /* _TRANSACTION_HELPER_H_ */
lib/Makefile +1 -1
@@ -13,7 +13,7 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \ sha1.o md5.o irq_regs.o argv_split.o \ proportions.o flex_proportions.o ratelimit.o show_mem.o \ is_single_threaded.o plist.o decompress.o kobject_uevent.o \ - earlycpio.o seq_buf.o + earlycpio.o seq_buf.o transaction_helper.o obj-$(CONFIG_ARCH_HAS_DEBUG_STRICT_USER_COPY_CHECKS) += usercopy.o lib-$(CONFIG_MMU) += ioremap.o
lib/transaction_helper.c new file +137 -0
137 lines not on this page · raw message
Signature · 1 line
2.1.4
Greg Kroah-Hartman
maintainerRe: [RFC 2/3] firmware_class: split out transaction helpers
Quoted 8 lines · James Bottomley
From: James Bottomley <[email protected]>
The firmware class contains code to manage an arbitrary sized buffer for discrete read and write operations. We need precisely this ability to update firmware capsule files (and likely for other transactions as well), so split out the capability into a library helper
Signed-off-by James Bottomley <[email protected]>
Quoted 8 lines
--- drivers/base/firmware_class.c | 117 ++++--------------------------- include/linux/transaction_helper.h | 26 +++++++ lib/Makefile | 2 +- lib/transaction_helper.c | 137 +++++++++++++++++++++++++++++++++++++ 4 files changed, 179 insertions(+), 103 deletions(-) create mode 100644 include/linux/transaction_helper.h create mode 100644 lib/transaction_helper.c
Interesting, do you think there are other places that can use this "transaction_helper" library? And a bit more documentation would be nice, but on the whole, no objection from me.
thanks,
greg k-h
Re: [RFC 2/3] firmware_class: split out transaction helpers
Quoted 18 lines · Greg Kroah-Hartman
Quoted 8 lines · James Bottomley
From: James Bottomley <[email protected]>
The firmware class contains code to manage an arbitrary sized buffer for discrete read and write operations. We need precisely this ability to update firmware capsule files (and likely for other transactions as well), so split out the capability into a library helper
Signed-off-by James Bottomley <[email protected]>Quoted 8 lines
--- drivers/base/firmware_class.c | 117 ++++--------------------------- include/linux/transaction_helper.h | 26 +++++++ lib/Makefile | 2 +- lib/transaction_helper.c | 137 +++++++++++++++++++++++++++++++++++++ 4 files changed, 179 insertions(+), 103 deletions(-) create mode 100644 include/linux/transaction_helper.h create mode 100644 lib/transaction_helper.c
Quoted 2 lines
Interesting, do you think there are other places that can use this "
transaction_helper" library?
Certainly, everywhere binary attributes are used for more than a page of data, I think. Certainly things like the PPC nvram dump read stuff and probably a few other architecture specific bin files for nvram. I imagine there'll be a long series of janatorial patches if people are interested.
Quoted 2 lines
And a bit more documentation would be
nice, but on the whole, no objection from me.
Yes, that's why it's an RFC.I wanted to show the plan in code, get all the modification suggestions and then do the proper implementation. Docs come after the API is agreed.
James
Re: [RFC 2/3] firmware_class: split out transaction helpers
In reply to James BottomleyQuoted 8 lines · James Bottomley
From: James Bottomley <[email protected]>
The firmware class contains code to manage an arbitrary sized buffer for discrete read and write operations. We need precisely this ability to update firmware capsule files (and likely for other transactions as well), so split out the capability into a library helper
Signed-off-by James Bottomley <[email protected]>
Quoted 8 lines
--- drivers/base/firmware_class.c | 117 ++++--------------------------- include/linux/transaction_helper.h | 26 +++++++ lib/Makefile | 2 +- lib/transaction_helper.c | 137 +++++++++++++++++++++++++++++++++++++ 4 files changed, 179 insertions(+), 103 deletions(-) create mode 100644 include/linux/transaction_helper.h create mode 100644 lib/transaction_helper.c
(Sorry, I'm coming to this incredibly late)
This patch is pretty neat and I wish something like this had existed when I originally wrote the EFI capsule patches.
James, do you have any plans to resubmit this as a non-RFC? If not, do you mind if I pick this up and rebase my capsule patches ontop of it?
Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
Re: [RFC 2/3] firmware_class: split out transaction helpers
Quoted 18 lines · Matt Fleming
Quoted 8 lines · James Bottomley
From: James Bottomley <[email protected]>
The firmware class contains code to manage an arbitrary sized buffer for discrete read and write operations. We need precisely this ability to update firmware capsule files (and likely for other transactions as well), so split out the capability into a library helper
Signed-off-by James Bottomley <[email protected]>Quoted 8 lines
--- drivers/base/firmware_class.c | 117 ++++--------------------------- include/linux/transaction_helper.h | 26 +++++++ lib/Makefile | 2 +- lib/transaction_helper.c | 137 +++++++++++++++++++++++++++++++++++++ 4 files changed, 179 insertions(+), 103 deletions(-) create mode 100644 include/linux/transaction_helper.h create mode 100644 lib/transaction_helper.c
Quoted 7 lines
(Sorry, I'm coming to this incredibly late)
This patch is pretty neat and I wish something like this had existed when I originally wrote the EFI capsule patches.
James, do you have any plans to resubmit this as a non-RFC? If not, do you mind if I pick this up and rebase my capsule patches ontop of it?
Sort of, but I was stalled trying to work out how to combine with your capsule patches. If you're going to do that work, I can resubmit my stuff as a patch with all the changes based on the review comments and then you can do the real work ...
James
Re: [RFC 2/3] firmware_class: split out transaction helpers
Quoted 7 lines · James Bottomley
Quoted 6 lines · Matt Fleming
(Sorry, I'm coming to this incredibly late)
This patch is pretty neat and I wish something like this had existed when I originally wrote the EFI capsule patches.
Quoted 7 lines
Quoted 2 lines
James, do you have any plans to resubmit this as a non-RFC? If not, do you mind if I pick this up and rebase my capsule patches ontop of it?
Sort of, but I was stalled trying to work out how to combine with your capsule patches. If you're going to do that work, I can resubmit my stuff as a patch with all the changes based on the review comments and then you can do the real work ...
Yeah, that sounds great. I'm happy to respin my patches on top.
Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
RFC 3/3 efi: add capsule update capability via sysfs
In reply to James BottomleyFrom: James Bottomley <[email protected]>
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.
--- 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
drivers/firmware/efi/Makefile +1 -1
@@ -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
drivers/firmware/efi/capsule.c new file +78 -0
78 lines not on this page · raw message
drivers/firmware/efi/capsule.h new file +2 -0
@@ -0,0 +1,2 @@ +int efi_capsule_init (struct kobject *efi_kobj); +void efi_capsule_remove (struct kobject *efi_kobj);
drivers/firmware/efi/efi.c +8 -0
@@ -25,6 +25,8 @@#include <linux/io.h> #include <linux/platform_device.h> +#include "capsule.h" +struct efi __read_mostly efi = { .mps =EFI_INVALID_TABLE_ADDR , .acpi =EFI_INVALID_TABLE_ADDR ,
@@ -219,8 +221,14 @@ static int __init efisubsys_init(void) goto err_remove_group; } + error =efi_capsule_init (efi_kobj); +if (error) + goto err_remove_capsule; +return 0 ; +err_remove_capsule: +efi_capsule_remove (efi_kobj); err_remove_group:sysfs_remove_group (efi_kobj, &efi_subsys_attr_group); err_unregister:
Signature · 1 line
2.1.4
Re: [RFC 3/3] efi: add capsule update capability via sysfs
Quoted 10 lines · James Bottomley
From: James Bottomley <[email protected]>
The firmware update should be applied simply by doing
cat
fw_file>/sys/firmware/capsule/updateWith 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.
Quoted 42 lines
Signed-off-by James Bottomley <[email protected]>--- 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.hdrivers/firmware/efi/Makefile +1
-1@@ -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.odrivers/firmware/efi/capsule.c new file +12
-0@@ -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)
drivers/firmware/efi/capsule.c line 22
Quoted 10 lines
drivers/firmware/efi/capsule.c new file +10
-0+{ +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
Re: [RFC 3/3] efi: add capsule update capability via sysfs
Quoted 13 lines · Andy Lutomirski
Quoted 10 lines · James Bottomley
From: James Bottomley <[email protected]>
The firmware update should be applied simply by doing
cat
fw_file>/sys/firmware/capsule/updateWith 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.
Quoted 4 lines
s/util-linux/coreutils/
This still makes my API sense itch. It's kind of an abuse of open/write/close.
It works ... and according to Alan, NFS is already doing it. I suppose we can have a do over of the whole debate again ...
Quoted 47 lines
Quoted 47 lines
Signed-off-by James Bottomley <[email protected]>--- 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.hdrivers/firmware/efi/Makefile +1
-1@@ -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.odrivers/firmware/efi/capsule.c new file +17
-0@@ -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 ;
Quoted 8 lines
drivers/firmware/efi/capsule.c line 22
Quoted 5 lines
drivers/firmware/efi/capsule.c new file +5
-0+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.
I thought of threading ->open and using that to make it exclusive. But then I thought caveat emptor.
I think for multiple files, I need a mutex in the transaction code just to ensure orderly access.
Quoted 1 line
Also, I think you crash if you open and close without calling write,
yes there should be an if (!capsule_buf) return -EINVAL in flush
Quoted 1 line
and I don't know what whether there can be spurious flushes (fsync?).
It turns out that the bdi flusher and the fop->flush() operation are totally different things. ->flush() is used mostly just to do stuff on close (NFS uses it to tidy up for instance).
James
Re: [RFC 3/3] efi: add capsule update capability via sysfs
Quoted 14 lines · James Bottomley
Quoted 13 lines · Andy Lutomirski
Quoted 10 lines · James Bottomley
From: James Bottomley <[email protected]>
The firmware update should be applied simply by doing
cat
fw_file>/sys/firmware/capsule/updateWith 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.
Quoted 7 lines
Quoted 4 lines
s/util-linux/coreutils/
This still makes my API sense itch. It's kind of an abuse of open/write/close.
It works ... and according to Alan, NFS is already doing it. I suppose we can have a do over of the whole debate again ...
I think that NFS is at least writing to actual files as opposed to trying to implement some kind of transactions.
Blech, whatever. This approach certainly works, as long as no one trips over the busybox thing. Maybe there should also be /sys/something_that_errors_on_close that people can use as a test.
--Andy
Quoted 38 lines
-----Original Message-----
From: James Bottomley [mailto:[email protected]]
Sent: Thursday, April 30, 2015 7:08 AM
To: [email protected]
Cc: Kweh, Hock Leong; LKML; Andy Lutomirski; Greg Kroah-Hartman; Peter Jones
Subject: [RFC 0/3] Add capsule update using error on close semanticsThis is a straw man implementation. The three patches firstly thread the needed ->flush() file op through sysfs and kernfs. The next one extracts transaction buffer handling from
firmware_class.cand makes it generic in a lib helper and the third patch adds a bare bones capsule update (I suspect the latter needs more work, since it doesn't implement the scatterlist).James Bottomley (3):
sysfs,kernfs: add flush operation
firmware_class: split out transaction helpers
efi: add capsule update capability via sysfsdrivers/base/firmware_class.c | 117 ++++--------------------------- drivers/firmware/efi/Makefile | 2 +- drivers/firmware/efi/capsule.c | 78 +++++++++++++++++++++ drivers/firmware/efi/capsule.h | 2 + drivers/firmware/efi/efi.c | 8 +++ fs/kernfs/file.c | 16 +++++ fs/sysfs/file.c | 16 +++++ include/linux/kernfs.h | 2 + include/linux/sysfs.h | 2 + include/linux/transaction_helper.h | 26 +++++++ lib/Makefile | 2 +- lib/transaction_helper.c | 137 +++++++++++++++++++++++++++++++++++++ 12 files changed, 304 insertions(+), 104 deletions(-) create mode 100644 drivers/firmware/efi/capsule.c create mode 100644 drivers/firmware/efi/capsule.h create mode 100644 include/linux/transaction_helper.h create mode 100644 lib/transaction_helper.c
Quoted 1 line
James
Hi James,
I like the sysfs enhancement but require Greg to buy in the idea.
For the efi capsule part, Matt has implemented some APIs where
you could get the patch at http://permalink.gmane.org/gmane.linux.kernel.efi/4837.
So, I would think that leveraging the APIs that Matt has created
is a better choice.
Thanks & Regards,
Wilson