Reviewed-by: Ray Ni <ray...@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Zhiguang Liu
> Sent: Tuesday, April 25, 2023 3:03 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang....@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desim...@intel.com>; Ni, Ray <ray...@intel.com>
> Subject: [edk2-devel] [PATCH 2/5] SimicsOpenBoardPkg: Move
> AcpiVariableGuid hob to MemDetect
>
> Currently, MemDetect create gEfiSmmSmramMemoryGuid Hob containing
> one
> descriptor, which should be updated later, when AcpiVariableGuid hob
> use some buffer from SmRam. However, the Hob doesn't get updated, and
> this is a bug.
>
> Move the logic creating AcpiVariableGuid hob from PEIM SmmAccessPei.inf
> to MemDetect, so that in the same file, it has both knowleage about
> the smmram and the acpi data structure. So it can create the
> gEfiSmmSmramMemoryGuid Hob containing two descriptors.
>
> Cc: Nate DeSimone <nathaniel.l.desim...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang....@intel.com>
> ---
> .../SimicsOpenBoardPkg/SimicsPei/MemDetect.c | 36 +++++++++++--------
> .../SimicsPei/SimicsPei.inf | 1 +
> .../SimicsX58SktPkg/Smm/Access/SmmAccessPei.c | 8 -----
> .../Smm/Access/SmmAccessPei.inf | 3 --
> 4 files changed, 22 insertions(+), 26 deletions(-)
>
> diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> index d80ac1d213..13ee415f40 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/MemDetect.c
> @@ -391,11 +391,10 @@ QemuInitializeRam (
> UINT64 LowerMemorySize;
> UINT64 UpperMemorySize;
> UINTN BufferSize;
> - UINT8 SmramIndex;
> UINT8 SmramRanges;
> EFI_PEI_HOB_POINTERS Hob;
> EFI_SMRAM_HOB_DESCRIPTOR_BLOCK *SmramHobDescriptorBlock;
> - UINT8 Index;
> + VOID *GuidHob;
>
> DEBUG ((EFI_D_INFO, "%a called\n", __FUNCTION__));
>
> @@ -418,8 +417,8 @@ QemuInitializeRam (
> AddReservedMemoryBaseSizeHob (LowerMemorySize - TsegSize,
> TsegSize,
> TRUE);
>
> - BufferSize = sizeof(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK);
> - SmramRanges = 1;
> + SmramRanges = 2;
> + BufferSize = sizeof(EFI_SMRAM_HOB_DESCRIPTOR_BLOCK) +
> (SmramRanges - 1) * sizeof(EFI_SMRAM_DESCRIPTOR);
>
> Hob.Raw = BuildGuidHob(
> &gEfiSmmSmramMemoryGuid,
> @@ -430,18 +429,25 @@ QemuInitializeRam (
> SmramHobDescriptorBlock = (EFI_SMRAM_HOB_DESCRIPTOR_BLOCK
> *)(Hob.Raw);
> SmramHobDescriptorBlock->NumberOfSmmReservedRegions =
> SmramRanges;
>
> - SmramIndex = 0;
> - for (Index = 0; Index < SmramRanges; Index++) {
> - //
> - // This is an SMRAM range, create an SMRAM descriptor
> - //
> - SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalStart =
> LowerMemorySize - TsegSize;
> - SmramHobDescriptorBlock->Descriptor[SmramIndex].CpuStart =
> LowerMemorySize - TsegSize;
> - SmramHobDescriptorBlock->Descriptor[SmramIndex].PhysicalSize =
> TsegSize;
> - SmramHobDescriptorBlock->Descriptor[SmramIndex].RegionState =
> EFI_SMRAM_CLOSED | EFI_CACHEABLE;
> - SmramIndex++;
> - }
> + //
> + // Create first SMRAM descriptor, which contains data structures used in
> S3 resume.
> + // One page is enough for the data structure
> + //
> + SmramHobDescriptorBlock->Descriptor[0].PhysicalStart =
> LowerMemorySize - TsegSize;
> + SmramHobDescriptorBlock->Descriptor[0].CpuStart = LowerMemorySize -
> TsegSize;
> + SmramHobDescriptorBlock->Descriptor[0].PhysicalSize = EFI_PAGE_SIZE;
> + SmramHobDescriptorBlock->Descriptor[0].RegionState =
> EFI_SMRAM_CLOSED | EFI_CACHEABLE | EFI_ALLOCATED;
> + GuidHob = BuildGuidHob (&gEfiAcpiVariableGuid,
> sizeof(EFI_SMRAM_DESCRIPTOR));
> + ASSERT (GuidHob != NULL);
> + CopyMem (GuidHob, &SmramHobDescriptorBlock->Descriptor[0],
> sizeof(EFI_SMRAM_DESCRIPTOR));
>
> + //
> + // Create second SMRAM descriptor, which is free and will be used by
> SMM foundation.
> + //
> + SmramHobDescriptorBlock->Descriptor[1].PhysicalStart =
> SmramHobDescriptorBlock->Descriptor[0].PhysicalStart + EFI_PAGE_SIZE;
> + SmramHobDescriptorBlock->Descriptor[1].CpuStart =
> SmramHobDescriptorBlock->Descriptor[0].CpuStart + EFI_PAGE_SIZE;
> + SmramHobDescriptorBlock->Descriptor[1].PhysicalSize = TsegSize -
> EFI_PAGE_SIZE;
> + SmramHobDescriptorBlock->Descriptor[1].RegionState =
> EFI_SMRAM_CLOSED | EFI_CACHEABLE;
> } else {
> AddMemoryRangeHob (BASE_1MB, LowerMemorySize);
> }
> diff --git a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> index 710fa680be..618ad4075f 100644
> --- a/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> +++ b/Platform/Intel/SimicsOpenBoardPkg/SimicsPei/SimicsPei.inf
> @@ -40,6 +40,7 @@
> [Guids]
> gEfiMemoryTypeInformationGuid
> gEfiSmmSmramMemoryGuid ## CONSUMES
> + gEfiAcpiVariableGuid
>
> [LibraryClasses]
> BaseLib
> diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
> b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
> index c54026b4d1..d489cc7513 100644
> --- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
> +++ b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.c
> @@ -241,7 +241,6 @@ SmmAccessPeiEntryPoint (
> EFI_STATUS Status;
> UINTN SmramMapSize;
> EFI_SMRAM_DESCRIPTOR SmramMap[DescIdxCount];
> - VOID *GuidHob;
>
> //
> // This module should only be included if SMRAM support is required.
> @@ -322,13 +321,6 @@ SmmAccessPeiEntryPoint (
> }
> DEBUG_CODE_END ();
>
> - GuidHob = BuildGuidHob (&gEfiAcpiVariableGuid, sizeof
> SmramMap[DescIdxSmmS3ResumeState]);
> - if (GuidHob == NULL) {
> - return EFI_OUT_OF_RESOURCES;
> - }
> -
> - CopyMem (GuidHob, &SmramMap[DescIdxSmmS3ResumeState], sizeof
> SmramMap[DescIdxSmmS3ResumeState]);
> -
> //
> // We're done. The next step should succeed, but even if it fails, we can't
> // roll back the above BuildGuidHob() allocation, because PEI doesn't
> support
> diff --git a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
> b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
> index 2b6b14f437..3c71e64fe9 100644
> --- a/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
> +++ b/Silicon/Intel/SimicsX58SktPkg/Smm/Access/SmmAccessPei.inf
> @@ -38,9 +38,6 @@
> SimicsX58SktPkg/SktPkg.dec
> SimicsIch10Pkg/Ich10Pkg.dec
>
> -[Guids]
> - gEfiAcpiVariableGuid
> -
> [LibraryClasses]
> BaseLib
> BaseMemoryLib
> --
> 2.31.1.windows.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103543): https://edk2.groups.io/g/devel/message/103543
Mute This Topic: https://groups.io/mt/98488192/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
