search

OP-TEE / manifest

Manifests to use OP-TEE on various platforms
69 stars 176 forks source link

fvp: add Trusted Services support #215

Closed balint-dobszay-arm closed 2 years ago

balint-dobszay-arm commented 2 years ago

Add new manifest file derived from the existing fvp.xml which includes the Trusted Services project and its dependencies.

Related PR: https://github.com/OP-TEE/build/pull/571

jenswi-linaro commented 2 years ago

What is the purpose with this? How will it be maintained? Can't all this be put in the normal fvp.xml instead?

balint-dobszay-arm commented 2 years ago

What is the purpose with this? How will it be maintained? Can't all this be put in the normal fvp.xml instead?

My goal was just to not clutter up the normal fvp.xml with Trusted Services specific stuff, since not all people who use OP-TEE on FVP need these extra repos. But I'm happy to move it into fvp.xml if that's preferred.

jbech-linaro commented 2 years ago

I think inherit and include this is fine as such, but could you please restructure it a bit so it follow what is stated here? https://optee.readthedocs.io/en/latest/building/gits/manifest.html#sections

Just as Jens, I'm interested in how it will be maintained

gyuri-szing commented 2 years ago

What is the purpose with this?

Good question, something we might have forgotten to ask ourselves. Since our team is responsible to develop the OP-TEE SPMC and PSA SPs, we started using the OP-TEE SW integration methodology, which is to use this repo. What we try to do here is to eliminate our fork of this repository and up-stream what we have. This has two main components:

Taking a step back, what we try to do here is to make the PSA SPs a first class citizen of the OP-TEE ecosystem. That is inevitable as some stakeholders are interested, and perhaps more will be in the future. If the best way for this is the OP-TEE SW integration method and thus this change as-is, is questionable. Going for the full solution (including TS integration) would have the following benefits:

How will it be maintained?

We are using the op-tee SW integration method for daily development of PSA SPs, and thus we have first class interest in keeping this in a good shape. As long as this does not change, it is natural for us to maintain the TS integration and OP-TEE SPMC related content for platforms we use. This currently means SPMC and TS related parts targeting the base FVP. If that changes and we migrate to e.g. yocto then the value of this becomes questionable. If running TS SPs in the OP-TEE CI will become a thing, then maintaining that and this integration as a dependency my remain our responsibility. If someone will add support for another platform maintaining that will probably be beyond our powers. We have no plans ready to abandon the OP-TEE integration based workflow, but mainly due to TS CI efficiency issues, we are investigating the topic. But even if we found something better for the TS CI, for daily development we may still stick with these makefiles.

Can't all this be put in the normal fvp.xml instead?

Perhaps it could, but making the FVP platform TS and SPMC specific might be bad for existing users of the platform. That might mean having some ugly conditional directives in the makefile. Adding a layer on top might be a cleaner approach and a pattern easier to be followed with other platforms in the future. This does not mean we are not open to changing the structure if you think this is needed.

gyuri-szing commented 2 years ago

Perhaps it could, but making the FVP platform TS and SPMC specific might be bad for existing users of the platform. That might mean having some ugly conditional directives in the makefile. Adding a layer on top might be a cleaner approach and a pattern easier to be followed with other platforms in the future. This does not mean we are not open to changing the structure if you think this is needed.

Apologies, I made a mistake here! This is a manifest change and I am talking about makefiles. Please ignore that. The point I try to make is still valid. SPMC and TS integration is a platform config option, and making a platform wide change is not the right option.

balint-dobszay-arm commented 2 years ago

I think inherit and include this is fine as such, but could you please restructure it a bit so it follow what is stated here? https://optee.readthedocs.io/en/latest/building/gits/manifest.html#sections

I refactored the file, now it should be in sync with these requirements.

jbech-linaro commented 2 years ago

Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

balint-dobszay-arm commented 2 years ago

Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

Thanks, applied tag and rebased on master.

jenswi-linaro commented 2 years ago

So I guess this is something "temporary" until we know more about how things will play out, that's fine.

jenswi-linaro commented 2 years ago

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

balint-dobszay-arm commented 2 years ago

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

Thanks, applied tag and rebased on master.

balint-dobszay-arm commented 2 years ago

A review comment on the related build repo patch (https://github.com/OP-TEE/build/pull/571#discussion_r895986648) suggested to rename the Linux FF-A driver targets and paths.

I updated this patch to get in sync with the path renaming. No major change, only linux-ffa-debugfs -> linux-arm-ffa-user and linux-ffa-tee -> linux-arm-ffa-tee.

balint-dobszay-arm commented 2 years ago

@jenswi-linaro are there any pending issues with this PR? Could we get it merged? Thanks!

jenswi-linaro commented 2 years ago

@jforissier ping?

jforissier commented 2 years ago

@balint-dobszay-arm @jenswi-linaro sorry, I missed that one!