Hi Andrew,
I have submitted the change as you have passed it through the ML as a base for the discussion. https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/11002
The issue is acknowledged, we had a brief discussion internally as to how best to refactor if need be.
It looks to us the main problem is that SPM-MM (pre-dating FF-A) has aged a bit and mixes standard and impdef func ids.
e.g. MM is an Arm standard and only defines two func ids (0x84000040, 0x84000041) so it may just be a matter of updating SPM_MM_FID_MAX_VALUE to 0x41 such that MM related services go through.
The other ids 0xX4000060, 61, 64, 65 are purely impdef for the SPM-MM to/from SP communication. Thus we may define SP_MM_FID_MIN_VALUE/SP_MM_FID_MAX_VALUE and a corresponding is_sp_mm_fid macro. This would avoid the clash with TRNG IDs (0xX4000050, 51, 52, 53).
What do you reckon?
Btw out of curiosity how did you discover this? Do you have a setup enabling both SPM_MM and TRNG_SUPPORT option? Or maybe this is because of Trusty SPD reuse of spm_mm_smc_handler?
Regards, Olivier.
________________________________________ From: TF-A tf-a-bounces@lists.trustedfirmware.org on behalf of Andrew Scull via TF-A tf-a@lists.trustedfirmware.org Sent: 04 August 2021 22:50 To: Manish Pandey2 Cc: tf-a@lists.trustedfirmware.org; Jimmy Brisson; Andre Przywara Subject: Re: [TF-A] TRNG SMCs intercepted by SPM-MM
I'm seeing server errors when I try "Generate Password" or setting the ssh key so I'm not sure how to push and authenticate. I've sent the patch directly to you, Manish, so the formatting doesn't get messed up and I don't know how to make git-send-email add it to a thread nicely..
On Wed, 4 Aug 2021 at 05:51, Manish Pandey2 <Manish.Pandey2@arm.commailto:Manish.Pandey2@arm.com> wrote: Hi Andrew,
Thanks for reporting the bug, "DEN0060A_ARM_MM_Interface_Specification.pdf" does not talk about range for SPM_MM but don't know how it's mentioned in the comments.
Will you be able to push a patch following instructions at https://trustedfirmware-a.readthedocs.io/en/latest/process/contributing.html... 5. Contributor’s Guide — Trusted Firmware-A documentationhttps://trustedfirmware-a.readthedocs.io/en/latest/process/contributing.html#submitting-changes 5. Contributor’s Guide¶ 5.1. Getting Started¶. Make sure you have a Github account and you are logged on both developer.trustedfirmware.orghttp://developer.trustedfirmware.org and review.trustedfirmware.orghttp://review.trustedfirmware.org. If you plan to contribute a major piece of work, it is usually a good idea to start a discussion around it on the mailing list. trustedfirmware-a.readthedocs.iohttp://trustedfirmware-a.readthedocs.io Repository: https://review.trustedfirmware.org/admin/repos/TF-A/trusted-firmware-a , you will be able to login to gerrit using github credentials. TF-A/trusted-firmware-a · Gerrit Code Reviewhttps://review.trustedfirmware.org/admin/repos/TF-A/trusted-firmware-a Gerrit Code Review review.trustedfirmware.orghttp://review.trustedfirmware.org
If not, then could you please send me the patch file (it appears copying directly from email generates corrupt patch file)
Thanks Manish
________________________________ From: TF-A <tf-a-bounces@lists.trustedfirmware.orgmailto:tf-a-bounces@lists.trustedfirmware.org> on behalf of Andrew Scull via TF-A <tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org> Sent: 03 August 2021 22:32 To: tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org <tf-a@lists.trustedfirmware.orgmailto:tf-a@lists.trustedfirmware.org> Cc: Andre Przywara <Andre.Przywara@arm.commailto:Andre.Przywara@arm.com>; Jimmy Brisson <Jimmy.Brisson@arm.commailto:Jimmy.Brisson@arm.com> Subject: [TF-A] TRNG SMCs intercepted by SPM-MM
I've failed to figure out how to upload a CL so I'm resorting to this, it's more of a bug report anyway. There seems to be a conflict in how the standard SMCs are claimed with the TRNG SMCs claimed by SPM-MM before TRNG would get a chance to handle them properly.
The patch below might fix the issue but I've not tested it or even built against ToT.
----
The TRNG SMCs use 0x84000050 to 0x84000053 which is in the range that SPM-MM claims for itself. Resolve this conflict by making SMC-MM much more selective about the SMCs it claims for itself.
Signed-off-by: Andrew Scull <ascull@google.commailto:ascull@google.com> Change-Id: If86b0d6a22497d34315c61fe72645b642c6e35f3 --- include/services/spm_mm_svc.h | 12 ++---------- services/std_svc/spm_mm/spm_mm_main.c | 12 ++++++++++++ 2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/include/services/spm_mm_svc.h b/include/services/spm_mm_svc.h index 3148beb80..4247c95a1 100644 --- a/include/services/spm_mm_svc.h +++ b/include/services/spm_mm_svc.h @@ -38,17 +38,8 @@ #define SPM_MM_VERSION_COMPILED SPM_MM_VERSION_FORM(SPM_MM_VERSION_MAJOR, \ SPM_MM_VERSION_MINOR)
-/* These macros are used to identify SPM-MM calls using the SMC function ID */ -#define SPM_MM_FID_MASK U(0xffff) -#define SPM_MM_FID_MIN_VALUE U(0x40) -#define SPM_MM_FID_MAX_VALUE U(0x7f) -#define is_spm_mm_fid(_fid) \ - ((((_fid) & SPM_MM_FID_MASK) >= SPM_MM_FID_MIN_VALUE) && \ - (((_fid) & SPM_MM_FID_MASK) <= SPM_MM_FID_MAX_VALUE)) - /* * SMC IDs defined in [1] for accessing MM services from the Non-secure world. - * These FIDs occupy the range 0x40 - 0x5f. * [1] DEN0060A_ARM_MM_Interface_Specification.pdf */ #define MM_VERSION_AARCH32 U(0x84000040) @@ -59,7 +50,6 @@ * SMC IDs defined for accessing services implemented by the Secure Partition * Manager from the Secure Partition(s). These services enable a partition to * handle delegated events and request privileged operations from the manager. - * They occupy the range 0x60-0x7f. */ #define SPM_MM_VERSION_AARCH32 U(0x84000060) #define MM_SP_EVENT_COMPLETE_AARCH64 U(0xC4000061) @@ -94,6 +84,8 @@
int32_t spm_mm_setup(void);
+bool is_spm_mm_fid(uint32_t smc_fid); + uint64_t spm_mm_smc_handler(uint32_t smc_fid, uint64_t x1, uint64_t x2, diff --git a/services/std_svc/spm_mm/spm_mm_main.c b/services/std_svc/spm_mm/spm_mm_main.c index 14c0038ba..07226b0fb 100644 --- a/services/std_svc/spm_mm/spm_mm_main.c +++ b/services/std_svc/spm_mm/spm_mm_main.c @@ -266,6 +266,18 @@ static uint64_t mm_communicate(uint32_t smc_fid, uint64_t mm_cookie, SMC_RET1(handle, rc); }
+/* Predicate indicating that a function id is part of SPM-MM */ +bool is_spm_mm_fid(uint32_t smc_fid) +{ + return ((smc_fid == MM_VERSION_AARCH32) || + (smc_fid == MM_COMMUNICATE_AARCH32) || + (smc_fid == MM_COMMUNICATE_AARCH64) || + (smc_fid == SPM_MM_VERSION_AARCH32) || + (smc_fid == MM_SP_EVENT_COMPLETE_AARCH64) || + (smc_fid == MM_SP_MEMORY_ATTRIBUTES_GET_AARCH64) || + (smc_fid == MM_SP_MEMORY_ATTRIBUTES_SET_AARCH64)); +} + /******************************************************************************* * Secure Partition Manager SMC handler. ******************************************************************************/ -- 2.32.0.554.ge1b32706d8-goog -- TF-A mailing list TF-A@lists.trustedfirmware.orgmailto:TF-A@lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a
On Tue, 10 Aug 2021 at 11:51, Olivier Deprez Olivier.Deprez@arm.com wrote:
Hi Andrew,
I have submitted the change as you have passed it through the ML as a base for the discussion. https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/11002
Thanks, it sounds like you have better ideas for addressing this more neatly which is great!
The issue is acknowledged, we had a brief discussion internally as to how best to refactor if need be.
It looks to us the main problem is that SPM-MM (pre-dating FF-A) has aged a bit and mixes standard and impdef func ids.
e.g. MM is an Arm standard and only defines two func ids (0x84000040, 0x84000041) so it may just be a matter of updating SPM_MM_FID_MAX_VALUE to 0x41 such that MM related services go through.
The other ids 0xX4000060, 61, 64, 65 are purely impdef for the SPM-MM to/from SP communication. Thus we may define SP_MM_FID_MIN_VALUE/SP_MM_FID_MAX_VALUE and a corresponding is_sp_mm_fid macro. This would avoid the clash with TRNG IDs (0xX4000050, 51, 52, 53).
What do you reckon?
Yep, that makes sense to me.
Btw out of curiosity how did you discover this? Do you have a setup enabling both SPM_MM and TRNG_SUPPORT option? Or maybe this is because of Trusty SPD reuse of spm_mm_smc_handler?
For pKVM, we rely on FF-A memory sharing with secure world and TRNG. This setup was based on the Trusty fork [1], rather than enabling SPM_MM, and when I then tried to enable TRNG_SUPPORT it wasn't getting through because of this issue.
[1] -- https://android.googlesource.com/trusty/external/trusted-firmware-a/+/refs/h...
Hi Andrew,
I see. Please have a look at the follow up discussion through gerrit [1]
[1] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/11002/comment...
Regards, Olivier.
________________________________________ From: Andrew Scull ascull@google.com Sent: 10 August 2021 22:21 To: Olivier Deprez Cc: tf-a@lists.trustedfirmware.org; Jimmy Brisson; Andre Przywara; Manish Pandey2 Subject: Re: [TF-A] TRNG SMCs intercepted by SPM-MM
On Tue, 10 Aug 2021 at 11:51, Olivier Deprez Olivier.Deprez@arm.com wrote:
Hi Andrew,
I have submitted the change as you have passed it through the ML as a base for the discussion. https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/11002
Thanks, it sounds like you have better ideas for addressing this more neatly which is great!
The issue is acknowledged, we had a brief discussion internally as to how best to refactor if need be.
It looks to us the main problem is that SPM-MM (pre-dating FF-A) has aged a bit and mixes standard and impdef func ids.
e.g. MM is an Arm standard and only defines two func ids (0x84000040, 0x84000041) so it may just be a matter of updating SPM_MM_FID_MAX_VALUE to 0x41 such that MM related services go through.
The other ids 0xX4000060, 61, 64, 65 are purely impdef for the SPM-MM to/from SP communication. Thus we may define SP_MM_FID_MIN_VALUE/SP_MM_FID_MAX_VALUE and a corresponding is_sp_mm_fid macro. This would avoid the clash with TRNG IDs (0xX4000050, 51, 52, 53).
What do you reckon?
Yep, that makes sense to me.
Btw out of curiosity how did you discover this? Do you have a setup enabling both SPM_MM and TRNG_SUPPORT option? Or maybe this is because of Trusty SPD reuse of spm_mm_smc_handler?
For pKVM, we rely on FF-A memory sharing with secure world and TRNG. This setup was based on the Trusty fork [1], rather than enabling SPM_MM, and when I then tried to enable TRNG_SUPPORT it wasn't getting through because of this issue.
[1] -- https://android.googlesource.com/trusty/external/trusted-firmware-a/+/refs/h...
tf-a@lists.trustedfirmware.org