-
Notifications
You must be signed in to change notification settings - Fork 17
Add Transfer Entry for Devicetree Overlay #74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Add blob type for DT overlay according to the update of Firmware Handoff spec[1]. [1] FirmwareHandoff/firmware_handoff#74 Signed-off-by: Raymond Mao <[email protected]>
Add blob type for DT overlay according to the update of Firmware Handoff spec[1]. [1] FirmwareHandoff/firmware_handoff#74 Signed-off-by: Raymond Mao <[email protected]>
source/transfer_list.rst
Outdated
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| The Devicetree Overlay is defined in [DTO]_. The Devicetree Overlay TE contains | ||
| the Device Overlay Note in the data section. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this refer to https://docs.kernel.org/devicetree/dynamic-resolution-notes.html ? If so, can you please add a link to it?
Is this information available in [DT]. I couldn't find it but I might have missed something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reference on [DTO] is fair enough, since it refers to https://docs.kernel.org/devicetree/dynamic-resolution-notes.html.
I cannot find this information from [DT] too.
| The Devicetree Overlay is defined in [DTO]_. The Devicetree Overlay TE contains | ||
| the Device Overlay Note in the data section. | ||
| The intent of the Devicetree Overlay entry is to modify the live devicetree in | ||
| U-Boot or kernel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a few more details about the lifefcycle here?
- Where does the overlay come from?
- Where does the base devicetree come from?
- Why the overlay is needed in addition to the base devicetree (different types of hardware / variants?)
- Are the devicetrees signed? How does security work with this?
- When should the overlay be applied? In U-Boot, or Linux, or something else?
- Give an example of projects in the boot chain and how this would be used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a few more details about the lifefcycle here?
I think the details and concept about overlay should be in the DT document (linked at the end) instead of this document.
This document focus on the definitions of each TE. But I can answer some of your questions as below.
- Where does the overlay come from?
Any boot stage can add overlays for the followng stages.
- Where does the base devicetree come from?
The previous boot stage.
- Why the overlay is needed in addition to the base devicetree (different types of hardware / variants?)
The reasons for patching a DT in the following stages are varied and totally implementation-defined.
- Are the devicetrees signed? How does security work with this?
If this is a concern, we should think about to sign the complete TL instead of DT only.
- When should the overlay be applied? In U-Boot, or Linux, or something else?
The overlay is self-described, so when to apply is implementation-defined.
- Give an example of projects in the boot chain and how this would be used
We already have usecases (AMD) to import an overlay as a TE during BL2 and then passed and applied in U-Boot.
| DT overlay entry layout (XFERLIST_DT_OVERLAY) | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| The Devicetree Overlay is defined in [DTO]_. The Devicetree Overlay TE contains |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it important that that this TE specifically refers to a DT overlay as oppose to a base DT? Can the receiver deduce this? The reason I ask is that I previously heard about the use-case for sending auxiliary (base) DTs, which looks similar to this. So can this be renamed to something like XFERLIST_FDT_AUXILIARY to cater for both cases (multiple base DTs and multiple overlays)? If there's a need for the sender to communicate the the distinct cases, then I guess it's OK to have separate TEs. An alternative might to have a flags field to indicate this but that might be overkill.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need XFERLIST_FDT_AUXILIARY as the spec v1.0 implies to support "multiple TEs with the same tag ID" - that means a TL is able to contain multiple XFERLIST_FDT for bases and multiple XFERLIST_DT_OVERLAY for overlays.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current spec doesn't have provision for multiple TEs of the same tag id:
It is strongly recommended that entries of a given type are unique in the TL. If firmware designs require multiple TE instances of a given type, then that TE type definition should provide sufficient information for the TE consumer to disambiguate between all TE instances. That information can be, for example, a sub-type field or contained within a self-describing data blob.
However, I guess for XFERLIST_FDT, the "model = " node might be enough to distinguish between instances.
I'll assume it is important for you to distinguish between base and overlay DTs (though you didn't explicitly say that).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current spec doesn't have provision for multiple TEs of the same tag id:
It is strongly recommended that entries of a given type are unique in the TL. If firmware designs require multiple TE instances of a given type, then that TE type definition should provide sufficient information for the TE consumer to disambiguate between all TE instances. That information can be, for example, a sub-type field or contained within a self-describing data blob.
However, I guess for XFERLIST_FDT, the "model = " node might be enough to distinguish between instances.
I'll assume it is important for you to distinguish between base and overlay DTs (though you didn't explicitly say that).
Yes, I mean, the current spec implies that multiple TE with same tag ID is possible, though it encourages a sub-type to be implementation-specific. As for a general solution for both self-described and non-self-described data, I would expect to have it as part of the TE header instead of placing it into the payload.
But anyway, this shouldn't block the DT or DTO, as you commented, both of them are self-described.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So can this be renamed to something like XFERLIST_FDT_AUXILIARY to cater for both cases (multiple base DTs and multiple overlays)?
Honestly, I don't think it's a good idea to try to make tags more flexible so they can be used for many different cases... on the contrary, I think it is important to make sure each tag ID clearly identifies one single application. I agree with Simon above that this tag seems underspecified as it is already — it just says that there's a device tree overlay in this TE, but not what that overlay is for or what the receiving firmware is supposed to do with it.
If the interoperability goal is that two boot stages which haven't intentionally been co-designed can hand off data to each other and use what they were given appropriately, then the spec needs to be very precise about what each piece of data means and in what ways it may (and may not) be used. A device tree that's supposed to be used by a kernel-loading boot stage to pass to the kernel is not the same as a device tree that's supposed to be used by the boot stage itself to enumerate devices. An overlay that's supposed to be merged into a device tree that the following boot stage got from somewhere else (e.g. hardcoded in the image) is probably not the same as an overlay that's supposed to be merged into a device tree that was also passed in the TL by the same stage.
Trying to make a single tag that could be valid for all these cases probably just means that boot stages will always ignore it unless they have been specifically hardcoded to interpret it as one specific of these uses, and that kinda defeats the purpose of self-describing data (because then an important piece of the information, how exactly to use it, is still passed out of band through compile-time configuration). If the tags were more precise (e.g. XFERLIST_DTBO_MERGE_INTO_KERNEL), the chance that things would interoperate correctly without both sides being specifically hardcoded on what to expect from the other would be higher.
If the idea is that some description inside the DTBO should resolve this ambiguity, then I think this spec should exactly describe the fields that should be used for that (so that all implementations can be sure to agree on the same thing).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is true that all of DTOs finally go to kernel but being applied in which boot stage is important.
But XFERLIST_HW_FDT_OVERLAY or XFERLIST_KERNEL_FDT_OVERLAY is still not clear for which boot stage should apply it.
Shouldn't the overlay always be applied and used by the first stage that has the base DT (whether that's the current receiver or a subsequent stage)? I don't see a use-case for a stage to apply the overlay for use by subsequent stages (e.g. the kernel) but not itself. The way you want to use this overlay in the kernel loader is the same way that earlier stages would use it, so I don't see how the name XFERLIST_DT_OVERLAY_KERNEL_LOADER helps clarify.
Multiple base DT is another topic and we can assume that single base DT is being used here as we don't have a real usecase now.
OK, understood.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is true that all of DTOs finally go to kernel but being applied in which boot stage is important.
But XFERLIST_HW_FDT_OVERLAY or XFERLIST_KERNEL_FDT_OVERLAY is still not clear for which boot stage should apply it.Shouldn't the overlay always be applied and used by the first stage that has the base DT (whether that's the current receiver or a subsequent stage)? I don't see a use-case for a stage to apply the overlay for use by subsequent stages (e.g. the kernel) but not itself. The way you want to use this overlay in the kernel loader is the same way that earlier stages would use it, so I don't see how the name XFERLIST_DT_OVERLAY_KERNEL_LOADER helps clarify.
We do have usecase that BL2 generates an overlay for applying in BL33, but it is not expected to be applied in BL31/32.
Maybe naming it to XFERLIST_FDT_OVERLAY_BL33 is more helpful.
Multiple base DT is another topic and we can assume that single base DT is being used here as we don't have a real usecase now.
OK, understood.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do have usecase that BL2 generates an overlay for applying in BL33, but it is not expected to be applied in BL31/32.
Interesting, but are you just saying the overlay is probably not relevant for BL31/32 or the overlay must not be applied to BL31/32? For the former case, I don't think the spec needs to make a distinction between BL33 and earlier stages - it can be IMP DEF whether a stage applies the overlay. For the latter case, I agree a distinction is necessary.
Maybe naming it to XFERLIST_FDT_OVERLAY_BL33 is more helpful.
BL33 is TF-A terminology. If a distinction is necessary, perhaps XFERLIST_FDT_NORMAL_WORLD_OVERLAY?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do have usecase that BL2 generates an overlay for applying in BL33, but it is not expected to be applied in BL31/32.
Interesting, but are you just saying the overlay is probably not relevant for BL31/32 or the overlay must not be applied to BL31/32? For the former case, I don't think the spec needs to make a distinction between BL33 and earlier stages - it can be IMP DEF whether a stage applies the overlay. For the latter case, I agree a distinction is necessary.
I mean the former case. @apalos Do we have the usecase of the latter one that Dan mentioned? But even for the former case, the DTO producers still need a distinction to identify which stage is the preferred concumer.
If this is not a cosideration, and multiple base DT is out-of-scope here, then we can simply go back to the original idea with just a general entry XFERLIST_FDT_OVERLAY for all.
Maybe naming it to XFERLIST_FDT_OVERLAY_BL33 is more helpful.
BL33 is TF-A terminology. If a distinction is necessary, perhaps XFERLIST_FDT_NORMAL_WORLD_OVERLAY?
It looks good for me. Can we unify it as XFERLIST_FDT_OVERLAY_<STAGE>? Then all later DTO can just follow the same pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...DTO producers still need a distinction to identify which stage is the preferred concumer.
I don't see that. This overlay is specifically for the hardware device tree (that is eventually passed to the kernel); any firmware that has both the base DT and overlay (wherever they came from) should apply and use it, right?
This excludes any other TEs that use the DT format (for example, XFERLIST_DT_SPMC_MANIFEST or XFERLIST_DT_FFA_MANIFEST).
If this is not a cosideration, and multiple base DT is out-of-scope here, then we can simply go back to the original idea with just a general entry XFERLIST_FDT_OVERLAY for all.
We already said multiple base DTs are out of scope. XFERLIST_FDT_OVERLAY works for me.
Can we unify it as XFERLIST_FDT_OVERLAY_...
That would work for me if we needed the but I don't think we do.
| * - dto | ||
| - data_size | ||
| - hdr_size | ||
| - The dto field contains the DT overlay. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, the TL may contain multiple overlays (or multiple base DTs if we cater for that use-case too), in which case there should be a way of distinguishing between these multiple instances as stated in section 2.4. For example, we may need a subtype field that the consumer understands, like a name or an id.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, an index field (or subtype) will be added into the TE header (or the unused bits of tag ID) as a general solution to all multiple TEs. @manish-pandey-arm has proposed the solution.
So we don't need to add this specially for DT or DTO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that we are going to extend the TE header at this stage so we may need to add a subtype to specific TEs like this. I believe there's a plan to schedule a call on this subject with the other maintainers. So let's keep this on hold until then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that we are going to extend the TE header at this stage so we may need to add a subtype to specific TEs like this. I believe there's a plan to schedule a call on this subject with the other maintainers. So let's keep this on hold until then.
FMO, having it in the header is more convenient than in the payload. Yes, this PR is not urgent to be merged. I just want to reserve the placeholder of ID to represent DTO as I need the ID for code implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, the TL may contain multiple overlays (or multiple base DTs if we cater for that use-case too), in which case there should be a way of distinguishing between these multiple instances as stated in section 2.4. For example, we may need a subtype field that the consumer understands, like a name or an id.
So that's an interesting case. I think we will need multiple DTs.
Think of a baseboard with multiple SOMs. It can have 1 firmware as long as that firmware contains all DTs and potential overlays.
The DT selection should be 'solved' we are going to use the compatible node right?
Why don't we add something similar to TL entry for overlays? Or even better ask the same question in the DT spec. We could then use that to correlate DTs to overlays
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robherring I think we should do what I mention above eventually and have a compatible node in the DTBO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think using a compatible node in the DTBO sounds like good idea. However, even then there may be multiple overlays targeting the same compatible node, right? So a sub-type still might be needed here.
Add a transfer entry ID to represent devicetree overly according to Firmware Handoff spec pull request[1]. [1] FirmwareHandoff/firmware_handoff#74 Signed-off-by: Raymond Mao <[email protected]>
|
I just wanted to mention here (as I did in the meeting) the FIT spec, which could be a good format for the information needed here. https://fitspec.osfw.foundation/#document-chapter2-source-file-format |
Add XFERLIST_DT_OVERLAY to represent Devicetree Overlay Notes which are being used in U-Boot and kernel to update the live devicetree. Signed-off-by: Raymond Mao <[email protected]>
4212d29 to
2bbf128
Compare
|
The patch is amended with below items per our discussions on Jun 19 sync-up:
A patch was posted to linux-doc to update the top-level compatible in DTSO at: |
Add blob type for DT overlay according to the update of Firmware Handoff spec[1]. Add an inline header to represent the 'subtype' in a DT overlay blob payload. [1] Add Transfer Entry for Devicetree Overlay FirmwareHandoff/firmware_handoff#74 Signed-off-by: Raymond Mao <[email protected]>
During FDT setup, apply all existing DT overlays from the bloblist to the base FDT if bloblist is being used for handoff from previous boot stage. According to the spec update for DT overlay handoff[1], an overlay must have the same top-level compatible string as its target base DT has. Before applying the DTO, it checks whether sufficient space is reserved in the base FDT region. A margin (0x400) is used during estimating the space size required by the merged DT. A resizing happens if the reserved space is insufficient. After all overlays are applied, it resizes to the actual size of the merged DT. Note that the margin (0x400) is arbitrary from experience, it might not cover all possible scenarios as complex overlays with many properties might require extra spaces and lead to FDT_ERR_NOSPACE error. [1] Add Transfer Entry for Devicetree Overlay FirmwareHandoff/firmware_handoff#74 Signed-off-by: Raymond Mao <[email protected]>
During FDT setup, apply all existing DT overlays from the bloblist to the base FDT if bloblist is being used for handoff from previous boot stage. According to the Firmware Handoff spec update to support DT overlay [1], an overlay must have the same top-level compatible string as its target base DT has. Before applying the overlays, check whether sufficient space is reserved in the base DT blob, if not, resize the blob to the allowed padded size, which is limited by CONFIG_SYS_FDT_PAD and the bloblist spare space size. After all overlays are applied, resize the merged DT to its actual size. [1] Add Transfer Entry for Devicetree Overlay FirmwareHandoff/firmware_handoff#74 Signed-off-by: Raymond Mao <[email protected]>
During FDT setup, apply all existing DT overlays from the bloblist to the base FDT if bloblist is being used for handoff from previous boot stage. According to the Firmware Handoff spec update to support DT overlay [1], an overlay must have the same top-level compatible string as its target base DT has. Before applying the overlays, check whether sufficient space is reserved in the base DT blob, if not, resize the blob to the allowed padded size, which is limited by CONFIG_SYS_FDT_PAD and the bloblist spare space size. After all overlays are applied, resize the merged DT to its actual size. [1] Add Transfer Entry for Devicetree Overlay FirmwareHandoff/firmware_handoff#74 Signed-off-by: Raymond Mao <[email protected]>
Add blob type for DT overlay according to the update of Firmware Handoff spec[1]. Add an inline header to represent the 'subtype' in a DT overlay blob payload. [1] Add Transfer Entry for Devicetree Overlay FirmwareHandoff/firmware_handoff#74 Signed-off-by: Raymond Mao <[email protected]>
During FDT setup, apply all existing DT overlays from the bloblist to the base FDT if bloblist is being used for handoff from previous boot stage. According to the Firmware Handoff spec update to support DT overlay [1], an overlay must have the same top-level compatible string as its target base DT has. Before applying the overlays, check whether sufficient space is reserved in the base DT blob, if not, resize the blob to the allowed padded size, which is limited by CONFIG_SYS_FDT_PAD and the bloblist spare space size. After all overlays are applied, resize the merged DT to its actual size. [1] Add Transfer Entry for Devicetree Overlay FirmwareHandoff/firmware_handoff#74 Signed-off-by: Raymond Mao <[email protected]>
During FDT setup, apply all existing DT overlays from the bloblist to the base FDT if bloblist is being used for handoff from previous boot stage. According to the Firmware Handoff spec update to support DT overlay [1], an overlay must have the same top-level compatible string as its target base DT has. Before applying the overlays, check whether sufficient space is reserved in the base DT blob, if not, resize the blob to the allowed padded size, which is limited by CONFIG_SYS_FDT_PAD and the bloblist spare space size. After all overlays are applied, resize the merged DT to its actual size. [1] Add Transfer Entry for Devicetree Overlay FirmwareHandoff/firmware_handoff#74 Signed-off-by: Raymond Mao <[email protected]>
When managing multiple base device trees and overlays in a structured way (e.g. bundled in firmware or tools), it is helpful to identify the intended target base DT for each overlay, which can be done via a top-level compatible string in the overlay. This provides a way to identify which overlays should be applied once the DT is selected for the case when a device have a common firmware binary which only differs on the DT and overlays. This patch updates the document with a note and example for this practice. For more information on this firmware requirement, please see [1]. [1] FirmwareHandoff/firmware_handoff#74 Suggested-by: Ilias Apalodimas <[email protected]> Acked-by: Conor Dooley <[email protected]> Signed-off-by: Raymond Mao <[email protected]>
Add XFERLIST_DT_OVERLAY to represent Devicetree Overlay Notes which are being used in U-Boot and kernel to update the live devicetree.