Re: [PATCH 3/6] ArmPkg/Include: Add MM interface SVC return codes.

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Jul 03, 2018 at 03:25:12PM +0530, Supreeth Venkatesh wrote:
> This patch adds the Management Mode(MM) - Secure Partition
> Manager(SPM) SVC return codes.
> 
> Also, It corrects SVC ID for retrieving SPM version information.

That sounds unrelated. And looking at it, worthy of a longer
explanation, so probably worth breaking out as a separate patch.

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Achin Gupta <achin.gupta@arm.com>
> Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Include/IndustryStandard/ArmMmSvc.h | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/ArmPkg/Include/IndustryStandard/ArmMmSvc.h b/ArmPkg/Include/IndustryStandard/ArmMmSvc.h
> index 4c7b6c3..a64b9ec 100644
> --- a/ArmPkg/Include/IndustryStandard/ArmMmSvc.h
> +++ b/ArmPkg/Include/IndustryStandard/ArmMmSvc.h
> @@ -20,7 +20,7 @@
>   * delegated events and request the Secure partition manager to perform
>   * privileged operations on its behalf.
>   */
> -#define ARM_SVC_ID_SPM_VERSION_AARCH64             0xC4000060
> +#define ARM_SVC_ID_SPM_VERSION_AARCH32             0x84000060

So, this isn't "correcting" a value - it's replacing a 64-bit call
with a 32-bit one. Which is potentially fine, but then it deserves a
separate patch and a commit message clearly describing why this is the
right thing to do.

Regards,

Leif

>  #define ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64       0xC4000061
>  #define ARM_SVC_ID_SP_GET_MEM_ATTRIBUTES_AARCH64   0xC4000064
>  #define ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64   0xC4000065
> @@ -40,4 +40,11 @@
>      ((((c_perm) & SET_MEM_ATTR_CODE_PERM_MASK) << SET_MEM_ATTR_CODE_PERM_SHIFT) | \
>      (( (d_perm) & SET_MEM_ATTR_DATA_PERM_MASK) << SET_MEM_ATTR_DATA_PERM_SHIFT))
>  
> +/* MM SVC Return error codes */
> +#define ARM_SVC_SPM_RET_SUCCESS               0
> +#define ARM_SVC_SPM_RET_NOT_SUPPORTED        -1
> +#define ARM_SVC_SPM_RET_INVALID_PARAMS       -2
> +#define ARM_SVC_SPM_RET_DENIED               -3
> +#define ARM_SVC_SPM_RET_NO_MEMORY            -5
> +
>  #endif
> -- 
> 2.7.4
>