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

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

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@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.

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.

-Takahiro Akashi

> Does it make sense to make enabling the partition support an option,
> instead of mandatory?
> 
> Regards,
> Simon