On 19 February 2016 at 13:33, Laszlo Ersek <ler...@redhat.com> wrote: > On 02/19/16 13:18, Ard Biesheuvel wrote: >> On 19 February 2016 at 13:14, Laszlo Ersek <ler...@redhat.com> wrote: >>> On 02/19/16 12:52, Ard Biesheuvel wrote: >>>> On 19 February 2016 at 10:56, Laszlo Ersek <ler...@redhat.com> wrote: >>>>> 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 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. >
OK, fair enough. That still means a PCD with different default values for X64 and other archs, but that is apparently not something to care about. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel