On Mon, 26 Nov 2018 at 11:22, Laszlo Ersek <[email protected]> wrote: > > 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. >
Yes, I think that makes sense. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
