Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tee client api errata #362

Merged
merged 3 commits into from
Dec 4, 2023

Conversation

vincent-mailhol
Copy link
Contributor

@vincent-mailhol vincent-mailhol commented Nov 1, 2023

Global Platform published the TEE Client API Specification v1.0 Errata and Precisions Version 2.0 in 2014. This document is not yet reflected in optee_client.

The document contains two errata:

  • E.1 Correct reference to TEEC_MemoryReference
  • E.2 Correct reference to TEEC_PARAMS_TYPE

and four precisions:

  • P.1 Clarify State of Operation Parameters on Return from
  • P.2 Add Normative Reference
  • P.3 Return Code When a Fatal Error Occurs in a Trusted Application
  • P.4 Define Additional Return Codes

The first patch of this series covers errata E.1, the second patch covers precision P.4. The third and last patch deprecates two existing return codes and suggest to use instead the ones introduced in precision P.4.

The remaining errata E.2 and precision P.1 to P.3 do not impact the implementation.

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Etienne Carriere <[email protected]> for commit
"tee_client_api: correct reference to TEEC_MemoryReference".

Reviewed-by: Etienne Carriere <[email protected]> for commit
"tee_client_api: define additional return codes" once comments are addressed.

@@ -186,6 +197,15 @@ extern "C" {
#define TEEC_ERROR_EXTERNAL_CANCEL 0xFFFF0011
#define TEEC_ERROR_TARGET_DEAD 0xFFFF3024
#define TEEC_ERROR_STORAGE_NO_SPACE 0xFFFF3041
#define TEE_ERROR_EXTERNAL_CANCEL 0xFFFF0011
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this line right above line 197.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference would have been to keep the order the same as it appears in Global Platform specification. But I do not want to argue on that. I applied your suggestion in cb92e7f.

#define TEE_ERROR_EXTERNAL_CANCEL 0xFFFF0011
#define TEE_ERROR_OVERFLOW 0xFFFF300F
#define TEE_ERROR_TARGET_DEAD 0xFFFF3024
#define TEEC_ERROR_TARGET_DEAD 0xFFFF3024
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move TEE_ERROR_TARGET_DEAD definition right above line 198 and remove below line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting the duplicate. Fixed in cb92e7f.

#define TEE_ERROR_OVERFLOW 0xFFFF300F
#define TEE_ERROR_TARGET_DEAD 0xFFFF3024
#define TEEC_ERROR_TARGET_DEAD 0xFFFF3024
#define TEE_ERROR_STORAGE_NO_SPACE 0xFFFF3041
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer TEE_ERROR_STORAGE_NO_SPACE definition right above TEEC_ERROR_STORAGE_NO_SPACE definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above. I applied the suggestion in cb92e7f.

Apply the errata from [1] which fixes two typos in the definition of
the paramTypes.

[1] TEE Client API Specification v1.0 Errata and Precisions
    Version 2.0, §E.1 Correct Reference to TEEC_MemoryReference

Signed-off-by: Vincent Mailhol <[email protected]>
Reviewed-by: Etienne Carriere <[email protected]>
@vincent-mailhol
Copy link
Contributor Author

@etienne-lms

Reviewed-by: Etienne Carriere <[email protected]> for commit "tee_client_api: correct reference to TEEC_MemoryReference".

Done

Reviewed-by: Etienne Carriere <[email protected]> for commit "tee_client_api: define additional return codes" once comments are addressed.

Done but I rewrote the description to explain the reordering. Thanks for your confirmation.

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last change in commit "tee_client_api: define additional return codes" and I'm fine with the series.

#define TEEC_ERROR_STORAGE_NO_SPACE 0xFFFF3041
#define TEE_ERROR_OVERFLOW 0xFFFF300F
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To meet value ordering:

 (...)
 #define TEEC_ERROR_EXTERNAL_CANCEL         0xFFFF0011
+#define TEE_ERROR_OVERFLOW                 0xFFFF300F
 #define TEE_ERROR_TARGET_DEAD              0xFFFF3024
 #define TEEC_ERROR_TARGET_DEAD             0xFFFF3024
 #define TEE_ERROR_STORAGE_NO_SPACE         0xFFFF3041
-#define TEE_ERROR_OVERFLOW                 0xFFFF300F
 #define TEE_ERROR_MAC_INVALID              0xFFFF3071
 (...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in a9cfb99. I also reorder the entry in the documentation and amended the commit description accordingly.

The precision from [1] defines 9 additional return codes. One of them,
TEEC_ERROR_TARGET_DEAD was already added in [2]. Apply the 8 other
ones and reorder them by values.

The added documentation is a verbatim copy of the description from
[1]. Overwrite the existing documentation of TEEC_ERROR_TARGET_DEAD
with the one from the specification.

[1] TEE Client API Specification v1.0 Errata and Precisions
    Version 2.0, §P.4 Define Additional Return Codes

[2] commit f2b0ed4 ("Updated related Linux Driver Refactoring")
Link: OP-TEE@f2b0ed41c8c7b3

Signed-off-by: Vincent Mailhol <[email protected]>
Reviewed-by: Etienne Carriere <[email protected]>
@vincent-mailhol
Copy link
Contributor Author

vincent-mailhol commented Nov 3, 2023

For what it is worth: I added 6f76f14 to deprecate TEE_ERROR_EXTERNAL_CANCEL and TEE_ERROR_STORAGE_NO_SPACE.

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acked-by: Etienne Carriere <[email protected]> for commit
"tee_client_api: deprecate two TEE_ERROR_* macros not in the specification".

…tion

TEE_ERROR_EXTERNAL_CANCEL and TEE_ERROR_STORAGE_NO_SPACE are not part
of the specification [1]. TEEC_ERROR_EXTERNAL_CANCEL and
TEEC_ERROR_STORAGE_NO_SPACE should be preferred instead.

Add a message in the description to deprecate these two macros, but
keep them for backward compatibility.

[1] TEE Client API Specification v1.0 Errata and Precisions
    Version 2.0, §P.4 Define Additional Return Codes

Signed-off-by: Vincent Mailhol <[email protected]>
Acked-by: Etienne Carriere <[email protected]>
Copy link

github-actions bot commented Dec 4, 2023

This pull request has been marked as a stale pull request because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this pull request will automatically be closed in 5 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Dec 4, 2023
@etienne-lms
Copy link
Contributor

I think this P-R is ready to be merged. @jenswi-linaro, @jforissier, any comment on these changes?

@jforissier
Copy link
Contributor

I think this P-R is ready to be merged. @jenswi-linaro, @jforissier, any comment on these changes?

It is. Thanks.

@jforissier jforissier merged commit a8381cf into OP-TEE:master Dec 4, 2023
1 check passed
@vincent-mailhol vincent-mailhol deleted the tee-client-api-errata branch December 4, 2023 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants