Hi,
On Wed, 30 Nov 2022 at 14:52, Julius Werner jwerner@chromium.org wrote:
Hi Jose,
Apologies for the late response, I had to find some time to dig back into this topic first.
The proposal is that the tag assignments are handled via a PR to [1]. A PR should provide reasoning for the proposed entry layout as well as a description of the use-case being serviced. The community is invited to comment on the pull requests. We should aim to have community consensus before merging any PR.
I think this is a good approach if we understand "community consensus" to be a quick, couple-of-days review cycle like we have for TF-A patches that doesn't create a huge threshold for additions, and it's fine for entries to be very specific to the needs of a single platform when necessary. But I think the document should still have more guidance about when standard vs. non-standard tags should be used, and in particular just strongly discourage the use of non-standard tags in general (basically I think they should only be used for experiments that won't reach production code, or for cases where all code reading or writing that tag is closed source). I think the spec should work
That's right
really hard in its language and guidance to discourage a situation where firmware components just assign a non-standard tag to something because it's easier and avoids the hassle of submitting something you don't think you'd want to share at the time, and then later it becomes entrenched and reused and eventually everyone has to pay attention to not reuse the non-standard tags that accidentally acquired a fixed meaning in enough implementations to become a problem. (It may also make sense to just make the standard tag range much bigger than the non-standard range, to make it clearer that the majority of things are expected to go in there.)
One way to enforce that is for firmware components to refuse to accept a transfer list with unknown tags, assuming that all firmware is built from source at the same time. It might be a pain, but I suspect it could help avoid security problems, where bad actors use the structure to pass code to a firmware blob, etc.
It seems sensible that data contained in an entry should not exceed 32MB. Could we still accommodate the notion of a hdr_size in the TL entry header? We'd have the following fields on an 8-byte TE header: tag_id -- 4 bytes hdr_size -- 1 byte Data_size -- 3 bytes
Yeah, that seems like a reasonable compromise, if you really think that variable header length is useful.
I still think the table header could be smaller as well (down to 16 bytes from 32 by just avoiding so much reserved space).
Do you want me to send those suggestions as pull requests to make them easier to discuss?
I have to disagree here. The context is gone in this email, so I cannot reply to the comments there. But I think it was talking about using the transfer list for very tiny structures, just a few words. That is not the intent. Firstly we should really have TF-A use the device tree (embedded in the transfer list) instead of replicating bl_params in this structure. Secondly, we should group things into a C structure that is at least 16-32 bytes long.
So I don't see a need for a smaller header. The 16-byte alignment works well with ACPI tables and we should not be using this structure for tiny things where the 8-byte overhead is significant. Believe me I have spent plenty of time dealing with memory space in U-Boot TPL, but
As to the maximum size, I can see how things might grow larger, for example to pass large amounts of data between firmware pieces, such as images, executables, even a ramdisk. It seems short-sighted to limit the size to 32MB.
For the header size, this provides for backwards compatibility in the case where we need to store more there. I don't think this is a big deal and certainly one byte should be enough...
But overall I think having a 16-byte header makes sense.
The document states that any tag in the standard range must be allocated in the spec before being used in code [4] -- what this is really saying is: until a tag id is allocated anyone can request it. So code written assuming a particular standard tag id (absent from the spec) may have to change later if someone in the meantime allocates that tag id for a different entry type. I believe it’s a per-community policy to prevent code submissions containing standard tag ids until these have been reserved in this spec. This is the advised policy, but this spec cannot mandate it, rather we must rely on the communities to enforce this.
I would say the spec should mandate that, honestly. If there is a standardized range, no firmware implementing transfer lists according to this spec should use tags from that range until they have been defined in the global repository. On the flip side, we need to make the process of defining new tags fast and painless so that this requirement doesn't become a burden.
Agreed on both points.
For the current set of entries, the data is included within the TL. Note that this does not prevent newer entries from pointing to off-TL data. Could this be handled on a case-by-case basis, via pull request, after an initial full release of the spec?
Yes, we can leave it like this and later add tags for the off-list equivalent of the tags defined there if necessary.
Ideally we should avoid this, since it makes it hard to relocate things. Anyway I agree we can add it later if really needed.
Regards, Simon