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

[Soeren Moch <smoch@web.de>]

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

Regards,
Soeren
>
> Regards,
> Simon