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

[Heinrich Schuchardt]

On 3/9/22 06:07, AKASHI Takahiro wrote:
> 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.

All major distros support booting via UEFI. Fedora and Suse have
specifically opted to make this the preferred way to boot on ARM. Same
is true for BSD. So why do you have doubts?

Best regards

Heinrich

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