[RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[Jeremy Kerr]

From: Matthew Garrett <mjg@redhat.com>

The existing EFI variables code only supports 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. So,
instead, let's add a filesystem. Variables can be read, written and
created, with the first 4 bytes of each variable representing its UEFI
attributes. The create() method doesn't actually commit to flash since
zero-length variables can't exist per-spec.

Updates from Jeremy Kerr <jeremy.kerr@canonical.com>.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>

---
v2: Add check for < 4-byte writes

---
 drivers/firmware/efivars.c |  384 ++++++++++++++++++++++++++++++++++++-
 include/linux/efi.h        |    5 
 2 files changed, 383 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 47408e8..1e1aad0 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -80,6 +80,10 @@
 #include <linux/slab.h>
 #include <linux/pstore.h>
 
+#include <linux/fs.h>
+#include <linux/ramfs.h>
+#include <linux/pagemap.h>
+
 #include <asm/uaccess.h>
 
 #define EFIVARS_VERSION "0.08"
@@ -91,6 +95,7 @@ MODULE_LICENSE("GPL");
 MODULE_VERSION(EFIVARS_VERSION);
 
 #define DUMP_NAME_LEN 52
+#define GUID_LEN 37
 
 /*
  * The maximum size of VariableName + Data = 1024
@@ -108,7 +113,6 @@ struct efi_variable {
 	__u32         Attributes;
 } __attribute__((packed));
 
-
 struct efivar_entry {
 	struct efivars *efivars;
 	struct efi_variable var;
@@ -122,6 +126,9 @@ struct efivar_attribute {
 	ssize_t (*store)(struct efivar_entry *entry, const char *buf, size_t count);
 };
 
+static struct efivars __efivars;
+static struct efivar_operations ops;
+
 #define PSTORE_EFI_ATTRIBUTES \
 	(EFI_VARIABLE_NON_VOLATILE | \
 	 EFI_VARIABLE_BOOTSERVICE_ACCESS | \
@@ -618,14 +625,380 @@ static struct kobj_type efivar_ktype = {
 	.default_attrs = def_attrs,
 };
 
-static struct pstore_info efi_pstore_info;
-
 static inline void
 efivar_unregister(struct efivar_entry *var)
 {
 	kobject_put(&var->kobj);
 }
 
+static int efivars_file_open(struct inode *inode, struct file *file)
+{
+	file->private_data = inode->i_private;
+	return 0;
+}
+
+static ssize_t efivars_file_write(struct file *file, const char __user *userbuf,
+				  size_t count, loff_t *ppos)
+{
+	struct efivar_entry *var = file->private_data;
+	struct efivars *efivars;
+	efi_status_t status;
+	void *data;
+	u32 attributes;
+	struct inode *inode = file->f_mapping->host;
+	int datasize = count - sizeof(attributes);
+
+	if (count < sizeof(attributes))
+		return -EINVAL;
+
+	data = kmalloc(datasize, GFP_KERNEL);
+
+	if (!data)
+		return -ENOMEM;
+
+	efivars = var->efivars;
+
+	if (copy_from_user(&attributes, userbuf, sizeof(attributes))) {
+		count = -EFAULT;
+		goto out;
+	}
+
+	if (attributes & ~(EFI_VARIABLE_MASK)) {
+		count = -EINVAL;
+		goto out;
+	}
+
+	if (copy_from_user(data, userbuf + sizeof(attributes), datasize)) {
+		count = -EFAULT;
+		goto out;
+	}
+
+	if (validate_var(&var->var, data, datasize) == false) {
+		count = -EINVAL;
+		goto out;
+	}
+
+	status = efivars->ops->set_variable(var->var.VariableName,
+					    &var->var.VendorGuid,
+					    attributes, datasize,
+					    data);
+
+	switch (status) {
+	case EFI_SUCCESS:
+		mutex_lock(&inode->i_mutex);
+		i_size_write(inode, count);
+		mutex_unlock(&inode->i_mutex);
+		break;
+	case EFI_INVALID_PARAMETER:
+		count = -EINVAL;
+		break;
+	case EFI_OUT_OF_RESOURCES:
+		count = -ENOSPC;
+		break;
+	case EFI_DEVICE_ERROR:
+		count = -EIO;
+		break;
+	case EFI_WRITE_PROTECTED:
+		count = -EROFS;
+		break;
+	case EFI_SECURITY_VIOLATION:
+		count = -EACCES;
+		break;
+	case EFI_NOT_FOUND:
+		count = -ENOENT;
+		break;
+	default:
+		count = -EINVAL;
+		break;
+	}
+out:
+	kfree(data);
+
+	return count;
+}
+
+static ssize_t efivars_file_read(struct file *file, char __user *userbuf,
+				 size_t count, loff_t *ppos)
+{
+	struct efivar_entry *var = file->private_data;
+	struct efivars *efivars = var->efivars;
+	efi_status_t status;
+	unsigned long datasize = 0;
+	u32 attributes;
+	void *data;
+	ssize_t size;
+
+	status = efivars->ops->get_variable(var->var.VariableName,
+					    &var->var.VendorGuid,
+					    &attributes, &datasize, NULL);
+
+	if (status != EFI_BUFFER_TOO_SMALL)
+		return 0;
+
+	data = kmalloc(datasize + 4, GFP_KERNEL);
+
+	if (!data)
+		return 0;
+
+	status = efivars->ops->get_variable(var->var.VariableName,
+					    &var->var.VendorGuid,
+					    &attributes, &datasize,
+					    (data + 4));
+
+	if (status != EFI_SUCCESS)
+		return 0;
+
+	memcpy(data, &attributes, 4);
+	size = simple_read_from_buffer(userbuf, count, ppos,
+					data, datasize + 4);
+	kfree(data);
+
+	return size;
+}
+
+static void efivars_evict_inode(struct inode *inode)
+{
+	clear_inode(inode);
+}
+
+static const struct super_operations efivars_ops = {
+	.statfs = simple_statfs,
+	.drop_inode = generic_delete_inode,
+	.evict_inode = efivars_evict_inode,
+	.show_options = generic_show_options,
+};
+
+static struct super_block *efivars_sb;
+
+static const struct inode_operations efivars_dir_inode_operations;
+
+static const struct file_operations efivars_file_operations = {
+	.open	= efivars_file_open,
+	.read	= efivars_file_read,
+	.write	= efivars_file_write,
+	.llseek	= default_llseek,
+};
+
+static struct inode *efivars_get_inode(struct super_block *sb,
+				const struct inode *dir, int mode, dev_t dev)
+{
+	struct inode *inode = new_inode(sb);
+
+	if (inode) {
+		inode->i_ino = get_next_ino();
+		inode->i_uid = inode->i_gid = 0;
+		inode->i_mode = mode;
+		inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
+		switch (mode & S_IFMT) {
+		case S_IFREG:
+			inode->i_fop = &efivars_file_operations;
+			break;
+		case S_IFDIR:
+			inode->i_op = &efivars_dir_inode_operations;
+			inode->i_fop = &simple_dir_operations;
+			inc_nlink(inode);
+			break;
+		}
+	}
+	return inode;
+}
+
+static void efivars_hex_to_guid(const char *str, efi_guid_t *guid)
+{
+	guid->b[0] = hex_to_bin(str[6]) << 4 | hex_to_bin(str[7]);
+	guid->b[1] = hex_to_bin(str[4]) << 4 | hex_to_bin(str[5]);
+	guid->b[2] = hex_to_bin(str[2]) << 4 | hex_to_bin(str[3]);
+	guid->b[3] = hex_to_bin(str[0]) << 4 | hex_to_bin(str[1]);
+	guid->b[4] = hex_to_bin(str[11]) << 4 | hex_to_bin(str[12]);
+	guid->b[5] = hex_to_bin(str[9]) << 4 | hex_to_bin(str[10]);
+	guid->b[6] = hex_to_bin(str[16]) << 4 | hex_to_bin(str[17]);
+	guid->b[7] = hex_to_bin(str[14]) << 4 | hex_to_bin(str[15]);
+	guid->b[8] = hex_to_bin(str[19]) << 4 | hex_to_bin(str[20]);
+	guid->b[9] = hex_to_bin(str[21]) << 4 | hex_to_bin(str[22]);
+	guid->b[10] = hex_to_bin(str[24]) << 4 | hex_to_bin(str[25]);
+	guid->b[11] = hex_to_bin(str[26]) << 4 | hex_to_bin(str[27]);
+	guid->b[12] = hex_to_bin(str[28]) << 4 | hex_to_bin(str[29]);
+	guid->b[13] = hex_to_bin(str[30]) << 4 | hex_to_bin(str[31]);
+	guid->b[14] = hex_to_bin(str[32]) << 4 | hex_to_bin(str[33]);
+	guid->b[15] = hex_to_bin(str[34]) << 4 | hex_to_bin(str[35]);
+}
+
+static int efivars_create(struct inode *dir, struct dentry *dentry,
+			  umode_t mode, bool excl)
+{
+	struct inode *inode = efivars_get_inode(dir->i_sb, dir, mode, 0);
+	struct efivars *efivars = &__efivars;
+	struct efivar_entry *var;
+	int namelen, i = 0, err = 0;
+
+	if (dentry->d_name.len < 38)
+		return -EINVAL;
+
+	if (!inode)
+		return -ENOSPC;
+
+	var = kzalloc(sizeof(struct efivar_entry), GFP_KERNEL);
+
+	if (!var)
+		return -ENOMEM;
+
+	namelen = dentry->d_name.len - GUID_LEN;
+
+	efivars_hex_to_guid(dentry->d_name.name + namelen + 1,
+			&var->var.VendorGuid);
+
+	for (i = 0; i < namelen; i++)
+		var->var.VariableName[i] = dentry->d_name.name[i];
+
+	var->var.VariableName[i] = '\0';
+
+	inode->i_private = var;
+	var->efivars = efivars;
+	var->kobj.kset = efivars->kset;
+
+	err = kobject_init_and_add(&var->kobj, &efivar_ktype, NULL, "%s",
+			     dentry->d_name.name);
+	if (err)
+		goto out;
+
+	kobject_uevent(&var->kobj, KOBJ_ADD);
+	spin_lock(&efivars->lock);
+	list_add(&var->list, &efivars->list);
+	spin_unlock(&efivars->lock);
+	d_instantiate(dentry, inode);
+	dget(dentry);
+out:
+	if (err)
+		kfree(var);
+	return err;
+}
+
+static int efivars_unlink(struct inode *dir, struct dentry *dentry)
+{
+	struct efivar_entry *var = dentry->d_inode->i_private;
+	struct efivars *efivars = var->efivars;
+	efi_status_t status;
+
+	spin_lock(&efivars->lock);
+
+	status = efivars->ops->set_variable(var->var.VariableName,
+					    &var->var.VendorGuid,
+					    0, 0, NULL);
+
+	if (status == EFI_SUCCESS || status == EFI_NOT_FOUND) {
+		list_del(&var->list);
+		spin_unlock(&efivars->lock);
+		efivar_unregister(var);
+		drop_nlink(dir);
+		dput(dentry);
+		return 0;
+	}
+
+	spin_unlock(&efivars->lock);
+	return -EINVAL;
+};
+
+int efivars_fill_super(struct super_block *sb, void *data, int silent)
+{
+	struct inode *inode = NULL;
+	struct dentry *root;
+	struct efivar_entry *entry, *n;
+	struct efivars *efivars = &__efivars;
+	int err;
+
+	efivars_sb = sb;
+
+	sb->s_maxbytes          = MAX_LFS_FILESIZE;
+	sb->s_blocksize         = PAGE_CACHE_SIZE;
+	sb->s_blocksize_bits    = PAGE_CACHE_SHIFT;
+	sb->s_magic             = PSTOREFS_MAGIC;
+	sb->s_op                = &efivars_ops;
+	sb->s_time_gran         = 1;
+
+	inode = efivars_get_inode(sb, NULL, S_IFDIR | 0755, 0);
+	if (!inode) {
+		err = -ENOMEM;
+		goto fail;
+	}
+	inode->i_op = &efivars_dir_inode_operations;
+
+	root = d_make_root(inode);
+	sb->s_root = root;
+	if (!root) {
+		err = -ENOMEM;
+		goto fail;
+	}
+
+	list_for_each_entry_safe(entry, n, &efivars->list, list) {
+		struct inode *inode;
+		struct dentry *dentry, *root = efivars_sb->s_root;
+		char *name;
+		unsigned long size = 0;
+		int len, i;
+
+		len = utf16_strlen(entry->var.VariableName);
+
+		/* GUID plus trailing NULL */
+		name = kmalloc(len + 38, GFP_ATOMIC);
+
+		for (i = 0; i < len; i++)
+			name[i] = entry->var.VariableName[i] & 0xFF;
+
+		name[len] = '-';
+
+		efi_guid_unparse(&entry->var.VendorGuid, name + len + 1);
+
+		name[len+GUID_LEN] = '\0';
+
+		inode = efivars_get_inode(efivars_sb, root->d_inode,
+					  S_IFREG | 0644, 0);
+		dentry = d_alloc_name(root, name);
+
+		efivars->ops->get_variable(entry->var.VariableName,
+					   &entry->var.VendorGuid,
+					   &entry->var.Attributes,
+					   &size,
+					   NULL);
+
+		mutex_lock(&inode->i_mutex);
+		inode->i_private = entry;
+		i_size_write(inode, size+4);
+		mutex_unlock(&inode->i_mutex);
+		d_add(dentry, inode);
+	}
+
+	return 0;
+fail:
+	iput(inode);
+	return err;
+}
+
+static struct dentry *efivars_mount(struct file_system_type *fs_type,
+				    int flags, const char *dev_name, void *data)
+{
+	return mount_single(fs_type, flags, data, efivars_fill_super);
+}
+
+static void efivars_kill_sb(struct super_block *sb)
+{
+	kill_litter_super(sb);
+	efivars_sb = NULL;
+}
+
+static struct file_system_type efivars_fs_type = {
+	.name    = "efivars",
+	.mount   = efivars_mount,
+	.kill_sb = efivars_kill_sb,
+};
+
+static const struct inode_operations efivars_dir_inode_operations = {
+	.lookup = simple_lookup,
+	.unlink = efivars_unlink,
+	.create = efivars_create,
+};
+
+static struct pstore_info efi_pstore_info;
+
 #ifdef CONFIG_PSTORE
 
 static int efi_pstore_open(struct pstore_info *psi)
