On 02/19/16 09:53, Ard Biesheuvel wrote:
> On 19 February 2016 at 09:45, Yao, Jiewen <jiewen....@intel.com> wrote:
>> I can explain the reason on allocating <4G. It is because this data will be
>> used in PEI phase in S3 resume.
>>
>> On most X86 platform, PEI phase is 32bit, and DXE phase is 32bit or 64bit.
>> So we have to limit the allocation <4G.
>>
>
> OK, got it.
>
>> Using ACPI version PCD looks strange here. Maybe another PCD, like
>> PcdDxeIplSwitchToLongMode?
>>
>
> But that does not tell us if we are running a 32-bit PEI, right? It
> only tells us if a 32-bit PEI should load a 32-bit DXE core on a
> 64-bit capable machine.
32->32 and 64->64 builds have PcdDxeIplSwitchToLongMode set to FALSE.
Only 32->64 builds have PcdDxeIplSwitchToLongMode set to TRUE.
(This is what we have in the three DSC files of OVMF.)
So, in order to see in DXE if your PEI is 32-bit, you can do:
BOOLEAN PeiIs32Bit;
PeiIs32Bit = FALSE;
if (FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
//
// this implies a 32->64 switch
//
PeiIs32Bit = TRUE;
} else {
//
// otherwise, PEI and DXE have the same bitness,
// so derive it from DXE's bitness
//
#if defined (MDE_CPU_IA32) || defined (MDE_CPU_ARM)
PeiIs32Bit = TRUE;
#endif
}
Alternatively:
PeiIs32Bit = FeaturePcdGet (PcdDxeIplSwitchToLongMode) ||
sizeof (UINTN) == sizeof (UINT32);
I'll admit I'm not really sure about EBC. The above mostly treats EBC as
nonexistent. :)
Thanks
Laszlo
>
>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Friday, February 19, 2016 4:03 AM
>> To: edk2-devel@lists.01.org; Tian, Feng; Zeng, Star;
>> leif.lindh...@linaro.org; graeme.greg...@linaro.org; ler...@redhat.com; Fan,
>> Jeff; Yao, Jiewen
>> Cc: mark.rutl...@arm.com; Gao, Liming; samer.el-haj-mahm...@hpe.com; Ard
>> Biesheuvel
>> Subject: [PATCH v2 2/3] IntelFrameworkModulePkg: BdsDxe: only allocate below
>> 4 GB on ACPI 1.0
>>
>> It is not entirely clear from the code why, but the reserved region for
>> storing performance data is allocated below 4 GB, and the variable to hold
>> the address of the allocation is called 'AcpiLowMemoryBase'
>> (which is the only mention of ACPI in the entire file).
>>
>> Let's make this allocation dependent on PcdAcpiExposedTableVersions as well,
>> since systems that can deal with ACPI table in high memory are also just as
>> likely to be able to deal with performance data in high memory. This
>> prevents memory fragmentation on systems that don't care about the 4 GB
>> limit.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
>> ---
>> IntelFrameworkModulePkg/Universal/BdsDxe/BdsDxe.inf | 1 +
>> IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c | 11 ++++++++++-
>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsDxe.inf
>> b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsDxe.inf
>> index 6afb8a09df9c..38172780fb49 100644
>> --- a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsDxe.inf
>> +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsDxe.inf
>> @@ -215,6 +215,7 @@ [Pcd]
>> gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution
>> ## CONSUMES
>> gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution
>> ## CONSUMES
>> gEfiMdeModulePkgTokenSpaceGuid.PcdErrorCodeSetVariable
>> ## CONSUMES
>> + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions
>> ## CONSUMES
>>
>> [Depex]
>> TRUE
>> diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
>> b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
>> index ae7ad2153c51..bf5bd6524a90 100644
>> --- a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
>> +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
>> @@ -22,6 +22,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>> EXPRESS OR IMPLIED.
>> #include "Hotkey.h"
>> #include "HwErrRecSupport.h"
>>
>> +#include <Protocol/AcpiSystemDescriptionTable.h>
>> +
>> ///
>> /// BDS arch protocol instance initial value.
>> ///
>> @@ -474,14 +476,21 @@ BdsAllocateMemoryForPerformanceData (
>> EFI_STATUS Status;
>> EFI_PHYSICAL_ADDRESS AcpiLowMemoryBase;
>> EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock;
>> + EFI_ALLOCATE_TYPE AcpiTableAllocType;
>>
>> AcpiLowMemoryBase = 0x0FFFFFFFFULL;
>>
>> + if ((PcdGet32 (PcdAcpiExposedTableVersions) &
>> EFI_ACPI_TABLE_VERSION_1_0B) != 0) {
>> + AcpiTableAllocType = AllocateMaxAddress; } else {
>> + AcpiTableAllocType = AllocateAnyPages; }
>> +
>> //
>> // Allocate a block of memory that will contain performance data to OS.
>> //
>> Status = gBS->AllocatePages (
>> - AllocateMaxAddress,
>> + AcpiTableAllocType,
>> EfiReservedMemoryType,
>> EFI_SIZE_TO_PAGES (PERF_DATA_MAX_LENGTH),
>> &AcpiLowMemoryBase
>> --
>> 2.5.0
>>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
