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

Tom Rini
2022-03-14T13:05:18Z

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

On Mon, Mar 14, 2022 at 09:27:40AM +0100, Soeren Moch 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@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@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@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@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@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@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.

I just want to emphasize the last point here. We need to make sure
we're making platforms better, not just bigger. And to preempt "EFI
keeps growing", yes, in reasonably small amounts, implementing a spec
(or fixing bugs against a spec) that the semi vendor is telling everyone
to make sure their hardware is compliant with. And I check all of the
growth for everyone, all of the time, for everything.

--
Tom

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