Re: [PATCH] wic: bootimg-efi-isar: Add support for loading device tree files

[Jan Kiszka <jan.kiszka@siemens.com>]

On 11.07.22 18:12, Henning Schild wrote:
> Am Mon, 11 Jul 2022 17:14:09 +0200
> schrieb Jan Kiszka <jan.kiszka@siemens.com>:
> 
>> On 11.07.22 15:12, Henning Schild wrote:
>>> Am Sun, 10 Jul 2022 22:51:44 +0200
>>> schrieb Jan Kiszka <jan.kiszka@siemens.com>:
>>>   
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> For device tree using systems, add support to set a custom devices
>>>> tree during UEFI boot. This requires to copy the DTB file to the
>>>> boot partition and to add the respective loader entries to the
>>>> configuration files.
>>>>
>>>> Both grub and systemd-boot support only loading a specific device
>>>> tree. Therefore refuse to work if DTB_FILES contains more than one
>>>> entry.
>>>>
>>>> This has been tested with grub from bullseye and systemd from
>>>> bullseye-backport. The latter is needed as only systemd 250 gained
>>>> the required feature.
>>>>
>>>> Out of scope are overlays (only supported by systemd-boot) as well
>>>> as unified kernel images (practically broken on non-x86 with
>>>> current toolchains).
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>  meta/classes/imagetypes_wic.bbclass           |  2 +-
>>>>  .../wic/plugins/source/bootimg-efi-isar.py    | 39
>>>> +++++++++++++------ 2 files changed, 29 insertions(+), 12
>>>> deletions(-)
>>>>
>>>> diff --git a/meta/classes/imagetypes_wic.bbclass
>>>> b/meta/classes/imagetypes_wic.bbclass index 61a74d4a..3a577f71
>>>> 100644 --- a/meta/classes/imagetypes_wic.bbclass
>>>> +++ b/meta/classes/imagetypes_wic.bbclass
>>>> @@ -106,7 +106,7 @@ WICVARS += "\
>>>>             ROOTFS_SIZE STAGING_DATADIR STAGING_DIR STAGING_LIBDIR
>>>> TARGET_SYS TRANSLATED_TARGET_ARCH" 
>>>>  # Isar specific vars used in our plugins
>>>> -WICVARS += "DISTRO DISTRO_ARCH"
>>>> +WICVARS += "DISTRO DISTRO_ARCH DTB_FILES"
>>>>  
>>>>  python do_rootfs_wicenv () {
>>>>      wicvars = d.getVar('WICVARS', True)
>>>> diff --git
>>>> a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
>>>> b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py index
>>>> a24e04f3..aff51e81 100644 ---
>>>> a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py +++
>>>> b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py   
>>>
>>> This file is a fork from OE with very minimal non-functional
>>> changes to make it into Isar. I am afraid that feature will have to
>>> make it via OE. 
>>
>> Right, from
>>
>>  bootimg-efi-isar.py |   65
>> +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed,
>> 59 insertions(+), 6 deletions(-)
>>
>> to
>>
>>  bootimg-efi-isar.py |  104
>> +++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed,
>> 87 insertions(+), 17 deletions(-)
> 
> If you look carefully you will see that what you propose is the only
> new feature. And most of the other code is a "trivial" grub template
> ... which one could shrink, but it is not worth it. In other places i
> invested a lot to make it hopefully easy to maintain, not to make it
> look small.
> 
> What you suggest is a pretty big change, which is going to be pretty
> hard to maintain. If we really need to go down that road ... we need
> testing for that.

The change is not "big".

> 
>> The problem is that OE core has no DTB_FILES, and seems like not even
>> a concept of injecting DTBs via the bootloader, thus passing them to
>> wic.
> 
> Then prepare an "additional boot files" patch for OE. ... but wait. It
> already has "custom_cfg". There is "initrd" which can specify multiple
> files to copy, so i guess all you need is to write it down differently.
> Carry the generated config+yourdiff as custom_cfg and set initrd=foo to
> initrd=foo;my.dtb to get that file copied, done without a patch.

Look, we surely don't want to ask users for much more than setting
DTB_FILES in order to inject their DTB into the boot loader. That rules
out custom configs and the existing primitive interface.

But I will explore if OE has an opinion on setting "devicetree" in grub
and systemd-boot configs. If that is at least positive in general, maybe
some less elegant API than this one is possible, and it will generate
one line less of deviation that this patch. If not, we will move forward
with our API.

Meanwhile: Happy Isar forking...

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux