From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.1792.1689182464231573544 for ; Wed, 12 Jul 2023 10:21:04 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=O25fOp6H; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: osde@linux.microsoft.com) Received: from [10.137.194.171] (unknown [131.107.1.171]) by linux.microsoft.com (Postfix) with ESMTPSA id 9371F21C44E1; Wed, 12 Jul 2023 10:21:03 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 9371F21C44E1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1689182463; bh=0aTeJPvd1GsS2EcMdzW23rCyBeHIW+Ni7DEnBBQTEnM=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=O25fOp6HRkyzgd46tKjX2n1shBe9kpeS25qWXkrC0ScUoh75Vv4hbApJZmyrc0j4E nXn+pr8l8zNAKkfkAtG/uXnVCBzSMSZnVrt+oWec7lou043AMnSp0w935eCQcj2yLW 09GNzB7ZPPm/W82CA0KOeYfBqSM+0MklRNy7UF+U= Message-ID: Date: Wed, 12 Jul 2023 10:21:03 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [edk2-devel] [edk2-platforms][PATCH V1 01/20] ArmPkg: Change PcdFfaEnable flag datatype To: devel@edk2.groups.io, nishant.sharma@arm.com Cc: Ard Biesheuvel , Sami Mujawar , Thomas Abraham , Sayanta Pattanayak , Achin Gupta References: <20230711143658.781597-1-nishant.sharma@arm.com> <20230711143658.781597-2-nishant.sharma@arm.com> From: "Oliver Smith-Denny" In-Reply-To: <20230711143658.781597-2-nishant.sharma@arm.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 7/11/2023 7:36 AM, Nishant Sharma wrote: > FeatureFlag type PCD flags are declared by typecasting an integer value > to BOOLEAN. These flags cannot be use in assembly code as assembler does > not recognise C primitive types. Change the flag data type from BOOLEAN > to UINT32. > > Signed-off-by: Nishant Sharma > --- > ArmPkg/ArmPkg.dec | 14 +++++++------- > ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf | 4 ++-- > StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf | 4 ++-- > ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c | 8 ++++---- > StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c | 8 ++++---- > 5 files changed, 19 insertions(+), 19 deletions(-) > > diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec > index 1a16d044c94b..c36c23e2e059 100644 > --- a/ArmPkg/ArmPkg.dec > +++ b/ArmPkg/ArmPkg.dec > @@ -155,13 +155,6 @@ > # hardware coherency (i.e., no virtualization or cache coherent DMA) > gArmTokenSpaceGuid.PcdNormalMemoryNonshareableOverride|FALSE|BOOLEAN|0x00000043 > > -[PcdsFeatureFlag.AARCH64, PcdsFeatureFlag.ARM] > - ## Used to select method for requesting services from S-EL1.

> - # TRUE - Selects FF-A calls for communication between S-EL0 and SPMC.
> - # FALSE - Selects SVC calls for communication between S-EL0 and SPMC.
> - # @Prompt Enable FF-A support. > - gArmTokenSpaceGuid.PcdFfaEnable|FALSE|BOOLEAN|0x0000005B > - > [PcdsFixedAtBuild.common] > gArmTokenSpaceGuid.PcdTrustzoneSupport|FALSE|BOOLEAN|0x00000006 > > @@ -298,6 +291,13 @@ > # not currently supported. > gArmTokenSpaceGuid.PcdArmNonSecModeTransition|0x3c9|UINT32|0x0000003E > > + ## Used to select method for requesting services from S-EL1.

