Hi Ard, On Thu, Sep 24, 2020 at 12:47:46PM +0200, Ard Biesheuvel wrote: > On Thu, 24 Sep 2020 at 10:28, Lee, Chun-Yi <joeyli.kernel@xxxxxxxxx> wrote: > > > > This patch moved the logic of creating efivars mount point to the > > registration of efivars abstraction. It's useful for userland to > > determine the availability of efivars filesystem by checking the > > existence of mount point. > > > > The 'efivars' platform device be created on generic EFI runtime services > > platform, so it can be used to determine the availability of efivarfs. > > But this approach is not available for google gsmi efivars abstraction. > > > > This patch be tested on Here on qemu-OVMF and qemu-uboot. > > > > Cc: Ard Biesheuvel <ardb@xxxxxxxxxx> > > Cc: Matthias Brugger <mbrugger@xxxxxxxx> > > Cc: Fabian Vogt <fvogt@xxxxxxxx> > > Cc: Ilias Apalodimas <ilias.apalodimas@xxxxxxxxxx> > > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > Cc: Arthur Heymans <arthur@xxxxxxxxxxxx> > > Cc: Patrick Rudolph <patrick.rudolph@xxxxxxxxxxxxx> > > Signed-off-by: "Lee, Chun-Yi" <jlee@xxxxxxxx> > > --- > > I take it this is v3 of [0]? If so, please explain how it deviates > from v2. If it doesn't deviate from v2, it is better to continue the > discussion in the other thread. > > For the sake of discussion, it helps to clarify the confusing nomenclature: > > a) 'efivars abstraction' - an internal kernel API that exposes EFI > variables, and can potentially be backed by an implementation that is > not EFI based (i.e., Google gsmi) > > b) efivars.ko module, built on top of the efivars abstraction, which > exposes EFI variables (real ones or gsmi ones) via the deprecated > sysfs interface > > c) efivarfs filesystem, also built on top of the efivars abstraction, > which exposes EFI variables (real ones or gsmi ones) via a special > filesystem independently of sysfs. > > Of course, the sysfs mount point we create for efivarfs is not called > 'efivarfs' but 'efivars'. The sysfs subdirectory we create for > efivars.ko is called 'vars'. Sigh. > Thanks for your clarification. It's useful to me! > > In this patch, you create the mount point for c) based on whether a) > gets registered (which occurs on systems with EFI Get/SetVariable > support or GSMI), right? So, to Greg's point, wouldn't it be easier to > simply check whether efivarfs is listed in /proc/filesystems? > Yes, I think that Greg's suggestion is good enough for a userland tool to detect the availability of efivarfs. You can ignore my patch. Thanks for your help! Joey Lee > It also helps if you could clarify what the actual use case is, rather > than saying that it is generally useful. > > > > > > [0] https://lore.kernel.org/linux-efi/20200825160719.7188-1-jlee@xxxxxxxx/ > > > drivers/firmware/efi/efi.c | 7 ------- > > drivers/firmware/efi/vars.c | 17 +++++++++++++++++ > > 2 files changed, 17 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c > > index 3aa07c3b5136..23c11a2a3f4d 100644 > > --- a/drivers/firmware/efi/efi.c > > +++ b/drivers/firmware/efi/efi.c > > @@ -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(); > > > > diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c > > index 973eef234b36..6fa7f288d635 100644 > > --- a/drivers/firmware/efi/vars.c > > +++ b/drivers/firmware/efi/vars.c > > @@ -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: > > -- > > 2.16.4 > >
