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

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Ignore my previous mail.

You could use AllocatePages() instead of directly calling gBS->AllocatePages().
Then rename the AllocateBootServiceMemory() to AllocateZeroPages() to better
reflect the function behavior.

Thanks/Ray

> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Ni, Ruiyu
> Sent: Monday, August 13, 2018 1:39 PM
> To: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> edk2-devel@lists.01.org
> Subject: Re: [edk2] [Patch v3 3/5] UefiCpuPkg/CpuS3DataDxe: Change
> Memory Type and address limitation.
> 
> Eric,
> I think you could change:
> AcpiCpuDataEx = AllocateBootServiceMemory (sizeof
> (ACPI_CPU_DATA_EX));
> ->
> AcpiCpuDataEx = AllocatePages (EFI_SIZE_TO_PAGES (sizeof
> (ACPI_CPU_DATA_EX)));
> 
> So the AllocateBootServiceMemory() is not needed.
> 
> Thanks/Ray
> 
> > -----Original Message-----
> > From: Dong, Eric
> > Sent: Monday, August 13, 2018 9:51 AM
> > To: Laszlo Ersek <lersek@redhat.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.
> >
> > 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
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel