Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751942AbbD2Xgf (ORCPT ); Wed, 29 Apr 2015 19:36:35 -0400 Received: from bedivere.hansenpartnership.com ([66.63.167.143]:38515 "EHLO bedivere.hansenpartnership.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751894AbbD2Xgd (ORCPT ); Wed, 29 Apr 2015 19:36:33 -0400 Message-ID: <1430350592.2189.50.camel@HansenPartnership.com> Subject: Re: [RFC 3/3] efi: add capsule update capability via sysfs From: James Bottomley To: Andy Lutomirski Cc: "linux-efi@vger.kernel.org" , "Kweh, Hock Leong" , LKML , Greg Kroah-Hartman , Peter Jones Date: Wed, 29 Apr 2015 16:36:32 -0700 In-Reply-To: References: <1430348859.2189.37.camel@HansenPartnership.com> <1430349130.2189.43.camel@HansenPartnership.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.11 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3834 Lines: 102 On Wed, 2015-04-29 at 16:25 -0700, Andy Lutomirski wrote: > On Wed, Apr 29, 2015 at 4:12 PM, James Bottomley > wrote: > > From: James Bottomley > > > > 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. It works ... and according to Alan, NFS is already doing it. I suppose we can have a do over of the whole debate again ... > > > > Signed-off-by: James Bottomley > > --- > > 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 > > +#include > > +#include > > + > > +#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. 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. > 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 > 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 -- 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/