Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200826004607.7483-1-jlee@suse.com>
In-Reply-To: <20200826004607.7483-1-jlee@suse.com>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Wed, 26 Aug 2020 14:08:18 +0200
Message-ID: <CAMj1kXHAmUVPs=P5ptb3J95+4epP5fQXkBZ_+vLP8xfxri8VBw@mail.gmail.com>
Subject: Re: [PATCH v2] efi/efivars: Create efivars mount point via efivars abstraction
To:     "Lee, Chun-Yi" <joeyli.kernel@gmail.com>
Cc:     Ard Biesheuvel <ard.biesheuvel@linaro.org>,
        linux-efi <linux-efi@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "Lee, Chun-Yi" <jlee@suse.com>,
        Matthias Brugger <mbrugger@suse.com>,
        Fabian Vogt <fvogt@suse.com>,
        Ilias Apalodimas <ilias.apalodimas@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 26 Aug 2020 at 02:46, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
>
> 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 <mbrugger@suse.com>
> Cc: Fabian Vogt <fvogt@suse.com>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>

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.


> ---
>
> 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(-)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 3aa07c3b5136..db483fc68501 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -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
>