Hi Dan, Julius,

On Tue, 6 Dec 2022 at 06:51, Dan Handley <dan.hand...@arm.com> wrote:
>
> Hi
>
> Just a few comments:
>
> * I agree with Julius that in general it isn't feasible for firmware to 
> refuse to accept a TL with unknown tags. The structures are defined to help 
> with backwards/forwards compatibility. To assume that all firmware is built 
> at the same time is a special case. A particular firmware implementation 
> could have such a policy, but I don't think we can specify this. I like the 
> idea of defining a tag allocation policy but I'm not sure it's a good idea 
> for this to be in the main specification as this could rapidly change 
> depending on how the spec is used in practice. I've commented on the PR to 
> this effect.

That all sounds reasonable to me.

>
> * I don't think we’re all going to agree on how we're going to use the TL, 
> which is part of the reason for its existence. At least one of the use-cases 
> is combining the passing of standard structures like FDT/HOBs through to the 
> normal world firmware and/or OS, with the passing of smaller structures 
> between more tightly integrated firmware components. In a system where BL31 
> (for example) has to parse FDT anyway, it might make sense to put all data in 
> FDT format, but in other systems, BL31 might just relay the FDT, so adding an 
> FDT parser might add unnecessary overhead. I don't think we need to mandate 
> usage for now - patterns will emerge.

The main concern I have here is that TL not be used for non-trivial
binary formats (say more than a C struct or table of them). We have so
many of those sorts of things. The overhead of FDT is certainly a
concern, but some of these components have hundreds of KB of code
anyway. Adding another 5KB for all the convenience and visibility
shouldn't matter.

>
> * Using TEs throughout is probably only going to work for vertically 
> integrated systems, but that is clearly also an important case, so optimizing 
> the structures for size in those cases is reasonable. I've also commented on 
> the PR about this.

OK.

- Simon

>
> Cheers
>
> Dan.
>
> > -----Original Message-----
> > From: Simon Glass <s...@chromium.org>
> > Sent: 02 December 2022 17:54
> > To: Julius Werner <jwer...@chromium.org>
> > Cc: Jose Marinho <jose.mari...@arm.com>; t...@lists.trustedfirmware.org;
> > u-boot@lists.denx.de; boot-architect...@lists.linaro.org; Manish Pandey2
> > <manish.pand...@arm.com>; Joanna Farley <joanna.far...@arm.com>;
> > Ilias Apalodimas <ilias.apalodi...@linaro.org>; Matteo Carlini
> > <matteo.carl...@arm.com>; Dan Handley <dan.hand...@arm.com>; Rob
> > Herring <rob.herr...@arm.com>; Harb Abdulhamid
> > (h...@amperecomputing.com) <h...@amperecomputing.com>;
> > Sivasakthivel Nainar <snai...@amperecomputing.com>; Samer El-Haj-
> > Mahmoud <samer.el-haj-mahm...@arm.com>; nd <n...@arm.com>
> > Subject: Re: [TF-A] [RFC] Proposed location to host the firmware handoff
> > specification.
> >
> > Hi,
> >
> > On Wed, 30 Nov 2022 at 16:48, Julius Werner <jwer...@chromium.org>
> > wrote:
> > >
> > > Okay, FWIW I created a pull request with my suggestions here:
> > > https://github.com/FirmwareHandoff/firmware_handoff/pull/4
> > >
> > > That should make it easier to discuss specific details, hopefully. As
> > > I was looking at the header size and format more closely I noticed
> > > that the checksum calculation and some details about what the size
> > > fields mean seems underspecified right now, so I suggested a change
> > > for that as well. Let me know what you think.
> > >
> > > > 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.
> > >
> > > I don't think that's feasible, particularly if tag IDs are not
> > > allocated in order (which I think is useful so that we can try to
> > > group similar tags by vendor, e.g. if say MediaTek wanted to add some
> > > tags they could just find some nice round number in the tag space, say
> > > 0x200, and then start allocating 0x201, 0x202, 0x203, etc. while the
> > > next vendor can do their own thing at 0x300 instead). It also creates
> > > automatic incompatibilities if you put newer and older components in
> > > the same image, which I think is something we explicitly don't want. I
> > > think the right policy to encourage compatibility is that unknown tags
> > > just get ignored.
> > >
> > > > 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.
> > >
> > > Uhh... okay, sorry, but then you're going against everything that was
> > > discussed in this thread and in the previous discussions last year.
> > > The whole idea of making this a tagged list came from the argument
> > > that other structures (like FDT) are unnecessarily bulky and complex
> > > for some of the intended use cases (like replacing TF-A
> > > bl_aux_params). I mean, what's the point of even having a transfer
> > > list if all it does is wrap an FDT? You could just use the FDT
> > > directly! I think all the 4 tags that are currently specified in that
> > > document are pathological edge cases that may be necessary for legacy
> > > applications but represent the opposite of how this structure *should*
> > > be used in the ideal case. We don't need just more wrappers for the
> > > stuff that we already have, that doesn't promote interoperability in
> > > any way. If project A only wants to use HOBs but not FDT, and project
> > > B only wants to use FDT but not HOBs, then creating a transfer list
> > > structure that can wrap both an FDT and a HOB in a TE is pointless and
> > > not going to fix anything (project A will still not be able to parse
> > > the FDT-wrapping TE, and project B will still not be able to parse the
> > > HOB-wrapping TE). The only way this whole exercise can actually be
> > > useful and improve anything is if we can use it to eventually (slowly)
> > > get away from both HOBs and FDTs towards a point where all data is
> > > encoded in TEs directly, without any extra layers.
> >
> > I don't know where that idea came from and I thought I read the same
> > threads. I think FDT is the best data structure in general, because it is 
> > self-
> > describing, can be understood just by dumping it and we can use schema
> > tools to validate things, and many other reasons. Overall, as an industry, I
> > believe it is best to get away from ad-hoc binary structures.
> >
> > So all projects which need to send or receive complex data should use FDT.
> >
> > For replacing bl_aux_params, I think FDT makes sense in general, but there
> > may be situations where a few TEs are useful though.
> >
> > The point of the transfer list is to deal with cases where we don't have
> > enough memory / CPU power to use libfdt. This happens very early in boot,
> > and in that case it is helpful to pass small data structures containing 
> > useful
> > information.
> >
> > Actually even just grouping data structures (FDT, ACPI things) in a TL is 
> > useful,
> > since we have a unified way to find them all.
> >
> > I replied on the github PR also.
> >
> > >
> > > > 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.
> > >
> > > FWIW, the compromise proposal now preserves the 16-byte TE alignment.
> > > (8 byte header, then small entries can use the next 8 bytes for data
> > > and the whole TE is only 16 bytes with only up to 7 bytes of forced
> > > padding. Once you need more than 8 bytes of data it gets a bit more
> > > ugly but at that point I guess you're not really a super small entry
> > > anymore.)
> >
> > I initially thought that was fine, but then I realised that this loses the 
> > 16-byte
> > alignment of the entries.
> >
> > Regards,
> > SImon

Reply via email to