On 02/19/16 13:40, Ard Biesheuvel wrote: > On 19 February 2016 at 13:33, Laszlo Ersek <[email protected]> wrote:
>> 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. Sorry, I don't understand; perhaps I missed something earlier. Your patch changes AllocateMaxAddress into AllocateAnyPages, conditionally, for the "AcpiLowMemoryBase" allocation. Why is it not enough to enable this change (with the PCD) for AARCH64 only? Why must X64 differ? Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
