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

["Kun Qin" <kuqin12@gmail.com>]

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

Hi Achin,

The main MM communicate usage in PEI we have today is the variable service,
because non secure UEFI cannot access flash directly.


We had a few use cases where we need to check the variable from the previous

boot in PEI, update some configuration data and consume it for the rest 
of the
boot. I think it will break as is if we switch over to FFA aware framework?


Please let me know how you think about the potential support in PEI.

Regards,
Kun

On 7/12/2023 7:21 AM, Achin Gupta wrote:
> 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: 18830 bytes --]