Thanks for the suggestions, will update them in V3.
Thanks! Chasel > -----Original Message----- > From: Kubacki, Michael A <michael.a.kuba...@intel.com> > Sent: Thursday, October 31, 2019 2:01 PM > To: Chiu, Chasel <chasel.c...@intel.com>; devel@edk2.groups.io > Cc: Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Gao, Liming > <liming....@intel.com> > Subject: RE: [edk2-platforms: PATCH v2 1/6] MinPlatformPkg: Add > SetCacheLib library class. > > 1. It would be good to list the new library class in the > [LibraryClasses] section of MinPlatformPkg.dec. > 2. I think it would be more descriptive to call these library > instances BaseSetCacheLibNull and BaseSetCacheLib. > 3. It would be helpful to provide a description somewhere such as > SetCacheLib.inf that explains in what scenarios that library > instance would be useful. > 4. The [Pcd] tag can be removed from SetCacheLibNull.inf since the > section is empty. > > Thanks, > Michael > > > -----Original Message----- > > From: Chiu, Chasel <chasel.c...@intel.com> > > Sent: Wednesday, October 30, 2019 5:30 PM > > To: devel@edk2.groups.io > > Cc: Kubacki, Michael A <michael.a.kuba...@intel.com>; Desimone, > > Nathaniel L <nathaniel.l.desim...@intel.com>; Gao, Liming > > <liming....@intel.com> > > Subject: [edk2-platforms: PATCH v2 1/6] MinPlatformPkg: Add > > SetCacheLib library class. > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2314 > > > > MinPlatformPkg should contain the library class header (API) and the > > NULL library class instance. > > > > Cc: Michael Kubacki <michael.a.kuba...@intel.com> > > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> > > Cc: Liming Gao <liming....@intel.com> > > Signed-off-by: Chasel Chiu <chasel.c...@intel.com> > > --- > > Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLib.c > | 325 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > +++++++++++++++++++++++++++++++++++ > > Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLibNull.c > | > > 37 +++++++++++++++++++++++++++++++++++++ > > Platform/Intel/MinPlatformPkg/Include/Library/SetCacheLib.h > | 34 > > ++++++++++++++++++++++++++++++++++ > > Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLib.inf > | 44 > > ++++++++++++++++++++++++++++++++++++++++++++ > > Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLibNull.inf > > | > > 30 ++++++++++++++++++++++++++++++ > > 5 files changed, 470 insertions(+) > > > > diff --git > > a/Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLib.c > > b/Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLib.c > > new file mode 100644 > > index 0000000000..b5c5041430 > > --- /dev/null > > +++ b/Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLib.c > > @@ -0,0 +1,325 @@ > > +/** @file > > + > > +SetCache library functions. > > + > > +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> > > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#include <Uefi.h> > > +#include <PiPei.h> > > +#include <Library/HobLib.h> > > +#include <Library/PcdLib.h> > > +#include <Library/DebugLib.h> > > +#include <Library/BaseMemoryLib.h> > > +#include <Library/MtrrLib.h> > > +#include <Library/PeiServicesLib.h> > > +#include <Guid/SmramMemoryReserve.h> > > + > > +/** > > + Set Cache Mtrr. > > +**/ > > +VOID > > +EFIAPI > > +SetCacheMtrr ( > > + VOID > > + ) > > +{ > > + EFI_STATUS Status; > > + EFI_PEI_HOB_POINTERS Hob; > > + MTRR_SETTINGS MtrrSetting; > > + UINT64 MemoryBase; > > + UINT64 MemoryLength; > > + UINT64 LowMemoryLength; > > + UINT64 HighMemoryLength; > > + EFI_BOOT_MODE BootMode; > > + EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttribute; > > + UINT64 CacheMemoryLength; > > + > > + /// > > + /// Reset all MTRR setting. > > + /// > > + ZeroMem(&MtrrSetting, sizeof(MTRR_SETTINGS)); > > + > > + /// > > + /// Cache the Flash area as WP to boost performance /// Status = > > + MtrrSetMemoryAttributeInMtrrSettings ( > > + &MtrrSetting, > > + (UINTN) PcdGet32 (PcdFlashAreaBaseAddress), > > + (UINTN) PcdGet32 (PcdFlashAreaSize), > > + CacheWriteProtected > > + ); > > + ASSERT_EFI_ERROR (Status); > > + > > + /// > > + /// Update MTRR setting from MTRR buffer for Flash Region to be WP > > + to boost performance /// MtrrSetAllMtrrs (&MtrrSetting); > > + > > + /// > > + /// Set low to 1 MB. Since 1MB cacheability will always be set /// > > + until override by CSM. > > + /// Initialize high memory to 0. > > + /// > > + LowMemoryLength = 0x100000; > > + HighMemoryLength = 0; > > + ResourceAttribute = ( > > + EFI_RESOURCE_ATTRIBUTE_PRESENT | > > + EFI_RESOURCE_ATTRIBUTE_INITIALIZED | > > + EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE | > > + > EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE | > > + > EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE | > > + > EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE > > + ); > > + > > + Status = PeiServicesGetBootMode (&BootMode); ASSERT_EFI_ERROR > > + (Status); > > + > > + if (BootMode != BOOT_ON_S3_RESUME) { > > + ResourceAttribute |= EFI_RESOURCE_ATTRIBUTE_TESTED; } > > + > > + Status = PeiServicesGetHobList ((VOID **) &Hob.Raw); while > > + (!END_OF_HOB_LIST (Hob)) { > > + if (Hob.Header->HobType == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR) > { > > + if ((Hob.ResourceDescriptor->ResourceType == > > EFI_RESOURCE_SYSTEM_MEMORY) || > > + ((Hob.ResourceDescriptor->ResourceType == > > EFI_RESOURCE_MEMORY_RESERVED) && > > + (Hob.ResourceDescriptor->ResourceAttribute == > ResourceAttribute)) > > + ) { > > + if (Hob.ResourceDescriptor->PhysicalStart >= 0x100000000ULL) { > > + HighMemoryLength += > Hob.ResourceDescriptor->ResourceLength; > > + } else if (Hob.ResourceDescriptor->PhysicalStart >= 0x100000) { > > + LowMemoryLength += > Hob.ResourceDescriptor->ResourceLength; > > + } > > + } > > + } > > + > > + Hob.Raw = GET_NEXT_HOB (Hob); > > + } > > + > > + DEBUG ((DEBUG_INFO, "Memory Length (Below 4GB) = %lx.\n", > > + LowMemoryLength)); DEBUG ((DEBUG_INFO, "Memory Length (Above > > 4GB) = > > + %lx.\n", HighMemoryLength)); > > + > > + /// > > + /// Assume size of main memory is multiple of 256MB /// > > + MemoryLength = (LowMemoryLength + 0xFFFFFFF) & 0xF0000000; > > MemoryBase > > + = 0; > > + > > + CacheMemoryLength = MemoryLength; > > + /// > > + /// Programming MTRRs to avoid override SPI region with UC when > MAX > > + TOLUD Length >= 3.5GB /// if (MemoryLength > 0xDC000000) { > > + CacheMemoryLength = 0xC0000000; > > + Status = MtrrSetMemoryAttributeInMtrrSettings ( > > + &MtrrSetting, > > + MemoryBase, > > + CacheMemoryLength, > > + CacheWriteBack > > + ); > > + ASSERT_EFI_ERROR (Status); > > + > > + MemoryBase = 0xC0000000; > > + CacheMemoryLength = MemoryLength - 0xC0000000; > > + if (MemoryLength > 0xE0000000) { > > + CacheMemoryLength = 0x20000000; > > + Status = MtrrSetMemoryAttributeInMtrrSettings ( > > + &MtrrSetting, > > + MemoryBase, > > + CacheMemoryLength, > > + CacheWriteBack > > + ); > > + ASSERT_EFI_ERROR (Status); > > + > > + MemoryBase = 0xE0000000; > > + CacheMemoryLength = MemoryLength - 0xE0000000; > > + } > > + } > > + > > + Status = MtrrSetMemoryAttributeInMtrrSettings ( > > + &MtrrSetting, > > + MemoryBase, > > + CacheMemoryLength, > > + CacheWriteBack > > + ); > > + ASSERT_EFI_ERROR (Status); > > + > > + if (LowMemoryLength != MemoryLength) { > > + MemoryBase = LowMemoryLength; > > + MemoryLength -= LowMemoryLength; > > + Status = MtrrSetMemoryAttributeInMtrrSettings ( > > + &MtrrSetting, > > + MemoryBase, > > + MemoryLength, > > + CacheUncacheable > > + ); > > + ASSERT_EFI_ERROR (Status); > > + } > > + > > + /// > > + /// VGA-MMIO - 0xA0000 to 0xC0000 to be UC /// Status = > > + MtrrSetMemoryAttributeInMtrrSettings ( > > + &MtrrSetting, > > + 0xA0000, > > + 0x20000, > > + CacheUncacheable > > + ); > > + ASSERT_EFI_ERROR (Status); > > + > > + /// > > + /// Update MTRR setting from MTRR buffer /// MtrrSetAllMtrrs > > + (&MtrrSetting); > > + > > + return ; > > +} > > + > > +/** > > + Update MTRR setting and set write back as default memory attribute. > > + > > + @retval EFI_SUCCESS The function completes successfully. > > + @retval Others Some error occurs. > > +**/ > > +EFI_STATUS > > +EFIAPI > > +SetCacheMtrrAfterEndOfPei ( > > + VOID > > + ) > > +{ > > + EFI_STATUS Status; > > + MTRR_SETTINGS MtrrSetting; > > + EFI_PEI_HOB_POINTERS Hob; > > + UINT64 MemoryBase; > > + UINT64 MemoryLength; > > + UINT64 Power2Length; > > + EFI_BOOT_MODE BootMode; > > + UINTN Index; > > + UINT64 SmramSize; > > + UINT64 SmramBase; > > + EFI_SMRAM_HOB_DESCRIPTOR_BLOCK > *SmramHobDescriptorBlock; > > + Status = PeiServicesGetBootMode (&BootMode); > > + ASSERT_EFI_ERROR (Status); > > + > > + if (BootMode == BOOT_ON_S3_RESUME) { > > + return EFI_SUCCESS; > > + } > > + // > > + // Clear the CAR Settings > > + // > > + ZeroMem(&MtrrSetting, sizeof(MTRR_SETTINGS)); > > + > > + // > > + // Default Cachable attribute will be set to WB to support large > > + memory size/hot plug memory // MtrrSetting.MtrrDefType &= > > + ~((UINT64)(0xFF)); MtrrSetting.MtrrDefType |= (UINT64) > > + CacheWriteBack; > > + > > + // > > + // Set fixed cache for memory range below 1MB // Status = > > + MtrrSetMemoryAttributeInMtrrSettings ( > > + &MtrrSetting, > > + 0x0, > > + 0xA0000, > > + CacheWriteBack > > + ); > > + ASSERT_EFI_ERROR (Status); > > + > > + Status = MtrrSetMemoryAttributeInMtrrSettings ( > > + &MtrrSetting, > > + 0xA0000, > > + 0x20000, > > + CacheUncacheable > > + ); > > + ASSERT_EFI_ERROR (Status); > > + > > + Status = MtrrSetMemoryAttributeInMtrrSettings ( > > + &MtrrSetting, > > + 0xC0000, > > + 0x40000, > > + CacheWriteProtected > > + ); > > + ASSERT_EFI_ERROR ( Status); > > + > > + // > > + // PI SMM IPL can't set SMRAM to WB because at that time CPU ARCH > > protocol is not available. > > + // Set cacheability of SMRAM to WB here to improve SMRAM > > + initialization > > performance. > > + // > > + SmramSize = 0; > > + SmramBase = 0; > > + Status = PeiServicesGetHobList ((VOID **) &Hob.Raw); while > > + (!END_OF_HOB_LIST (Hob)) { > > + if (Hob.Header->HobType == EFI_HOB_TYPE_GUID_EXTENSION) { > > + if (CompareGuid (&Hob.Guid->Name, > &gEfiSmmSmramMemoryGuid)) { > > + SmramHobDescriptorBlock = > (EFI_SMRAM_HOB_DESCRIPTOR_BLOCK > > *) (Hob.Guid + 1); > > + for (Index = 0; Index < SmramHobDescriptorBlock- > > >NumberOfSmmReservedRegions; Index++) { > > + if > > + (SmramHobDescriptorBlock->Descriptor[Index].PhysicalStart > > > 0x100000) { > > + SmramSize += SmramHobDescriptorBlock- > > >Descriptor[Index].PhysicalSize; > > + if (SmramBase == 0 || SmramBase > > > + SmramHobDescriptorBlock- > > >Descriptor[Index].CpuStart) { > > + SmramBase = SmramHobDescriptorBlock- > > >Descriptor[Index].CpuStart; > > + } > > + } > > + } > > + break; > > + } > > + } > > + Hob.Raw = GET_NEXT_HOB (Hob); > > + } > > + > > + // > > + // Set non system memory as UC > > + // > > + MemoryBase = 0x100000000; > > + > > + // > > + // Add IED size to set whole SMRAM as WB to save MTRR count // > > + MemoryLength = MemoryBase - (SmramBase + SmramSize); while > > + (MemoryLength != 0) { > > + Power2Length = GetPowerOfTwo64 (MemoryLength); > > + MemoryBase -= Power2Length; > > + Status = MtrrSetMemoryAttributeInMtrrSettings ( > > + &MtrrSetting, > > + MemoryBase, > > + Power2Length, > > + CacheUncacheable > > + ); > > + ASSERT_EFI_ERROR (Status); > > + MemoryLength -= Power2Length; > > + } > > + > > + DEBUG ((DEBUG_INFO, "PcdPciReservedMemAbove4GBLimit - 0x%lx\n", > > + PcdGet64 (PcdPciReservedMemAbove4GBLimit))); > > + DEBUG ((DEBUG_INFO, "PcdPciReservedMemAbove4GBBase - 0x%lx\n", > > + PcdGet64 (PcdPciReservedMemAbove4GBBase))); > > + if (PcdGet64 (PcdPciReservedMemAbove4GBLimit) > PcdGet64 > > (PcdPciReservedMemAbove4GBBase)) { > > + Status = MtrrSetMemoryAttributeInMtrrSettings ( > > + &MtrrSetting, > > + PcdGet64 > (PcdPciReservedMemAbove4GBBase), > > + PcdGet64 > (PcdPciReservedMemAbove4GBLimit) > > + - PcdGet64 > > (PcdPciReservedMemAbove4GBBase) + 1, > > + CacheUncacheable > > + ); > > + ASSERT_EFI_ERROR ( Status); > > + } > > + > > + DEBUG ((DEBUG_INFO, "PcdPciReservedPMemAbove4GBLimit - > 0x%lx\n", > > + PcdGet64 (PcdPciReservedPMemAbove4GBLimit))); > > + DEBUG ((DEBUG_INFO, "PcdPciReservedPMemAbove4GBBase - > 0x%lx\n", > > + PcdGet64 (PcdPciReservedPMemAbove4GBBase))); > > + if (PcdGet64 (PcdPciReservedPMemAbove4GBLimit) > PcdGet64 > > (PcdPciReservedPMemAbove4GBBase)) { > > + Status = MtrrSetMemoryAttributeInMtrrSettings ( > > + &MtrrSetting, > > + PcdGet64 > (PcdPciReservedPMemAbove4GBBase), > > + PcdGet64 > (PcdPciReservedPMemAbove4GBLimit) > > + - PcdGet64 > > (PcdPciReservedPMemAbove4GBBase) + 1, > > + CacheUncacheable > > + ); > > + ASSERT_EFI_ERROR ( Status); > > + } > > + > > + // > > + // Update MTRR setting from MTRR buffer // MtrrSetAllMtrrs > > + (&MtrrSetting); > > + > > + return Status; > > +} > > diff --git > > a/Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLibNull.c > > b/Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLibNull.c > > new file mode 100644 > > index 0000000000..581bc7648b > > --- /dev/null > > +++ > b/Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLibNull. > > +++ c > > @@ -0,0 +1,37 @@ > > +/** @file > > + > > +NULL instances of SetCache library functions. > > + > > +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> > > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > +#include <Uefi.h> > > +#include <Library/PcdLib.h> > > +#include <Library/DebugLib.h> > > + > > +/** > > + Set Cache Mtrr. > > +**/ > > +VOID > > +EFIAPI > > +SetCacheMtrr ( > > + VOID > > + ) > > +{ > > + return; > > +} > > + > > +/** > > + Update MTRR setting and set write back as default memory attribute. > > + > > + @retval EFI_SUCCESS The function completes successfully. > > +**/ > > +EFI_STATUS > > +EFIAPI > > +SetCacheMtrrAfterEndOfPei ( > > + VOID > > + ) > > +{ > > + return EFI_SUCCESS; > > +} > > diff --git > > a/Platform/Intel/MinPlatformPkg/Include/Library/SetCacheLib.h > > b/Platform/Intel/MinPlatformPkg/Include/Library/SetCacheLib.h > > new file mode 100644 > > index 0000000000..d67426cef7 > > --- /dev/null > > +++ b/Platform/Intel/MinPlatformPkg/Include/Library/SetCacheLib.h > > @@ -0,0 +1,34 @@ > > +/** @file > > + > > +Header for SetCache library functions. > > + > > +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> > > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#ifndef _SET_CACHE_LIB_H_ > > +#define _SET_CACHE_LIB_H_ > > + > > +/** > > + Set Cache Mtrr. > > +**/ > > +VOID > > +EFIAPI > > +SetCacheMtrr ( > > + VOID > > + ); > > + > > +/** > > + Update MTRR setting and set write back as default memory attribute. > > + > > + @retval EFI_SUCCESS The function completes successfully. > > + @retval Others Some error occurs. > > +**/ > > +EFI_STATUS > > +EFIAPI > > +SetCacheMtrrAfterEndOfPei ( > > + VOID > > + ); > > + > > +#endif > > diff --git > > a/Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLib.inf > > b/Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLib.inf > > new file mode 100644 > > index 0000000000..a53aed858f > > --- /dev/null > > +++ b/Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLib.in > > +++ f > > @@ -0,0 +1,44 @@ > > +## @file > > +# Component information file for Platform SetCache Library # # > > +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> # # > > +SPDX-License-Identifier: BSD-2-Clause-Patent # ## > > + > > +[Defines] > > + INF_VERSION = 0x00010005 > > + BASE_NAME = SetCacheLib > > + FILE_GUID = > 9F2A2899-3AD7-4176-9B89-33B3AC456A99 > > + MODULE_TYPE = BASE > > + VERSION_STRING = 1.0 > > + LIBRARY_CLASS = SetCacheLib > > + > > +[LibraryClasses] > > + BaseLib > > + PcdLib > > + DebugLib > > + HobLib > > + MtrrLib > > + PeiServicesLib > > + BaseMemoryLib > > + > > +[Packages] > > + MinPlatformPkg/MinPlatformPkg.dec > > + MdePkg/MdePkg.dec > > + UefiCpuPkg/UefiCpuPkg.dec > > + > > +[Sources] > > + SetCacheLib.c > > + > > +[Guids] > > + gEfiSmmSmramMemoryGuid ## > CONSUMES > > + > > +[Pcd] > > + gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress > ## > > CONSUMES > > + gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize > ## > > CONSUMES > > + gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBBase > > ## CONSUMES > > + gMinPlatformPkgTokenSpaceGuid.PcdPciReservedMemAbove4GBLimit > > ## > > +CONSUMES > > + gMinPlatformPkgTokenSpaceGuid.PcdPciReservedPMemAbove4GBBase > > ## > > +CONSUMES > > + gMinPlatformPkgTokenSpaceGuid.PcdPciReservedPMemAbove4GBLimit > > ## > > +CONSUMES > > diff --git > > a/Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLibNull.in > > f > > b/Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLibNull.in > > f > > new file mode 100644 > > index 0000000000..50419b398b > > --- /dev/null > > +++ > b/Platform/Intel/MinPlatformPkg/Library/SetCacheLib/SetCacheLibNull. > > +++ inf > > @@ -0,0 +1,30 @@ > > +## @file > > +# Component information file for Platform SetCache Library # # > > +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> # # > > +SPDX-License-Identifier: BSD-2-Clause-Patent # ## > > + > > +[Defines] > > + INF_VERSION = 0x00010005 > > + BASE_NAME = SetCacheLibNull > > + FILE_GUID = > D1ED4CD7-AD20-4943-9192-3ABE766A9411 > > + MODULE_TYPE = BASE > > + VERSION_STRING = 1.0 > > + LIBRARY_CLASS = SetCacheLib > > + > > +[LibraryClasses] > > + BaseLib > > + PcdLib > > + DebugLib > > + > > +[Packages] > > + MinPlatformPkg/MinPlatformPkg.dec > > + MdePkg/MdePkg.dec > > + > > +[Sources] > > + SetCacheLibNull.c > > + > > +[Pcd] > > -- > > 2.13.3.windows.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#49751): https://edk2.groups.io/g/devel/message/49751 Mute This Topic: https://groups.io/mt/40049898/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
