On 11/23/18 13:14, Ard Biesheuvel wrote: > The ArmVirtQemu targets currently limit the size of the IPA space to > 40 bits because that is all what KVM supports. However, this is about > to change, and so we need to update the code if we want to ensure that > our UEFI firmware builds can keep running on systems that set values > other than 40 (which could be > 40 or < 40) > > So add a helper to ArmLib to read the number of supported address bits (#1) > and take this into account in the page table code (#2), which allows > PcdPrePiCpuMemorySize to assume a value that exceeds the capabilities of > the CPU. > > Patch #3 is mostly a cleanup patch, to switch to the new helper added in > patch #1. No functional changes intended. > > Patch #4 builds the CPU hob (and thus declares the size of the GCD memory > space) based on the CPU capabilities rather than the value of > PcdPrePiCpuMemorySize, to prevent any potential regressions in memory > utilization when we bump PcdPrePiCpuMemorySize back to 48. > > Patch #5 drops the definitions of PcdPrePiCpuMemorySize, reverting its > value back to the default 48.
Staring a bit more at this, I wonder if it were helpful to reorder the patches like this (just thinking loudly, I'm not even suggesting it, I'm just curious about your opinion): - patch #1: keep in place (introduce new helper) - patch #2: current patch #3 (ArmVirtPkg refactoring), to benefit from the new helper at once, without any relation to the feature that's the end goal here. - patch #3: current patch #2, build page tables with CPU PA limits accounted for - patch #4: current patch #4, build GCD memory space map with CPU PA limits accounted for - patch #5: remove the traces of the now useless PCD from ArmVirt So basically, swap around #2 and #3. It's not really important; the reason I'm thinking of it is the following though: while removing the 40-bit limitation, my first thought is, "what are the current consumers, what needs to be updated". The current structuring of the series, with patch #3 in the middle, suggests that ArmVirtMemInfoLib instances are consumers. That's not the case however, they already fetch the CPU PA limits dynamically. So they need no functional updates, just a cleanup / centralization. That's why I'd find it helpful to handle ArmVirtMemInfoLib right after the introduction of the helper. The actual consumers (in need of functional updates) are the page tables and the GCD memory space map (two concepts, not three). If you think this would be an improvement, please consider the reordering. No need to repost just for this. Thanks! Laszlo > > Cc: Laszlo Ersek <[email protected]> > Cc: Leif Lindholm <[email protected]> > Cc: Eric Auger <[email protected]> > Cc: Andrew Jones <[email protected]> > Cc: Philippe Mathieu-Daude <[email protected]> > Cc: Julien Grall <[email protected]> > > Ard Biesheuvel (5): > ArmPkg/ArmLib: add support for reading the max physical address space > size > ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account > ArmVirtPkg: refactor reading of the physical address space size > ArmVirtPkg: disregard PcdPrePiCpuMemorySize PCD when sizing the GCD > space > ArmVirtPkg: revert PcdPrePiCpuMemorySize to is default value of 48 > > ArmPkg/Include/Library/ArmLib.h | 6 +++ > ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 16 ++++++++ > ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S | 8 ++++ > .../Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 5 ++- > ArmVirtPkg/ArmVirtQemu.dsc | 5 --- > ArmVirtPkg/ArmVirtQemu.fdf | 1 - > ArmVirtPkg/ArmVirtQemuKernel.dsc | 4 -- > .../Include/Library/ArmVirtMemInfoLib.h | 1 + > .../ArmVirtMemoryInitPeiLib.c | 7 +++- > .../ArmVirtMemoryInitPeiLib.inf | 1 + > .../QemuVirtMemInfoLib/AArch64/PhysAddrTop.S | 39 ------------------- > .../QemuVirtMemInfoLib/Arm/PhysAddrTop.S | 24 ------------ > .../QemuVirtMemInfoLib/QemuVirtMemInfoLib.c | 6 +-- > .../QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf | 7 ---- > .../QemuVirtMemInfoPeiLib.inf | 7 ---- > .../XenVirtMemInfoLib/AArch64/PhysAddrTop.S | 39 ------------------- > .../XenVirtMemInfoLib/Arm/PhysAddrTop.S | 24 ------------ > .../XenVirtMemInfoLib/XenVirtMemInfoLib.c | 8 +--- > .../XenVirtMemInfoLib/XenVirtMemInfoLib.inf | 6 --- > .../PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 3 -- > ArmVirtPkg/PrePi/PrePi.c | 3 -- > 21 files changed, 46 insertions(+), 174 deletions(-) > delete mode 100644 > ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S > delete mode 100644 ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S > delete mode 100644 ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S > delete mode 100644 ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
