[PATCH] Add syste-firmware-efi-efivars.mount for support automount EFI variable filesystem

[PATCH] Add syste-firmware-efi-efivars.mount for support automount EFI variable filesystem

[Lee, Chun-Yi]

Add units/sys-firmware-efi-efivars.mount rule for support automount EFI variable filesystem

Cc: Kay Sievers <kay-tD+1rO4QERM@public.gmane.org>
Cc: Lennart Poettering <lennart-mdGvqq1h2p+GdvJs77BJ7Q@public.gmane.org>
Cc: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Jeremy Kerr <jeremy.kerr-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Cc: Matthew Garrett <mjg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Lee, Chun-Yi <jlee-IBi9RG/b67k@public.gmane.org>
---
 units/sys-firmware-efi-efivars.mount |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)
 create mode 100644 units/sys-firmware-efi-efivars.mount

diff --git a/units/sys-firmware-efi-efivars.mount b/units/sys-firmware-efi-efivars.mount
new file mode 100644
index 0000000..641d771
--- /dev/null
+++ b/units/sys-firmware-efi-efivars.mount
@@ -0,0 +1,17 @@
+#  This file is part of systemd.
+#
+#  systemd is free software; you can redistribute it and/or modify it
+#  under the terms of the GNU General Public License as published by
+#  the Free Software Foundation; either version 2 of the License, or
+#  (at your option) any later version.
+
+[Unit]
+Description=EFI Variables File System
+DefaultDependencies=no
+ConditionPathExists=/sys/firmware/efi/efivars
+Before=sysinit.target
+
+[Mount]
+What=efivarfs
+Where=/sys/firmware/efi/efivars
+Type=efivarfs
-- 
1.6.0.2

Re: [systemd-devel] [PATCH] Add syste-firmware-efi-efivars.mount for support automount EFI variable filesystem

[Mantas Mikulėnas]

On Wed, Oct 24, 2012 at 1:42 PM, Lee, Chun-Yi <joeyli.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Add units/sys-firmware-efi-efivars.mount rule for support automount EFI variable filesystem
> [...]
> +
> +[Mount]
> +What=efivarfs
> +Where=/sys/firmware/efi/efivars
> +Type=efivarfs

Out of curiosity: Which kernels actually use a separate filesystem
type for this? I'm using 3.6 (Arch) here, and it doesn't even
recognize "efivarfs" -- it exports the variables directly via sysfs at
/sys/firmware/efi/vars...

-- 
Mantas Mikulėnas

Re: [systemd-devel] [PATCH] Add syste-firmware-efi-efivars.mount for support automount EFI variable filesystem

[joeyli]

Hi Mantas, 

於 三，2012-10-24 於 14:07 +0300，Mantas Mikulėnas 提到：
> On Wed, Oct 24, 2012 at 1:42 PM, Lee, Chun-Yi <joeyli.kernel-Re5JQEeQqe8@public.gmane.orgm> wrote:
> > Add units/sys-firmware-efi-efivars.mount rule for support automount EFI variable filesystem
> > [...]
> > +
> > +[Mount]
> > +What=efivarfs
> > +Where=/sys/firmware/efi/efivars
> > +Type=efivarfs
> 
> Out of curiosity: Which kernels actually use a separate filesystem
> type for this? I'm using 3.6 (Arch) here, and it doesn't even
> recognize "efivarfs" -- it exports the variables directly via sysfs at
> /sys/firmware/efi/vars...
> 

Sorry for I didn't mention to EFI variable filesystem
(/sys/firmware/efi/vars) will show up in v3.7 or v3.8 kernel.

Those patches already merged in EFI git branch:
http://git.kernel.org/?p=linux/kernel/git/mfleming/efi.git;a=summary


Thanks a lot!
Joey Lee

Re: [systemd-devel] [PATCH] Add syste-firmware-efi-efivars.mount for support automount EFI variable filesystem

[joeyli]

於 三，2012-10-24 於 19:32 +0800，joeyli 提到：
> Hi Mantas, 
> 
> 於 三，2012-10-24 於 14:07 +0300，Mantas Mikulėnas 提到：
> > On Wed, Oct 24, 2012 at 1:42 PM, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> > > Add units/sys-firmware-efi-efivars.mount rule for support automount EFI variable filesystem
> > > [...]
> > > +
> > > +[Mount]
> > > +What=efivarfs
> > > +Where=/sys/firmware/efi/efivars
> > > +Type=efivarfs
> > 
> > Out of curiosity: Which kernels actually use a separate filesystem
> > type for this? I'm using 3.6 (Arch) here, and it doesn't even
> > recognize "efivarfs" -- it exports the variables directly via sysfs at
> > /sys/firmware/efi/vars...
> > 
> 
> Sorry for I didn't mention to EFI variable filesystem
> (/sys/firmware/efi/vars) will show up in v3.7 or v3.8 kernel.
> 
> Those patches already merged in EFI git branch:
> http://git.kernel.org/?p=linux/kernel/git/mfleming/efi.git;a=summary
> 

The exact patch is here:
http://git.kernel.org/?p=linux/kernel/git/mfleming/efi.git;a=commit;h=b003aaf799c991295b8b73e8f940d20bda2c1bbb


Thanks a lot!
Joey Lee

Re: [PATCH] Add syste-firmware-efi-efivars.mount for support automount EFI variable filesystem

[Zbigniew Jędrzejewski-Szmek]

On Wed, Oct 24, 2012 at 06:42:02PM +0800, Lee, Chun-Yi wrote:
> Add units/sys-firmware-efi-efivars.mount rule for support automount EFI variable filesystem
> ---
Hi,

in systemd parlance, automount means autofs mount, but to have that, a
second .automount unit is needed.  Please have a look at
http://cgit.freedesktop.org/systemd/systemd/tree/units/proc-sys-fs-binfmt_misc.automount
vs
http://cgit.freedesktop.org/systemd/systemd/tree/units/proc-sys-fs-binfmt_misc.mount .
Now the question is whether the loading of the fs is slow enough to
matter, ie. if it is actually beneficial to use automount instead of
mounting directly.  Since the fs can be compiled as a module, than it
probably is.

Also, would be nice to add a Documentation= line like in
proc-sys-fs-binfmt_misc.automount.

>  units/sys-firmware-efi-efivars.mount |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
>  create mode 100644 units/sys-firmware-efi-efivars.mount

Zbyszek

Re: [systemd-devel] [PATCH] Add syste-firmware-efi-efivars.mount for support automount EFI variable filesystem

[Kay Sievers]

On Wed, Oct 24, 2012 at 2:12 PM, Zbigniew Jędrzejewski-Szmek
<zbyszek-wrcVdnn0TatmR6Xm/wNWPw@public.gmane.org> wrote:
> On Wed, Oct 24, 2012 at 06:42:02PM +0800, Lee, Chun-Yi wrote:
>> Add units/sys-firmware-efi-efivars.mount rule for support automount EFI variable filesystem
>> ---
> Hi,
>
> in systemd parlance, automount means autofs mount, but to have that, a
> second .automount unit is needed.  Please have a look at
> http://cgit.freedesktop.org/systemd/systemd/tree/units/proc-sys-fs-binfmt_misc.automount
> vs
> http://cgit.freedesktop.org/systemd/systemd/tree/units/proc-sys-fs-binfmt_misc.mount .
> Now the question is whether the loading of the fs is slow enough to
> matter, ie. if it is actually beneficial to use automount instead of
> mounting directly.  Since the fs can be compiled as a module, than it
> probably is.
>
> Also, would be nice to add a Documentation= line like in
> proc-sys-fs-binfmt_misc.automount.

It might all not be worth it, and we might just add it to the
"unconditionally mounted" list in the compiled-in code (marked with
allowed-to-fail). It seems like the better option than having a mount
unit, and a module-load force option.

