On 02/19/16 13:50, Ard Biesheuvel wrote: > On 19 February 2016 at 13:48, Laszlo Ersek <[email protected]> wrote: >> 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? >> > > The only downside is that AARCH64 platforms don't get the change 'for > free' but have to set this new PCD. But thinking about it, that is not > entirely unreasonable ...
Yes: they are the freaks with no RAM under 4GB! ;) Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
