Hi Ilias,
Just minor coding style nit picking:
* Drivers/OpTee/OpteeRpmbPkg/FixupPcd.c:FixPcdMemory() I think the
error codes are missing in the function header
* Thanks for adding the IN/OUT function parameter descriptors. Is it
possible to also add them in the function headers [1] ?
About the FFA/SVC call:
>> If this is an FFA call, is it possible to:
>> - put a reference in the header to the spec (it should be similar
to the
>> one at
>> edk2/ArmPkg/Library/StandaloneMmMmuLib/AArch64/ArmMmuStandaloneMmLib.c)
>> - check the return status of the SVC call against the ones available at
>> edk2/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
>> - if possible, remove the dependency to <IndustryStandard/ArmMmSvc.h>
>
> The call is technically an FFA one but at the moment OP-TEE returns
the StMM
> return code which is defined in the last header you mention.
> The relevant code is in ./core/arch/arm/kernel/stmm_sp.c function
> tee2stmm_ret_val().
> So unless we redefine that in OP-TEE or (better imho), wait for a
full FFA
> mechanism to be in place, I'd prefer leaving it as is.
> Keep in mind that adding the full FFA will also get rid of the
hardcoded IDs
> on the beginning of the file.
The description of a FFA_MSG_SEND_DIRECT_REQ (s10.2 of [2]) doesn't seem
to return the same error codes as the ones optee returns (in
optee_os/core/arch/arm/kernel/stmm_sp.c:tee2stmm_ret_val()). I am not
sure a new FFA specification will change these error codes.
Another thing is that I think the mMemMgrId variable described in
edk2-platforms/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c is currently
defined as
edk2/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h:ARM_FFA_DESTINATION_ENDPOINT_ID
(the name seems to be misleading).
I think it would be better to:
* align the optee error codes with what is in the FFA spec
* handle these error codes in edk2 with what is in
edk2/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h and remove the
dependendy to edk2/ArmPkg/Include/IndustryStandard/ArmMmSvc.h if
possible
* rename
edk2/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h:ARM_FFA_DESTINATION_ENDPOINT_ID
define to a proper name, according to what is in
optee_os/core/arch/arm/kernel/stmm_sp.c, and add one define for
'mem_mgr_id'
* remove the mMemMgrId and mStorageId variables from
edk2-platforms/Drivers/OpTee/OpteeRpmbPkg/OpTeeRpmbFvb.c and use the
newly created defines from
edk2/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
This would allow to be aligned with the current FFA spec and when a new
one comes, these endpoints IDs (mMemMgrId/mStorageId) can just be
removed from one location (as you said).
In the end it seems you and Sami will maintain this, so I will let you
decide what is best.
[1]
https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/6_documenting_software/610_special_commands#6-10-4-param-in-out-parameter-description
[2] https://developer.arm.com/documentation/den0077/a
Regards,
Pierre
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72622): https://edk2.groups.io/g/devel/message/72622
Mute This Topic: https://groups.io/mt/81201374/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