The current mount unit would never trigger for modules, because the
path will not exist. An internal API mount will cause a trransparent
kernel-forked modprobe with the mount() syscall.

Kay

Re: [systemd-devel] [PATCH] Add syste-firmware-efi-efivars.mount for support automount EFI variable filesystem

[Zbigniew Jędrzejewski-Szmek]

On Wed, Oct 24, 2012 at 02:17:39PM +0200, Kay Sievers wrote:
> On Wed, Oct 24, 2012 at 2:12 PM, Zbigniew Jędrzejewski-Szmek
> <zbyszek-wrcVdnn0TatmR6Xm/wNWPw@public.gmane.org> wrote:
> > On Wed, Oct 24, 2012 at 06:42:02PM +0800, Lee, Chun-Yi wrote:
> >> Add units/sys-firmware-efi-efivars.mount rule for support automount EFI variable filesystem
> >> ---
> > Hi,
> >
> > in systemd parlance, automount means autofs mount, but to have that, a
> > second .automount unit is needed.  Please have a look at
> > http://cgit.freedesktop.org/systemd/systemd/tree/units/proc-sys-fs-binfmt_misc.automount
> > vs
> > http://cgit.freedesktop.org/systemd/systemd/tree/units/proc-sys-fs-binfmt_misc.mount .
> > Now the question is whether the loading of the fs is slow enough to
> > matter, ie. if it is actually beneficial to use automount instead of
> > mounting directly.  Since the fs can be compiled as a module, than it
> > probably is.
> >
> > Also, would be nice to add a Documentation= line like in
> > proc-sys-fs-binfmt_misc.automount.
> 
> It might all not be worth it, and we might just add it to the
> "unconditionally mounted" list in the compiled-in code (marked with
> allowed-to-fail). It seems like the better option than having a mount
> unit, and a module-load force option.
Probably should be measured by someone with UEFI. This will likely be
a very rarely used fs, so if the loading time is actually measureable,
than automount probably makes sense.

> The current mount unit would never trigger for modules, because the
> path will not exist. An internal API mount will cause a trransparent
> kernel-forked modprobe with the mount() syscall.
Yeah, the ConditionPathExists would have to be dropped.

Zbyszek

Re: [systemd-devel] [PATCH] Add syste-firmware-efi-efivars.mount for support automount EFI variable filesystem

[Kay Sievers]

On Wed, Oct 24, 2012 at 2:23 PM, Zbigniew Jędrzejewski-Szmek
<zbyszek-wrcVdnn0TatmR6Xm/wNWPw@public.gmane.org> wrote:
> On Wed, Oct 24, 2012 at 02:17:39PM +0200, Kay Sievers wrote:
>> On Wed, Oct 24, 2012 at 2:12 PM, Zbigniew Jędrzejewski-Szmek
>> <zbyszek-wrcVdnn0TatmR6Xm/wNWPw@public.gmane.org> wrote:
>> > On Wed, Oct 24, 2012 at 06:42:02PM +0800, Lee, Chun-Yi wrote:
>> >> Add units/sys-firmware-efi-efivars.mount rule for support automount EFI variable filesystem
>> >
>> > in systemd parlance, automount means autofs mount, but to have that, a
>> > second .automount unit is needed.  Please have a look at
>> > http://cgit.freedesktop.org/systemd/systemd/tree/units/proc-sys-fs-binfmt_misc.automount
>> > vs
>> > http://cgit.freedesktop.org/systemd/systemd/tree/units/proc-sys-fs-binfmt_misc.mount .
>> > Now the question is whether the loading of the fs is slow enough to
>> > matter, ie. if it is actually beneficial to use automount instead of
>> > mounting directly.  Since the fs can be compiled as a module, than it
>> > probably is.
>> >
>> > Also, would be nice to add a Documentation= line like in
>> > proc-sys-fs-binfmt_misc.automount.
>>
>> It might all not be worth it, and we might just add it to the
>> "unconditionally mounted" list in the compiled-in code (marked with
>> allowed-to-fail). It seems like the better option than having a mount
>> unit, and a module-load force option.
> Probably should be measured by someone with UEFI.

