From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 73096941BE5 for ; Thu, 28 Sep 2023 18:16:19 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=kxMvIZbsG7Y/pKaz0WplfscZiLlEFFBS75LJljQyLJ8=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1695924978; v=1; b=hqEpOIx5vDq+b549PDehI8r450WbMYgG1gefEDok78/VnFdjbyZC1gZmvDgfySXuXpn8R3Xx TKz304Na4tuwUuJonM20OLUTjA6dq4k7DMUEPad0oKMUwyq2Fq7ITEoKoXEcZKO0YiQCmlsX+tj OkxY3mOEMFMEBXI40xvFUUzg= X-Received: by 127.0.0.2 with SMTP id cctrYY7687511xeq3qfgmNwi; Thu, 28 Sep 2023 11:16:18 -0700 X-Received: from mail-ed1-f51.google.com (mail-ed1-f51.google.com [209.85.208.51]) by mx.groups.io with SMTP id smtpd.web11.73.1695924977642487234 for ; Thu, 28 Sep 2023 11:16:18 -0700 X-Received: by mail-ed1-f51.google.com with SMTP id 4fb4d7f45d1cf-5230a22cfd1so16552195a12.1 for ; Thu, 28 Sep 2023 11:16:17 -0700 (PDT) X-Gm-Message-State: 5w6SxIzf4lfMcBn4b2Ymbvk1x7686176AA= X-Google-Smtp-Source: AGHT+IFg21vC53yab3P37vgnYDUBZKwdEOA01Y5mUYGMfMrFjFYEFIeC5DiAIBQyXEOgDIgyoNkFBD1jClMj9EyrCgk= X-Received: by 2002:aa7:d388:0:b0:525:76fc:f559 with SMTP id x8-20020aa7d388000000b0052576fcf559mr1738510edq.41.1695924975894; Thu, 28 Sep 2023 11:16:15 -0700 (PDT) MIME-Version: 1.0 References: <20230914231037.23950-1-tphan@ventanamicro.com> <20230914231037.23950-3-tphan@ventanamicro.com> <74654aa1-3d9f-23ba-f233-0a3941315d4d@arm.com> In-Reply-To: <74654aa1-3d9f-23ba-f233-0a3941315d4d@arm.com> From: "Tuan Phan" Date: Thu, 28 Sep 2023 11:16:04 -0700 Message-ID: Subject: Re: [edk2-devel] [PATCH v2 2/2] StandaloneMmPkg: Arm: Update to use the new StandaloneMmCpu driver To: Sami Mujawar Cc: devel@edk2.groups.io, ardb+tianocore@kernel.org, ray.ni@intel.com, huangming@linux.alibaba.com, sunilvl@ventanamicro.com, yong.li@intel.com, "nd@arm.com" , yeoreum.yun@arm.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,tphan@ventanamicro.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: multipart/alternative; boundary="000000000000c161a006066f4d1c" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=hqEpOIx5; dmarc=none; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io --000000000000c161a006066f4d1c Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Sami, Please see my comments below. On Thu, Sep 28, 2023 at 9:16=E2=80=AFAM Sami Mujawar = wrote: > Hi Tuan, > > Thank you for this patch. > > Please see my response inline marked [SAMI]. > > Regards, > > Sami Mujawar > > On 15/09/2023 12:10 am, Tuan Phan wrote: > > Update entry point library for Arm to use the new platform independent > > [SAMI] Should this be worded as architecture independent instead of > platform independent? > > Can you also check the subject line and commit message for patch 1/2, > please? > [Tuan] Sure, that makes sense. > > [/SAMI] > > > StandaloneMmCpu driver. > > > > Signed-off-by: Tuan Phan > > --- > > .../Library/Arm/StandaloneMmCoreEntryPoint.h | 17 ++------ > > .../Arm/CreateHobList.c | 43 ++++++++++--------= - > > .../Arm/StandaloneMmCoreEntryPoint.c | 15 ++++++- > > .../StandaloneMmCoreEntryPoint.inf | 2 +- > > 4 files changed, 40 insertions(+), 37 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 =3D 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 =3D (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR > *)BuildGuidHob ( > > > > - > &gEfiArmTfCpuDriverEpDescriptorGuid, > > > > - sizeo= f > (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR) > > > > - ); > > > > + CpuDriverEntryPointDesc =3D (MM_CPU_DRIVER_EP_DESCRIPTOR *)BuildGuid= Hob > ( > > > > + > &gEfiMmCpuDriverEpDescriptorGuid, > > > > + sizeof > (MM_CPU_DRIVER_EP_DESCRIPTOR) > > > > + ); > > > > > > > > - *CpuDriverEntryPoint =3D NULL; > > > > - CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr =3D CpuDriverEntryPoint= ; > > > > + *CpuDriverEntryPoint =3D NULL; > > > > + CpuDriverEntryPointDesc->MmCpuDriverEpPtr =3D CpuDriverEntryPoint; > > > > > > > > // Find the size of the GUIDed HOB with SRAM ranges > > > > BufferSize =3D sizeof (EFI_MMRAM_HOB_DESCRIPTOR_BLOCK); > > > > diff --git > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCore= EntryPoint.c > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCore= EntryPoint.c > > index 96de10405af8..900e0252ef9f 100644 > > --- > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCore= EntryPoint.c > > +++ > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCore= EntryPoint.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 =3D > SPM_MINOR_VERSION_FFA; > > > > > > #define BOOT_PAYLOAD_VERSION 1 > > > > > > > > -PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT CpuDriverEntryPoint =3D NULL; > > > > +PI_MM_CPU_DRIVER_ENTRYPOINT CpuDriverEntryPoint =3D NULL; > > > > > > > > /** > > > > Retrieve a pointer to and print the boot information passed by > privileged > > > > @@ -140,6 +141,18 @@ DelegatedEventLoop ( > > DEBUG ((DEBUG_INFO, "X6 : 0x%x\n", > (UINT32)EventCompleteSvcArgs->Arg6)); > > > > DEBUG ((DEBUG_INFO, "X7 : 0x%x\n", > (UINT32)EventCompleteSvcArgs->Arg7)); > > > > > > > > + // > > > > + // 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 !=3D > (UINT32)EventCompleteSvcArgs->Arg0) && > > > > + (ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ !=3D > (UINT32)EventCompleteSvcArgs->Arg0)) > > > > + { > > > > + DEBUG ((DEBUG_ERROR, "UnRecognized Event - 0x%x\n", > (UINT32)EventCompleteSvcArgs->Arg0)); > > > > + continue; > > [SAMI] I think an error needs to be returned instead of continuing > otherwise this changes the original behaviour. > > Status needs to be set to EFI_INVALID_PARAMETER here. > > [/SAMI] > > > > + } > > [SAMI] The code from this point needs to be enclosed in an else block > until before the switch statement. > > That way the proper error code would be returned. Can you check, please? > > [/SAMI] > [Tuan] Thanks for the catch. Will fix it > > > > > + > > > > FfaEnabled =3D FeaturePcdGet (PcdFfaEnable); > > > > if (FfaEnabled) { > > > > Status =3D CpuDriverEntryPoint ( > > > > diff --git > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntr= yPoint.inf > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntr= yPoint.inf > > index 75cfb98c0e75..d41d7630b614 100644 > > --- > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntr= yPoint.inf > > +++ > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntr= yPoint.inf > > @@ -49,7 +49,7 @@ > > gMpInformationHobGuid > > > > gEfiMmPeiMmramMemoryReserveGuid > > > > gEfiStandaloneMmNonSecureBufferGuid > > > > - gEfiArmTfCpuDriverEpDescriptorGuid > > > > + gEfiMmCpuDriverEpDescriptorGuid > > > > > > > > [FeaturePcd.ARM, FeaturePcd.AARCH64] > > > > gArmTokenSpaceGuid.PcdFfaEnable > > > -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109174): https://edk2.groups.io/g/devel/message/109174 Mute This Topic: https://groups.io/mt/101369647/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --000000000000c161a006066f4d1c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Sami,
Please see my com= ments below.

On Thu, Sep 28, 2023 at 9:16=E2=80=AFAM Sami Mujawar <sami.mujawar@arm.com> wrote:
Hi Tuan,

Thank you for this patch.

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 15/09/2023 12:10 am, Tuan Phan wrote:
> Update entry point library for Arm to use the new platform independent=

[SAMI] Should this be worded as architecture independent instead of
platform independent?

Can you also check the subject line and commit message for patch 1/2,
please?
[Tuan] Sure, that makes sense.=C2=A0

[/SAMI]

> StandaloneMmCpu driver.
>
> Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
> ---
>=C2=A0 =C2=A0.../Library/Arm/StandaloneMmCoreEntryPoint.h=C2=A0 | 17 ++= ------
>=C2=A0 =C2=A0.../Arm/CreateHobList.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| 43 ++++++++++--------- >=C2=A0 =C2=A0.../Arm/StandaloneMmCoreEntryPoint.c=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 | 15 ++++++-
>=C2=A0 =C2=A0.../StandaloneMmCoreEntryPoint.inf=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 |=C2=A0 2 +-
>=C2=A0 =C2=A04 files changed, 40 insertions(+), 37 deletions(-)
>
> diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntry= Point.h b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h<= br> > 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
>=C2=A0 =C2=A0#ifndef __STANDALONEMMCORE_ENTRY_POINT_H__
>
>=C2=A0 =C2=A0#define __STANDALONEMMCORE_ENTRY_POINT_H__
>
>=C2=A0 =C2=A0
>
> +#include <StandaloneMmCpu.h>
>
>=C2=A0 =C2=A0#include <Library/PeCoffLib.h>
>
>=C2=A0 =C2=A0#include <Library/FvLib.h>
>
>=C2=A0 =C2=A0
>
> @@ -47,18 +48,6 @@ typedef struct {
>=C2=A0 =C2=A0 =C2=A0EFI_SECURE_PARTITION_CPU_INFO=C2=A0 =C2=A0 *CpuInfo= ;
>
>=C2=A0 =C2=A0} EFI_SECURE_PARTITION_BOOT_INFO;
>
>=C2=A0 =C2=A0
>
> -typedef
>
> -EFI_STATUS
>
> -(*PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT) (
>
> -=C2=A0 IN UINTN=C2=A0 EventId,
>
> -=C2=A0 IN UINTN=C2=A0 CpuNumber,
>
> -=C2=A0 IN UINTN=C2=A0 NsCommBufferAddr
>
> -=C2=A0 );
>
> -
>
> -typedef struct {
>
> -=C2=A0 PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT=C2=A0 =C2=A0 *ArmTfCpuDrive= rEpPtr;
>
> -} ARM_TF_CPU_DRIVER_EP_DESCRIPTOR;
>
> -
>
>=C2=A0 =C2=A0typedef RETURN_STATUS (*REGION_PERMISSION_UPDATE_FUNC) ( >
>=C2=A0 =C2=A0 =C2=A0IN=C2=A0 EFI_PHYSICAL_ADDRESS=C2=A0 BaseAddress, >
>=C2=A0 =C2=A0 =C2=A0IN=C2=A0 UINT64=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 Length
>
> @@ -145,8 +134,8 @@ LocateStandaloneMmCorePeCoffData (
>=C2=A0 =C2=A0VOID *
>
>=C2=A0 =C2=A0EFIAPI
>
>=C2=A0 =C2=A0CreateHobListFromBootInfo (
>
> -=C2=A0 IN=C2=A0 OUT=C2=A0 PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT=C2=A0 *C= puDriverEntryPoint,
>
> -=C2=A0 IN=C2=A0 =C2=A0 =C2=A0 =C2=A0EFI_SECURE_PARTITION_BOOT_INFO=C2= =A0 =C2=A0 =C2=A0 *PayloadBootInfo
>
> +=C2=A0 IN=C2=A0 OUT=C2=A0 PI_MM_CPU_DRIVER_ENTRYPOINT=C2=A0 =C2=A0 = =C2=A0*CpuDriverEntryPoint,
>
> +=C2=A0 IN=C2=A0 =C2=A0 =C2=A0 =C2=A0EFI_SECURE_PARTITION_BOOT_INFO=C2= =A0 *PayloadBootInfo
>
>=C2=A0 =C2=A0 =C2=A0);
>
>=C2=A0 =C2=A0
>
>=C2=A0 =C2=A0/**
>
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/Cr= eateHobList.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/Crea= teHobList.c
> index 2ac2d354f06a..80ed532352af 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHob= List.c
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHob= List.c
> @@ -13,6 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>=C2=A0 =C2=A0#include <Guid/MmramMemoryReserve.h>
>
>=C2=A0 =C2=A0#include <Guid/MpInformation.h>
>
>=C2=A0 =C2=A0
>
> +#include <StandaloneMmCpu.h>
>
>=C2=A0 =C2=A0#include <Library/Arm/StandaloneMmCoreEntryPoint.h><= br> >
>=C2=A0 =C2=A0#include <Library/ArmMmuLib.h>
>
>=C2=A0 =C2=A0#include <Library/ArmSvcLib.h>
>
> @@ -39,7 +40,7 @@ extern EFI_GUID=C2=A0 gEfiStandaloneMmNonSecureBuffe= rGuid;
>=C2=A0 =C2=A0// GUID to identify HOB where the entry point of the CPU d= river will be
>
>=C2=A0 =C2=A0// populated to allow this entry point driver to invoke it= upon receipt of an
>
>=C2=A0 =C2=A0// event
>
> -extern EFI_GUID=C2=A0 gEfiArmTfCpuDriverEpDescriptorGuid;
>
> +extern EFI_GUID=C2=A0 gEfiMmCpuDriverEpDescriptorGuid;
>
>=C2=A0 =C2=A0
>
>=C2=A0 =C2=A0/**
>
>=C2=A0 =C2=A0 =C2=A0Use the boot information passed by privileged firmw= are to populate a HOB list
>
> @@ -52,22 +53,22 @@ extern EFI_GUID=C2=A0 gEfiArmTfCpuDriverEpDescript= orGuid;
>=C2=A0 =C2=A0**/
>
>=C2=A0 =C2=A0VOID *
>
>=C2=A0 =C2=A0CreateHobListFromBootInfo (
>
> -=C2=A0 IN=C2=A0 OUT=C2=A0 PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT=C2=A0 *C= puDriverEntryPoint,
>
> -=C2=A0 IN=C2=A0 =C2=A0 =C2=A0 =C2=A0EFI_SECURE_PARTITION_BOOT_INFO=C2= =A0 =C2=A0 =C2=A0 *PayloadBootInfo
>
> +=C2=A0 IN=C2=A0 OUT=C2=A0 PI_MM_CPU_DRIVER_ENTRYPOINT=C2=A0 =C2=A0 = =C2=A0*CpuDriverEntryPoint,
>
> +=C2=A0 IN=C2=A0 =C2=A0 =C2=A0 =C2=A0EFI_SECURE_PARTITION_BOOT_INFO=C2= =A0 *PayloadBootInfo
>
>=C2=A0 =C2=A0 =C2=A0)
>
>=C2=A0 =C2=A0{
>
> -=C2=A0 EFI_HOB_HANDOFF_INFO_TABLE=C2=A0 =C2=A0 =C2=A0 =C2=A0*HobStart= ;
>
> -=C2=A0 EFI_RESOURCE_ATTRIBUTE_TYPE=C2=A0 =C2=A0 =C2=A0 Attributes; >
> -=C2=A0 UINT32=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Index;
>
> -=C2=A0 UINT32=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0BufferSize;
>
> -=C2=A0 UINT32=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Flags;
>
> -=C2=A0 EFI_MMRAM_HOB_DESCRIPTOR_BLOCK=C2=A0 =C2=A0*MmramRangesHob; >
> -=C2=A0 EFI_MMRAM_DESCRIPTOR=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0*MmramRanges;
>
> -=C2=A0 EFI_MMRAM_DESCRIPTOR=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0*NsCommBufMmramRange;
>
> -=C2=A0 MP_INFORMATION_HOB_DATA=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 *MpI= nformationHobData;
>
> -=C2=A0 EFI_PROCESSOR_INFORMATION=C2=A0 =C2=A0 =C2=A0 =C2=A0 *ProcInfo= Buffer;
>
> -=C2=A0 EFI_SECURE_PARTITION_CPU_INFO=C2=A0 =C2=A0 *CpuInfo;
>
> -=C2=A0 ARM_TF_CPU_DRIVER_EP_DESCRIPTOR=C2=A0 *CpuDriverEntryPointDesc= ;
>
> +=C2=A0 EFI_HOB_HANDOFF_INFO_TABLE=C2=A0 =C2=A0 =C2=A0 *HobStart;
>
> +=C2=A0 EFI_RESOURCE_ATTRIBUTE_TYPE=C2=A0 =C2=A0 =C2=A0Attributes;
>
> +=C2=A0 UINT32=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Index;
>
> +=C2=A0 UINT32=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BufferSize;
>
> +=C2=A0 UINT32=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Flags;
>
> +=C2=A0 EFI_MMRAM_HOB_DESCRIPTOR_BLOCK=C2=A0 *MmramRangesHob;
>
> +=C2=A0 EFI_MMRAM_DESCRIPTOR=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = *MmramRanges;
>
> +=C2=A0 EFI_MMRAM_DESCRIPTOR=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = *NsCommBufMmramRange;
>
> +=C2=A0 MP_INFORMATION_HOB_DATA=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*MpIn= formationHobData;
>
> +=C2=A0 EFI_PROCESSOR_INFORMATION=C2=A0 =C2=A0 =C2=A0 =C2=A0*ProcInfoB= uffer;
>
> +=C2=A0 EFI_SECURE_PARTITION_CPU_INFO=C2=A0 =C2=A0*CpuInfo;
>
> +=C2=A0 MM_CPU_DRIVER_EP_DESCRIPTOR=C2=A0 =C2=A0 =C2=A0*CpuDriverEntry= PointDesc;
>
>=C2=A0 =C2=A0
>
>=C2=A0 =C2=A0 =C2=A0// Create a hoblist with a PHIT and EOH
>
>=C2=A0 =C2=A0 =C2=A0HobStart =3D HobConstructor (
>
> @@ -144,13 +145,13 @@ CreateHobListFromBootInfo (
>=C2=A0 =C2=A0
>
>=C2=A0 =C2=A0 =C2=A0// Create a Guided HOB to enable the ARM TF CPU dri= ver to share its entry
>
>=C2=A0 =C2=A0 =C2=A0// point and populate it with the address of the sh= ared buffer
>
> -=C2=A0 CpuDriverEntryPointDesc =3D (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR *= )BuildGuidHob (
>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0&gEfiArmTfCpuDriverEpDescriptorGuid,
>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0sizeof (ARM_TF_CPU_DRIVER_EP_DESCRIPTOR)
>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0);
>
> +=C2=A0 CpuDriverEntryPointDesc =3D (MM_CPU_DRIVER_EP_DESCRIPTOR *)Bui= ldGuidHob (
>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0&a= mp;gEfiMmCpuDriverEpDescriptorGuid,
>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0si= zeof (MM_CPU_DRIVER_EP_DESCRIPTOR)
>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0);=
>
>=C2=A0 =C2=A0
>
> -=C2=A0 *CpuDriverEntryPoint=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D NULL;
>
> -=C2=A0 CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr =3D CpuDriverE= ntryPoint;
>
> +=C2=A0 *CpuDriverEntryPoint=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D NULL;
>
> +=C2=A0 CpuDriverEntryPointDesc->MmCpuDriverEpPtr =3D CpuDriverEntr= yPoint;
>
>=C2=A0 =C2=A0
>
>=C2=A0 =C2=A0 =C2=A0// Find the size of the GUIDed HOB with SRAM ranges=
>
>=C2=A0 =C2=A0 =C2=A0BufferSize=C2=A0 =3D sizeof (EFI_MMRAM_HOB_DESCRIPT= OR_BLOCK);
>
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/St= andaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryP= oint/Arm/StandaloneMmCoreEntryPoint.c
> index 96de10405af8..900e0252ef9f 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/Standalon= eMmCoreEntryPoint.c
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/Standalon= eMmCoreEntryPoint.c
> @@ -15,6 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>=C2=A0 =C2=A0#include <Guid/MmramMemoryReserve.h>
>
>=C2=A0 =C2=A0#include <Guid/MpInformation.h>
>
>=C2=A0 =C2=A0
>
> +#include <StandaloneMmCpu.h>
>
>=C2=A0 =C2=A0#include <Library/ArmSvcLib.h>
>
>=C2=A0 =C2=A0#include <Library/DebugLib.h>
>
>=C2=A0 =C2=A0#include <Library/HobLib.h>
>
> @@ -41,7 +42,7 @@ STATIC CONST UINT32=C2=A0 mSpmMinorVerFfa =3D SPM_MI= NOR_VERSION_FFA;
>=C2=A0 =C2=A0
>
>=C2=A0 =C2=A0#define BOOT_PAYLOAD_VERSION=C2=A0 1
>
>=C2=A0 =C2=A0
>
> -PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT=C2=A0 CpuDriverEntryPoint =3D NULL= ;
>
> +PI_MM_CPU_DRIVER_ENTRYPOINT=C2=A0 CpuDriverEntryPoint =3D NULL;
>
>=C2=A0 =C2=A0
>
>=C2=A0 =C2=A0/**
>
>=C2=A0 =C2=A0 =C2=A0Retrieve a pointer to and print the boot informatio= n passed by privileged
>
> @@ -140,6 +141,18 @@ DelegatedEventLoop (
>=C2=A0 =C2=A0 =C2=A0 =C2=A0DEBUG ((DEBUG_INFO, "X6 :=C2=A0 0x%x\n&= quot;, (UINT32)EventCompleteSvcArgs->Arg6));
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0DEBUG ((DEBUG_INFO, "X7 :=C2=A0 0x%x\n&= quot;, (UINT32)EventCompleteSvcArgs->Arg7));
>
>=C2=A0 =C2=A0
>
> +=C2=A0 =C2=A0 //
>
> +=C2=A0 =C2=A0 // ARM TF passes SMC FID of the MM_COMMUNICATE interfac= e as the Event ID upon
>
> +=C2=A0 =C2=A0 // receipt of a synchronous MM request. Use the Event I= D to distinguish
>
> +=C2=A0 =C2=A0 // between synchronous and asynchronous events.
>
> +=C2=A0 =C2=A0 //
>
> +=C2=A0 =C2=A0 if ((ARM_SMC_ID_MM_COMMUNICATE !=3D (UINT32)EventComple= teSvcArgs->Arg0) &&
>
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 (ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ !=3D = (UINT32)EventCompleteSvcArgs->Arg0))
>
> +=C2=A0 =C2=A0 {
>
> +=C2=A0 =C2=A0 =C2=A0 DEBUG ((DEBUG_ERROR, "UnRecognized Event - = 0x%x\n", (UINT32)EventCompleteSvcArgs->Arg0));
>
> +=C2=A0 =C2=A0 =C2=A0 continue;

[SAMI] I think an error needs to be returned instead of continuing
otherwise this changes the original behaviour.

Status needs to be set to EFI_INVALID_PARAMETER here.

[/SAMI]

>
> +=C2=A0 =C2=A0 }

[SAMI] The code from this point needs to be enclosed in an else block
until before the switch statement.

That way the proper error code would be returned. Can you check, please?
[/SAMI]
[Tuan] Thanks for the catch. Will fix it=C2=A0= =C2=A0

>
> +
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0FfaEnabled =3D FeaturePcdGet (PcdFfaEnable);=
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (FfaEnabled) {
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Status =3D CpuDriverEntryPoint (
>
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Standa= loneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoi= nt/StandaloneMmCoreEntryPoint.inf
> index 75cfb98c0e75..d41d7630b614 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmC= oreEntryPoint.inf
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmC= oreEntryPoint.inf
> @@ -49,7 +49,7 @@
>=C2=A0 =C2=A0 =C2=A0gMpInformationHobGuid
>
>=C2=A0 =C2=A0 =C2=A0gEfiMmPeiMmramMemoryReserveGuid
>
>=C2=A0 =C2=A0 =C2=A0gEfiStandaloneMmNonSecureBufferGuid
>
> -=C2=A0 gEfiArmTfCpuDriverEpDescriptorGuid
>
> +=C2=A0 gEfiMmCpuDriverEpDescriptorGuid
>
>=C2=A0 =C2=A0
>
>=C2=A0 =C2=A0[FeaturePcd.ARM, FeaturePcd.AARCH64]
>
>=C2=A0 =C2=A0 =C2=A0gArmTokenSpaceGuid.PcdFfaEnable
>
_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#109174) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--000000000000c161a006066f4d1c--