[PATCH v3 00/19] efi_loader: more tightly integrate UEFI disks to driver model

[AKASHI Takahiro]

On Tue, Mar 08, 2022 at 08:10:01PM -0700, Simon Glass wrote:
> Hi Takahiro,
> 
> On Tue, 8 Mar 2022 at 19:48, AKASHI Takahiro <takahiro.akashi at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Tue, Mar 08, 2022 at 07:34:15PM -0700, Simon Glass wrote:
> > > Hi Takahiro,
> > >
> > > On Tue, 8 Mar 2022 at 19:10, AKASHI Takahiro <takahiro.akashi at linaro.org> wrote:
> > > >
> > > > Heinrich, Simon,
> > > >
> > > > On Tue, Mar 08, 2022 at 05:49:13PM +0100, Heinrich Schuchardt wrote:
> > > > > On 3/8/22 12:36, AKASHI Takahiro wrote:
> > > > > > With this patch set[1] applied, UEFI subsystem maintains a list of its
> > > > > > disk objects dynamically at runtime based on block device's probing.
> > > > > > (See "issues" below.)
> > > > > >
> > > > > > [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk
> > > > >
> > > > > This series together with Simon's series breaks multiple boards due to
> > > > > size constraints:
> > > > >
> > > > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197
> > > > >
> > > > > Please, investigate how to work around this issue.
> > > >
> > > > I have already mentioned this size issue in my cover-letter
> > > > in order to let reviewers aware of it and discuss a possible solution:
> > > >
> > > > ===8<===
> > > > Issues:
> > > > =======
> > > > * The image size of U-Boot may increase. CI build test complains,
> > > >   for instance,
> > > >     rcar3_salvator-x:
> > > >       "u-boot.img exceeds file size limit: ... excess: 0x79c bytes"
> > > >     phycore-rk3288:
> > > >       "SPL image is too large (size 0x8800 than 0x8000)"
> > > >
> > > >   See [2].
> > > >
> > > > [2] https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3770&view=results
> > > > ===>8===
> > > >
> > > > I have dug into rcar3_salvator-x case; I removed *all* the commits
> > > > in this series and yet enabled CONFIG_EVENT, CONFIG_EVENT_DYNAMIC
> > > > and CONFIG_DM_EVENT, which are all required for enabling my patch,
> > > > with Simon's patch applied on top of v2022.04-rc3.
> > > >
> > > > Then I still see this size problem:
> > > > ===8<===
> > > >   ...
> > > >   MKIMAGE u-boot.img
> > > > u-boot.img exceeds file size limit:
> > > >   limit:  0x100000 bytes
> > > >   actual: 0x100036 bytes
> > > >   excess: 0x36 bytes
> > > > ===>8===
> > > >
> > > > So I have no way to deal with it.
> > > >
> > > > FYI, the combination of EVENT, EVENT_DYNAMIC and DM_EVENT will
> > > > increase the binary size by up to 0x1b2 for rcar3_salvator-x and
> > > > it seems the binary has almost already reached its maximum size
> > > > even now.
> > >
> > > So you do need EVENT_DYNAMIC?
> >
> > Unfortunately, yes.
> > When I rebased my patch set to your v2, I tried to use *static*
> > bindings, but some of ut tests, including dm_test_blk_base and
> > dm_test_blk_usb, failed.
> 
> OK. Well maybe there is a filesystem in there that is not needed? 1MB
> is a huge size! Can we disable EFI_LOADER on that board?

Well, EFI_LOADER is by default 'y' for arm64.
Basically, I doubt that this default is reasonable.

> >
> > This can happen because, with static bindings, efi's callback function
> > (efi_disk_probe) is unconditionally called even when UEFI subsystem has
> > not been initialized yet.
> 
> Yes, I have seen things like that too.
> 
> >
> > -Takahiro Akashi
> >
> > > Does it make sense to make enabling the partition support an option,
> > > instead of mandatory?
> 
> What about this one? ^^

First of all, according to my rough attempt,
the patches for adding efi_disk callback functions may increase
the binary size by 0x31c, while the patches for adding UCLASS_PARTITION
adds another 0x3ba.
This means that "enabling the partition support an option" can help a bit
but doesn't help well enough overall.

FYI, adding dev_read/write(udev) interfaces costs another 0x1df.

-Takahiro Akashi

> Regards,
> Simon