I'll check that when it's merged; in 3.8, I guess.

> This will likely be
> a very rarely used fs

We will likely end up using it ourselves from PID1 to extract boot
loader performance data, just like we do the initrd and other timing
data there. So an automount might not give us any real advantage in
the end.

> so if the loading time is actually measureable,
> than automount probably makes sense.

On EFI systems it will not really be measurable, I guess. The mount is
almost free, because the superblock is in the kernel anyway, very much
like like proc, sysfs, devtmpfs.

For non-EFI systems, it might cause a "useless" kernel-initiated
modprobe, which we probably want to avoid by some condition.

>> The current mount unit would never trigger for modules, because the
>> path will not exist. An internal API mount will cause a trransparent
>> kernel-forked modprobe with the mount() syscall.
> Yeah, the ConditionPathExists would have to be dropped.

Right, but maybe we can key-off some other condition, which indicates
an EFI bootup, so we can avoid trying to mount things on systems where
it can't be there.

Looking at if from a distance:
I guess the kernel should just start mounting its "crap" on its own,
instead of relying on userspace to know anything about this
ever-changing stuff. This filesytem is just a directory in /sys,
nothing else for userspace to know about it.

Requiring to patch userspace to mirror the fashion-of-the-month kernel
setup really does not scale in the longer run. Userspace really does
not want to know all these things. It has the right to, and wants to
be "stupid" here, and does not really want to fiddle around in such
kernel-internals.

It might be time to think all this through, it seems very much like an
entirely needless loop to jump through userspace here.

Kay

Re: [systemd-devel] [PATCH] Add syste-firmware-efi-efivars.mount for support automount EFI variable filesystem

[Zbigniew Jędrzejewski-Szmek]

On Wed, Oct 24, 2012 at 03:00:14PM +0200, Kay Sievers wrote:
> On Wed, Oct 24, 2012 at 2:23 PM, Zbigniew Jędrzejewski-Szmek
> <zbyszek-wrcVdnn0TatmR6Xm/wNWPw@public.gmane.org> wrote:
> > On Wed, Oct 24, 2012 at 02:17:39PM +0200, Kay Sievers wrote:
> >> On Wed, Oct 24, 2012 at 2:12 PM, Zbigniew Jędrzejewski-Szmek
> >> <zbyszek-wrcVdnn0TatmR6Xm/wNWPw@public.gmane.org> wrote:
> >> > On Wed, Oct 24, 2012 at 06:42:02PM +0800, Lee, Chun-Yi wrote:
> >> >> Add units/sys-firmware-efi-efivars.mount rule for support automount EFI variable filesystem
> >> >
> >> > in systemd parlance, automount means autofs mount, but to have that, a
> >> > second .automount unit is needed.  Please have a look at
> >> > http://cgit.freedesktop.org/systemd/systemd/tree/units/proc-sys-fs-binfmt_misc.automount
> >> > vs
> >> > http://cgit.freedesktop.org/systemd/systemd/tree/units/proc-sys-fs-binfmt_misc.mount .
> >> > Now the question is whether the loading of the fs is slow enough to
> >> > matter, ie. if it is actually beneficial to use automount instead of
> >> > mounting directly.  Since the fs can be compiled as a module, than it
> >> > probably is.
> >> >
> >> > Also, would be nice to add a Documentation= line like in
> >> > proc-sys-fs-binfmt_misc.automount.
> >>
> >> It might all not be worth it, and we might just add it to the
> >> "unconditionally mounted" list in the compiled-in code (marked with
> >> allowed-to-fail). It seems like the better option than having a mount
> >> unit, and a module-load force option.
> > Probably should be measured by someone with UEFI.
> 
> I'll check that when it's merged; in 3.8, I guess.
> 
> > This will likely be
> > a very rarely used fs
> 
> We will likely end up using it ourselves from PID1 to extract boot
> loader performance data, just like we do the initrd and other timing
> data there. So an automount might not give us any real advantage in
> the end.
OK.

