* [Patch v2 0/2] Change CpuS3Data memory type and address limitation @ 2018-08-08 7:34 Eric Dong 2018-08-08 7:34 ` [Patch 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Combine implementation Eric Dong 2018-08-08 7:34 ` [Patch 2/2] UefiCpuPkg/CpuS3DataDxe: Change Memory Type and address limitation Eric Dong 0 siblings, 2 replies; 5+ messages in thread From: Eric Dong @ 2018-08-08 7:34 UTC (permalink / raw) To: edk2-devel; +Cc: Marvin H�user, Fan Jeff, Laszlo Ersek, Ruiyu Ni [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 1044 bytes --] Because CpuS3Data memory will be copy to smram at SmmReadToLock point by PiSmmCpuDxeSmm driver, 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. Bugz: https://bugzilla.tianocore.org/show_bug.cgi?id=959 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> Eric Dong (2): UefiCpuPkg/RegisterCpuFeaturesLib: Combine implementation. UefiCpuPkg/CpuS3DataDxe: Change Memory Type and address limitation. UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c | 60 ++-------- UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf | 1 + .../DxeRegisterCpuFeaturesLib.c | 67 ----------- .../PeiRegisterCpuFeaturesLib.c | 131 --------------------- .../RegisterCpuFeaturesLib.c | 90 ++++++++++++++ 5 files changed, 104 insertions(+), 245 deletions(-) -- 2.15.0.windows.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Patch 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Combine implementation. 2018-08-08 7:34 [Patch v2 0/2] Change CpuS3Data memory type and address limitation Eric Dong @ 2018-08-08 7:34 ` Eric Dong 2018-08-08 7:39 ` Dong, Eric 2018-08-08 7:34 ` [Patch 2/2] UefiCpuPkg/CpuS3DataDxe: Change Memory Type and address limitation Eric Dong 1 sibling, 1 reply; 5+ messages in thread From: Eric Dong @ 2018-08-08 7:34 UTC (permalink / raw) To: edk2-devel; +Cc: Marvin Häuser, Fan Jeff, Laszlo Ersek, Ruiyu Ni V1 changes: > Current code logic can't confirm CpuS3DataDxe driver start before > CpuFeaturesDxe driver. So the assumption in CpuFeaturesDxe not valid. > Add implementation for AllocateAcpiCpuData function to remove this > assumption. V2 changes: > 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> --- .../DxeRegisterCpuFeaturesLib.c | 67 ----------- .../PeiRegisterCpuFeaturesLib.c | 131 --------------------- .../RegisterCpuFeaturesLib.c | 90 ++++++++++++++ 3 files changed, 90 insertions(+), 198 deletions(-) diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c index 902a339529..1f34a3f489 100644 --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c @@ -197,70 +197,3 @@ GetNumberOfProcessor ( ASSERT_EFI_ERROR (Status); } -/** - Allocates ACPI NVS memory to save ACPI_CPU_DATA. - - @return Pointer to allocated ACPI_CPU_DATA. -**/ -ACPI_CPU_DATA * -AllocateAcpiCpuData ( - VOID - ) -{ - // - // CpuS3DataDxe will do it. - // - ASSERT (FALSE); - return NULL; -} - -/** - Enlarges CPU register table for each processor. - - @param[in, out] RegisterTable Pointer processor's CPU register table -**/ -VOID -EnlargeRegisterTable ( - IN OUT CPU_REGISTER_TABLE *RegisterTable - ) -{ - EFI_STATUS Status; - EFI_PHYSICAL_ADDRESS Address; - UINTN AllocatePages; - - Address = BASE_4GB - 1; - AllocatePages = RegisterTable->AllocatedSize / EFI_PAGE_SIZE; - Status = gBS->AllocatePages ( - AllocateMaxAddress, - EfiACPIMemoryNVS, - AllocatePages + 1, - &Address - ); - ASSERT_EFI_ERROR (Status); - - // - // If there are records existing in the register table, then copy its contents - // to new region and free the old one. - // - if (RegisterTable->AllocatedSize > 0) { - CopyMem ( - (VOID *) (UINTN) Address, - (VOID *) (UINTN) RegisterTable->RegisterTableEntry, - RegisterTable->AllocatedSize - ); - // - // RegisterTableEntry is allocated by gBS->AllocatePages() service. - // So, gBS->FreePages() service is used to free it. - // - gBS->FreePages ( - RegisterTable->RegisterTableEntry, - AllocatePages - ); - } - - // - // Adjust the allocated size and register table base address. - // - RegisterTable->AllocatedSize += EFI_PAGE_SIZE; - RegisterTable->RegisterTableEntry = Address; -} diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c index 6804eddf65..82fe268812 100644 --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c @@ -257,134 +257,3 @@ GetNumberOfProcessor ( ); ASSERT_EFI_ERROR (Status); } - -/** - Allocates ACPI NVS memory to save ACPI_CPU_DATA. - - @return Pointer to allocated ACPI_CPU_DATA. -**/ -ACPI_CPU_DATA * -AllocateAcpiCpuData ( - VOID - ) -{ - EFI_STATUS Status; - EFI_PEI_MP_SERVICES_PPI *CpuMpPpi; - UINTN NumberOfCpus; - UINTN NumberOfEnabledProcessors; - ACPI_CPU_DATA *AcpiCpuData; - EFI_PHYSICAL_ADDRESS Address; - UINTN TableSize; - CPU_REGISTER_TABLE *RegisterTable; - UINTN Index; - EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer; - - Status = PeiServicesAllocatePages ( - EfiACPIMemoryNVS, - EFI_SIZE_TO_PAGES (sizeof (ACPI_CPU_DATA)), - &Address - ); - ASSERT_EFI_ERROR (Status); - AcpiCpuData = (ACPI_CPU_DATA *) (UINTN) Address; - ASSERT (AcpiCpuData != NULL); - - // - // Get MP Services Protocol - // - Status = PeiServicesLocatePpi ( - &gEfiPeiMpServicesPpiGuid, - 0, - NULL, - (VOID **)&CpuMpPpi - ); - ASSERT_EFI_ERROR (Status); - - // - // Get the number of CPUs - // - Status = CpuMpPpi->GetNumberOfProcessors ( - GetPeiServicesTablePointer (), - CpuMpPpi, - &NumberOfCpus, - &NumberOfEnabledProcessors - ); - ASSERT_EFI_ERROR (Status); - AcpiCpuData->NumberOfCpus = (UINT32)NumberOfCpus; - - // - // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for all CPUs - // - TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE); - Status = PeiServicesAllocatePages ( - EfiACPIMemoryNVS, - EFI_SIZE_TO_PAGES (TableSize), - &Address - ); - ASSERT_EFI_ERROR (Status); - RegisterTable = (CPU_REGISTER_TABLE *) (UINTN) Address; - - for (Index = 0; Index < NumberOfCpus; Index++) { - Status = CpuMpPpi->GetProcessorInfo ( - GetPeiServicesTablePointer (), - CpuMpPpi, - Index, - &ProcessorInfoBuffer - ); - ASSERT_EFI_ERROR (Status); - - RegisterTable[Index].InitialApicId = (UINT32)ProcessorInfoBuffer.ProcessorId; - RegisterTable[Index].TableLength = 0; - RegisterTable[Index].AllocatedSize = 0; - RegisterTable[Index].RegisterTableEntry = 0; - - RegisterTable[NumberOfCpus + Index].InitialApicId = (UINT32)ProcessorInfoBuffer.ProcessorId; - RegisterTable[NumberOfCpus + Index].TableLength = 0; - RegisterTable[NumberOfCpus + Index].AllocatedSize = 0; - RegisterTable[NumberOfCpus + Index].RegisterTableEntry = 0; - } - AcpiCpuData->RegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTable; - AcpiCpuData->PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN)(RegisterTable + NumberOfCpus); - - return AcpiCpuData; -} - -/** - Enlarges CPU register table for each processor. - - @param[in, out] RegisterTable Pointer processor's CPU register table -**/ -VOID -EnlargeRegisterTable ( - IN OUT CPU_REGISTER_TABLE *RegisterTable - ) -{ - EFI_STATUS Status; - EFI_PHYSICAL_ADDRESS Address; - UINTN AllocatePages; - - AllocatePages = RegisterTable->AllocatedSize / EFI_PAGE_SIZE; - Status = PeiServicesAllocatePages ( - EfiACPIMemoryNVS, - AllocatePages + 1, - &Address - ); - ASSERT_EFI_ERROR (Status); - - // - // If there are records existing in the register table, then copy its contents - // to new region and free the old one. - // - if (RegisterTable->AllocatedSize > 0) { - CopyMem ( - (VOID *) (UINTN) Address, - (VOID *) (UINTN) RegisterTable->RegisterTableEntry, - RegisterTable->AllocatedSize - ); - } - - // - // Adjust the allocated size and register table base address. - // - RegisterTable->AllocatedSize += EFI_PAGE_SIZE; - RegisterTable->RegisterTableEntry = Address; -} diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c index dd6a82be7a..b87eb280f8 100644 --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c @@ -488,6 +488,96 @@ RegisterCpuFeature ( return RETURN_SUCCESS; } +/** + Allocates ACPI NVS memory to save ACPI_CPU_DATA. + + @return Pointer to allocated ACPI_CPU_DATA. +**/ +ACPI_CPU_DATA * +AllocateAcpiCpuData ( + VOID + ) +{ + EFI_STATUS Status; + UINTN NumberOfCpus; + UINTN NumberOfEnabledProcessors; + ACPI_CPU_DATA *AcpiCpuData; + UINTN TableSize; + CPU_REGISTER_TABLE *RegisterTable; + UINTN Index; + EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer; + + AcpiCpuData = AllocatePages (EFI_SIZE_TO_PAGES (sizeof (ACPI_CPU_DATA))); + ASSERT (AcpiCpuData != NULL); + + GetNumberOfProcessor (&NumberOfCpus, &NumberOfEnabledProcessors); + AcpiCpuData->NumberOfCpus = (UINT32) NumberOfCpus; + + // + // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for all CPUs + // + TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE); + RegisterTable = AllocatePages (EFI_SIZE_TO_PAGES (TableSize)); + ASSERT (RegisterTable != NULL); + + for (Index = 0; Index < NumberOfCpus; Index++) { + Status = GetProcessorInformation (Index, &ProcessorInfoBuffer); + ASSERT_EFI_ERROR (Status); + + RegisterTable[Index].InitialApicId = (UINT32)ProcessorInfoBuffer.ProcessorId; + RegisterTable[Index].TableLength = 0; + RegisterTable[Index].AllocatedSize = 0; + RegisterTable[Index].RegisterTableEntry = 0; + + RegisterTable[NumberOfCpus + Index].InitialApicId = (UINT32)ProcessorInfoBuffer.ProcessorId; + RegisterTable[NumberOfCpus + Index].TableLength = 0; + RegisterTable[NumberOfCpus + Index].AllocatedSize = 0; + RegisterTable[NumberOfCpus + Index].RegisterTableEntry = 0; + } + AcpiCpuData->RegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN) RegisterTable; + AcpiCpuData->PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN) (RegisterTable + NumberOfCpus); + + return AcpiCpuData; +} + +/** + Enlarges CPU register table for each processor. + + @param[in, out] RegisterTable Pointer processor's CPU register table +**/ +VOID +EnlargeRegisterTable ( + IN OUT CPU_REGISTER_TABLE *RegisterTable + ) +{ + EFI_PHYSICAL_ADDRESS Address; + UINTN UsedPages; + + UsedPages = RegisterTable->AllocatedSize / EFI_PAGE_SIZE; + Address = (UINTN) AllocatePages (UsedPages + 1); + ASSERT (Address != 0); + + // + // If there are records existing in the register table, then copy its contents + // to new region and free the old one. + // + if (RegisterTable->AllocatedSize > 0) { + CopyMem ( + (VOID *) (UINTN) Address, + (VOID *) (UINTN) RegisterTable->RegisterTableEntry, + RegisterTable->AllocatedSize + ); + + FreePages ((VOID *) (UINTN) RegisterTable->RegisterTableEntry, UsedPages); + } + + // + // Adjust the allocated size and register table base address. + // + RegisterTable->AllocatedSize += EFI_PAGE_SIZE; + RegisterTable->RegisterTableEntry = Address; +} + /** Add an entry in specified register table. -- 2.15.0.windows.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Patch 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Combine implementation. 2018-08-08 7:34 ` [Patch 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Combine implementation Eric Dong @ 2018-08-08 7:39 ` Dong, Eric 0 siblings, 0 replies; 5+ messages in thread From: Dong, Eric @ 2018-08-08 7:39 UTC (permalink / raw) To: Dong, Eric, edk2-devel@lists.01.org; +Cc: Laszlo Ersek, Ni, Ruiyu 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 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Combine > implementation. > > V1 changes: > > Current code logic can't confirm CpuS3DataDxe driver start before > > CpuFeaturesDxe driver. So the assumption in CpuFeaturesDxe not valid. > > Add implementation for AllocateAcpiCpuData function to remove this > > assumption. > > V2 changes: > > 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> > --- > .../DxeRegisterCpuFeaturesLib.c | 67 ----------- > .../PeiRegisterCpuFeaturesLib.c | 131 --------------------- > .../RegisterCpuFeaturesLib.c | 90 ++++++++++++++ > 3 files changed, 90 insertions(+), 198 deletions(-) > > diff --git > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c > index 902a339529..1f34a3f489 100644 > --- > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLi > +++ b.c > @@ -197,70 +197,3 @@ GetNumberOfProcessor ( > ASSERT_EFI_ERROR (Status); > } > > -/** > - Allocates ACPI NVS memory to save ACPI_CPU_DATA. > - > - @return Pointer to allocated ACPI_CPU_DATA. > -**/ > -ACPI_CPU_DATA * > -AllocateAcpiCpuData ( > - VOID > - ) > -{ > - // > - // CpuS3DataDxe will do it. > - // > - ASSERT (FALSE); > - return NULL; > -} > - > -/** > - Enlarges CPU register table for each processor. > - > - @param[in, out] RegisterTable Pointer processor's CPU register table > -**/ > -VOID > -EnlargeRegisterTable ( > - IN OUT CPU_REGISTER_TABLE *RegisterTable > - ) > -{ > - EFI_STATUS Status; > - EFI_PHYSICAL_ADDRESS Address; > - UINTN AllocatePages; > - > - Address = BASE_4GB - 1; > - AllocatePages = RegisterTable->AllocatedSize / EFI_PAGE_SIZE; > - Status = gBS->AllocatePages ( > - AllocateMaxAddress, > - EfiACPIMemoryNVS, > - AllocatePages + 1, > - &Address > - ); > - ASSERT_EFI_ERROR (Status); > - > - // > - // If there are records existing in the register table, then copy its contents > - // to new region and free the old one. > - // > - if (RegisterTable->AllocatedSize > 0) { > - CopyMem ( > - (VOID *) (UINTN) Address, > - (VOID *) (UINTN) RegisterTable->RegisterTableEntry, > - RegisterTable->AllocatedSize > - ); > - // > - // RegisterTableEntry is allocated by gBS->AllocatePages() service. > - // So, gBS->FreePages() service is used to free it. > - // > - gBS->FreePages ( > - RegisterTable->RegisterTableEntry, > - AllocatePages > - ); > - } > - > - // > - // Adjust the allocated size and register table base address. > - // > - RegisterTable->AllocatedSize += EFI_PAGE_SIZE; > - RegisterTable->RegisterTableEntry = Address; -} diff --git > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c > index 6804eddf65..82fe268812 100644 > --- > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLi > +++ b.c > @@ -257,134 +257,3 @@ GetNumberOfProcessor ( > ); > ASSERT_EFI_ERROR (Status); > } > - > -/** > - Allocates ACPI NVS memory to save ACPI_CPU_DATA. > - > - @return Pointer to allocated ACPI_CPU_DATA. > -**/ > -ACPI_CPU_DATA * > -AllocateAcpiCpuData ( > - VOID > - ) > -{ > - EFI_STATUS Status; > - EFI_PEI_MP_SERVICES_PPI *CpuMpPpi; > - UINTN NumberOfCpus; > - UINTN NumberOfEnabledProcessors; > - ACPI_CPU_DATA *AcpiCpuData; > - EFI_PHYSICAL_ADDRESS Address; > - UINTN TableSize; > - CPU_REGISTER_TABLE *RegisterTable; > - UINTN Index; > - EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer; > - > - Status = PeiServicesAllocatePages ( > - EfiACPIMemoryNVS, > - EFI_SIZE_TO_PAGES (sizeof (ACPI_CPU_DATA)), > - &Address > - ); > - ASSERT_EFI_ERROR (Status); > - AcpiCpuData = (ACPI_CPU_DATA *) (UINTN) Address; > - ASSERT (AcpiCpuData != NULL); > - > - // > - // Get MP Services Protocol > - // > - Status = PeiServicesLocatePpi ( > - &gEfiPeiMpServicesPpiGuid, > - 0, > - NULL, > - (VOID **)&CpuMpPpi > - ); > - ASSERT_EFI_ERROR (Status); > - > - // > - // Get the number of CPUs > - // > - Status = CpuMpPpi->GetNumberOfProcessors ( > - GetPeiServicesTablePointer (), > - CpuMpPpi, > - &NumberOfCpus, > - &NumberOfEnabledProcessors > - ); > - ASSERT_EFI_ERROR (Status); > - AcpiCpuData->NumberOfCpus = (UINT32)NumberOfCpus; > - > - // > - // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for > all CPUs > - // > - TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE); > - Status = PeiServicesAllocatePages ( > - EfiACPIMemoryNVS, > - EFI_SIZE_TO_PAGES (TableSize), > - &Address > - ); > - ASSERT_EFI_ERROR (Status); > - RegisterTable = (CPU_REGISTER_TABLE *) (UINTN) Address; > - > - for (Index = 0; Index < NumberOfCpus; Index++) { > - Status = CpuMpPpi->GetProcessorInfo ( > - GetPeiServicesTablePointer (), > - CpuMpPpi, > - Index, > - &ProcessorInfoBuffer > - ); > - ASSERT_EFI_ERROR (Status); > - > - RegisterTable[Index].InitialApicId = > (UINT32)ProcessorInfoBuffer.ProcessorId; > - RegisterTable[Index].TableLength = 0; > - RegisterTable[Index].AllocatedSize = 0; > - RegisterTable[Index].RegisterTableEntry = 0; > - > - RegisterTable[NumberOfCpus + Index].InitialApicId = > (UINT32)ProcessorInfoBuffer.ProcessorId; > - RegisterTable[NumberOfCpus + Index].TableLength = 0; > - RegisterTable[NumberOfCpus + Index].AllocatedSize = 0; > - RegisterTable[NumberOfCpus + Index].RegisterTableEntry = 0; > - } > - AcpiCpuData->RegisterTable = > (EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTable; > - AcpiCpuData->PreSmmInitRegisterTable = > (EFI_PHYSICAL_ADDRESS)(UINTN)(RegisterTable + NumberOfCpus); > - > - return AcpiCpuData; > -} > - > -/** > - Enlarges CPU register table for each processor. > - > - @param[in, out] RegisterTable Pointer processor's CPU register table > -**/ > -VOID > -EnlargeRegisterTable ( > - IN OUT CPU_REGISTER_TABLE *RegisterTable > - ) > -{ > - EFI_STATUS Status; > - EFI_PHYSICAL_ADDRESS Address; > - UINTN AllocatePages; > - > - AllocatePages = RegisterTable->AllocatedSize / EFI_PAGE_SIZE; > - Status = PeiServicesAllocatePages ( > - EfiACPIMemoryNVS, > - AllocatePages + 1, > - &Address > - ); > - ASSERT_EFI_ERROR (Status); > - > - // > - // If there are records existing in the register table, then copy its contents > - // to new region and free the old one. > - // > - if (RegisterTable->AllocatedSize > 0) { > - CopyMem ( > - (VOID *) (UINTN) Address, > - (VOID *) (UINTN) RegisterTable->RegisterTableEntry, > - RegisterTable->AllocatedSize > - ); > - } > - > - // > - // Adjust the allocated size and register table base address. > - // > - RegisterTable->AllocatedSize += EFI_PAGE_SIZE; > - RegisterTable->RegisterTableEntry = Address; -} diff --git > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c > index dd6a82be7a..b87eb280f8 100644 > --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c > @@ -488,6 +488,96 @@ RegisterCpuFeature ( > return RETURN_SUCCESS; > } > > +/** > + Allocates ACPI NVS memory to save ACPI_CPU_DATA. > + > + @return Pointer to allocated ACPI_CPU_DATA. > +**/ > +ACPI_CPU_DATA * > +AllocateAcpiCpuData ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + UINTN NumberOfCpus; > + UINTN NumberOfEnabledProcessors; > + ACPI_CPU_DATA *AcpiCpuData; > + UINTN TableSize; > + CPU_REGISTER_TABLE *RegisterTable; > + UINTN Index; > + EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer; > + > + AcpiCpuData = AllocatePages (EFI_SIZE_TO_PAGES (sizeof > + (ACPI_CPU_DATA))); ASSERT (AcpiCpuData != NULL); > + > + GetNumberOfProcessor (&NumberOfCpus, &NumberOfEnabledProcessors); > + AcpiCpuData->NumberOfCpus = (UINT32) NumberOfCpus; > + > + // > + // Allocate buffer for empty RegisterTable and > + PreSmmInitRegisterTable for all CPUs // TableSize = 2 * NumberOfCpus > + * sizeof (CPU_REGISTER_TABLE); RegisterTable = AllocatePages > + (EFI_SIZE_TO_PAGES (TableSize)); ASSERT (RegisterTable != NULL); > + > + for (Index = 0; Index < NumberOfCpus; Index++) { > + Status = GetProcessorInformation (Index, &ProcessorInfoBuffer); > + ASSERT_EFI_ERROR (Status); > + > + RegisterTable[Index].InitialApicId = > (UINT32)ProcessorInfoBuffer.ProcessorId; > + RegisterTable[Index].TableLength = 0; > + RegisterTable[Index].AllocatedSize = 0; > + RegisterTable[Index].RegisterTableEntry = 0; > + > + RegisterTable[NumberOfCpus + Index].InitialApicId = > (UINT32)ProcessorInfoBuffer.ProcessorId; > + RegisterTable[NumberOfCpus + Index].TableLength = 0; > + RegisterTable[NumberOfCpus + Index].AllocatedSize = 0; > + RegisterTable[NumberOfCpus + Index].RegisterTableEntry = 0; } > + AcpiCpuData->RegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN) > RegisterTable; > + AcpiCpuData->PreSmmInitRegisterTable = (EFI_PHYSICAL_ADDRESS)(UINTN) > + (RegisterTable + NumberOfCpus); > + > + return AcpiCpuData; > +} > + > +/** > + Enlarges CPU register table for each processor. > + > + @param[in, out] RegisterTable Pointer processor's CPU register table > +**/ > +VOID > +EnlargeRegisterTable ( > + IN OUT CPU_REGISTER_TABLE *RegisterTable > + ) > +{ > + EFI_PHYSICAL_ADDRESS Address; > + UINTN UsedPages; > + > + UsedPages = RegisterTable->AllocatedSize / EFI_PAGE_SIZE; Address = > + (UINTN) AllocatePages (UsedPages + 1); ASSERT (Address != 0); > + > + // > + // If there are records existing in the register table, then copy its > + contents // to new region and free the old one. > + // > + if (RegisterTable->AllocatedSize > 0) { > + CopyMem ( > + (VOID *) (UINTN) Address, > + (VOID *) (UINTN) RegisterTable->RegisterTableEntry, > + RegisterTable->AllocatedSize > + ); > + > + FreePages ((VOID *) (UINTN) RegisterTable->RegisterTableEntry, > + UsedPages); } > + > + // > + // Adjust the allocated size and register table base address. > + // > + RegisterTable->AllocatedSize += EFI_PAGE_SIZE; > + RegisterTable->RegisterTableEntry = Address; } > + > /** > Add an entry in specified register table. > > -- > 2.15.0.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Patch 2/2] UefiCpuPkg/CpuS3DataDxe: Change Memory Type and address limitation. 2018-08-08 7:34 [Patch v2 0/2] Change CpuS3Data memory type and address limitation Eric Dong 2018-08-08 7:34 ` [Patch 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Combine implementation Eric Dong @ 2018-08-08 7:34 ` Eric Dong 2018-08-08 7:39 ` Dong, Eric 1 sibling, 1 reply; 5+ messages in thread From: Eric Dong @ 2018-08-08 7:34 UTC (permalink / raw) To: edk2-devel; +Cc: Marvin Häuser, Fan Jeff, Laszlo Ersek, Ruiyu Ni 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Patch 2/2] UefiCpuPkg/CpuS3DataDxe: Change Memory Type and address limitation. 2018-08-08 7:34 ` [Patch 2/2] UefiCpuPkg/CpuS3DataDxe: Change Memory Type and address limitation Eric Dong @ 2018-08-08 7:39 ` Dong, Eric 0 siblings, 0 replies; 5+ messages in thread From: Dong, Eric @ 2018-08-08 7:39 UTC (permalink / raw) To: Dong, Eric, edk2-devel@lists.01.org; +Cc: Laszlo Ersek, Ni, Ruiyu 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-08-08 7:39 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-08-08 7:34 [Patch v2 0/2] Change CpuS3Data memory type and address limitation Eric Dong 2018-08-08 7:34 ` [Patch 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Combine implementation Eric Dong 2018-08-08 7:39 ` Dong, Eric 2018-08-08 7:34 ` [Patch 2/2] UefiCpuPkg/CpuS3DataDxe: Change Memory Type and address limitation Eric Dong 2018-08-08 7:39 ` Dong, Eric
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox