Hi Ard, Thank you for the feedback. Please find my response inline marked [SAMI].
Regards, Sami Mujawar On 10/05/2023, 12:39, "Ard Biesheuvel" <a...@kernel.org <mailto:a...@kernel.org>> wrote: On Tue, 25 Apr 2023 at 18:04, Sami Mujawar <sami.muja...@arm.com <mailto:sami.muja...@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? [SAMI] For Realm code a SMC is required for PSCI calls. Following extract from Section A1.3 of the RMM A-bet0 specification (https://developer.arm.com/documentation/den0137/1-0bet0/?lang=en) The RMM exposes the following interfaces, which are accessed via SMC instructions, to Realms: * The Realm Services Interface (RSI), which provides services used to manage resources allocated to the Realm, and to request an attestation report. * The Power State Coordination Interface (PSCI), which provides services used to control power states of VPEs within a Realm. Note that the HVC conduit for PSCI is not supported for Realms. [/SAMI] 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.muja...@arm.com > <mailto:sami.muja...@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)' > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#104864): https://edk2.groups.io/g/devel/message/104864 Mute This Topic: https://groups.io/mt/98495955/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
