Received: by 2002:a05:6a10:22f:0:0:0:0 with SMTP id 15csp209523pxk; Thu, 24 Sep 2020 03:50:03 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzUl02BvSSoxa43BsShL3DuNL+3ffWETXx3qjR4aazLFuG7mEf5gnB5l/e4fCObujD2ekVr X-Received: by 2002:aa7:ca46:: with SMTP id j6mr309968edt.155.1600944603510; Thu, 24 Sep 2020 03:50:03 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1600944603; cv=none; d=google.com; s=arc-20160816; b=ULo+HWkCEgr0JLpTmqCql6xGhTi3DjVXxxqTgYaN8GXeFp0dHHr2r8SKgWw3ZDrM1I x/W16smk7V3kgdItB+2HkTe1Cpa3WdD8+KyxS8UFWzV5bQsCOGKbbHeAhNvvo/ZjtDW0 d4j7Rfiq2Ca+oykP7CS4LK3rFJGgtXI8AsToP/cSzndOiY3dDb30ij4EwdKv69leCG3s oEX3ZgpUQwSKXgNY6XLaOUeAaCuQcTRNg6NnwWrIgfWIO0pklViruTxyziljXJzKKr8c Fl0+nY1J1kazuQhUL8tuZUkQGeyXpv0Jcdq+F7ckTFg4x1vU0A+qczm+ei/EVZgln8yR KuJw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=DxjOk/PKVj8B5oyexLSm7J2XNuljsFbwFNpfNIbVqvM=; b=Bgsb2umH3NTfIz6kfZvyoOBQoVFj79UVdjTwOKlu8GcDqZJGVowxfIHs/yWJH8yp4W g4rne69TniNYSsm7dzIwaP5mn01CPof+sqAVLvm1ofqlW8m5UpJiHt3xnY5ICf2rP0pw 3z9j10+KMXnQFTukbrmRbY4iHCZnIzDa9ikvL3p1aa6R2JZoP1zGLdtBGCMLybkqtgsa pnBwEXV1pifrshTgv+OWDKrHan41M6XfyocQO6Q90e6xYmh620xQXXdGHAqC1yR3OxVW mr1Xc1z/mihw3hYTMUUw6wodIdFhUun+nio1qS9KPBRmgAZPZD/oMr3CgwGNY7RnTRKl Ij9A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=bKPDntr9; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id f2si1864823edr.143.2020.09.24.03.49.39; Thu, 24 Sep 2020 03:50:03 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=bKPDntr9; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726652AbgIXKr7 (ORCPT + 99 others); Thu, 24 Sep 2020 06:47:59 -0400 Received: from mail.kernel.org ([198.145.29.99]:60762 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726303AbgIXKr7 (ORCPT ); Thu, 24 Sep 2020 06:47:59 -0400 Received: from mail-ot1-f42.google.com (mail-ot1-f42.google.com [209.85.210.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 98F7A239A1; Thu, 24 Sep 2020 10:47:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600944478; bh=crA6jQgUS1FFfr6DRg0MQCRnOUDeb7w6bUzRsFm5xKE=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=bKPDntr9XYycPElj3RXGS8Km6PrEOQQzu7sg5MsQWDi1Ki9DE9qD1W5I+MKQWdZJC /qm9XZyDoUOqK+LxF8HigXlZY7JxaqEEtvxixVdmq0UPYTwcoVVdY0o8aY4BBBXgb+ T7Xv0tonCuc1WPT4hyhR2yKq9UnnO1dLSFTBKCXE= Received: by mail-ot1-f42.google.com with SMTP id s66so2736743otb.2; Thu, 24 Sep 2020 03:47:58 -0700 (PDT) X-Gm-Message-State: AOAM532cbIt0Fx7dMG7XFsewB7cp4h8Ig0X3waYMRy4jlGG9IJWJNkXu kudbLAr1bfjxKTGx5uMr93aCJnNTEMHFf66ZGYA= X-Received: by 2002:a9d:6193:: with SMTP id g19mr2623454otk.108.1600944477888; Thu, 24 Sep 2020 03:47:57 -0700 (PDT) MIME-Version: 1.0 References: <20200924082833.12722-1-jlee@suse.com> In-Reply-To: <20200924082833.12722-1-jlee@suse.com> From: Ard Biesheuvel Date: Thu, 24 Sep 2020 12:47:46 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] efi/efivars: Create efivars mount point in the registration of efivars abstraction To: "Lee, Chun-Yi" Cc: Ard Biesheuvel , linux-efi , Linux Kernel Mailing List , "Lee, Chun-Yi" , Matthias Brugger , Fabian Vogt , Ilias Apalodimas , Greg Kroah-Hartman , Arthur Heymans , Patrick Rudolph Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 24 Sep 2020 at 10:28, Lee, Chun-Yi 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 > Cc: Matthias Brugger > Cc: Fabian Vogt > Cc: Ilias Apalodimas > Cc: Greg Kroah-Hartman > Cc: Arthur Heymans > Cc: Patrick Rudolph > Signed-off-by: "Lee, Chun-Yi" > --- 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. 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? 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@suse.com/ > 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 >