| Message ID | 20181123121431.22353-1-ard.biesheuvel@linaro.org |
|---|---|
| Headers | show |
| Series | ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit | expand |
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. > > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Leif Lindholm <leif.lindholm@linaro.org> > Cc: Eric Auger <eric.auger@redhat.com> > Cc: Andrew Jones <drjones@redhat.com> > Cc: Philippe Mathieu-Daude <philmd@redhat.com> > Cc: Julien Grall <julien.grall@linaro.org> > > 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 > I've worked way too much over this weekend & now I'm tired; I'll try to get to this series some time next week. Thanks for your patience! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Hi Ard, 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. > > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Leif Lindholm <leif.lindholm@linaro.org> > Cc: Eric Auger <eric.auger@redhat.com> > Cc: Andrew Jones <drjones@redhat.com> > Cc: Philippe Mathieu-Daude <philmd@redhat.com> > Cc: Julien Grall <julien.grall@linaro.org> > > 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 > sorry, I'm confused. In the first place, I don't understand what PcdPrePiCpuMemorySize stands for. I checked the declaration in "EmbeddedPkg/EmbeddedPkg.dec", and there is no comment on it. And, this patch set doesn't clear it up for me. According to patch #2, it will not dictate the size of the address space that we map 1:1. According to patch #3 it will also not dictate the size of the GCD memory address space. So: what *is* it good for? Can you add a patch for EmbeddedPkg/EmbeddedPkg.dec that supplies the precise specification? Thanks, Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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 <lersek@redhat.com> > Cc: Leif Lindholm <leif.lindholm@linaro.org> > Cc: Eric Auger <eric.auger@redhat.com> > Cc: Andrew Jones <drjones@redhat.com> > Cc: Philippe Mathieu-Daude <philmd@redhat.com> > Cc: Julien Grall <julien.grall@linaro.org> > > 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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Mon, 26 Nov 2018 at 11:22, Laszlo Ersek <lersek@redhat.com> 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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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. Cc: Laszlo Ersek <lersek@redhat.com> Cc: Leif Lindholm <leif.lindholm@linaro.org> Cc: Eric Auger <eric.auger@redhat.com> Cc: Andrew Jones <drjones@redhat.com> Cc: Philippe Mathieu-Daude <philmd@redhat.com> Cc: Julien Grall <julien.grall@linaro.org> 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 -- 2.17.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel