Doran,
Which platform are you using? I thought those platforms are quite old and no
one is using them.
> -----Original Message-----
> From: Oram, Isaac W <isaac.w.o...@intel.com>
> Sent: Tuesday, August 30, 2022 6:27 AM
> To: Benjamin Doron <benjamin.doro...@gmail.com>; devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Sinha, Ankit
> <ankit.si...@intel.com>; Ni, Ray
> <ray...@intel.com>; Chaganty, Rangasai V <rangasai.v.chaga...@intel.com>
> Subject: RE: [edk2-devel][edk2-platforms][PATCH v1 1/5]
> IntelSiliconPkg/Feature/PeiSmmAccessLibSmramc: Implement
> chipset support
>
> Reviewed-by: Isaac Oram <isaac.w.o...@intel.com>
>
> I would prefer to see contents of sections indented, but it is a nit.
> It might be slightly better to have PcdsFixedAtBuild type PCD for the
> register information, but this is pretty stable HW, so it
> is ok.
>
> Regards,
> Isaac
>
> -----Original Message-----
> From: Benjamin Doron <benjamin.doro...@gmail.com>
> Sent: Monday, August 29, 2022 1:36 PM
> To: devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Sinha, Ankit
> <ankit.si...@intel.com>; Ni, Ray
> <ray...@intel.com>; Chaganty, Rangasai V <rangasai.v.chaga...@intel.com>;
> Oram, Isaac W <isaac.w.o...@intel.com>
> Subject: [edk2-devel][edk2-platforms][PATCH v1 1/5]
> IntelSiliconPkg/Feature/PeiSmmAccessLibSmramc: Implement
> chipset support
>
> SMRAM must be opened to retrieve the lockbox for S3, and SMM communication
> depends on this PPI. For security
> purposes, SMRAM lock must be performed before EndOfPei (although FSP notify
> performs lockdown too).
>
> It seems to me that this library is generic and applicable to all Intel
> platforms in the tree using the MCH SMRAMC register.
>
> Cc: Nate DeSimone <nathaniel.l.desim...@intel.com>
> Cc: Ankit Sinha <ankit.si...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaga...@intel.com>
> Cc: Isaac Oram <isaac.w.o...@intel.com>
> Signed-off-by: Benjamin Doron <benjamin.doro...@gmail.com>
> ---
> .../PeiSmmAccessLibSmramc/PeiSmmAccessLib.c | 430 ++++++++++++++++++
> .../PeiSmmAccessLibSmramc/PeiSmmAccessLib.inf | 41 ++
> 2 files changed, 471 insertions(+)
> create mode 100644
> Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLibSmramc/PeiSmmAccessLib.c
> create mode 100644
> Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLibSmramc/PeiSmmAccessLib.inf
>
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLibSmramc/PeiSmmAccessLib.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLibSmramc/PeiSmmAccessLib.c
> new file mode 100644
> index 000000000000..5b472bf86abf
> --- /dev/null
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAcce
> +++ ssLibSmramc/PeiSmmAccessLib.c
> @@ -0,0 +1,430 @@
> +/** @file+ This is to publish the SMM Access Ppi instance.++ Copyright (c)
> 2019 - 2020, Intel Corporation. All rights
> reserved.<BR>+ SPDX-License-Identifier: BSD-2-Clause-Patent++**/+#include
> <Library/BaseMemoryLib.h>+#include
> <Library/DebugLib.h>+#include <Library/MemoryAllocationLib.h>+#include
> <Library/PciSegmentLib.h>+#include
> <Library/PeiServicesLib.h>+#include <Library/HobLib.h>+#include
> <Uefi/UefiBaseType.h>+#include
> <Guid/SmramMemoryReserve.h>++#include <Ppi/MmAccess.h>+#include
> <IndustryStandard/Pci22.h>++#define
> SMM_ACCESS_PRIVATE_DATA_SIGNATURE SIGNATURE_32 ('4', '5', 's', 'a')++///+///
> Private data+///+typedef struct {+
> UINTN Signature;+ EFI_HANDLE Handle;+
> EFI_PEI_MM_ACCESS_PPI SmmAccess;+ //+ // Local Data for
> SMM Access interface goes here+ //+ UINTN NumberRegions;+
> EFI_SMRAM_DESCRIPTOR *SmramDesc;+}
> SMM_ACCESS_PRIVATE_DATA;++#define SMM_ACCESS_PRIVATE_DATA_FROM_THIS(a) \+
> CR (a, \+
> SMM_ACCESS_PRIVATE_DATA, \+ SmmAccess, \+
> SMM_ACCESS_PRIVATE_DATA_SIGNATURE \+ )++//+//
> Common registers:+//+// DEVICE 0 (Memory Controller Hub)+//+#define SA_MC_BUS
> 0x00+#define SA_MC_DEV
> 0x00+#define SA_MC_FUN 0x00+///+/// Description:+/// The SMRAMC
> register controls how accesses to Compatible
> SMRAM spaces are treated. The Open, Close and Lock bits function only when
> G_SMRAME bit is set to 1. Also, the Open
> bit must be reset before the Lock bit is set.+///+#define R_SA_SMRAMC
> (0x88)+#define B_SA_SMRAMC_D_LCK_MASK
> (0x10)+#define B_SA_SMRAMC_D_CLS_MASK (0x20)+#define
> B_SA_SMRAMC_D_OPEN_MASK (0x40)++/**+ This
> routine accepts a request to "open" a region of SMRAM. The+ region could be
> legacy ABSEG, HSEG, or TSEG near top of
> physical memory.+ The use of "open" means that the memory is visible from
> all PEIM+ and SMM agents.++ @param[in]
> PeiServices - General purpose services available to every PEIM.+
> @param[in] This - Pointer to the SMM Access
> Interface.+ @param[in] DescriptorIndex - Region of SMRAM to Open.++
> @retval EFI_SUCCESS - The region was
> successfully opened.+ @retval EFI_DEVICE_ERROR - The region could not
> be opened because locked by+
> chipset.+ @retval EFI_INVALID_PARAMETER - The descriptor index was out of
> bounds.+**/+EFI_STATUS+EFIAPI+Open
> (+ IN EFI_PEI_SERVICES **PeiServices,+ IN EFI_PEI_MM_ACCESS_PPI
> *This,+ IN UINTN
> DescriptorIndex+ )+{+ SMM_ACCESS_PRIVATE_DATA *SmmAccess;+ UINT8
> Index;+ UINT64 Address;+
> UINT8 SmramControl;++ SmmAccess =
> SMM_ACCESS_PRIVATE_DATA_FROM_THIS (This);+ if
> (DescriptorIndex >= SmmAccess->NumberRegions) {+ DEBUG ((DEBUG_WARN,
> "SMRAM region out of range\n"));++
> return EFI_INVALID_PARAMETER;+ } else if
> (SmmAccess->SmramDesc[DescriptorIndex].RegionState &
> EFI_SMRAM_LOCKED) {+ //+ // Cannot open a "locked" region+ //+
> DEBUG ((DEBUG_WARN, "Cannot open a locked
> SMRAM region\n"));++ return EFI_DEVICE_ERROR;+ }++ ///+ /// BEGIN
> CHIPSET CODE+ ///+ ///+ /// SMRAM register
> is PCI 0:0:0:88, SMRAMC (8 bit)+ ///+ Address = PCI_SEGMENT_LIB_ADDRESS (0,
> SA_MC_BUS, SA_MC_DEV, SA_MC_FUN,
> R_SA_SMRAMC);+ SmramControl = PciSegmentRead8 (Address);+ ///+ /// Is
> SMRAM locked?+ ///+ if ((SmramControl
> & B_SA_SMRAMC_D_LCK_MASK) != 0) {+ ///+ /// Cannot Open a locked
> region+ ///+ for (Index = 0; Index <
> SmmAccess->NumberRegions; Index++) {+
> SmmAccess->SmramDesc[Index].RegionState |=
> EFI_SMRAM_LOCKED;+ }+ DEBUG ((DEBUG_WARN, "Cannot open a locked SMRAM
> region\n"));+ return
> EFI_DEVICE_ERROR;+ }+ ///+ /// Open SMRAM region+ ///+ SmramControl |=
> B_SA_SMRAMC_D_OPEN_MASK;+
> SmramControl &= ~(B_SA_SMRAMC_D_CLS_MASK);++ PciSegmentWrite8 (Address,
> SmramControl);+ ///+ /// END
> CHIPSET CODE+ ///++ SmmAccess->SmramDesc[DescriptorIndex].RegionState &=
> (UINT64) ~(EFI_SMRAM_CLOSED |
> EFI_ALLOCATED);+ SmmAccess->SmramDesc[DescriptorIndex].RegionState |=
> (UINT64) EFI_SMRAM_OPEN;+
> SmmAccess->SmmAccess.OpenState = TRUE;+ return EFI_SUCCESS;+}++/**+ This
> routine accepts a request to "close" a
> region of SMRAM. This is valid for+ compatible SMRAM region.++ @param[in]
> PeiServices - General purpose services
> available to every PEIM.+ @param[in] This - Pointer to the
> SMM Access Interface.+ @param[in] DescriptorIndex
> - Region of SMRAM to Close.++ @retval EFI_SUCCESS - The region
> was successfully closed.+ @retval
> EFI_DEVICE_ERROR - The region could not be closed because locked by+
> chipset.+ @retval
> EFI_INVALID_PARAMETER - The descriptor index was out of
> bounds.+**/+EFI_STATUS+EFIAPI+Close (+ IN
> EFI_PEI_SERVICES **PeiServices,+ IN EFI_PEI_MM_ACCESS_PPI *This,+
> IN UINTN DescriptorIndex+ )+{+
> SMM_ACCESS_PRIVATE_DATA *SmmAccess;+ BOOLEAN OpenState;+
> UINT8 Index;+ UINT64
> Address;+ UINT8 SmramControl;++ SmmAccess =
> SMM_ACCESS_PRIVATE_DATA_FROM_THIS (This);+ if
> (DescriptorIndex >= SmmAccess->NumberRegions) {+ DEBUG ((DEBUG_WARN,
> "SMRAM region out of range\n"));++
> return EFI_INVALID_PARAMETER;+ } else if
> (SmmAccess->SmramDesc[DescriptorIndex].RegionState &
> EFI_SMRAM_LOCKED) {+ //+ // Cannot close a "locked" region+ //+
> DEBUG ((DEBUG_WARN, "Cannot close a locked
> SMRAM region\n"));++ return EFI_DEVICE_ERROR;+ }++ if
> (SmmAccess->SmramDesc[DescriptorIndex].RegionState &
> EFI_SMRAM_CLOSED) {+ return EFI_DEVICE_ERROR;+ }++ ///+ /// BEGIN
> CHIPSET CODE+ ///+ ///+ /// SMRAM
> register is PCI 0:0:0:88, SMRAMC (8 bit)+ ///+ Address =
> PCI_SEGMENT_LIB_ADDRESS (0, SA_MC_BUS, SA_MC_DEV,
> SA_MC_FUN, R_SA_SMRAMC);+ SmramControl = PciSegmentRead8 (Address);+ ///+
> /// Is SMRAM locked?+ ///+ if
> ((SmramControl & B_SA_SMRAMC_D_LCK_MASK) != 0) {+ ///+ /// Cannot Close
> a locked region+ ///+ for (Index = 0;
> Index < SmmAccess->NumberRegions; Index++) {+
> SmmAccess->SmramDesc[Index].RegionState |=
> EFI_SMRAM_LOCKED;+ }+ DEBUG ((DEBUG_WARN, "Cannot close a locked SMRAM
> region\n"));+ return
> EFI_DEVICE_ERROR;+ }+ ///+ /// Close SMRAM region+ ///+ SmramControl &=
> ~(B_SA_SMRAMC_D_OPEN_MASK);++
> PciSegmentWrite8 (Address, SmramControl);+ ///+ /// END CHIPSET CODE+
> ///++ SmmAccess-
> >SmramDesc[DescriptorIndex].RegionState &= (UINT64) ~EFI_SMRAM_OPEN;+
> >SmmAccess-
> >SmramDesc[DescriptorIndex].RegionState |= (UINT64) (EFI_SMRAM_CLOSED |
> >EFI_ALLOCATED);++ //+ // Find out if any
> regions are still open+ //+ OpenState = FALSE;+ for (Index = 0; Index <
> SmmAccess->NumberRegions; Index++) {+ if
> ((SmmAccess->SmramDesc[Index].RegionState & EFI_SMRAM_OPEN) ==
> EFI_SMRAM_OPEN) {+ OpenState =
> TRUE;+ }+ }++ SmmAccess->SmmAccess.OpenState = OpenState;+ return
> EFI_SUCCESS;+}++/**+ This routine accepts a
> request to "lock" SMRAM. The+ region could be legacy AB or TSEG near top of
> physical memory.+ The use of "lock" means
> that the memory can no longer be opened+ to PEIM.++ @param[in] PeiServices
> - General purpose services available
> to every PEIM.+ @param[in] This - Pointer to the SMM Access
> Interface.+ @param[in] DescriptorIndex - Region
> of SMRAM to Lock.++ @retval EFI_SUCCESS - The region was
> successfully locked.+ @retval EFI_DEVICE_ERROR -
> The region could not be locked because at least+
> one range is still open.+ @retval
> EFI_INVALID_PARAMETER - The descriptor index was out of
> bounds.+**/+EFI_STATUS+EFIAPI+Lock (+ IN
> EFI_PEI_SERVICES **PeiServices,+ IN EFI_PEI_MM_ACCESS_PPI
> *This,+ IN UINTN DescriptorIndex+ )+{+
> SMM_ACCESS_PRIVATE_DATA *SmmAccess;+ UINT64 Address;+
> UINT8 SmramControl;++ SmmAccess =
> SMM_ACCESS_PRIVATE_DATA_FROM_THIS (This);+ if (DescriptorIndex >=
> SmmAccess->NumberRegions) {+ DEBUG
> ((DEBUG_WARN, "SMRAM region out of range\n"));++ return
> EFI_INVALID_PARAMETER;+ } else if (SmmAccess-
> >SmmAccess.OpenState) {+ DEBUG ((DEBUG_WARN, "Cannot lock SMRAM when SMRAM
> >regions are still open\n"));++
> return EFI_DEVICE_ERROR;+ }++
> SmmAccess->SmramDesc[DescriptorIndex].RegionState |= (UINT64)
> EFI_SMRAM_LOCKED;+ SmmAccess->SmmAccess.LockState = TRUE;++ ///+ /// BEGIN
> CHIPSET CODE+ ///+ ///+ ///
> SMRAM register is PCI 0:0:0:88, SMRAMC (8 bit)+ ///+ Address =
> PCI_SEGMENT_LIB_ADDRESS (0, SA_MC_BUS,
> SA_MC_DEV, SA_MC_FUN, R_SA_SMRAMC);+ SmramControl = PciSegmentRead8
> (Address);++ ///+ /// Lock the
> SMRAM+ ///+ SmramControl |= B_SA_SMRAMC_D_LCK_MASK;++ PciSegmentWrite8
> (Address, SmramControl);+ ///+
> /// END CHIPSET CODE+ ///++ return EFI_SUCCESS;+}++/**+ This routine
> services a user request to discover the
> SMRAM+ capabilities of this platform. This will report the possible+
> ranges that are possible for SMRAM access, based
> upon the+ memory controller capabilities.++ @param[in] PeiServices -
> General purpose services available to every
> PEIM.+ @param[in] This - Pointer to the SMRAM Access
> Interface.+ @param[in, out] SmramMapSize - Pointer to
> the variable containing size of the+ buffer
> to contain the description information.+ @param[in, out]
> SmramMap - Buffer containing the data describing the Smram+
> region descriptors.++ @retval
> EFI_BUFFER_TOO_SMALL - The user did not provide a sufficient buffer.+
> @retval EFI_SUCCESS - The user provided a
> sufficiently-sized buffer.+**/+EFI_STATUS+EFIAPI+GetCapabilities (+ IN
> EFI_PEI_SERVICES **PeiServices,+ IN
> EFI_PEI_MM_ACCESS_PPI *This,+ IN OUT UINTN
> *SmramMapSize,+ IN OUT EFI_SMRAM_DESCRIPTOR
> *SmramMap+ )+{+ EFI_STATUS Status;+ SMM_ACCESS_PRIVATE_DATA
> *SmmAccess;+ UINTN
> NecessaryBufferSize;++ SmmAccess =
> SMM_ACCESS_PRIVATE_DATA_FROM_THIS (This);+ NecessaryBufferSize =
> SmmAccess->NumberRegions * sizeof (EFI_SMRAM_DESCRIPTOR);+ if (*SmramMapSize
> < NecessaryBufferSize) {+
> DEBUG ((DEBUG_WARN, "SMRAM Map Buffer too small\n"));++ Status =
> EFI_BUFFER_TOO_SMALL;+ } else {+
> CopyMem (SmramMap, SmmAccess->SmramDesc, NecessaryBufferSize);+ Status =
> EFI_SUCCESS;+ }++ *SmramMapSize
> = NecessaryBufferSize;+ return Status;+}++/**+ This function is to install
> an SMM Access PPI+ - <b>Introduction</b> \n+
> An API to install an instance of EFI_PEI_MM_ACCESS_PPI. This PPI is commonly
> used to control SMM mode memory access
> for S3 resume.++ @retval EFI_SUCCESS - Ppi successfully started
> and installed.+ @retval EFI_NOT_FOUND - Ppi
> can't be found.+ @retval EFI_OUT_OF_RESOURCES - Ppi does not have enough
> resources to initialize the
> driver.+**/+EFI_STATUS+EFIAPI+PeiInstallSmmAccessPpi (+ VOID+ )+{+
> EFI_STATUS Status;+ UINTN
> Index;+ EFI_PEI_PPI_DESCRIPTOR *PpiList;+
> EFI_SMRAM_HOB_DESCRIPTOR_BLOCK *DescriptorBlock;+
> SMM_ACCESS_PRIVATE_DATA *SmmAccessPrivate;+ VOID
> *HobList;++ //+ // Initialize private data+
> //+ SmmAccessPrivate = AllocateZeroPool (sizeof (*SmmAccessPrivate));+
> ASSERT (SmmAccessPrivate != NULL);+ if
> (SmmAccessPrivate == NULL) {+ return EFI_OUT_OF_RESOURCES;+ }+ PpiList
> = AllocateZeroPool (sizeof
> (*PpiList));+ ASSERT (PpiList != NULL);+ if (PpiList == NULL) {+ return
> EFI_OUT_OF_RESOURCES;+ }++
> SmmAccessPrivate->Signature = SMM_ACCESS_PRIVATE_DATA_SIGNATURE;+
> SmmAccessPrivate->Handle = NULL;++
> //+ // Get Hob list+ //+ HobList = GetFirstGuidHob
> (&gEfiSmmSmramMemoryGuid);+ if (HobList == NULL) {+ DEBUG
> ((DEBUG_WARN, "SmramMemoryReserve HOB not found\n"));+ return
> EFI_NOT_FOUND;+ }++ DescriptorBlock =
> (EFI_SMRAM_HOB_DESCRIPTOR_BLOCK *) ((UINT8 *) HobList + sizeof
> (EFI_HOB_GUID_TYPE));++ //+ // Alloc space for
> SmmAccessPrivate->SmramDesc+ //+ SmmAccessPrivate->SmramDesc =
> AllocateZeroPool ((DescriptorBlock-
> >NumberOfSmmReservedRegions) * sizeof (EFI_SMRAM_DESCRIPTOR));+ if
> >(SmmAccessPrivate->SmramDesc == NULL)
> {+ DEBUG ((DEBUG_WARN, "Alloc SmmAccessPrivate->SmramDesc fail.\n"));+
> return EFI_OUT_OF_RESOURCES;+ }++
> DEBUG ((DEBUG_INFO, "Alloc SmmAccessPrivate->SmramDesc success.\n"));++ //+
> // use the hob to publish SMRAM
> capabilities+ //+ for (Index = 0; Index <
> DescriptorBlock->NumberOfSmmReservedRegions; Index++) {+
> SmmAccessPrivate->SmramDesc[Index].PhysicalStart =
> DescriptorBlock->Descriptor[Index].PhysicalStart;+
> SmmAccessPrivate->SmramDesc[Index].CpuStart =
> DescriptorBlock->Descriptor[Index].CpuStart;+
> SmmAccessPrivate->SmramDesc[Index].PhysicalSize =
> DescriptorBlock->Descriptor[Index].PhysicalSize;+
> SmmAccessPrivate->SmramDesc[Index].RegionState =
> DescriptorBlock->Descriptor[Index].RegionState;+ }++
> SmmAccessPrivate->NumberRegions = Index;+
> SmmAccessPrivate->SmmAccess.Open = Open;+
> SmmAccessPrivate->SmmAccess.Close = Close;+
> SmmAccessPrivate->SmmAccess.Lock = Lock;+
> SmmAccessPrivate->SmmAccess.GetCapabilities = GetCapabilities;+
> SmmAccessPrivate->SmmAccess.LockState =
> FALSE;+ SmmAccessPrivate->SmmAccess.OpenState = FALSE;++ //+ //
> Install PPI+ //+ PpiList->Flags =
> (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST);+
> PpiList->Guid =
> &gEfiPeiMmAccessPpiGuid;+ PpiList->Ppi = &SmmAccessPrivate->SmmAccess;++
> Status = PeiServicesInstallPpi
> (PpiList);+ ASSERT_EFI_ERROR (Status);++ return EFI_SUCCESS;+}diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLibSmramc/PeiSmmAccessLib.inf
> b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAccessLibSmramc/PeiSmmAccessLib.inf
> new file mode 100644
> index 000000000000..916346aacff3
> --- /dev/null
> +++ b/Silicon/Intel/IntelSiliconPkg/Feature/SmmAccess/Library/PeiSmmAcce
> +++ ssLibSmramc/PeiSmmAccessLib.inf
> @@ -0,0 +1,41 @@
> +## @file+# Library description file for the SmmAccess PPI+#+# Copyright
> +(c) 2019, Intel Corporation. All rights reserved.<BR>+#
> +SPDX-License-Identifier:
> +BSD-2-Clause-Patent+#+##++[Defines]+INF_VERSION = 0x00010017+BASE_NAME
> += PeiSmmAccessLibSmramc+FILE_GUID =
> +3D28FD4B-F46F-4E24-88AA-9DA09C51BE87+VERSION_STRING = 1.0+MODULE_TYPE =
> PEIM+LIBRARY_CLASS =
> SmmAccessLib+++[LibraryClasses]+BaseMemoryLib+MemoryAllocationLib+DebugLib+HobLib+PciSegmentLib+PeiServicesL
> ib+++[Packages]+MdePkg/MdePkg.dec+IntelSiliconPkg/IntelSiliconPkg.dec+++[Sources]+PeiSmmAccessLib.c+++[Ppis]+gE
> fiPeiMmAccessPpiGuid ## PRODUCES+++[Guids]+gEfiSmmSmramMemoryGuid--
> 2.37.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92938): https://edk2.groups.io/g/devel/message/92938
Mute This Topic: https://groups.io/mt/93335518/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
