[TF-A] [RFC] Proposed location to host the firmware handoff specification.

[Simon Glass]

Hi,

On Wed, 30 Nov 2022 at 14:52, Julius Werner <jwerner at 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