> > so if the loading time is actually measureable,
> > than automount probably makes sense.
> 
> On EFI systems it will not really be measurable, I guess. The mount is
> almost free, because the superblock is in the kernel anyway, very much
> like like proc, sysfs, devtmpfs.
> 
> For non-EFI systems, it might cause a "useless" kernel-initiated
> modprobe, which we probably want to avoid by some condition.
> 
> >> The current mount unit would never trigger for modules, because the
> >> path will not exist. An internal API mount will cause a trransparent
> >> kernel-forked modprobe with the mount() syscall.
> > Yeah, the ConditionPathExists would have to be dropped.
> 
> Right, but maybe we can key-off some other condition, which indicates
> an EFI bootup, so we can avoid trying to mount things on systems where
> it can't be there.
> 
> Looking at if from a distance:
> I guess the kernel should just start mounting its "crap" on its own,
> instead of relying on userspace to know anything about this
> ever-changing stuff. This filesytem is just a directory in /sys,
> nothing else for userspace to know about it.
Actually, in a container, it might make sense to not mount
/sys/firmware/efi/efivars, since it opens an avenue to modify kernel
state.

> Requiring to patch userspace to mirror the fashion-of-the-month kernel
> setup really does not scale in the longer run. Userspace really does
> not want to know all these things. It has the right to, and wants to
> be "stupid" here, and does not really want to fiddle around in such
> kernel-internals.
> 
> It might be time to think all this through, it seems very much like an
> entirely needless loop to jump through userspace here.

Zbyszek

Re: [systemd-devel] [PATCH] Add syste-firmware-efi-efivars.mount for support automount EFI variable filesystem

[Kay Sievers]

On Wed, Oct 24, 2012 at 3:09 PM, Zbigniew Jędrzejewski-Szmek
<zbyszek-wrcVdnn0TatmR6Xm/wNWPw@public.gmane.org> wrote:
> On Wed, Oct 24, 2012 at 03:00:14PM +0200, Kay Sievers wrote:
>> On Wed, Oct 24, 2012 at 2:23 PM, Zbigniew Jędrzejewski-Szmek
>> <zbyszek-wrcVdnn0TatmR6Xm/wNWPw@public.gmane.org> wrote:
>> > On Wed, Oct 24, 2012 at 02:17:39PM +0200, Kay Sievers wrote:
>> >> On Wed, Oct 24, 2012 at 2:12 PM, Zbigniew Jędrzejewski-Szmek
>> >> <zbyszek-wrcVdnn0TatmR6Xm/wNWPw@public.gmane.org> wrote:
>> >> > On Wed, Oct 24, 2012 at 06:42:02PM +0800, Lee, Chun-Yi wrote:
>> >> >> Add units/sys-firmware-efi-efivars.mount rule for support automount EFI variable filesystem
>> >> >
>> >> > in systemd parlance, automount means autofs mount, but to have that, a
>> >> > second .automount unit is needed.  Please have a look at
>> >> > http://cgit.freedesktop.org/systemd/systemd/tree/units/proc-sys-fs-binfmt_misc.automount
>> >> > vs
>> >> > http://cgit.freedesktop.org/systemd/systemd/tree/units/proc-sys-fs-binfmt_misc.mount .
>> >> > Now the question is whether the loading of the fs is slow enough to
>> >> > matter, ie. if it is actually beneficial to use automount instead of
>> >> > mounting directly.  Since the fs can be compiled as a module, than it
>> >> > probably is.
>> >> >
>> >> > Also, would be nice to add a Documentation= line like in
>> >> > proc-sys-fs-binfmt_misc.automount.
>> >>
>> >> It might all not be worth it, and we might just add it to the
>> >> "unconditionally mounted" list in the compiled-in code (marked with
>> >> allowed-to-fail). It seems like the better option than having a mount
>> >> unit, and a module-load force option.
>> > Probably should be measured by someone with UEFI.
>>
>> I'll check that when it's merged; in 3.8, I guess.
>>
>> > This will likely be
>> > a very rarely used fs
>>
>> We will likely end up using it ourselves from PID1 to extract boot
>> loader performance data, just like we do the initrd and other timing
>> data there. So an automount might not give us any real advantage in
>> the end.
> OK.
>
>> > so if the loading time is actually measureable,
>> > than automount probably makes sense.
>>
>> On EFI systems it will not really be measurable, I guess. The mount is
>> almost free, because the superblock is in the kernel anyway, very much
>> like like proc, sysfs, devtmpfs.
>>
>> For non-EFI systems, it might cause a "useless" kernel-initiated
>> modprobe, which we probably want to avoid by some condition.
>>
>> >> The current mount unit would never trigger for modules, because the
>> >> path will not exist. An internal API mount will cause a trransparent
>> >> kernel-forked modprobe with the mount() syscall.
>> > Yeah, the ConditionPathExists would have to be dropped.
>>
>> Right, but maybe we can key-off some other condition, which indicates
>> an EFI bootup, so we can avoid trying to mount things on systems where
>> it can't be there.
>>
>> Looking at if from a distance:
>> I guess the kernel should just start mounting its "crap" on its own,
>> instead of relying on userspace to know anything about this
>> ever-changing stuff. This filesytem is just a directory in /sys,
>> nothing else for userspace to know about it.
> Actually, in a container, it might make sense to not mount
> /sys/firmware/efi/efivars, since it opens an avenue to modify kernel
> state.

