[PATCH] efi: add efivars kobject to efi sysfs folder

[PATCH] efi: add efivars kobject to efi sysfs folder

[Lee, Chun-Yi]

UEFI variable filesystem need a new mount point, so this patch add
efivars kobject to efi_kobj for create a /sys/firmware/efi/efivars
folder.

Cc: Matt Fleming <matt.fleming@intel.com>
Cc: Jeremy Kerr <jeremy.kerr@canonical.com>
Cc: Matthew Garrett <mjg@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
---
 drivers/firmware/efivars.c |   11 +++++++++++
 include/linux/efi.h        |    1 +
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 1e1aad0..7c1234e 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -1487,6 +1487,7 @@ void unregister_efivars(struct efivars *efivars)
 		sysfs_remove_bin_file(&efivars->kset->kobj, efivars->del_var);
 	kfree(efivars->new_var);
 	kfree(efivars->del_var);
+	kobject_put(efivars->kobject);
 	kset_unregister(efivars->kset);
 }
 EXPORT_SYMBOL_GPL(unregister_efivars);
@@ -1518,6 +1519,13 @@ int register_efivars(struct efivars *efivars,
 		goto out;
 	}
 
+	efivars->kobject = kobject_create_and_add("efivars", parent_kobj);
+	if (!efivars->kobject) {
+		pr_err("efivars: Subsystem registration failed.\n");
+		error = -ENOMEM;
+		goto err_unreg_vars;
+	}
+
 	/*
 	 * Per EFI spec, the maximum storage allocated for both
 	 * the variable name and variable data is 1024 bytes.
@@ -1562,6 +1570,9 @@ int register_efivars(struct efivars *efivars,
 
 	register_filesystem(&efivars_fs_type);
 
+err_unreg_vars:
+	kset_unregister(efivars->kset);
+
 out:
 	kfree(variable_name);
 
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 1829a97..c993f54 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -654,6 +654,7 @@ struct efivars {
 	spinlock_t lock;
 	struct list_head list;
 	struct kset *kset;
+	struct kobject *kobject;
 	struct bin_attribute *new_var, *del_var;
 	const struct efivar_operations *ops;
 	struct efivar_entry *walk_entry;
-- 
1.7.7

Re: [PATCH] efi: add efivars kobject to efi sysfs folder

[Matt Fleming]

On Thu, 2012-10-04 at 10:24 +0800, Lee, Chun-Yi wrote:
> UEFI variable filesystem need a new mount point, so this patch add
> efivars kobject to efi_kobj for create a /sys/firmware/efi/efivars
> folder.
> 
> Cc: Matt Fleming <matt.fleming@intel.com>
> Cc: Jeremy Kerr <jeremy.kerr@canonical.com>
> Cc: Matthew Garrett <mjg@redhat.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
> ---
>  drivers/firmware/efivars.c |   11 +++++++++++
>  include/linux/efi.h        |    1 +
>  2 files changed, 12 insertions(+), 0 deletions(-)

Jeremy, did you want to pick this up as part of your series?

Actually, shouldn't the new filesystem be called "efivarfs", or "efifs"
to make it more explicit that it is a filesystem?

We also need something in Documentation/filesystems/ describing the old
EFI variable method, why it's no longer any good, and why the new
filesystem is favoured. Does anybody have anything to add to the
following?

diff --git a/Documentation/filesystems/00-INDEX b/Documentation/filesystems/00-INDEX
index 8c624a1..ddf5a83 100644
--- a/Documentation/filesystems/00-INDEX
+++ b/Documentation/filesystems/00-INDEX
@@ -38,6 +38,8 @@ dnotify_test.c
 	- example program for dnotify
 ecryptfs.txt
 	- docs on eCryptfs: stacked cryptographic filesystem for Linux.
+efivars.txt
+	- info for the efivars filesystem.
 exofs.txt
 	- info, usage, mount options, design about EXOFS.
 ext2.txt
diff --git a/Documentation/filesystems/efivars.txt b/Documentation/filesystems/efivars.txt
new file mode 100644
index 0000000..4350c1a
--- /dev/null
+++ b/Documentation/filesystems/efivars.txt
@@ -0,0 +1,16 @@
+
+efivars - a (U)EFI variable filesystem
+
+The efivars filesystem was created to address the shortcomings of
+using entries in sysfs to maintain EFI variables. The old sysfs EFI
+variables code only supported variables of up to 1024 bytes. This
+limitation existed in version 0.99 of the EFI specification, but was
+removed before any full releases. Since variables can now be larger
+than a single page, sysfs isn't the best interface for this.
+
+Variables can be created, deleted and modified with the efivars
+filesystem.
+
+The efivars filesystem is typically mounted like this,
+
+	mount -t efivars none /sys/firmware/efi/efivars

Re: [PATCH] efi: add efivars kobject to efi sysfs folder

[Jeremy Kerr]

Hi Matt,

> Jeremy, did you want to pick this up as part of your series?

I have this in my series, yes. I'm just working on the authenticated 
delete code, then will send out the next revision.

Speaking of which - Peter: I've realised that doing a GetVariable() 
after the SetVariable is a much cleaner way of checking whether we need 
to drop the inode after an authenticated delete.

Because the inode's size may not be simply updated by the write(), we 
should probably be doing a GetVariable() after an APPEND_WRITE, to read 
the new size. Rather than having a separate (and quite complex) code 
path to check for the delete case, we may as well use the same logic to 
determine if the variable has been deleted.

> Actually, shouldn't the new filesystem be called "efivarfs", or "efifs"
> to make it more explicit that it is a filesystem?

I'm not too fussed about the name, but +1 for efivarfs.

> We also need something in Documentation/filesystems/ describing the old
> EFI variable method, why it's no longer any good, and why the new
> filesystem is favoured. Does anybody have anything to add to the
> following?

Looks good to me. We can always elaborate later, if necessary.

Cheers,


Jeremy

Re: [PATCH] efi: add efivars kobject to efi sysfs folder

[Peter Jones]

On Thu, Oct 04, 2012 at 05:08:48PM +0800, Jeremy Kerr wrote:
> Hi Matt,
> 
> >Jeremy, did you want to pick this up as part of your series?
> 
> I have this in my series, yes. I'm just working on the authenticated
> delete code, then will send out the next revision.
> 
> Speaking of which - Peter: I've realised that doing a GetVariable()
> after the SetVariable is a much cleaner way of checking whether we
> need to drop the inode after an authenticated delete.
> 
> Because the inode's size may not be simply updated by the write(),
> we should probably be doing a GetVariable() after an APPEND_WRITE,
> to read the new size. Rather than having a separate (and quite
> complex) code path to check for the delete case, we may as well use
> the same logic to determine if the variable has been deleted.

Yeah, this is fine.

> >Actually, shouldn't the new filesystem be called "efivarfs", or "efifs"
> >to make it more explicit that it is a filesystem?
> 
> I'm not too fussed about the name, but +1 for efivarfs.

I like efivarfs for the name, as well.

> >We also need something in Documentation/filesystems/ describing the old
> >EFI variable method, why it's no longer any good, and why the new
> >filesystem is favoured. Does anybody have anything to add to the
> >following?
> 
> Looks good to me. We can always elaborate later, if necessary.

Yep.

-- 
        Peter

Re: [PATCH] efi: add efivars kobject to efi sysfs folder

[joeyli]

於 四，2012-10-04 於 09:54 +0100，Matt Fleming 提到：
> On Thu, 2012-10-04 at 10:24 +0800, Lee, Chun-Yi wrote:
> > UEFI variable filesystem need a new mount point, so this patch add
> > efivars kobject to efi_kobj for create a /sys/firmware/efi/efivars
> > folder.
> > 
> > Cc: Matt Fleming <matt.fleming@intel.com>
> > Cc: Jeremy Kerr <jeremy.kerr@canonical.com>
> > Cc: Matthew Garrett <mjg@redhat.com>
> > Cc: H. Peter Anvin <hpa@zytor.com>
> > Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
> > ---
> >  drivers/firmware/efivars.c |   11 +++++++++++
> >  include/linux/efi.h        |    1 +
> >  2 files changed, 12 insertions(+), 0 deletions(-)
> 
> Actually, shouldn't the new filesystem be called "efivarfs", or "efifs"
> to make it more explicit that it is a filesystem?

I used "efivars" because this folder generated by efivars. But, I like
your suggestion for use "efivarfs", it's better then "efivars".

> 
> We also need something in Documentation/filesystems/ describing the old
> EFI variable method, why it's no longer any good, and why the new
> filesystem is favoured. Does anybody have anything to add to the
> following?
> 
> diff --git a/Documentation/filesystems/00-INDEX b/Documentation/filesystems/00-INDEX
> index 8c624a1..ddf5a83 100644
> --- a/Documentation/filesystems/00-INDEX
> +++ b/Documentation/filesystems/00-INDEX
> @@ -38,6 +38,8 @@ dnotify_test.c
>  	- example program for dnotify
>  ecryptfs.txt
>  	- docs on eCryptfs: stacked cryptographic filesystem for Linux.
> +efivars.txt
> +	- info for the efivars filesystem.
>  exofs.txt
>  	- info, usage, mount options, design about EXOFS.
>  ext2.txt
> diff --git a/Documentation/filesystems/efivars.txt b/Documentation/filesystems/efivars.txt
> new file mode 100644
> index 0000000..4350c1a
> --- /dev/null
> +++ b/Documentation/filesystems/efivars.txt
> @@ -0,0 +1,16 @@
> +
> +efivars - a (U)EFI variable filesystem
> +
> +The efivars filesystem was created to address the shortcomings of
> +using entries in sysfs to maintain EFI variables. The old sysfs EFI
> +variables code only supported variables of up to 1024 bytes. This
> +limitation existed in version 0.99 of the EFI specification, but was
> +removed before any full releases. Since variables can now be larger
> +than a single page, sysfs isn't the best interface for this.
> +
> +Variables can be created, deleted and modified with the efivars
> +filesystem.
> +
> +The efivars filesystem is typically mounted like this,
> +
> +	mount -t efivars none /sys/firmware/efi/efivars
> 

The above documentation is really good!


Thanks a lot!
Joey Lee