Hi Sami, Not sure if you can merge this series or let me know who can do it. Thanks, From: Sami Mujawar Date: Thursday, October 5, 2023 at 4:12 AM To: Tuan Phan , devel@edk2.groups.io Cc: ardb+tianocore@kernel.org , ray.ni@intel.com , huangming@linux.alibaba.com , sunilvl@ventanamicro.com , yong.li@intel.com , yeoreum.yun@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 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 > Reviewed-by: levi.yun > --- > .../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 > > #include > > #include > > > > @@ -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 > > #include > > > > +#include > > #include > > #include > > #include > > @@ -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 > > #include > > > > +#include > > #include > > #include > > #include > > @@ -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] -=-=-=-=-=-=-=-=-=-=-=-