Re: [Patch v3 3/5] UefiCpuPkg/CpuS3DataDxe: Change Memory Type and address limitation.

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

Hi Laszlo,

I checked the code base and don't find the API AllocateZeroPages (), and I agree to Zero the memory before using it. So I think update the original function is the best way now.

Thanks,
Eric

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Saturday, August 11, 2018 12:15 AM
> To: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org
> Cc: Marvin Häuser <Marvin.Haeuser@outlook.com>; Fan Jeff
> <vanjeff_919@hotmail.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: Re: [Patch v3 3/5] UefiCpuPkg/CpuS3DataDxe: Change Memory Type
> and address limitation.
> 
> On 08/10/18 18:12, Laszlo Ersek wrote:
> > On 08/10/18 06:19, Eric Dong wrote:
> >> Because CpuS3Data memory will be copy to smram at SmmReadyToLock
> >> point, 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.
> 
> [snip]
> 
> > (2) In the previous patch, we lifted the 4GB limitation on the stack
> > address (while preserving the memory type restriction as AcpiNVS).
> > However, you continue to allocate the stack with
> > AllocateAcpiNvsMemoryBelow4G().
> >
> > I don't think that's consistent with the purpose of this patch set, or
> > with the documentation change in the previous patch. We should
> > allocate the stack as AcpiNVS without address limitation.
> >
> > And then we can remove the AllocateAcpiNvsMemoryBelow4G() function
> > altogether.
> 
> Nevermind, I'm just seeing the next patch.
> 
> (You might want to add a hint about the next patch to the commit message of
> this patch -- "we'll handle the stack in the next patch".)
> 
> So only my question (1) remains for this patch, i.e. why not use
> AllocateZeroPages().
> 
> Thanks,
> Laszlo