efi/efivars: Expose RT service availability via efivars abstraction
Subsystems (3)
Ard Biesheuvel
maintainerPATCH v3 1/1 efi/efivars: Expose RT service availability via efivars abstraction
Commit
bf67fad19e493b ("efi: Use more granular check for availability for variable services")
introduced a check into the efivarfs, efi-pstore and other drivers that aborts loading of the module if not all three variable runtime services (GetVariable, SetVariable and GetNextVariable) are supported. However, this results in efivarfs being unavailable entirely if only SetVariable support is missing, which is only needed if you want to make any modifications. Also, efi-pstore and the sysfs EFI variable interface could be backed by another implementation of the 'efivars' abstraction, in which case it is completely irrelevant which services are supported by the EFI firmware.
So make the generic 'efivars' abstraction dependent on the availibility of the GetVariable and GetNextVariable EFI runtime services, and add a helper to find out whether the current 'efivars' abstraction supports writes (and wire it up to the availability of SetVariable for the generic one)
---
v3 of [0], which fixed things at the wrong layer - efivarfs does not care about EFI firmware, it only talks to the lower 'efivars' layer, which can be implemented in various different ways.
[0] https://lore.kernel.org/linux-efi/[email protected]/
drivers/firmware/efi/efi-pstore.c | 5 +---- drivers/firmware/efi/efi.c | 12 ++++++++---- drivers/firmware/efi/efivars.c | 5 +---- drivers/firmware/efi/vars.c | 6 ++++++ fs/efivarfs/super.c | 6 +++--- include/linux/efi.h | 1 + 6 files changed, 20 insertions(+), 15 deletions(-)
drivers/firmware/efi/efi-pstore.c +1 -4
@@ -356,10 +356,7 @@ static struct pstore_info efi_pstore_info = {static __init int efivars_pstore_init (void ) { -if (!efi_rt_services_supported (EFI_RT_SUPPORTED_VARIABLE_SERVICES )) -return 0 ; - -if (!efivars_kobject ()) +if (!efivars_kobject () || !efivar_supports_writes ())return 0 ;if (efivars_pstore_disable)
drivers/firmware/efi/efi.c +8 -4
@@ -176,11 +176,13 @@ static struct efivar_operations generic_ops;static int generic_ops_register (void ) { generic_ops.get_variable = efi.get_variable; - generic_ops.set_variable = efi.set_variable; - generic_ops.set_variable_nonblocking = efi.set_variable_nonblocking; generic_ops.get_next_variable = efi.get_next_variable; generic_ops.query_variable_store = efi_query_variable_store; +if (efi_rt_services_supported (EFI_RT_SUPPORTED_SET_VARIABLE )) { + generic_ops.set_variable = efi.set_variable; + generic_ops.set_variable_nonblocking = efi.set_variable_nonblocking; + }return efivars_register (&generic_efivars, &generic_ops, efi_kobj); }
@@ -382,7 +384,8 @@ static int __init efisubsys_init(void)return -ENOMEM ; } -if (efi_rt_services_supported (EFI_RT_SUPPORTED_VARIABLE_SERVICES )) { +if (efi_rt_services_supported (EFI_RT_SUPPORTED_GET_VARIABLE | +EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME )) {efivar_ssdt_load (); error =generic_ops_register ();if (error)
@@ -416,7 +419,8 @@ static int __init efisubsys_init(void) err_remove_group:sysfs_remove_group (efi_kobj, &efi_subsys_attr_group); err_unregister: -if (efi_rt_services_supported (EFI_RT_SUPPORTED_VARIABLE_SERVICES )) +if (efi_rt_services_supported (EFI_RT_SUPPORTED_GET_VARIABLE | +EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME ))generic_ops_unregister (); err_put:kobject_put (efi_kobj);
drivers/firmware/efi/efivars.c +1 -4
@@ -680,11 +680,8 @@ int efivars_sysfs_init(void)struct kobject *parent_kobj =efivars_kobject ();int error =0 ; -if (!efi_rt_services_supported (EFI_RT_SUPPORTED_VARIABLE_SERVICES )) -return -ENODEV ; -/* No efivars has been registered yet */ -if (!parent_kobj) +if (!parent_kobj || !efivar_supports_writes ())return 0 ; printk(KERN_INFO "EFI Variables Facility v%s %s\n", EFIVARS_VERSION,
drivers/firmware/efi/vars.c +6 -0
@@ -1229,3 +1229,9 @@ int efivars_unregister(struct efivars *efivars)return rv; }EXPORT_SYMBOL_GPL (efivars_unregister); + +int efivar_supports_writes (void ) +{ +return __efivars && __efivars->ops->set_variable; +} +EXPORT_SYMBOL_GPL (efivar_supports_writes);
fs/efivarfs/super.c +3 -3
@@ -201,6 +201,9 @@ static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc) sb->s_d_op = &efivarfs_d_ops; sb->s_time_gran =1 ; +if (!efivar_supports_writes ()) + sb->s_flags |=SB_RDONLY ; + inode =efivarfs_get_inode (sb,NULL ,S_IFDIR |0755 ,0 , true);if (!inode)return -ENOMEM ;
@@ -252,9 +255,6 @@ static struct file_system_type efivarfs_type = {static __init int efivarfs_init (void ) { -if (!efi_rt_services_supported (EFI_RT_SUPPORTED_VARIABLE_SERVICES )) -return -ENODEV ; -if (!efivars_kobject ())return -ENODEV ;
include/linux/efi.h +1 -0
@@ -994,6 +994,7 @@ int efivars_register(struct efivars *efivars,int efivars_unregister (struct efivars *efivars);struct kobject *efivars_kobject (void ); +int efivar_supports_writes (void );int efivar_init (int (*func)(efi_char16_t *,efi_guid_t ,unsigned long ,void *),void *data,bool duplicates,struct list_head *head);
Signature · 1 line
2.17.1
Quoted 163 lines · Ard Biesheuvel
Commit
bf67fad19e493b("efi: Use more granular check for availability for variable services")introduced a check into the efivarfs, efi-pstore and other drivers that aborts loading of the module if not all three variable runtime services (GetVariable, SetVariable and GetNextVariable) are supported. However, this results in efivarfs being unavailable entirely if only SetVariable support is missing, which is only needed if you want to make any modifications. Also, efi-pstore and the sysfs EFI variable interface could be backed by another implementation of the 'efivars' abstraction, in which case it is completely irrelevant which services are supported by the EFI firmware.
So make the generic 'efivars' abstraction dependent on the availibility of the GetVariable and GetNextVariable EFI runtime services, and add a helper to find out whether the current 'efivars' abstraction supports writes (and wire it up to the availability of SetVariable for the generic one)
Cc Ilias Apalodimas <[email protected]>Fixes bf67fad19e493b ("efi: Use more granular check for availability for variable services")Reported-by Heinrich Schuchardt <[email protected]>Signed-off-by Ard Biesheuvel <[email protected]>---
v3 of [0], which fixed things at the wrong layer - efivarfs does not care about EFI firmware, it only talks to the lower 'efivars' layer, which can be implemented in various different ways.[0] https://lore.kernel.org/linux-efi/[email protected]/
drivers/firmware/efi/efi-pstore.c | 5 +---- drivers/firmware/efi/efi.c | 12 ++++++++---- drivers/firmware/efi/efivars.c | 5 +---- drivers/firmware/efi/vars.c | 6 ++++++ fs/efivarfs/super.c | 6 +++--- include/linux/efi.h | 1 + 6 files changed, 20 insertions(+), 15 deletions(-)drivers/firmware/efi/efi-pstore.c +1
-4@@ -356,10 +356,7 @@ static struct pstore_info efi_pstore_info = {static __init int efivars_pstore_init (void ) { -if (!efi_rt_services_supported (EFI_RT_SUPPORTED_VARIABLE_SERVICES )) -return 0 ; - -if (!efivars_kobject ()) +if (!efivars_kobject () || !efivar_supports_writes ())return 0 ;if (efivars_pstore_disable)drivers/firmware/efi/efi.c +8
-4@@ -176,11 +176,13 @@ static struct efivar_operations generic_ops;static int generic_ops_register (void ) { generic_ops.get_variable = efi.get_variable; - generic_ops.set_variable = efi.set_variable; - generic_ops.set_variable_nonblocking = efi.set_variable_nonblocking; generic_ops.get_next_variable = efi.get_next_variable; generic_ops.query_variable_store = efi_query_variable_store; +if (efi_rt_services_supported (EFI_RT_SUPPORTED_SET_VARIABLE )) { + generic_ops.set_variable = efi.set_variable; + generic_ops.set_variable_nonblocking = efi.set_variable_nonblocking; + }return efivars_register (&generic_efivars, &generic_ops, efi_kobj); }@@ -382,7 +384,8 @@ static int __init efisubsys_init(void)return -ENOMEM ; } -if (efi_rt_services_supported (EFI_RT_SUPPORTED_VARIABLE_SERVICES )) { +if (efi_rt_services_supported (EFI_RT_SUPPORTED_GET_VARIABLE | +EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME )) {efivar_ssdt_load (); error =generic_ops_register ();if (error)@@ -416,7 +419,8 @@ static int __init efisubsys_init(void) err_remove_group:sysfs_remove_group (efi_kobj, &efi_subsys_attr_group); err_unregister: -if (efi_rt_services_supported (EFI_RT_SUPPORTED_VARIABLE_SERVICES )) +if (efi_rt_services_supported (EFI_RT_SUPPORTED_GET_VARIABLE | +EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME ))generic_ops_unregister (); err_put:kobject_put (efi_kobj);drivers/firmware/efi/efivars.c +1
-4@@ -680,11 +680,8 @@ int efivars_sysfs_init(void)struct kobject *parent_kobj =efivars_kobject ();int error =0 ; -if (!efi_rt_services_supported (EFI_RT_SUPPORTED_VARIABLE_SERVICES )) -return -ENODEV ; -/* No efivars has been registered yet */ -if (!parent_kobj) +if (!parent_kobj || !efivar_supports_writes ())return 0 ; printk(KERN_INFO "EFI Variables Facility v%s %s\n", EFIVARS_VERSION,drivers/firmware/efi/vars.c +6
-0@@ -1229,3 +1229,9 @@ int efivars_unregister(struct efivars *efivars)return rv; }EXPORT_SYMBOL_GPL (efivars_unregister); + +int efivar_supports_writes (void ) +{ +return __efivars && __efivars->ops->set_variable; +} +EXPORT_SYMBOL_GPL (efivar_supports_writes);fs/efivarfs/super.c +3
-3@@ -201,6 +201,9 @@ static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc) sb->s_d_op = &efivarfs_d_ops; sb->s_time_gran =1 ; +if (!efivar_supports_writes ()) + sb->s_flags |=SB_RDONLY ; + inode =efivarfs_get_inode (sb,NULL ,S_IFDIR |0755 ,0 , true);if (!inode)return -ENOMEM ;@@ -252,9 +255,6 @@ static struct file_system_type efivarfs_type = {static __init int efivarfs_init (void ) { -if (!efi_rt_services_supported (EFI_RT_SUPPORTED_VARIABLE_SERVICES )) -return -ENODEV ; -if (!efivars_kobject ())return -ENODEV ;include/linux/efi.h +1
-0@@ -994,6 +994,7 @@ int efivars_register(struct efivars *efivars,int efivars_unregister (struct efivars *efivars);struct kobject *efivars_kobject (void ); +int efivar_supports_writes (void );int efivar_init (int (*func)(efi_char16_t *,efi_guid_t ,unsigned long ,void *),void *data,bool duplicates,struct list_head *head);Signature · 1 line
2.17.1