> + # 1 - Selects FF-A calls for communication between S-EL0 and SPMC.
> + # 0 - Selects SVC calls for communication between S-EL0 and SPMC.
> + # Using unsigned integer as boolean does not work on assembler. > + # @Prompt Enable FF-A support. > + gArmTokenSpaceGuid.PcdFfaEnable|0|UINT32|0x0000005B > + A small nit: any reason not to go to UINT8 from BOOLEAN? It would seem the logical conversion, unless you envision it extending greatly in the future. Thanks, Oliver > > # > # These PCDs are also defined as 'PcdsDynamic' or 'PcdsPatchableInModule' to be > diff --git a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf > index ff20e5898051..3c733585f573 100644 > --- a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf > +++ b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf > @@ -1,6 +1,6 @@ > #/** @file > # > -# Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.
> +# Copyright (c) 2017 - 2023, Arm Limited. All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -23,7 +23,7 @@ > ArmPkg/ArmPkg.dec > MdePkg/MdePkg.dec > > -[FeaturePcd.ARM, FeaturePcd.AARCH64] > +[FixedPcd] > gArmTokenSpaceGuid.PcdFfaEnable > > [LibraryClasses] > diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf > index 75cfb98c0e75..dc6d3d859911 100644 > --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf > +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf > @@ -1,7 +1,7 @@ > ## @file > # Module entry point library for DXE core. > # > -# Copyright (c) 2017 - 2021, Arm Ltd. All rights reserved.
> +# Copyright (c) 2017 - 2023, Arm Ltd. All rights reserved. > # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -51,7 +51,7 @@ > gEfiStandaloneMmNonSecureBufferGuid > gEfiArmTfCpuDriverEpDescriptorGuid > > -[FeaturePcd.ARM, FeaturePcd.AARCH64] > +[FixedPcd] > gArmTokenSpaceGuid.PcdFfaEnable > > # > diff --git a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c > index d55aff76201e..1a41a289ef17 100644 > --- a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c > +++ b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c > @@ -1,7 +1,7 @@ > /** @file > File managing the MMU for ARMv8 architecture in S-EL0 > > - Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.
> + Copyright (c) 2017 - 2023, Arm Limited. All rights reserved. > Copyright (c) 2021, Linaro Limited > SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -54,7 +54,7 @@ SendMemoryPermissionRequest ( > } > > ArmCallSvc (SvcArgs); > - if (FeaturePcdGet (PcdFfaEnable)) { > + if (FixedPcdGet32 (PcdFfaEnable) != 0) { > // Get/Set memory attributes is an atomic call, with > // StandaloneMm at S-EL0 being the caller and the SPM > // core being the callee. Thus there won't be a > @@ -163,7 +163,7 @@ GetMemoryPermissions ( > // Prepare the message parameters. > // See [1], Section 13.5.5.1 MM_SP_MEMORY_ATTRIBUTES_GET_AARCH64. > ZeroMem (&SvcArgs, sizeof (ARM_SVC_ARGS)); > - if (FeaturePcdGet (PcdFfaEnable)) { > + if (FixedPcdGet32 (PcdFfaEnable) != 0) { > // See [2], Section 10.2 FFA_MSG_SEND_DIRECT_REQ. > SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ; > SvcArgs.Arg1 = ARM_FFA_DESTINATION_ENDPOINT_ID; > @@ -218,7 +218,7 @@ RequestMemoryPermissionChange ( > // Prepare the message parameters. > // See [1], Section 13.5.5.2 MM_SP_MEMORY_ATTRIBUTES_SET_AARCH64. > ZeroMem (&SvcArgs, sizeof (ARM_SVC_ARGS)); > - if (FeaturePcdGet (PcdFfaEnable)) { > + if (FixedPcdGet32 (PcdFfaEnable) != 0) { > // See [2], Section 10.2 FFA_MSG_SEND_DIRECT_REQ. > SvcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ; > SvcArgs.Arg1 = ARM_FFA_DESTINATION_ENDPOINT_ID; > diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c > index 96de10405af8..5dd1d9747995 100644 > --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c > +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c > @@ -2,7 +2,7 @@ > Entry point to the Standalone MM Foundation when initialized during the SEC > phase on ARM platforms > > -Copyright (c) 2017 - 2021, Arm Ltd. All rights reserved.
> +Copyright (c) 2017 - 2023, Arm Ltd. All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -140,7 +140,7 @@ DelegatedEventLoop ( > DEBUG ((DEBUG_INFO, "X6 : 0x%x\n", (UINT32)EventCompleteSvcArgs->Arg6)); > DEBUG ((DEBUG_INFO, "X7 : 0x%x\n", (UINT32)EventCompleteSvcArgs->Arg7)); > > - FfaEnabled = FeaturePcdGet (PcdFfaEnable); > + FfaEnabled = FixedPcdGet32 (PcdFfaEnable != 0); > if (FfaEnabled) { > Status = CpuDriverEntryPoint ( > EventCompleteSvcArgs->Arg0, > @@ -225,7 +225,7 @@ GetSpmVersion ( > UINT32 SpmVersion; > ARM_SVC_ARGS SpmVersionArgs; > > - if (FeaturePcdGet (PcdFfaEnable)) { > + if (FixedPcdGet32 (PcdFfaEnable) != 0) { > SpmVersionArgs.Arg0 = ARM_SVC_ID_FFA_VERSION_AARCH32; > SpmVersionArgs.Arg1 = mSpmMajorVerFfa << SPM_MAJOR_VER_SHIFT; > SpmVersionArgs.Arg1 |= mSpmMinorVerFfa; > @@ -293,7 +293,7 @@ InitArmSvcArgs ( > OUT INT32 *Ret > ) > { > - if (FeaturePcdGet (PcdFfaEnable)) { > + if (FixedPcdGet32 (PcdFfaEnable) != 0) { > InitMmFoundationSvcArgs->Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP; > InitMmFoundationSvcArgs->Arg1 = 0; > InitMmFoundationSvcArgs->Arg2 = 0;