Re: [Patch 2/2] UefiCpuPkg/CpuS3DataDxe: Change Memory Type and address limitation.

["Dong, Eric" <eric.dong@intel.com>]

Please ignore this change which forgot to change version to v2.

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Eric
> Dong
> Sent: Wednesday, August 8, 2018 3:34 PM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [edk2] [Patch 2/2] UefiCpuPkg/CpuS3DataDxe: Change Memory Type
> and address limitation.
> 
> Because CpuS3Data memory will be copy to smram at SmmReadToLock point,
> so the memory type no need to be ACPI NVS type, also the address not limit to
> below 4G.
> This change remove the limit of ACPI NVS memory type and below 4G.
> 
> Pass OS boot and resume from S3 test.
> 
> Bugz: https://bugzilla.tianocore.org/show_bug.cgi?id=959
> 
> Reported-by: Marvin Häuser <Marvin.Haeuser@outlook.com>
> Suggested-by: Fan Jeff <vanjeff_919@hotmail.com>
> Cc: Marvin Häuser <Marvin.Haeuser@outlook.com>
> Cc: Fan Jeff <vanjeff_919@hotmail.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c      | 60 +++++++-------------------------
>  UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf |  1 +
>  2 files changed, 14 insertions(+), 47 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> index dccb406b8d..d8eb8c976f 100644
> --- a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> +++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> @@ -31,6 +31,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/MtrrLib.h>
> +#include <Library/MemoryAllocationLib.h>
> 
>  #include <Protocol/MpService.h>
>  #include <Guid/EventGroup.h>
> @@ -45,42 +46,6 @@ typedef struct {
>    IA32_DESCRIPTOR           IdtrProfile;
>  } ACPI_CPU_DATA_EX;
> 
> -/**
> -  Allocate EfiACPIMemoryNVS below 4G memory address.
> -
> -  This function allocates EfiACPIMemoryNVS below 4G memory address.
> -
> -  @param[in] Size   Size of memory to allocate.
> -
> -  @return       Allocated address for output.
> -
> -**/
> -VOID *
> -AllocateAcpiNvsMemoryBelow4G (
> -  IN UINTN  Size
> -  )
> -{
> -  EFI_PHYSICAL_ADDRESS  Address;
> -  EFI_STATUS            Status;
> -  VOID                  *Buffer;
> -
> -  Address = BASE_4GB - 1;
> -  Status  = gBS->AllocatePages (
> -                   AllocateMaxAddress,
> -                   EfiACPIMemoryNVS,
> -                   EFI_SIZE_TO_PAGES (Size),
> -                   &Address
> -                   );
> -  if (EFI_ERROR (Status)) {
> -    return NULL;
> -  }
> -
> -  Buffer = (VOID *)(UINTN)Address;
> -  ZeroMem (Buffer, Size);
> -
> -  return Buffer;
> -}
> -
>  /**
>    Callback function executed when the EndOfDxe event group is signaled.
> 
> @@ -150,7 +115,6 @@ CpuS3DataInitialize (
>    EFI_MP_SERVICES_PROTOCOL   *MpServices;
>    UINTN                      NumberOfCpus;
>    UINTN                      NumberOfEnabledProcessors;
> -  VOID                       *Stack;
>    UINTN                      TableSize;
>    CPU_REGISTER_TABLE         *RegisterTable;
>    UINTN                      Index;
> @@ -171,10 +135,7 @@ CpuS3DataInitialize (
>    //
>    OldAcpiCpuData = (ACPI_CPU_DATA *) (UINTN) PcdGet64
> (PcdCpuS3DataAddress);
> 
> -  //
> -  // Allocate ACPI NVS memory below 4G memory for use on ACPI S3 resume.
> -  //
> -  AcpiCpuDataEx = AllocateAcpiNvsMemoryBelow4G (sizeof
> (ACPI_CPU_DATA_EX));
> +  AcpiCpuDataEx = AllocatePages (EFI_SIZE_TO_PAGES (sizeof
> + (ACPI_CPU_DATA_EX)));
>    ASSERT (AcpiCpuDataEx != NULL);
>    AcpiCpuData = &AcpiCpuDataEx->AcpiCpuData;
> 
> @@ -210,11 +171,16 @@ CpuS3DataInitialize (
>    AcpiCpuData->MtrrTable    =
> (EFI_PHYSICAL_ADDRESS)(UINTN)&AcpiCpuDataEx->MtrrTable;
> 
>    //
> -  // Allocate stack space for all CPUs
> +  // Allocate stack space for all CPUs, use ACPI NVS memory type
> + because it will  // not copy to smram at Smm ready to lock point.
>    //
> -  Stack = AllocateAcpiNvsMemoryBelow4G (NumberOfCpus * AcpiCpuData-
> >StackSize);
> -  ASSERT (Stack != NULL);
> -  AcpiCpuData->StackAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)Stack;
> +  Status  = gBS->AllocatePages (
> +                   AllocateAnyPages,
> +                   EfiACPIMemoryNVS,
> +                   EFI_SIZE_TO_PAGES (NumberOfCpus * AcpiCpuData->StackSize),
> +                   &AcpiCpuData->StackAddress
> +                   );
> +  ASSERT_EFI_ERROR (Status);
> 
>    //
>    // Get the boot processor's GDT and IDT @@ -227,7 +193,7 @@
> CpuS3DataInitialize (
>    //
>    GdtSize = AcpiCpuDataEx->GdtrProfile.Limit + 1;
>    IdtSize = AcpiCpuDataEx->IdtrProfile.Limit + 1;
> -  Gdt = AllocateAcpiNvsMemoryBelow4G (GdtSize + IdtSize);
> +  Gdt = AllocatePages (EFI_SIZE_TO_PAGES (GdtSize + IdtSize));
>    ASSERT (Gdt != NULL);
>    Idt = (VOID *)((UINTN)Gdt + GdtSize);
>    CopyMem (Gdt, (VOID *)AcpiCpuDataEx->GdtrProfile.Base, GdtSize); @@ -
> 243,7 +209,7 @@ CpuS3DataInitialize (
>      // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable
> for all CPUs
>      //
>      TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
> -    RegisterTable = (CPU_REGISTER_TABLE
> *)AllocateAcpiNvsMemoryBelow4G (TableSize);
> +    RegisterTable = AllocatePages (EFI_SIZE_TO_PAGES (TableSize));
>      ASSERT (RegisterTable != NULL);
> 
>      for (Index = 0; Index < NumberOfCpus; Index++) { diff --git
> a/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
> b/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
> index 480c98ebcd..c16731529c 100644
> --- a/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
> +++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
> @@ -51,6 +51,7 @@
>    DebugLib
>    BaseLib
>    MtrrLib
> +  MemoryAllocationLib
> 
>  [Guids]
>    gEfiEndOfDxeEventGroupGuid         ## CONSUMES   ## Event
> --
> 2.15.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel