From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web10.14822.1683720546568250358 for ; Wed, 10 May 2023 05:09:06 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=Asy/ujWw; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 07085630FC for ; Wed, 10 May 2023 12:09:06 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 56717C433A0 for ; Wed, 10 May 2023 12:09:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1683720545; bh=NsYiwvy5wiVeeDNgWDrHTIy4sdZy+/QSezBSGooa1ZM=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=Asy/ujWwGQv+Orc9IQYtY8UWY8LHynoObvqtMYxSxqWPPm96mm2C2SQ1Z8q1Q/Y0W CitI0/OrN441GKiBEw7TwrrQbZWEQxMiu4ii/Pb+++kCf/zsLzQTZg2hJJuwJgv0uz crcJRHtKIPhPrRkMwfL+FNGg666lAcfjFeE6ireTPuji0devwNbNGZ9MLxAY+ElmSz pj9GeY4USapTMgr0IdNnVyhFN0x6vLsCW148DtmTrSxPTtBdX83erCOZVl5YFmQk+y 40WSUYcMm/hX/gqgnlnzxgDiHQLS2wWFL5DYPs2SaksdNzPt3mEFnHBeFhIzW7iC65 VKPqpD8w/nL1g== Received: by mail-lf1-f51.google.com with SMTP id 2adb3069b0e04-4ec8eca56cfso7960088e87.0 for ; Wed, 10 May 2023 05:09:05 -0700 (PDT) X-Gm-Message-State: AC+VfDxbUtnyw7LCeSSZ183BpCxvF3IkryhD0++bo6GM/+MbEOJqFSrA zO6yq34R1S36WWQfHrJHUZeJCwWAwniDf99xK6g= X-Google-Smtp-Source: ACHHUZ4K5rrbpR/8YJSeuTaTa9BntYcLhi99QDgRo12BC8jynB7fKCCf/UdRFC6+QA0MeI0Ud0YCSaTOGvPK8WL8ZW8= X-Received: by 2002:ac2:4102:0:b0:4f1:4cdc:ec03 with SMTP id b2-20020ac24102000000b004f14cdcec03mr1598921lfi.18.1683720543271; Wed, 10 May 2023 05:09:03 -0700 (PDT) MIME-Version: 1.0 References: <20230425160428.27980-1-sami.mujawar@arm.com> <20230425160428.27980-26-sami.mujawar@arm.com> In-Reply-To: <20230425160428.27980-26-sami.mujawar@arm.com> From: "Ard Biesheuvel" Date: Wed, 10 May 2023 14:08:52 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC PATCH v1 25/30] ArmVirtPkg: Add ArmCcaDxe for early DXE phase initialisation To: Sami Mujawar Cc: devel@edk2.groups.io, ardb+tianocore@kernel.org, quic_llindhol@quicinc.com, kraxel@redhat.com, Pierre.Gondois@arm.com, Suzuki.Poulose@arm.com, jean-philippe@linaro.org, Matteo.Carlini@arm.com, Akanksha.Jain2@arm.com, Ben.Adderson@arm.com, nd@arm.com Content-Type: text/plain; charset="UTF-8" On Tue, 25 Apr 2023 at 18:05, Sami Mujawar wrote: > > Add ArmCcaDxe for early DXE phase initialisation like setting > up the monitor call conduit for Realm code > > The Realm code should use SMC as the conduit for monitor calls. > Therefore, set the PcdMonitorConduitHvc to FALSE if the code is > running in a Realm. > > Note: ArmCcaDxe is configured as an APRIORI DXE so that the DXE > dispatcher can schedule this to be loaded at the very beginning > of the Dxe phase. The DevicePathDxe.inf and Pcd.inf modules have > also been included to satisfy the required dependencies. > Please find a way to achieve this without relying on APRIORI - this is fragile and defeats the dependency based dispatch model that DXE is based on. IIUC the issue you are addressing is that the PCD must be set correctly before any library that [transitively] depends on it is used, right? That simply means that the SMC/HVC functionality must be exposed as a protocol rather than a library I.e., the 'monitor call' abstraction library should be backed by a protocol, that could be implemented in two different ways: using HVCs or using SMCs. The abstraction library will DEPEX on the protocol, and nothing gets dispatched until one of the two protocols is installed. That gets rid of the dynamic PCD as well. > Signed-off-by: Sami Mujawar > --- > ArmVirtPkg/ArmCcaDxe/ArmCcaDxe.c | 50 ++++++++++++++++++++ > ArmVirtPkg/ArmCcaDxe/ArmCcaDxe.inf | 39 +++++++++++++++ > ArmVirtPkg/ArmVirtKvmTool.dsc | 5 +- > ArmVirtPkg/ArmVirtKvmTool.fdf | 10 ++++ > 4 files changed, 102 insertions(+), 2 deletions(-) > > diff --git a/ArmVirtPkg/ArmCcaDxe/ArmCcaDxe.c b/ArmVirtPkg/ArmCcaDxe/ArmCcaDxe.c > new file mode 100644 > index 0000000000000000000000000000000000000000..36a74f2521d2d92d404c42e86d5d37dd31a1972d > --- /dev/null > +++ b/ArmVirtPkg/ArmCcaDxe/ArmCcaDxe.c > @@ -0,0 +1,50 @@ > +/** @file > + ArmCcaDxe > + > + Copyright (c) 2022 - 2023, ARM Ltd. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/** Entrypoint of Arm CCA Dxe. > + > + @param [in] ImageHandle Image handle of this driver. > + @param [in] SystemTable Pointer to the EFI System Table. > + > + @retval RETURN_SUCCESS Success. > + @retval EFI_NOT_FOUND Required HOB not found. > +**/ > +EFI_STATUS > +EFIAPI > +ArmCcaDxe ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + > + if (!IsRealm ()) { > + // Nothing to do here, return SUCCESS. > + return EFI_SUCCESS; > + } > + > + // Setup the conduit to be used by Realm code to SMC. > + Status = PcdSetBoolS (PcdMonitorConduitHvc, FALSE); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "ERROR - Failed to set PcdMonitorConduitHvc\n")); > + ASSERT (0); > + return Status; > + } > + > + return Status; > +} > diff --git a/ArmVirtPkg/ArmCcaDxe/ArmCcaDxe.inf b/ArmVirtPkg/ArmCcaDxe/ArmCcaDxe.inf > new file mode 100644 > index 0000000000000000000000000000000000000000..df110ae54ce54f792fe9cf9420334dd1e6a3fc2c > --- /dev/null > +++ b/ArmVirtPkg/ArmCcaDxe/ArmCcaDxe.inf > @@ -0,0 +1,39 @@ > +## @file > +# ArmCcaDxe > +# > +# Copyright (c) 2022 - 2023, Arm Limited. All rights reserved.
> +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x0001001B > + BASE_NAME = ArmCcaDxe > + FILE_GUID = 6E474F73-7D50-46A8-9AEB-996B71599FE9 > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.0 > + ENTRY_POINT = ArmCcaDxe > + > +[Sources] > + ArmCcaDxe.c > + > +[LibraryClasses] > + ArmCcaLib > + BaseLib > + DebugLib > + HobLib > + PcdLib > + UefiDriverEntryPoint > + > +[Packages] > + ArmPkg/ArmPkg.dec > + ArmVirtPkg/ArmVirtPkg.dec > + MdeModulePkg/MdeModulePkg.dec > + MdePkg/MdePkg.dec > + > +[Pcd] > + gArmTokenSpaceGuid.PcdMonitorConduitHvc > + > +[Depex] > + TRUE > diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc > index 9bc857ea88d00431bf4223f588f908eab7561a19..acf4ede48da2d33d50b5593a857f3815f427707c 100644 > --- a/ArmVirtPkg/ArmVirtKvmTool.dsc > +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc > @@ -404,9 +404,10 @@ [Components.common] > # > SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf > > -!if $(ARCH) == AARCH64 > +[Components.AARCH64] > # > # ACPI Support > # > ArmVirtPkg/KvmtoolCfgMgrDxe/ConfigurationManagerDxe.inf > -!endif > + > + ArmVirtPkg/ArmCcaDxe/ArmCcaDxe.inf > diff --git a/ArmVirtPkg/ArmVirtKvmTool.fdf b/ArmVirtPkg/ArmVirtKvmTool.fdf > index 8ccbccd71e134e0ea97d49380293687aca43e8b9..68bd0e9d82dc83a337d8127a598018381888d894 100644 > --- a/ArmVirtPkg/ArmVirtKvmTool.fdf > +++ b/ArmVirtPkg/ArmVirtKvmTool.fdf > @@ -117,6 +117,16 @@ [FV.FvMain] > READ_LOCK_CAP = TRUE > READ_LOCK_STATUS = TRUE > > +!if $(ARCH) == AARCH64 > + APRIORI DXE { > + INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf > + INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf > + INF ArmVirtPkg/ArmCcaDxe/ArmCcaDxe.inf > + } > + > + INF ArmVirtPkg/ArmCcaDxe/ArmCcaDxe.inf > +!endif > + > INF MdeModulePkg/Core/Dxe/DxeMain.inf > INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf > INF OvmfPkg/Fdt/VirtioFdtDxe/VirtioFdtDxe.inf > -- > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' >