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

Mon Mar 14 20:21:09 CET 2022

Hi Soeren,

On Mon, 14 Mar 2022 at 13:17, Soeren Moch <smoch at web.de> wrote:
>
> Hi Simon,
>
> On 14.03.22 18:08, Simon Glass wrote:
> > Hi Soeren,
> >
> > [I think you sent your email with html or something so it is a big
> > mangled. I'll just add one comment]
> >
> > On Mon, 14 Mar 2022 at 02:27, Soeren Moch <smoch at web.de> wrote:
> >> Hi Simon,
> >>
> >> On 12.03.22 06:02, Simon Glass wrote:
> >>
> >> Hi Soeren,
> >>
> >> On Fri, 11 Mar 2022 at 15:43, Soeren Moch <smoch at web.de> wrote:
> >>
> >>
> >>
> >> On 09.03.22 16:33, Simon Glass wrote:
> >>
> >> Hi Tom,
> >>
> >> On Wed, 9 Mar 2022 at 07:25, Tom Rini <trini at konsulko.com> wrote:
> >>
> >> On Tue, Mar 08, 2022 at 08:10:38PM -0700, Simon Glass wrote:
> >>
> >> Hi Tom,
> >>
> >> On Tue, 8 Mar 2022 at 20:00, Tom Rini <trini at konsulko.com> wrote:
> >>
> >> On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote:
> >>
> >>    Hi Tom,
> >>
> >> On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini at konsulko.com> wrote:
> >>
> >> On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote:
> >>
> >> Hi Soeren,
> >>
> >> On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch at web.de> wrote:
> >>
> >> On 08.03.22 17:56, Simon Glass wrote:
> >>
> >> Hi,
> >>
> >> On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk at gmx.de> 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.
> >>
> >> tbs2910 - perhaps we should just drop this board? It doesn't use
> >> DM_SERIAL and still uses OF_EMBED
> >>
> >> Are we again at the same point? You are breaking working boards with
> >> (for these boards) useless additions, and all you come up with is
> >> "remove this board". Of course without adding the board maintainer.
> >>
> >> I'm just expressing reasonable frustration that this board uses
> >> OF_EMBED and does not use DM_SERIAL, after all of this time. Why
> >> should the rest of the U-Boot developers care more about this board
> >> than the maintainer?
> >>
> >> I cannot see what is is reasonable here.
> >>
> >> I care about this board, what you can simply see by the fact that I even
> >> picked this thread from the mailing list while you "forgot" to cc me.
> >>
> >> This is the patch I sent:
> >>
> >> [PATCH 2/5] tbs2910: Disable ext4 write
> >>
> >> It shows that you are on cc. What are you referring to?
> >>
> >> I'm referring to the exact same email thread that I answered in, which btw. is still this exact same thread for this answer. Why should I refer to the totally different email thread you cited here?
> >>
> >> OF_EMBED and DM_SERIAL are not at all related to EFI or size constraints.
> >>
> >> I'm surprised that you can speak for "the rest of the U-Boot
> >> developers", and that you want to push your frustration onto tbs2910
> >> developers and users. Why is it my fault that other people add code size
> >> without guarding config options? Why is it my fault that nobody informed
> >> me that there is again a size problem?
> >>
> >> Your board is up against the limit and this causes problems. Please
> >> take a look and see how you can add some margin. Takahiro's series
> >> does add size and this is unavoidable. See my series of today for some
> >> fixes for the SPL size, but for U-Boot proper we have to accept the
> >> growth.
> >>
> >> As it stands here this is just your opinion. Why exactly is this unavoidable?
> >>
> >> Please keep in mind Simon that we've had zero releases with the
> >> DM_SERIAL migration warning being posted, v2022.04 will be the first
> >> one.
> >>
> >> Yes, understood :-) For OF_EMBED though...?
> >>
> >> No deadline and 50 boards.
> >>
> >> Er, there has been a build message about that since the beginning, so
> >> people ignored it. Do we really need to make the build fail for these
> >> sorts of things? Perhaps so, but it is a sad situation.
> >>
> >> Yes, in hind-sight, "don't do that" wasn't the right path.  It would be
> >> a good idea to start a different thread and see what / how the platforms
> >> can be migrated away.
> >>
> >> For tbs2910 this is just a workaround for a strange property of the imx
> >> build system. OF_SEPARATE created a broken u-boot.imx when I tried last
> >> time.
> >>
> >> OK, that is worth digging in to.
> >>
> >> Probably. I'm happy to test whatever someone comes up with.
> >>
> >>
> >> I think there is a use case for it now - e.g. booting Apple M1 which
> >> uses a separate bootloader. IMO a .img or .fit file would be better in
> >> some cases but people seem to be allergic to implementing U-Boot
> >> things in their code bases. We have the same requirement for the EFI
> >> app since UEFI does not implement the U-Boot .img file.
> >>
> >> So if we are going to support this, perhaps we should create a new
> >> option for it. But honestly I am just too weary to consider yet
> >> another migration. We need to finish some, e.g. Kconfig.
> >>
> >> It was actually quite hard to add a migration message until we added
> >> the CONFIG_SERIAL base thing and that was a pain to do.
> >>
> >> For those of us who take on larger refactors etc., we end up spending
> >> a lot of our time on these few platforms. I'm not picking on tbs2910in
> >> in particular.
> >>
> >> Well, the flip side of the problem here is that there's a number of
> >> platforms with real constraints to them and it keeps being "can we drop
> >> this yet?" without CC'ing the board maintainer on the series that once
> >> again pushes a given platform to the limit.  I would expect no size
> >> growth to tbs2910 for the topic of this series since it disables
> >> EFI_LOADER entirely, so why is it a problem?
> >>
> >> The partition changes are going to add some size anyway, I expect. I
> >> have not actually analysed it though. Perhaps we can just disable a
> >> filesystem?
> >>
> >> OK, you did not even analyse where the problem comes from. But disabling
> >> user visible functionality on my board is the natural solution to that?
> >> Strange.
> >>
> >> As above, please create some space so people can continue to develop.
> >> There are refactors and features updates which require more code
> >> space. It is somewhat rare, but it happens perhaps every year.
> >>
> >> It has always been u-boot policy that additional new features should not break existing boards, usually by disabling these new features in defconfig.
> >> It is also not new that there are boards with size constraints.
> >>
> >> If someone causes regressions, then I at least expect that this is thoroughly analysed.
> >>
> >> I was a bit too absolutist there, sorry.  Yes, a few hundreds of bytes
> >> here-and-there is probably a non issue.  But it shouldn't be kilobytes.
> >> It really shouldn't push things over the line.
> >>
> >> And on the tbs2910 side, Soeren, can you look at enabling LTO for this
> >> platform?  That would likely buy a good bit of space savings.  That
> >> might well be needed to do further DM migrations/etc.
> >>
> >> I'm not familiar with LTO in U-Boot, but will have a look at the weekend.
> >>
> >> OK, I suggest getting it several KB under the limit if you can, or
> >> perhaps even drop the limit.
> >>
> >> I already reduced tbs2910 image size several times by substantial amounts. And this is becoming more and more difficult. The size limit is real.
> >>
> >> Thanks Tom for the LTO suggestion, this will buy us another round. I sent a patch for that.
> >>
> >> But please, everyone, be careful with additional code size for existing boards. Additional code size is not unavoidable for disabled new features. You just did not try hard enough.
> > Please take a look at Tahahiro's series and tell me how we can avoid
> > adding a driver for partitions, when the whole point of the series is
> > to add a driver for partitions :-)
> If this is just a new driver that I don't need (as before), why is it
> enabled for my board and causing regressions?

If you disable partition support, you don't need it. Please just take
a look at the series as I think it will make sense.

Regards,
SImon