Re: [PATCH 0/7] efi: Various tidy-ups and drop the default

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 12742 bytes --]

On Fri, Jul 02, 2021 at 09:48:27PM +0200, Mark Kettenis wrote:
> > From: Simon Glass <sjg@chromium.org>
> > Date: Fri, 2 Jul 2021 13:05:20 -0600
> > Cc: Tom Rini <trini@konsulko.com>, U-Boot Mailing List <u-boot@lists.denx.de>,
> >         Pali Rohár <pali@kernel.org>,
> >         Heinrich Schuchardt <xypron.glpk@gmx.de>,
> >         Ilias Apalodimas <ilias.apalodimas@linaro.org>,
> >         Alex Graf <agraf@csgraf.de>,
> >         Masahiro Yamada <yamada.masahiro@socionext.com>
> > Content-Type: text/plain; charset="UTF-8"
> > 
> > Hi Mark,
> > 
> > On Mon, 28 Jun 2021 at 11:49, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> > >
> > > > From: Simon Glass <sjg@chromium.org>
> > > > Date: Mon, 28 Jun 2021 08:18:26 -0600
> > > >
> > > > Hi Tom, Mark,
> > > >
> > > > On Mon, 28 Jun 2021 at 07:37, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Mon, Jun 28, 2021 at 10:38:50AM +0200, Mark Kettenis wrote:
> > > > > > > From: Simon Glass <sjg@chromium.org>
> > > > > > > Date: Sun, 27 Jun 2021 19:48:34 -0600
> > > > > > >
> > > > > > > It has come to light that EFI_LOADER adds an extraordinary amount of
> > > > > > > code to U-Boot. For example, with nokia_rx51 the size delta is about
> > > > > > > 90KB. About 170 boards explicitly disable the option, but is is clear
> > > > > > > that many more could, thus saving image size and boot time.
> > > > > >
> > > > > > EFI_LOADER used to be a lot smaller.  It is great to see that over the
> > > > > > years UEFI support has become more complete, but a lot of that new
> > > > > > code implements features that are not at all essential for just
> > > > > > booting an OS from storage.  If that growth leads to the suggestion to
> > > > > > disable EFI_LOADER completely by default, we're putting the cart
> > > > > > before the horse.
> > > > >
> > > > > Well, I see I forgot to prefix my patch with RFC, but I hadn't found
> > > > > EFI_LOADER being used in the wild on armv7, but wasn't sure about the
> > > > > BSD families.  I did see that Debian doesn't use it, and that Armbian
> > > > > doesn't even use it on aarch64.
> > > > >
> > > > > > > The current situation is affecting U-Boot's image as a svelt bootloader.
> > > > > >
> > > > > > Really?  I know UEFI has a bad reputation in the Open Source world,
> > > > > > and some of its Microsoft-isms are really annoying (yay UCS-2).  But
> > > > > > it works, it provides a standardized approach across several platforms
> > > > > > (ARMv7, AMRv8, RISC-V) and the industry seems to like it.  Personally
> > > > > > I'd wish the industry had standardized on Open Firmware instead, but
> > > > > > that ship sailed a long time ago...
> > > > > >
> > > > > > I find it hard to imagine that 90k is a serious amount of storage for
> > > > > > something that is going to include a multi-MB Linux kernel.  This
> > > > > > isn't code that lives in SPL or TPL where severe size restrictions
> > > > > > apply.
> > > > >
> > > > > In one of those cases where I need to pop back in to the other (Nokia
> > > > > N900 specific) thread and see if the big size reduction really was just
> > > > > disabling EFI_LOADER, it's perhaps just one of those "fun" things about
> > > > > Kconfig and anything other than "make oldconfig" for spotting new config
> > > > > options that default to enabled.
> > > >
> > > > Yes it will be interesting to see what you find there. My results on
> > > > nokia_rx51 were something like this:
> > > >
> > > > default
> > > >         arm: (for 1/1 boards) all +129370.0 bss +1136.0 data +7399.0
> > > > rodata +10989.0 text +109846.0
> > > >
> > > > without ebbr
> > > >        arm: (for 1/1 boards) all +38460.0 bss +1040.0 data +2375.0
> > > > rodata +5333.0 text +29712.0
> > > >
> > > > with various other things:
> > > > CONFIG_OF_LIBFDT_ASSUME_MASK=7
> > > > # CONFIG_OF_TRANSLATE is not set
> > > > # CONFIG_SIMPLE_BUS is not set
> > > > # CONFIG_TI_SYSC is not set
> > > > # CONFIG_CMD_FDT is not set
> > > >
> > > >        arm: (for 1/1 boards) all +19170.0 bss -16.0 data +360.0 rodata
> > > > +3274.0 text +15552.0
> > > >
> > > > (Mark, in the same email:)
> > > > > > FIT simply isn't fit for purpose (pun intended).  It only really works
> > > > > > for booting Linux, and forces people to combine u-boot, kernel,
> > > > > > initial ramdisk and other firmware components into a single image.
> > > > > > That is really undesirable as:
> > > > > > - This makes it sigificantly harder to update individual components of
> > > > > >   such an image.  Making it hard to update a kernel is obviously a
> > > > > >   serious security risk.
> > > > > > - This makes it impossible to build an OS install image that works om
> > > > > >   multiple boards/SoCs.
> > > >
> > > >
> > > > I would really like to understand this better. The whole thing is a
> > > > complete mystery to me.
> > > >
> > > > Firstly I have sometimes fiddled with booting other OSes using FIT. It
> > > > seemed OK. I can't see why it only works with Linux.
> > >
> > > Well, you could of course rework the boot flow of other OSes such that
> > > booting them includes some sort of FIT if you really wanted to.  I But
> > > an OS like OpenBSD comes with its own bootloader that is essential in
> > > the boot flow.  On OpenBSD armv7/arm64/riscv64 it adds some essential
> > > properties to the device tree.  Besides, the kernel itself relies on a
> > > valid EFI memory map.
> > 
> > (thanks for taking the time to reply with so much detail)
> > 
> > That's news to me; when did that happen? Anyway, I doubt that adds a
> > lot of code.
> 
> Shortly after EFI_LOADER support was added to U-Boot and there was a
> clear consensus that EFI_LOADER support was going to be enabled by
> default on armv7 and armv8 targets.  My initial commit of the EFI
> bootloader for armv7 is from May 2016.
> 
> > > > Secondly, I don't expect that U-Boot itself would be in the FIT.
> > >
> > > So the FIT would only contain the OS kernel and other OS components?
> > > What about the FIT that is used on some arm64 platforms to combine
> > > U-Boot and TF-A?  I guess you can have multiple FITs...
> > >
> > > > Thirdly, do you really want the kernel and initrd to be separate? At
> > > > least in the systems I have used, they are built together, even having
> > > > the same name, e.g.:
> > > >
> > > > initrd.img-5.10.40-1rodete1-amd64
> > > > System.map-5.10.40-1rodete1-amd64
> > > > vmlinuz-5.10.28-1rodete2-amd64
> > >
> > > I don't really use Linux on these platforms.  But I'd expect the
> > > normal package management tools of my Linux distribution to build
> > > those as necessary and place them in the root file system where the
> > > bootloader picks them up.  And as a distro developer, I'd like to have
> > > the same approach work on all Linux systems, regardless the specific
> > > firmware they're running (EDK2, U-Boot or something completely
> > > different).  Ideally that wouldn't even depend on the architecture.
> > >
> > > Now Armbian takes a different approach, and does treat most systems
> > > they provide as special snowflakes, providing flash images for each
> > > board.  But that doesn't scale and makes for a fairly crappy user
> > > experience.  They don't always support booting a mainline kernel.  And
> > > updating the kernel often requires installing special packages.
> > 
> > I don't think it is a requirement that things have to be special
> > snowflakes. There are a few common boot flows to support and we can
> > put that support in U-Boot and in userspace, without needing to make
> > everything special.
> > 
> > >
> > > > Finally, for the firmware components, do you mean system firmware? If
> > > > so, I would expect it to be more convenient to distribute updates to
> > > > that separately, although I suppose they could be combined with the
> > > > kernel if the combinatorial explosion can be contained. What is the
> > > > problem, exactly? (If you mean peripheral firmware, I would expect
> > > > fwupd to handle that.)
> > >
> > > I guess I mean system firmware.  Essentially everything that runs on
> > > the system before my OS bootloader runs.  So for me, U-Boot is part of
> > > the system firmware even if it sometimes happens to live on removable
> > > media.
> > >
> > > > What exactly is impossible? Can you please be more specific?
> > >
> > > So let me explain what we want for OpenBSD.  We really want a uniform
> > > user experience across platforms, and don't want to maintain different
> > > codepaths for special snowflake platforms that might exist for a
> > > specific architecture.
> > >
> > > Installing OpenBSD on a machine should be as simple as dd'ing the
> > > installer to some boot media, plugging it in and powering the machine
> > > on.  Now this is somewhat tricky to achieve on some hardware targetted
> > > by U-Boot as they don't come with usable system firmware on the board
> > > itself.  But on those boards you can mostly get away with having
> > > U-Boot on uSD or eMMC and the OS installer on USB.
> > >
> > > Updating the OpenBSD kernel should be as simple as copying the kernel
> > > to /bsd.  Since the root filesystem uses the UFS/FFS filesystem, this
> > > means that whatever we use as a bootloader must be able to read from
> > > that filesystem.  To make sure the kernel is properly seeded with
> > > entropy, the OpenBSD bootloader has some additional tricks up its
> > > sleeve.  It will replace a special segment in the kernel with random
> > > data before handing control to the kernel.  On platforms that support
> > > it, it will try to use a firmware-provided RNG to do this (and EFI
> > > supports this) but also mix in some random data from a file on the
> > > UFS/FFS filesystem.  It will actually mark that file as "used" after
> > > reading it to throw a warning when the file is reused when the machine
> > > is rebooted (it should have been filled with fresh new entropy).  So
> > > you really need to use the OpenBSD bootloader instead loading an
> > > OpenBSD kernel directly from system firmware.
> > >
> > > Updating the OpenBSD bootloader should be as simple as running
> > > installboot(8) from within the OS.
> > >
> > > This all works on pretty much any architecture that OpenBSD supports.
> > > And right now, thanks to EFI_LOADER support in U-Boot, we have a
> > > nearly uniform boot flow on amd64, arm64, armv7 and riscv64.
> > 
> > OK I see. Really it sounds like you have a pre-kernel loader. The
> > functionality (some fresh random data) could just as easily be
> > provided by U-Boot directly, with vastly less code and complexity. But
> > I do understand that trying to design across a whole system is a pain,
> > and it is attractive to try to use what hooks exist already to do what
> > you want.
> 
> What do you mean with vastly less code and complexity.  At this point
> 70% of the OpenBSD bootloader code is shared between most of the
> architectures OpenBSD runs on (alpha, amd64, arm64, armv7, hppa, i386,
> powerpc, riscv64, sh, sparc64).  And on platforms that support UEFI
> (amd64, arm64, armv7 and riscv64) this is closer to 95% shared code.
> 
> Having OS-dependent code in U-Boot doesn't work.  FreeBSD tried that
> with the U-Boot API.  It was never enabled by default, and got broken
> all the time.

That's good to know and I believe validates why we want EFI_LOADER as
much as possible.  This is a good example of how real world OSes can
make use of this to reduce their own overhead.  And I think this also
shows why we need EFI_LOADER, the U-boot API was never being
sufficiently tested and most importantly never had anyone championing
it's usage.  That's not the case with the EFI API.

> > How do you do verified boot?
> 
> We don't.  I don't think it makes sense for an Open Source OS where
> people like to tinker with things.  And it gets in the way of some of
> our own security features.  OpenBSD relinks the kernel upon every boot
> as a defence against attacks that depend on the kernel address space
> layout.  Doing that in an environment where all code needs to be
> signed is a big challenge.
> 
> There is a comany that has products based on OpenBSD.  They do
> verified boot.  I'm not really familliar with the details, but
> presumably they turned off address space randomization and I believe
> it is based on EFI_LOADER support.  Patrick Wildt contributed patches
> to implement this to U-Boot before the Linaro folks did their
> full-blown UEFI-based implementation.

That's good to know, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]