-----Original Message----- From: Julius Werner jwerner@google.com Sent: 24 June 2019 23:46 To: Soby Mathew Soby.Mathew@arm.com Cc: tf-a@lists.trustedfirmware.org; nd nd@arm.com Subject: Re: Header file re-org
Should the project recognize these special headers and have them organized
together in a folder ? It is important to recognize that the ABI can be extended by the platform. I would expect even if these “common” headers are organized into a folder, the platform specific ones need not go together with them.
Should the header be restricted from including standard C library headers ? Should these ABI headers be allowed to include each other ? Forward
declaration might be able to solve some of the issues, but good to have a policy on this.
Just to explain my reasoning for the proposal I uploaded: I think in order for this to be really usable, you cannot expect other projects to provide C library headers according to any specific standard, because in practice most of them don't (and in fact neither does Trusted Firmware... we have an include/lib/libc/, but it is missing plenty of headers required by ISO C and commonly available in other projects, like <ctype.h>). My intention was to require the absolute minimum that is needed to define handoff structures at all, namely fixed-width integer types like uint32_t. Even if another project doesn't use those, it's relatively easy for them to just write a wrapper header that defines those types before including the raw headers from TF. (They also need to pre-define the U(), __ASSEMBLY__ and AARCH32/AARCH64 macros because it would have been very hard to split out the code needed for this without those. I think the best way to do this is to just define a minimal list of types and macros that need to be pre-defined before one of these headers can be included, and try to not make any future additions to that list unless really necessary.)
Thanks Julius for the inputs. That makes sense. We already have a precedence for sharing headers between the cross compiler and the host toolchain for the host tools via `tools_share` directory. Similarly we can have a `xxx_share` directory in /include for these headers. I think it is worth factoring out the U() macro to this folder. Regarding the __ASSEMBLY__ and AARCH32/AARCH64 macros, we could split the header out for asm, aarch32, aarch64 variants of the file thus removing the need to define these macros.
Allowing these headers to include each other could be done as a convenience, but only if using "../../double/quote/relative/path.h" notation. Using <abosolute/path/notation.h> like normal TF headers wouldn't work because then the other project would have to add the Trusted Firmware toplevel include/ directory to their own include path, which they can't because it may cause header clashes with their own headers, or at least it would depend on include order and create a big risk of accidentally including the wrong thing and be overall far too messy. In my proposal, since some headers (like <stdint.h>) always need to be pre-included by the wrapper anyway, I opted to just pre-include everything to keep things consistent.
OK , so we can say that these headers can include others such "raw" headers as long as they stick to the relative path. This also provides another reason to group all such "raw" headers together in a folder as it becomes easier to include and export them.
I'm open to putting them in a separate folder if we prefer that. My current proposal just sets them apart by having them all use the "_raw.h" suffix, but we could also make it a prefix instead or create a raw/ subfolder to any folder that has such headers or anything like that. I don't think it makes a big difference either way.
I think these raw header are worth grouping together as it makes it clear the need to handle them separately and makes it easier to export these headers. A strawman proposal for the header hierarchy :
/include/tf_share --> the ep_info, param_header and other such common header could go in there /include/tf_share/ bl_aux_params --> folder for the new bl_aux param header /include/tf_share/bl_aux_params/vendor/rockchip --> folder for vendor extensions for the rockchip platform
Does that sound sensible ?
Best Regards Soby Mathew