search

Linaro / meta-qcom

OpenEmbedded/Yocto Project BSP layer for Qualcomm based platforms
MIT License
62 stars 70 forks source link

Generate vfat image by packing device-tree blob. #620

Closed quic-vkraleti closed 6 days ago

quic-vkraleti commented 1 month ago

This PR introduces a bbclass, which upon inherited by kernel will produce a vfat image for each device-tree blob generated by kernel.

This helps to meet one of the ARM System Ready IR requirements of keeping dtb out of kernel binary.

ricardosalveti commented 1 month ago

I would also split this change in at least 2 commits, one adding the recipe and another the image.

lumag commented 1 month ago

Also please make sure that your new recipes are included into the CI build.

quic-vkraleti commented 1 month ago
  • please split into two commits

Done.

  • please describe, how the dtb us going to be selected. Refer existing standards, etc. Note, that using qcom,msm-id and qcom,board-id is generally frowned upon by DT maintainers and are forbidden to be used for new devices.

Updated description in dtb-qcom-image.bb describing how combined-dtb is put into use.

As a general note, I dislike the idea of a single merged dtb image just as a concatenation of all DT files. It is not easily extensible, overriding it also is not easy. If not for the SecureBoot, I'd have suggested following DtbLoader way (dtb/CHID.dtb). However with SecureBoot being an important thing the DTB provisioning mechanism should leave place for the signature verification. I would suggest exploring either a set of PE files, each having one or several DTB sections and a signature, a FIT image with a signature or just external .pk7 signature being placed next to each dtb file.

Discussions in progress with other stake holders on alternative proposals like "keep a DTB in UKI and pass the same to Linux instead of what is provided by UEFI". Once there is a conclusion, will share a PR for the same.

Also please make sure that your new recipes are included into the CI build.

Sure. What is the best place for adding a dependency on esp & dtb images to get built by default?

lumag commented 1 month ago

Yes, I saw that. However it is not acceptable. The qcom,board-id property isn't allowed to be used on the affected SoCs, see arm/qcom.yaml

lumag commented 1 month ago

keep a DTB in UKI and pass the same to Linux instead of what is provided by UEFI

This is good as an override, not I don't think it will be acceptable for System ready. Please review the old DtbLoader proposal of passing multiple dtb files via the /dtb/.dtb, where CHID is generated from the platform properties (like Windows and fwupdate do).

lumag commented 1 month ago

Sure. What is the best place for adding a dependency on esp & dtb images to get built by default?

Please extend the CI config in this repo. Be sure not to enable new images for the branches which don't have corresponding recipe.

ricardosalveti commented 4 weeks ago

This is good as an override, not I don't think it will be acceptable for System ready.

SR just requires the firmware to provide a DTB that is compliant with the spec, the issue here is that the DTB provided by the firmware is not going to be compliant, but the way it gets loaded is outside the SR scope. It is possible to have the DTB available via the firmware directory (ESP/FIRMWARE), but that is optional.

IIRC the idea is also to have the firmware providing a generic and upstream aligned device tree when the partition is not available (or empty), which can be a tradeoff until we're done with the the board-id / compatible string upstream discussion.

ricardosalveti commented 4 weeks ago

Discussions in progress with other stake holders on alternative proposals like "keep a DTB in UKI and pass the same to Linux instead of what is provided by UEFI". Once there is a conclusion, will share a PR for the same.

Supporting DTB loading via UKI is going to be needed anyway, even if the firmware is or not providing any DTB before UKI. DTB via UKI is basically allowing the OS to take control of the DTB content (which is also a valid use case).

lumag commented 3 weeks ago

the DTB provided by the firmware is not going to be compliant

Could you please be more specific, which DTB is it and why isn't it compliant? Can we replace it with the proper, compliant DTB?

ricardosalveti commented 3 weeks ago

the DTB provided by the firmware is not going to be compliant

Could you please be more specific, which DTB is it and why isn't it compliant? Can we replace it with the proper, compliant DTB?

Selection done by the boot firmware is still done with qcom,board-id, for example. Moving away from that is a larger discussion that is going in parallel.

lumag commented 3 weeks ago

@ricardosalveti can we flash a single DTB and ignore selection completely?

lumag commented 3 weeks ago

I think there are two distinct questions:

From the SR perspective it should be fine if the UEFI provides a not-so-perfect DTB, especially if this DTB can later be upgraded via different mechanism, like fwupd

quic-vkraleti commented 3 weeks ago

@ricardosalveti can we flash a single DTB and ignore selection completely?

AFAIK, even if a single DTB is flashed, board-id check would still happen. Only way to avoid this check is by placing dtb inside UKI.

lumag commented 3 weeks ago

Oh, my. It would be really nice if such checks were skipped if there is just a single DTB. I think in the past we had to patch ABL to disable such checks.

ricardosalveti commented 3 weeks ago

We had discussions to allow:

But that firmware is still to be release iirc.

lumag commented 3 weeks ago

Well, from my POV the best option is a separate partition with a single DTB file, loaded by the firmware. Then we have a sane default, users can manually change it, fwupd can handle updates. And if at some point there is a need to go to multiple files, UEFI can use compat strings to make a selection.

The board-id will never fly and will never be accepted upstream as a generic solution. In the mobile world nearly every phone vendor mishandles those options, telling that the phone is MTP (or QRD).

Of course grub/systemdboot still can override the loaded DTB if it is specified in the config.

ricardosalveti commented 3 weeks ago

Well, from my POV the best option is a separate partition with a single DTB file, loaded by the firmware. Then we have a sane default, users can manually change it, fwupd can handle updates.

That is indeed a goal, but need to check on the details there as the name (and possibly board-id) needs to match what is expected by the firmware.

lumag commented 3 weeks ago

Name -- agreed, but isn't firmware already loading a combined DTB file? We can use that file name. Board-id -- make firmware ignore it if there is a single entry. Or maybe make a special case for something like default.dtb.

quic-vkraleti commented 3 weeks ago

@lumag @ricardosalveti Moved two commits for enhancing uki generation to accept optional DTB from this PR into https://github.com/Linaro/meta-qcom/pull/622

lumag commented 3 weeks ago
ricardosalveti commented 3 weeks ago
  • package description is still incorrect

The description was updated with "This recipe creates a vfat image with the 'combined-dtb.dtb' for UEFI consumption.", which is the intent.

  • DTB selection is based on the non-standard property

While not ideal, this is the current firmware behavior and will probably stay this way until an upstream compatible solution is found (discussion in progress). Not something that can be changed as part of this PR.

lumag commented 3 weeks ago

Ack for the description. For the DTB, can we pretty please have a single DTB with no additional selection. There is no reason to install several different revisions if we know in advance, which one is going to be selected.

ricardosalveti commented 3 weeks ago

Ack for the description. For the DTB, can we pretty please have a single DTB with no additional selection. There is no reason to install several different revisions if we know in advance, which one is going to be selected.

This feature will be supported in a newer firmware release, currently in development. Once available we can update the logic here.

lumag commented 3 weeks ago

I'm reluctant to merge a solution that depends on the patches that are not going to bed accepted upstream. And upstream DT has neither qcom, msm-id nor qcom,board-id properties.

lumag commented 1 week ago

LGTM now