public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Tuan Phan" <tphan@ventanamicro.com>
To: Sami Mujawar <sami.mujawar@arm.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "ardb+tianocore@kernel.org" <ardb+tianocore@kernel.org>,
	"ray.ni@intel.com" <ray.ni@intel.com>,
	"huangming@linux.alibaba.com" <huangming@linux.alibaba.com>,
	"sunilvl@ventanamicro.com" <sunilvl@ventanamicro.com>,
	"yong.li@intel.com" <yong.li@intel.com>,
	"yeoreum.yun@arm.com" <yeoreum.yun@arm.com>,
	"nd@arm.com" <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH v3 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver
Date: Wed, 18 Oct 2023 21:25:13 +0000	[thread overview]
Message-ID: <PH7PR10MB63358508AB8FC50C1C4785EAA7D5A@PH7PR10MB6335.namprd10.prod.outlook.com> (raw)
In-Reply-To: <8d8857ee-5a0e-6a1d-3ede-564e3c49092c@arm.com>

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

Hi Sami,
Not sure if you can merge this series or let me know who can do it.

Thanks,

From: Sami Mujawar <sami.mujawar@arm.com>
Date: Thursday, October 5, 2023 at 4:12 AM
To: Tuan Phan <tphan@ventanamicro.com>, devel@edk2.groups.io <devel@edk2.groups.io>
Cc: ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>, ray.ni@intel.com <ray.ni@intel.com>, huangming@linux.alibaba.com <huangming@linux.alibaba.com>, sunilvl@ventanamicro.com <sunilvl@ventanamicro.com>, yong.li@intel.com <yong.li@intel.com>, yeoreum.yun@arm.com <yeoreum.yun@arm.com>, nd@arm.com <nd@arm.com>
Subject: Re: [PATCH v3 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver

Hi Tuan,

Thank you for this patch.

These changes look good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 28/09/2023 10:14 pm, Tuan Phan wrote:
> Update entry point library for Arm to use the new architecture independent
> StandaloneMmCpu driver.
>
> Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
> Reviewed-by: levi.yun <yeoreum.yun@arm.com>
> ---
>   .../Library/Arm/StandaloneMmCoreEntryPoint.h  | 17 +----
>   .../Arm/CreateHobList.c                       | 43 ++++++------
>   .../Arm/StandaloneMmCoreEntryPoint.c          | 69 +++++++++++--------
>   .../StandaloneMmCoreEntryPoint.inf            |  2 +-
>   4 files changed, 67 insertions(+), 64 deletions(-)
>
> diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> index 41bf0f132b4f..dbb81610ff8e 100644
> --- a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> +++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
> @@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   #ifndef __STANDALONEMMCORE_ENTRY_POINT_H__
>
>   #define __STANDALONEMMCORE_ENTRY_POINT_H__
>
>
>
> +#include <StandaloneMmCpu.h>
>
>   #include <Library/PeCoffLib.h>
>
>   #include <Library/FvLib.h>
>
>
>
> @@ -47,18 +48,6 @@ typedef struct {
>     EFI_SECURE_PARTITION_CPU_INFO    *CpuInfo;
>
>   } EFI_SECURE_PARTITION_BOOT_INFO;
>
>
>
> -typedef
>
> -EFI_STATUS
>
> -(*PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT) (
>
> -  IN UINTN  EventId,
>
> -  IN UINTN  CpuNumber,
>
> -  IN UINTN  NsCommBufferAddr
>
> -  );
>
> -
>
> -typedef struct {
>
> -  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT    *ArmTfCpuDriverEpPtr;
>
> -} ARM_TF_CPU_DRIVER_EP_DESCRIPTOR;
>
> -
>
>   typedef RETURN_STATUS (*REGION_PERMISSION_UPDATE_FUNC) (
>
>     IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
>
>     IN  UINT64                Length
>
> @@ -145,8 +134,8 @@ LocateStandaloneMmCorePeCoffData (
>   VOID *
>
>   EFIAPI
>
>   CreateHobListFromBootInfo (
>
> -  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  *CpuDriverEntryPoint,
>
> -  IN       EFI_SECURE_PARTITION_BOOT_INFO      *PayloadBootInfo
>
> +  IN  OUT  PI_MM_CPU_DRIVER_ENTRYPOINT     *CpuDriverEntryPoint,
>
> +  IN       EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo
>
>     );
>
>
>
>   /**
>
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
> index 2ac2d354f06a..80ed532352af 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
> @@ -13,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   #include <Guid/MmramMemoryReserve.h>
>
>   #include <Guid/MpInformation.h>
>
>
>
> +#include <StandaloneMmCpu.h>
>
>   #include <Library/Arm/StandaloneMmCoreEntryPoint.h>
>
>   #include <Library/ArmMmuLib.h>
>
>   #include <Library/ArmSvcLib.h>
>
> @@ -39,7 +40,7 @@ extern EFI_GUID  gEfiStandaloneMmNonSecureBufferGuid;
>   // GUID to identify HOB where the entry point of the CPU driver will be
>
>   // populated to allow this entry point driver to invoke it upon receipt of an
>
>   // event
>
> -extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;
>
> +extern EFI_GUID  gEfiMmCpuDriverEpDescriptorGuid;
>
>
>
>   /**
>
>     Use the boot information passed by privileged firmware to populate a HOB list
>
> @@ -52,22 +53,22 @@ extern EFI_GUID  gEfiArmTfCpuDriverEpDescriptorGuid;
>   **/
>
>   VOID *
>
>   CreateHobListFromBootInfo (
>
> -  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  *CpuDriverEntryPoint,
>
> -  IN       EFI_SECURE_PARTITION_BOOT_INFO      *PayloadBootInfo
>
> +  IN  OUT  PI_MM_CPU_DRIVER_ENTRYPOINT     *CpuDriverEntryPoint,
>
> +  IN       EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo
>
>     )
>
>   {
>
> -  EFI_HOB_HANDOFF_INFO_TABLE       *HobStart;
>
> -  EFI_RESOURCE_ATTRIBUTE_TYPE      Attributes;
>
> -  UINT32                           Index;
>
> -  UINT32                           BufferSize;
>
> -  UINT32                           Flags;
>
> -  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK   *MmramRangesHob;
>
> -  EFI_MMRAM_DESCRIPTOR             *MmramRanges;
>
> -  EFI_MMRAM_DESCRIPTOR             *NsCommBufMmramRange;
>
> -  MP_INFORMATION_HOB_DATA          *MpInformationHobData;
>
> -  EFI_PROCESSOR_INFORMATION        *ProcInfoBuffer;
>
> -  EFI_SECURE_PARTITION_CPU_INFO    *CpuInfo;
>
> -  ARM_TF_CPU_DRIVER_EP_DESCRIPTOR  *CpuDriverEntryPointDesc;
>
> +  EFI_HOB_HANDOFF_INFO_TABLE      *HobStart;
>
> +  EFI_RESOURCE_ATTRIBUTE_TYPE     Attributes;
>
> +  UINT32                          Index;
>
> +  UINT32                          BufferSize;
>
> +  UINT32                          Flags;
>
> +  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK  *MmramRangesHob;
>
> +  EFI_MMRAM_DESCRIPTOR            *MmramRanges;
>
> +  EFI_MMRAM_DESCRIPTOR            *NsCommBufMmramRange;
>
> +  MP_INFORMATION_HOB_DATA         *MpInformationHobData;
>
> +  EFI_PROCESSOR_INFORMATION       *ProcInfoBuffer;
>
> +  EFI_SECURE_PARTITION_CPU_INFO   *CpuInfo;
>
> +  MM_CPU_DRIVER_EP_DESCRIPTOR     *CpuDriverEntryPointDesc;
>
>
>
>     // Create a hoblist with a PHIT and EOH
>
>     HobStart = HobConstructor (
>
> @@ -144,13 +145,13 @@ CreateHobListFromBootInfo (
>
>
>     // Create a Guided HOB to enable the ARM TF CPU driver to share its entry
>
>     // point and populate it with the address of the shared buffer
>
> -  CpuDriverEntryPointDesc = (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR *)BuildGuidHob (
>
> -                                                                 &gEfiArmTfCpuDriverEpDescriptorGuid,
>
> -                                                                 sizeof (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR)
>
> -                                                                 );
>
> +  CpuDriverEntryPointDesc = (MM_CPU_DRIVER_EP_DESCRIPTOR *)BuildGuidHob (
>
> +                                                             &gEfiMmCpuDriverEpDescriptorGuid,
>
> +                                                             sizeof (MM_CPU_DRIVER_EP_DESCRIPTOR)
>
> +                                                             );
>
>
>
> -  *CpuDriverEntryPoint                         = NULL;
>
> -  CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr = CpuDriverEntryPoint;
>
> +  *CpuDriverEntryPoint                      = NULL;
>
> +  CpuDriverEntryPointDesc->MmCpuDriverEpPtr = CpuDriverEntryPoint;
>
>
>
>     // Find the size of the GUIDed HOB with SRAM ranges
>
>     BufferSize  = sizeof (EFI_MMRAM_HOB_DESCRIPTOR_BLOCK);
>
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
> index 96de10405af8..70306be2495e 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
> @@ -15,6 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   #include <Guid/MmramMemoryReserve.h>
>
>   #include <Guid/MpInformation.h>
>
>
>
> +#include <StandaloneMmCpu.h>
>
>   #include <Library/ArmSvcLib.h>
>
>   #include <Library/DebugLib.h>
>
>   #include <Library/HobLib.h>
>
> @@ -41,7 +42,7 @@ STATIC CONST UINT32  mSpmMinorVerFfa = SPM_MINOR_VERSION_FFA;
>
>
>   #define BOOT_PAYLOAD_VERSION  1
>
>
>
> -PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;
>
> +PI_MM_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;
>
>
>
>   /**
>
>     Retrieve a pointer to and print the boot information passed by privileged
>
> @@ -140,34 +141,46 @@ DelegatedEventLoop (
>       DEBUG ((DEBUG_INFO, "X6 :  0x%x\n", (UINT32)EventCompleteSvcArgs->Arg6));
>
>       DEBUG ((DEBUG_INFO, "X7 :  0x%x\n", (UINT32)EventCompleteSvcArgs->Arg7));
>
>
>
> -    FfaEnabled = FeaturePcdGet (PcdFfaEnable);
>
> -    if (FfaEnabled) {
>
> -      Status = CpuDriverEntryPoint (
>
> -                 EventCompleteSvcArgs->Arg0,
>
> -                 EventCompleteSvcArgs->Arg6,
>
> -                 EventCompleteSvcArgs->Arg3
>
> -                 );
>
> -      if (EFI_ERROR (Status)) {
>
> -        DEBUG ((
>
> -          DEBUG_ERROR,
>
> -          "Failed delegated event 0x%x, Status 0x%x\n",
>
> -          EventCompleteSvcArgs->Arg3,
>
> -          Status
>
> -          ));
>
> -      }
>
> +    //
>
> +    // ARM TF passes SMC FID of the MM_COMMUNICATE interface as the Event ID upon
>
> +    // receipt of a synchronous MM request. Use the Event ID to distinguish
>
> +    // between synchronous and asynchronous events.
>
> +    //
>
> +    if ((ARM_SMC_ID_MM_COMMUNICATE != (UINT32)EventCompleteSvcArgs->Arg0) &&
>
> +        (ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ != (UINT32)EventCompleteSvcArgs->Arg0))
>
> +    {
>
> +      DEBUG ((DEBUG_ERROR, "UnRecognized Event - 0x%x\n", (UINT32)EventCompleteSvcArgs->Arg0));
>
> +      Status = EFI_INVALID_PARAMETER;
>
>       } else {
>
> -      Status = CpuDriverEntryPoint (
>
> -                 EventCompleteSvcArgs->Arg0,
>
> -                 EventCompleteSvcArgs->Arg3,
>
> -                 EventCompleteSvcArgs->Arg1
>
> -                 );
>
> -      if (EFI_ERROR (Status)) {
>
> -        DEBUG ((
>
> -          DEBUG_ERROR,
>
> -          "Failed delegated event 0x%x, Status 0x%x\n",
>
> -          EventCompleteSvcArgs->Arg0,
>
> -          Status
>
> -          ));
>
> +      FfaEnabled = FeaturePcdGet (PcdFfaEnable);
>
> +      if (FfaEnabled) {
>
> +        Status = CpuDriverEntryPoint (
>
> +                   EventCompleteSvcArgs->Arg0,
>
> +                   EventCompleteSvcArgs->Arg6,
>
> +                   EventCompleteSvcArgs->Arg3
>
> +                   );
>
> +        if (EFI_ERROR (Status)) {
>
> +          DEBUG ((
>
> +            DEBUG_ERROR,
>
> +            "Failed delegated event 0x%x, Status 0x%x\n",
>
> +            EventCompleteSvcArgs->Arg3,
>
> +            Status
>
> +            ));
>
> +        }
>
> +      } else {
>
> +        Status = CpuDriverEntryPoint (
>
> +                   EventCompleteSvcArgs->Arg0,
>
> +                   EventCompleteSvcArgs->Arg3,
>
> +                   EventCompleteSvcArgs->Arg1
>
> +                   );
>
> +        if (EFI_ERROR (Status)) {
>
> +          DEBUG ((
>
> +            DEBUG_ERROR,
>
> +            "Failed delegated event 0x%x, Status 0x%x\n",
>
> +            EventCompleteSvcArgs->Arg0,
>
> +            Status
>
> +            ));
>
> +        }
>
>         }
>
>       }
>
>
>
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> index 75cfb98c0e75..d41d7630b614 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> @@ -49,7 +49,7 @@
>     gMpInformationHobGuid
>
>     gEfiMmPeiMmramMemoryReserveGuid
>
>     gEfiStandaloneMmNonSecureBufferGuid
>
> -  gEfiArmTfCpuDriverEpDescriptorGuid
>
> +  gEfiMmCpuDriverEpDescriptorGuid
>
>
>
>   [FeaturePcd.ARM, FeaturePcd.AARCH64]
>
>     gArmTokenSpaceGuid.PcdFfaEnable
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109769): https://edk2.groups.io/g/devel/message/109769
Mute This Topic: https://groups.io/mt/101646679/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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

  reply	other threads:[~2023-10-18 21:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-28 21:14 [edk2-devel] [PATCH v3 0/2] Make StandaloneMmCpu architecture independent Tuan Phan
2023-09-28 21:14 ` [edk2-devel] [PATCH v3 1/2] StandaloneMmPkg: Make StandaloneMmCpu driver " Tuan Phan
2023-09-28 21:14 ` [edk2-devel] [PATCH v3 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver Tuan Phan
2023-10-05 11:11   ` Sami Mujawar
2023-10-18 21:25     ` Tuan Phan [this message]
2024-02-27 12:58 ` [edk2-devel] [PATCH v3 0/2] Make StandaloneMmCpu architecture independent Sami Mujawar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=PH7PR10MB63358508AB8FC50C1C4785EAA7D5A@PH7PR10MB6335.namprd10.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox