Re: [edk2-devel] [edk2-platforms][PATCH V1 19/20] ArmPkg/MmCommunicationDxe: Use the FF-A transport for MM requests

[achin.gupta@arm.com]

[-- Attachment #1: Type: text/plain, Size: 9101 bytes --]

Hi Kun,

IIUC, not much happens during the PEI phase on Arm platforms. Hence the patches focus on DXE in the Normal world. It would be difficult to provide an effort estimate as we do not see a use case and consequently this is not on the to-do list. Would you be able to provide more detail about the scenarios you have in mind?

cheers,
Achin
________________________________
From: Kun Qin <kuqin12@gmail.com>
Sent: 11 July 2023 20:22
To: devel@edk2.groups.io <devel@edk2.groups.io>; Nishant Sharma <Nishant.Sharma@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar <Sami.Mujawar@arm.com>; Thomas Abraham <thomas.abraham@arm.com>; Sayanta Pattanayak <Sayanta.Pattanayak@arm.com>; Achin Gupta <Achin.Gupta@arm.com>; Aditya Angadi <Aditya.Angadi@arm.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH V1 19/20] ArmPkg/MmCommunicationDxe: Use the FF-A transport for MM requests

Hi Nishant,

Thank you for sending out the patch. Can you please evaluate how much
effort it would be
to support the same for MmCommunicatePei? I think it would provide
better coverage for
the FFA support if we can have that change.

Thanks,
Kun

On 7/11/2023 7:36 AM, Nishant Sharma wrote:
> From: Achin Gupta <achin.gupta@arm.com>
>
> This patch packages requests for accessing a Standalone MM driver
> through the MM communication protocol as FF-A direct messages.
> Corresponding changes in Standalone MM Core ensure that responses are
> packaged in the same way.
>
> Signed-off-by: Achin Gupta <achin.gupta@arm.com>
> Co-developed-by: Aditya Angadi <aditya.angadi@arm.com>
> Signed-off-by: Nishant Sharma <nishant.sharma@arm.com>
> ---
>   ArmPkg/Include/IndustryStandard/ArmFfaSvc.h         |   2 +
>   ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 141 +++++++++++++-------
>   2 files changed, 97 insertions(+), 46 deletions(-)
>
> diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
> index 530af8bd3c2e..493997346143 100644
> --- a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
> +++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
> @@ -23,6 +23,7 @@
>   #define ARM_SVC_ID_FFA_RXTX_UNMAP_AARCH32            0x84000067
>   #define ARM_SVC_ID_FFA_PARTITION_INFO_GET_AARCH32    0x84000068
>   #define ARM_SVC_ID_FFA_ID_GET_AARCH32                0x84000069
> +#define ARM_SVC_ID_FFA_RUN_AARCH32                   0x8400006D
>   #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH32   0x8400006F
>   #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH32  0x84000070
>   #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64   0xC400006F
> @@ -31,6 +32,7 @@
>   #define ARM_SVC_ID_FFA_SUCCESS_AARCH64               0xC4000061
>   #define ARM_SVC_ID_FFA_MEM_PERM_SET_AARCH32          0x84000089
>   #define ARM_SVC_ID_FFA_MEM_PERM_GET_AARCH32          0x84000088
> +#define ARM_SVC_ID_FFA_INTERRUPT_AARCH32             0x84000062
>   #define ARM_SVC_ID_FFA_ERROR_AARCH32                 0x84000060
>   #define ARM_SVC_ID_FFA_ERROR_AARCH64                 0xC4000060
>   #define ARM_SVC_ID_FFA_MSG_WAIT_AARCH32              0x8400006B
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> index 94a5d96c051d..a70318581bd2 100644
> --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> @@ -100,6 +100,7 @@ MmCommunication2Communicate (
>     ARM_SMC_ARGS               CommunicateSmcArgs;
>     EFI_STATUS                 Status;
>     UINTN                      BufferSize;
> +  UINTN                      Ret;
>
>     Status     = EFI_ACCESS_DENIED;
>     BufferSize = 0;
> @@ -160,60 +161,108 @@ MmCommunication2Communicate (
>       return Status;
>     }
>
> -  // SMC Function ID
> -  CommunicateSmcArgs.Arg0 = ARM_SMC_ID_MM_COMMUNICATE_AARCH64;
> -
> -  // Cookie
> -  CommunicateSmcArgs.Arg1 = 0;
> -
>     // Copy Communication Payload
>     CopyMem ((VOID *)mNsCommBuffMemRegion.VirtualBase, CommBufferVirtual, BufferSize);
>
> -  // comm_buffer_address (64-bit physical address)
> -  CommunicateSmcArgs.Arg2 = (UINTN)mNsCommBuffMemRegion.PhysicalBase;
> +  // Use the FF-A interface if enabled.
> +  if (FixedPcdGet32 (PcdFfaEnable) != 0) {
> +    // FF-A Interface ID for direct message communication
> +    CommunicateSmcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64;
>
> -  // comm_size_address (not used, indicated by setting to zero)
> -  CommunicateSmcArgs.Arg3 = 0;
> +    // FF-A Destination EndPoint ID, not used as of now
> +    CommunicateSmcArgs.Arg1 = mFfaPartId << 16 | mStmmPartInfo.PartId;
>
> +    // Reserved for future use(MBZ)
> +    CommunicateSmcArgs.Arg2 = 0x0;
> +
> +    // comm_buffer_address (64-bit physical address)
> +    CommunicateSmcArgs.Arg3 = (UINTN)mNsCommBuffMemRegion.PhysicalBase;
> +
> +    // Cookie
> +    CommunicateSmcArgs.Arg4 = 0x0;
> +
> +    // Not Used
> +    CommunicateSmcArgs.Arg5 = 0;
> +
> +    // comm_size_address (not used, indicated by setting to zero)
> +    CommunicateSmcArgs.Arg6 = 0;
> +  } else {
> +    // SMC Function ID
> +    CommunicateSmcArgs.Arg0 = ARM_SMC_ID_MM_COMMUNICATE_AARCH64;
> +
> +    // Cookie
> +    CommunicateSmcArgs.Arg1 = 0;
> +
> +    // comm_buffer_address (64-bit physical address)
> +    CommunicateSmcArgs.Arg2 = (UINTN)mNsCommBuffMemRegion.PhysicalBase;
> +
> +    // comm_size_address (not used, indicated by setting to zero)
> +    CommunicateSmcArgs.Arg3 = 0;
> +  }
> +
> +ffa_intr_loop:
>     // Call the Standalone MM environment.
>     ArmCallSmc (&CommunicateSmcArgs);
>
> -  switch (CommunicateSmcArgs.Arg0) {
> -    case ARM_SMC_MM_RET_SUCCESS:
> -      ZeroMem (CommBufferVirtual, BufferSize);
> -      // On successful return, the size of data being returned is inferred from
> -      // MessageLength + Header.
> -      CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER *)mNsCommBuffMemRegion.VirtualBase;
> -      BufferSize        = CommunicateHeader->MessageLength +
> -                          sizeof (CommunicateHeader->HeaderGuid) +
> -                          sizeof (CommunicateHeader->MessageLength);
> -
> -      CopyMem (
> -        CommBufferVirtual,
> -        (VOID *)mNsCommBuffMemRegion.VirtualBase,
> -        BufferSize
> -        );
> -      Status = EFI_SUCCESS;
> -      break;
> -
> -    case ARM_SMC_MM_RET_INVALID_PARAMS:
> -      Status = EFI_INVALID_PARAMETER;
> -      break;
> -
> -    case ARM_SMC_MM_RET_DENIED:
> -      Status = EFI_ACCESS_DENIED;
> -      break;
> -
> -    case ARM_SMC_MM_RET_NO_MEMORY:
> -      // Unexpected error since the CommSize was checked for zero length
> -      // prior to issuing the SMC
> -      Status = EFI_OUT_OF_RESOURCES;
> -      ASSERT (0);
> -      break;
> -
> -    default:
> -      Status = EFI_ACCESS_DENIED;
> -      ASSERT (0);
> +  Ret = CommunicateSmcArgs.Arg0;
> +
> +  if (FixedPcdGet32 (PcdFfaEnable) != 0) {
> +    if (Ret == ARM_SVC_ID_FFA_INTERRUPT_AARCH32) {
> +      DEBUG ((DEBUG_INFO, "Resuming interrupted FF-A call \n"));
> +
> +      // FF-A Interface ID for running the interrupted partition
> +      CommunicateSmcArgs.Arg0 = ARM_SVC_ID_FFA_RUN_AARCH32;
> +
> +      // FF-A Destination EndPoint and vCPU ID, TODO: We are assuming vCPU0 of the
> +      // StMM SP since it is UP.
> +      CommunicateSmcArgs.Arg1 = mStmmPartInfo.PartId << 16;
> +
> +      // Loop if the call was interrupted
> +      goto ffa_intr_loop;
> +    }
> +  }
> +
> +  if (((FixedPcdGet32 (PcdFfaEnable) != 0) &&
> +      (Ret == ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP)) ||
> +      (Ret == ARM_SMC_MM_RET_SUCCESS)) {
> +    ZeroMem (CommBufferVirtual, BufferSize);
> +    // On successful return, the size of data being returned is inferred from
> +    // MessageLength + Header.
> +    CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER *)mNsCommBuffMemRegion.VirtualBase;
> +    BufferSize = CommunicateHeader->MessageLength +
> +                 sizeof (CommunicateHeader->HeaderGuid) +
> +                 sizeof (CommunicateHeader->MessageLength);
> +
> +    CopyMem (CommBufferVirtual, (VOID *)mNsCommBuffMemRegion.VirtualBase,
> +             BufferSize);
> +    Status = EFI_SUCCESS;
> +    return Status;
> +  }
> +
> +  if (FixedPcdGet32 (PcdFfaEnable) != 0) {
> +    Ret = CommunicateSmcArgs.Arg2;
> +  }
> +
> +  // Error Codes are same for FF-A and SMC interface
> +  switch (Ret) {
> +  case ARM_SMC_MM_RET_INVALID_PARAMS:
> +    Status = EFI_INVALID_PARAMETER;
> +    break;
> +
> +  case ARM_SMC_MM_RET_DENIED:
> +    Status = EFI_ACCESS_DENIED;
> +    break;
> +
> +  case ARM_SMC_MM_RET_NO_MEMORY:
> +    // Unexpected error since the CommSize was checked for zero length
> +    // prior to issuing the SMC
> +    Status = EFI_OUT_OF_RESOURCES;
> +    ASSERT (0);
> +    break;
> +
> +  default:
> +    Status = EFI_ACCESS_DENIED;
> +    ASSERT (0);
>     }
>
>     return Status;

[-- Attachment #2: Type: text/html, Size: 15933 bytes --]