@@ -1187,6 +1560,8 @@ int register_efivars(struct efivars *efivars,
 		pstore_register(&efivars->efi_pstore_info);
 	}
 
+	register_filesystem(&efivars_fs_type);
+
 out:
 	kfree(variable_name);
 
@@ -1194,9 +1569,6 @@ out:
 }
 EXPORT_SYMBOL_GPL(register_efivars);
 
-static struct efivars __efivars;
-static struct efivar_operations ops;
-
 /*
  * For now we register the efi subsystem with the firmware subsystem
  * and the vars subsystem with the efi subsystem.  In the future, it
diff --git a/include/linux/efi.h b/include/linux/efi.h
index ec45ccd..1829a97 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -29,7 +29,12 @@
 #define EFI_UNSUPPORTED		( 3 | (1UL << (BITS_PER_LONG-1)))
 #define EFI_BAD_BUFFER_SIZE     ( 4 | (1UL << (BITS_PER_LONG-1)))
 #define EFI_BUFFER_TOO_SMALL	( 5 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_NOT_READY		( 6 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_DEVICE_ERROR	( 7 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_WRITE_PROTECTED	( 8 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_OUT_OF_RESOURCES	( 9 | (1UL << (BITS_PER_LONG-1)))
 #define EFI_NOT_FOUND		(14 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_SECURITY_VIOLATION	(26 | (1UL << (BITS_PER_LONG-1)))
 
 typedef unsigned long efi_status_t;
 typedef u8 efi_bool_t;

Re: [RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[Matt Fleming]

On Thu, 2012-09-06 at 13:40 +0800, Jeremy Kerr wrote:
> From: Matthew Garrett <mjg@redhat.com>
> 
> The existing EFI variables code only supports 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. So,
> instead, let's add a filesystem. Variables can be read, written and
> created, with the first 4 bytes of each variable representing its UEFI
> attributes. The create() method doesn't actually commit to flash since
> zero-length variables can't exist per-spec.
> 
> Updates from Jeremy Kerr <jeremy.kerr@canonical.com>.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
> 
> ---
> v2: Add check for < 4-byte writes

Am I correct in thinking that this filesystem doesn't handle deletion of
authenticated variables?

Re: [RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[Jeremy Kerr]

Hi Matt,

> Am I correct in thinking that this filesystem doesn't handle deletion of
> authenticated variables?

Unless I'm missing something, this should work fine; we just pass the 
EFI_VARIABLE_AUTHENTICATION_2 descriptor (with the usual attribute 
header), but no data following the descriptor.

Cheers,


Jeremy

Re: [RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[Matt Fleming]

(Cc'ing Peter Jones who wrote a auth deletion patch for efivars)

On Thu, 2012-09-06 at 22:23 +0800, Jeremy Kerr wrote:
> Hi Matt,
> 
> > Am I correct in thinking that this filesystem doesn't handle deletion of
> > authenticated variables?
> 
> Unless I'm missing something, this should work fine; we just pass the 
> EFI_VARIABLE_AUTHENTICATION_2 descriptor (with the usual attribute 
> header), but no data following the descriptor.

I think this is the problematic chunk of code,

static int efivars_unlink(struct inode *dir, struct dentry *dentry)             
{                                                                               
        struct efivar_entry *var = dentry->d_inode->i_private;                  
        struct efivars *efivars = var->efivars;                                 
        efi_status_t status;                                                    
                                                                                
        spin_lock(&efivars->lock);                                              
                                                                                
        status = efivars->ops->set_variable(var->var.VariableName,              
                                            &var->var.VendorGuid,               
                                            0, 0, NULL);                        

For deleting authenticated variables Attributes, DataSize and Data need
to be non-zero. I think DataSize == AuthInfo descriptor. Peter?

Re: [RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[Jeremy Kerr]

Hi Matt,

> I think this is the problematic chunk of code,
>
> static int efivars_unlink(struct inode *dir, struct dentry *dentry)
> {
>          struct efivar_entry *var = dentry->d_inode->i_private;
>          struct efivars *efivars = var->efivars;
>          efi_status_t status;
>
>          spin_lock(&efivars->lock);
>
>          status = efivars->ops->set_variable(var->var.VariableName,
>                                              &var->var.VendorGuid,
>                                              0, 0, NULL);
>
> For deleting authenticated variables Attributes, DataSize and Data need
> to be non-zero. I think DataSize == AuthInfo descriptor. Peter?

That's correct, but we can't use unlink to delete a secure variable, we 
need to do a write instead, in order to provide the authentication 
descriptor.

But yes, we may need to alter the efivars_file_write path so that such a 
write actually results in the dentry being removed...

Cheers,


Jeremy

Re: [RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[Matt Fleming]

On Thu, 2012-09-06 at 22:37 +0800, Jeremy Kerr wrote:
> That's correct, but we can't use unlink to delete a secure variable, we 
> need to do a write instead, in order to provide the authentication 
> descriptor.

Ah, duh. Thanks for clearing that up.

Re: Fwd: [RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[joeyli]

On Thu, 2012-09-06 at 13:40 +0800, Jeremy Kerr wrote:
> From: Matthew Garrett <mjg@redhat.com>
> 
> The existing EFI variables code only supports 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.
> So,
> instead, let's add a filesystem. Variables can be read, written and
> created, with the first 4 bytes of each variable representing its UEFI
> attributes. The create() method doesn't actually commit to flash since
> zero-length variables can't exist per-spec.
> 
> Updates from Jeremy Kerr <jeremy.kerr@canonical.com>.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
> 
> ---
> v2: Add check for < 4-byte writes
> 

Do we have plan to create a new kobject add to /sys/firmware/efi for
provide a fixed mount point to efivars fs?
e.g. /sys/firmware/efi/efivars

Or we just direct reuse current /sys/firmeware/efi/vars? But, that means
we need think for the backward compatibility if choice reuse vars
folder.


Thanks a lot!
Joey Lee

Re: [RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[joeyli]

於 二，2012-09-11 於 15:23 +0800，lee joey 提到：
> From: Matthew Garrett <mjg@redhat.com>
> 
> The existing EFI variables code only supports 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.
> So,
> instead, let's add a filesystem. Variables can be read, written and
> created, with the first 4 bytes of each variable representing its UEFI
> attributes. The create() method doesn't actually commit to flash since
> zero-length variables can't exist per-spec.
> 
> Updates from Jeremy Kerr <jeremy.kerr@canonical.com>.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
> 
> ---
> v2: Add check for < 4-byte writes 

Do we have plan to create a new kobject add to /sys/firmware/efi for
provide a fixed mount point to efivars fs?
e.g. /sys/firmware/efi/efivars

Or we just direct reuse current /sys/firmeware/efi/vars? But, that means
we need think for the backward compatibility if choice reuse vars
folder.


Thanks a lot!
Joey Lee

Re: [RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[Peter Jones]

On Thu, 2012-09-13 at 16:10 +0800, joeyli wrote:

> Do we have plan to create a new kobject add to /sys/firmware/efi for
> provide a fixed mount point to efivars fs?
> e.g. /sys/firmware/efi/efivars
> 
> Or we just direct reuse current /sys/firmeware/efi/vars? But, that means
> we need think for the backward compatibility if choice reuse vars
> folder.

I'm not sure that's a terribly large concern - if you haven't updated
your tools to use the new efivars, don't mount the new efivars.

-- 
  Peter

Re: [RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[joeyli]

Hi Peter, 

於 四，2012-09-13 於 09:52 -0400，Peter Jones 提到：
> On Thu, 2012-09-13 at 16:10 +0800, joeyli wrote:
> 
> > Do we have plan to create a new kobject add to /sys/firmware/efi for
> > provide a fixed mount point to efivars fs?
> > e.g. /sys/firmware/efi/efivars
> > 
> > Or we just direct reuse current /sys/firmeware/efi/vars? But, that means
> > we need think for the backward compatibility if choice reuse vars
> > folder.
> 
> I'm not sure that's a terribly large concern - if you haven't updated
> your tools to use the new efivars, don't mount the new efivars.
> 

Thanks for your comment!

Joey Lee

Re: [RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[H. Peter Anvin]

On 09/13/2012 08:45 PM, joeyli wrote:
> Hi Peter,
>
> 於 四，2012-09-13 於 09:52 -0400，Peter Jones 提到：
>> On Thu, 2012-09-13 at 16:10 +0800, joeyli wrote:
>>
>>> Do we have plan to create a new kobject add to /sys/firmware/efi for
>>> provide a fixed mount point to efivars fs?
>>> e.g. /sys/firmware/efi/efivars
>>>
>>> Or we just direct reuse current /sys/firmeware/efi/vars? But, that means
>>> we need think for the backward compatibility if choice reuse vars
>>> folder.
>>
>> I'm not sure that's a terribly large concern - if you haven't updated
>> your tools to use the new efivars, don't mount the new efivars.
>>
>
> Thanks for your comment!
>

I think it is wrong.  The new API should have a different path.

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[joeyli]

於 二，2012-09-11 於 15:23 +0800，lee joey 提到：
> From: Matthew Garrett <mjg@redhat.com>
> 
> The existing EFI variables code only supports 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.
> So,
> instead, let's add a filesystem. Variables can be read, written and
> created, with the first 4 bytes of each variable representing its UEFI
> attributes. The create() method doesn't actually commit to flash since
> zero-length variables can't exist per-spec.
> 
> Updates from Jeremy Kerr <jeremy.kerr@canonical.com>.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> 

I and Gary tested create, update, delete unauthenticated variable, works
fine.

Tested-by: Lee, Chun-Yi <jlee@suse.com>


Thanks a lot!
Joey Lee

Re: [RFC,PATCH v2] efi: Add support for a UEFI variable filesystem

[joeyli]

於 四，2012-09-13 於 21:41 -0700，H. Peter Anvin 提到：
> On 09/13/2012 08:45 PM, joeyli wrote:
> > Hi Peter,
> >
> > 於 四，2012-09-13 於 09:52 -0400，Peter Jones 提到：
> >> On Thu, 2012-09-13 at 16:10 +0800, joeyli wrote:
> >>
> >>> Do we have plan to create a new kobject add to /sys/firmware/efi for
> >>> provide a fixed mount point to efivars fs?
> >>> e.g. /sys/firmware/efi/efivars
> >>>
> >>> Or we just direct reuse current /sys/firmeware/efi/vars? But, that means
> >>> we need think for the backward compatibility if choice reuse vars
> >>> folder.
> >>
> >> I'm not sure that's a terribly large concern - if you haven't updated
> >> your tools to use the new efivars, don't mount the new efivars.
> >>
> >
> > Thanks for your comment!
> >
> 
> I think it is wrong.  The new API should have a different path.
> 
> 	-hpa
> 

OK, so, if still keep the old API and have new mount point for UEFI
variable filesystem. Then how about add this simple patch for create new
mount point to /sys/firmware/efi/efivars.


Thanks a lot!
Joey Lee


>From 5e64382a9a4ad538dd5ca94072d19c7a70e4c650 Mon Sep 17 00:00:00 2001
From: Lee, Chun-Yi <jlee@suse.com>
Date: Sat, 15 Sep 2012 10:33:46 +0800
Subject: [PATCH] efi: add efivars kobject to efi sysfs folder

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.

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