/sys (and all below) should be read-only inside a container anyway, so
it would only be readable, which might be bad enough, but not that
much different from the rest of /sys.

"Make sure to pre-mount /sys, and /proc, /sys/fs/selinux before
invoking systemd, and mount /proc/sys and the entirety of /sys and
/sys/fs/selinux read-only in order to avoid that the container can
alter the host kernel's configuration settings."
  http://www.freedesktop.org/wiki/Software/systemd/ContainerInterface

Also, in a container we mount 'sysfs' explicitly, not with a
bind-mount from the host, so this would not be inherited anyway, I
guess.

Kay

Re: [PATCH] Add syste-firmware-efi-efivars.mount for support automount EFI variable filesystem

[Lennart Poettering]

On Wed, 24.10.12 18:42, Lee, Chun-Yi (joeyli.kernel@gmail.com) wrote:

> Add units/sys-firmware-efi-efivars.mount rule for support automount
> EFI variable filesystem

Good in principle, but as Kay mentioned we probably should not expose
this as units, but just mount it from PID1's C code. More specifically
add it to the table in src/core/mount-setup.c, but make sure to set the
"fatal" bool in the table to false for this entry, to clarify that this
is not a reason to fail.

You probably also want to add the kmod to the table in
src/core/kmod-setup.c since the mount point is not available if the kmod
is not loaded.

Thanks!

Lennart

-- 
Lennart Poettering - Red Hat, Inc.

Re: [PATCH] Add syste-firmware-efi-efivars.mount for support automount EFI variable filesystem

[joeyli]

於 三，2012-10-24 於 16:04 +0200，Lennart Poettering 提到：
> On Wed, 24.10.12 18:42, Lee, Chun-Yi (joeyli.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> 
> > Add units/sys-firmware-efi-efivars.mount rule for support automount
> > EFI variable filesystem
> 
> Good in principle, but as Kay mentioned we probably should not expose
> this as units, but just mount it from PID1's C code. More specifically
> add it to the table in src/core/mount-setup.c, but make sure to set the
> "fatal" bool in the table to false for this entry, to clarify that this
> is not a reason to fail.
> 
> You probably also want to add the kmod to the table in
> src/core/kmod-setup.c since the mount point is not available if the kmod
> is not loaded.
> 
> Thanks!
> 
> Lennart
> 

Thanks for all systemd experts' review and comments.

I will follow the above suggestions to create new patch and send it
after tested success.


Thanks a lot!
Joey Lee