Re: [RFC PATCH v1 05/30] ArmPkg & ArmVirtPkg: Make PcdMonitorConduitHvc a dynamic PCD

["Ard Biesheuvel" <ardb@kernel.org>]

On Tue, 25 Apr 2023 at 18:04, Sami Mujawar <sami.mujawar@arm.com> wrote:
>
> The monitor call conduit is fixed for a platform firmware in
> most scenarios. For a normal virtual machine guest firmware,
> the default conduit is HVC. However, for Arm CCA the Realm
> code must use SMC as the conduit.
>
> To have a common code base for Guest/Virtual firmware to be used
> by both normal VMs and Realm VMs, make PcdMonitorConduitHvc as a
> dynamic PCD. This allows the firmware to detect if it is running
> in a Realm and it can configure the PcdMonitorConduitHvc as FALSE
> (i.e. to use SMC as the conduit when running in a Realm).
>
> Also update the ArmVirtPkg/ArmVirtKvmTool.dsc workspace to move
> the PcdMonitorConduitHvc in the PcdsDynamic section to prevent
> the build from breaking.
>

Do you mean realm VMs will use SMC even for PSCI calls etc?

The change looks fine to me, given that other platforms that rely on
the default will still get a fixed PCD after this change.


> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>  ArmPkg/ArmPkg.dec                            | 10 +++++-----
>  ArmPkg/Library/ArmMonitorLib/ArmMonitorLib.c |  4 ++--
>  ArmVirtPkg/ArmVirtKvmTool.dsc                |  4 ++--
>  3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index f17ba913e6de1326d49b93d6a15378ff2f522d24..0730533e512d60fcba19c4cfa84944061d16f02e 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -139,11 +139,6 @@ [PcdsFeatureFlag.common]
>    # Define if the GICv3 controller should use the GICv2 legacy
>    gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042
>
> -  ## Define the conduit to use for monitor calls.
> -  # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
> -  # If PcdMonitorConduitHvc = TRUE, conduit = HVC
> -  gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x00000047
> -
>  [PcdsFeatureFlag.ARM]
>    # Whether to map normal memory as non-shareable. FALSE is the safe choice, but
>    # TRUE may be appropriate to fix performance problems if you don't care about
> @@ -393,6 +388,11 @@ [PcdsFixedAtBuild.common, PcdsDynamic.common]
>    gArmTokenSpaceGuid.PcdPciBusMin|0x0|UINT32|0x00000059
>    gArmTokenSpaceGuid.PcdPciBusMax|0x0|UINT32|0x0000005A
>
> +  ## Define the conduit to use for monitor calls.
> +  # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
> +  # If PcdMonitorConduitHvc = TRUE, conduit = HVC
> +  gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x00000047
> +
>  [PcdsDynamicEx]
>    #
>    # This dynamic PCD hold the GUID of a firmware FFS which contains
> diff --git a/ArmPkg/Library/ArmMonitorLib/ArmMonitorLib.c b/ArmPkg/Library/ArmMonitorLib/ArmMonitorLib.c
> index 741f5c615744dc5cc5381ff3848078f93858dd2b..221724125ce3a8f351a55a81f441409a99bcb5cf 100644
> --- a/ArmPkg/Library/ArmMonitorLib/ArmMonitorLib.c
> +++ b/ArmPkg/Library/ArmMonitorLib/ArmMonitorLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Arm Monitor Library.
>
> -  Copyright (c) 2022, Arm Limited. All rights reserved.<BR>
> +  Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -26,7 +26,7 @@ ArmMonitorCall (
>    IN OUT ARM_MONITOR_ARGS  *Args
>    )
>  {
> -  if (FeaturePcdGet (PcdMonitorConduitHvc)) {
> +  if (PcdGetBool (PcdMonitorConduitHvc)) {
>      ArmCallHvc ((ARM_HVC_ARGS *)Args);
>    } else {
>      ArmCallSmc ((ARM_SMC_ARGS *)Args);
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
> index d2228a95726b24fe5c2edfbc84b1f5c23a85feba..467e5c166e1bbad3acbae78f53c225f5bac525a9 100644
> --- a/ArmVirtPkg/ArmVirtKvmTool.dsc
> +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
> @@ -117,8 +117,6 @@ [PcdsFeatureFlag.common]
>    # Use MMIO for accessing RTC controller registers.
>    gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|TRUE
>
> -  gArmTokenSpaceGuid.PcdMonitorConduitHvc|TRUE
> -
>  [PcdsFixedAtBuild.common]
>    gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F
>
> @@ -237,6 +235,8 @@ [PcdsDynamicDefault.common]
>    gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0
>    gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0
>
> +  gArmTokenSpaceGuid.PcdMonitorConduitHvc|TRUE
> +
>  ################################################################################
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>