efi/efivars: Create efivars mount point via efivars abstraction
PATCH v2 1/1 efi/efivars: Create efivars mount point via efivars abstraction
This patch creates efivars mount point when active efivars abstraction be set. It is useful for userland to determine the availability of efivars filesystem.
---
v2:
Using efivars_kobject() helper instead of checking GetVariable or GetNextVariable EFI runtime services. Because the efivarfs code could be instantiated using a different efivars abstraction.
drivers/firmware/efi/efi.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/firmware/efi/efi.c +7 -5
@@ -405,11 +405,13 @@ static int __init efisubsys_init(void)if (error) goto err_remove_group; -/* and the standard mountpoint for efivarfs */ - error =sysfs_create_mount_point (efi_kobj,"efivars" ); -if (error) { -pr_err ("efivars: Subsystem registration failed.\n" ); - goto err_remove_group; +if (efivars_kobject ()) { +/* and the standard mountpoint for efivarfs */ + error =sysfs_create_mount_point (efi_kobj,"efivars" ); +if (error) { +pr_err ("efivars: Subsystem registration failed.\n" ); + goto err_remove_group; + } }if (efi_enabled (EFI_DBG ) &&efi_enabled (EFI_PRESERVE_BS_REGIONS ))
Signature · 1 line
2.16.4
Ard Biesheuvel
maintainerQuoted 10 lines · Chun-Yi
This patch creates efivars mount point when active efivars abstraction be set. It is useful for userland to determine the availability of efivars filesystem.
Cc Matthias Brugger <[email protected]>Cc Fabian Vogt <[email protected]>Cc Ilias Apalodimas <[email protected]>Cc Ard Biesheuvel <[email protected]>Signed-off-by "Lee, Chun-Yi" <[email protected]>
Apologies for not bringing this up before: while the patch seems fine, I wonder if we really need this if the purpose is to decide whether efivars is available or not. We already have the 'efivars' platform device for this, and so userland can simply check for the existence of
/sys/devices/platform/efivars.0
and so we don't need to make any changes for this.
Quoted 36 lines
---
v2:
Usingefivars_kobject()helper instead of checking GetVariable or GetNextVariable EFI runtime services. Because the efivarfs code could be instantiated using a different efivars abstraction.drivers/firmware/efi/efi.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)drivers/firmware/efi/efi.c +7
-6@@ -405,11 +405,13 @@ static int __init efisubsys_init(void)if (error) goto err_remove_group; -/* and the standard mountpoint for efivarfs */ - error =sysfs_create_mount_point (efi_kobj,"efivars" ); -if (error) { -pr_err ("efivars: Subsystem registration failed.\n" ); - goto err_remove_group; +if (efivars_kobject ()) { +/* and the standard mountpoint for efivarfs */ + error =sysfs_create_mount_point (efi_kobj,"efivars" ); +if (error) { +pr_err ("efivars: Subsystem registration failed.\n" ); + goto err_remove_group; + } }if (efi_enabled (EFI_DBG ) &&efi_enabled (EFI_PRESERVE_BS_REGIONS )) --2.16.4
Hi Ard,
Quoted 12 lines · Ard Biesheuvel
Quoted 10 lines · Chun-Yi
This patch creates efivars mount point when active efivars abstraction be set. It is useful for userland to determine the availability of efivars filesystem.
Cc Matthias Brugger <[email protected]>Cc Fabian Vogt <[email protected]>Cc Ilias Apalodimas <[email protected]>Cc Ard Biesheuvel <[email protected]>Signed-off-by "Lee, Chun-Yi" <[email protected]>
Quoted 9 lines
Apologies for not bringing this up before: while the patch seems fine, I wonder if we really need this if the purpose is to decide whether efivars is available or not. We already have the 'efivars' platform device for this, and so userland can simply check for the existence of
/sys/devices/platform/efivars.0and so we don't need to make any changes for this.
The platform device only be registered on generic EFI runtime services platform. If the efivars abstraction is google gsmi, then userland can not use efivars.0 to detectmine the availability of efivars filesystem.
If we only consider generic EFI platform, then efivars.0 is good enough. But if the gsmi implementation joins this game, then my patch should not blocks the creation of efivars mount point because gsmi_kobj is not initialized yet.
Maybe the creation of efivars mount point can be moved to efivars_register(), after the kobject be set. Then gsmi can also create mount point. How do you think?
On the other hand, the efisubsys_init() does not unregister efivars.0 platform device in err_unregister block. It causes that efivars and efi-pstore be loaded when no generic_ops.
Let me know if I miss understood any thing, please.
Thanks a lot!
Joey Lee
Quoted 38 lines
Quoted 38 lines
---
v2:
Usingefivars_kobject()helper instead of checking GetVariable or GetNextVariable EFI runtime services. Because the efivarfs code could be instantiated using a different efivars abstraction.drivers/firmware/efi/efi.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)drivers/firmware/efi/efi.c +7
-6@@ -405,11 +405,13 @@ static int __init efisubsys_init(void)if (error) goto err_remove_group; -/* and the standard mountpoint for efivarfs */ - error =sysfs_create_mount_point (efi_kobj,"efivars" ); -if (error) { -pr_err ("efivars: Subsystem registration failed.\n" ); - goto err_remove_group; +if (efivars_kobject ()) { +/* and the standard mountpoint for efivarfs */ + error =sysfs_create_mount_point (efi_kobj,"efivars" ); +if (error) { +pr_err ("efivars: Subsystem registration failed.\n" ); + goto err_remove_group; + } }if (efi_enabled (EFI_DBG ) &&efi_enabled (EFI_PRESERVE_BS_REGIONS )) --2.16.4
Quoted 29 lines · Joey Lee
Hi Ard,
Quoted 12 lines · Ard Biesheuvel
Quoted 10 lines · Chun-Yi
This patch creates efivars mount point when active efivars abstraction be set. It is useful for userland to determine the availability of efivars filesystem.
Cc Matthias Brugger <[email protected]>Cc Fabian Vogt <[email protected]>Cc Ilias Apalodimas <[email protected]>Cc Ard Biesheuvel <[email protected]>Signed-off-by "Lee, Chun-Yi" <[email protected]>Quoted 9 lines
Apologies for not bringing this up before: while the patch seems fine, I wonder if we really need this if the purpose is to decide whether efivars is available or not. We already have the 'efivars' platform device for this, and so userland can simply check for the existence of
/sys/devices/platform/efivars.0and so we don't need to make any changes for this.
The platform device only be registered on generic EFI runtime services platform. If the efivars abstraction is google gsmi, then userland can not use efivars.0 to detectmine the availability of efivars filesystem.
Quoted 9 lines
If we only consider generic EFI platform, then efivars.0 is good enough. But if the gsmi implementation joins this game, then my patch should not blocks the creation of efivars mount point because
gsmi_kobjis not initialized yet.Maybe the creation of efivars mount point can be moved to
efivars_register(), after the kobject be set. Then gsmi can also create mount point. How do you think?
Here is the patch for my concept. I simply tested on qemu-OVMF and qemu-uboot.
From 7c81d3f058a177bb17c4589fe1863b79940f94c3 Mon Sep 17 00:00:00 2001
From: "Lee, Chun-Yi" <[email protected]>
Date: Fri, 28 Aug 2020 00:05:22 +0800
Subject: [PATCH] efi/efivars: Create efivars mount point in the registration
of efivars abstraction
This patch moved the logic of creating efivars mount point to the registration of efivars abstraction. It is useful for userland to determine the availability of efivars filesystem by checking the existence of mount point.
--- drivers/firmware/efi/efi.c | 7 ------- drivers/firmware/efi/vars.c | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 7 deletions(-)
drivers/firmware/efi/efi.c +0 -7
@@ -405,13 +405,6 @@ static int __init efisubsys_init(void)if (error) goto err_remove_group; -/* and the standard mountpoint for efivarfs */ - error =sysfs_create_mount_point (efi_kobj,"efivars" ); -if (error) { -pr_err ("efivars: Subsystem registration failed.\n" ); - goto err_remove_group; - } -if (efi_enabled (EFI_DBG ) &&efi_enabled (EFI_PRESERVE_BS_REGIONS ))efi_debugfs_init ();
drivers/firmware/efi/vars.c +17 -0
@@ -1179,6 +1179,8 @@ int efivars_register(struct efivars *efivars,const struct efivar_operations *ops, struct kobject *kobject) { + int error; +if (down_interruptible (&efivars_lock))return -EINTR ;
@@ -1191,6 +1193,19 @@ int efivars_register(struct efivars *efivars,up (&efivars_lock); +/* and the standard mountpoint for efivarfs */ +if (efi_kobj) { + error =sysfs_create_mount_point (efi_kobj,"efivars" ); +if (error) { +if (down_interruptible (&efivars_lock)) +return -EINTR ; + __efivars =NULL ; +up (&efivars_lock); +pr_err ("efivars: Subsystem registration failed.\n" ); +return error; + } + } +return 0 ; }EXPORT_SYMBOL_GPL (efivars_register);
@@ -1222,6 +1237,8 @@ int efivars_unregister(struct efivars *efivars)pr_info ("Unregistered efivars operations\n" ); __efivars =NULL ; +if (efi_kobj) +sysfs_remove_mount_point (efi_kobj,"efivars" ); rv =0 ; out:
Signature · 4 lines
2.16.4 Thanks a lot! Joey Lee
Quoted 45 lines
On the other hand, the
efisubsys_init()does not unregister efivars.0 platform device inerr_unregisterblock. It causes that efivars and efi-pstore be loaded when nogeneric_ops.Let me know if I miss understood any thing, please.
Quoted 38 lines
Quoted 38 lines
---
v2:
Usingefivars_kobject()helper instead of checking GetVariable or GetNextVariable EFI runtime services. Because the efivarfs code could be instantiated using a different efivars abstraction.drivers/firmware/efi/efi.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)drivers/firmware/efi/efi.c +7
-6@@ -405,11 +405,13 @@ static int __init efisubsys_init(void)if (error) goto err_remove_group; -/* and the standard mountpoint for efivarfs */ - error =sysfs_create_mount_point (efi_kobj,"efivars" ); -if (error) { -pr_err ("efivars: Subsystem registration failed.\n" ); - goto err_remove_group; +if (efivars_kobject ()) { +/* and the standard mountpoint for efivarfs */ + error =sysfs_create_mount_point (efi_kobj,"efivars" ); +if (error) { +pr_err ("efivars: Subsystem registration failed.\n" ); + goto err_remove_group; + } }if (efi_enabled (EFI_DBG ) &&efi_enabled (EFI_PRESERVE_BS_REGIONS )) --2.16.4