On 02/19/16 13:18, Ard Biesheuvel wrote: > On 19 February 2016 at 13:14, Laszlo Ersek <[email protected]> wrote: >> On 02/19/16 12:52, Ard Biesheuvel wrote: >>> On 19 February 2016 at 10:56, Laszlo Ersek <[email protected]> wrote: >>>> On 02/19/16 09:53, Ard Biesheuvel wrote: >>>>> On 19 February 2016 at 09:45, Yao, Jiewen <[email protected]> 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 for clarifying. >>> >>> So the only case where we have to take care to allocate below 4 GB is >>> the 32->64 case, since in all other cases, the memory allocated in DXE >>> will always be addressable in PEI. >> >> I don't think so. PEI is not obliged to be able to address all of the >> memory. >> >> In OVMF for example, even if PEI is 64-bit, the reset vector builds page >> tables only for the first 4GB (for the SEC and PEI phases), and only the >> DXE core identify-maps the full memory. >> > > OK, so that would imply that we simply need to check for > defined(MDE_CPU_X64) here? Or does the same apply to IPF? > > Since this is a bit of a can of worms, I am perfectly happy to only > drop the limit for MDE_CPU_AARCH64 instead, and make everybody's lives > easier ...
Okay, so the new PCD idea has been raised several times. IIRC Jiewen mentioned that proliferation of PCDs is now frowned upon. On the other hand, in this thread, Jiewen mentioned the possibility of a new PCD: http://thread.gmane.org/gmane.comp.bios.edk2.devel/7817/focus=7871 I think customizing this code for MDE_CPU_AARCH64, on the source code level, is not good. Here we are under IntelFrameworkModulePkg/Universal/BdsDxe/, which has two consequences: - it says Universal, so it shouldn't be specific to architecture on the source code level, - it says IntelFrameworkModulePkg, so you have a PCD namespace that doesn't clutter MdePkg's / MdeModulePkg's. Also, you mentioned earlier that memory layout is not specific to ISA, but platform, hence MDE_CPU_AARCH64 would not be a perfect match anyway. My vote is to introduce a new PCD that controls this allocation limit. After all, it can depend on a bunch of things: - Whether you have S3 or not - whether PEI can address all of the memory or just 4GB - whether your platform has RAM under 4GB - whether allocating under 4GB is detrimental to performance and so on. I think this is the textbook case for a new PCD. Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
