| Message ID | 20181123121431.22353-6-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit | expand |
On Fri, 23 Nov 2018 at 14:36, Andrew Jones <drjones@redhat.com> wrote: > > On Fri, Nov 23, 2018 at 01:14:31PM +0100, Ard Biesheuvel wrote: > > Drop the PcdPrePiCpuMemorySize definitions that limit it to 40 > > bits on AArch64 targets. This is no longer appropriate now that > > KVM has been enhanced to permit any IPA space size permitted by > > the architecture. This means the value will revert back to its > > default of 48. > > If we're running on older KVM, then, since KVM just passes through > the host value of id_aa64mmfr0_el1, firmware will see whatever > the host supports and use that (I'm not sure if the 48-bit default > ever can come into play too). In either case, it probably doesn't > matter, because just like the guest kernel works today on older > KVM, as long as nothing is placed above 40 bits there isn't any > problem. Is that the case for edk2 too? > The value of 48 serves as a limit now, which makes sense given that 52-bit requires 64k pages, which we don't support. But as I said, it might make sense to permit the GCD space to describe that much, which is actually a nice side effect of the previous patch, which takes the value directly from the CPU system register on virt targets. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
(1) s/is/its/ in $SUBJECT, please. On 11/23/18 13:14, Ard Biesheuvel wrote: > Drop the PcdPrePiCpuMemorySize definitions that limit it to 40 > bits on AArch64 targets. Indeed, after this series is applied, we still have [PcdsFixedAtBuild.ARM] gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40 left, in "ArmVirt.dsc.inc". Therefore, highlighting AArch64 in the above sentence seems justified. (2) Now, I'm noticing that the DEC default for ARM (which we continue overriding as 40) is not 48 however, but 32. Can you please remind me why we do that? This is BTW not just for my own education -- the subject seems to imply that we "generally" revert the PCD to its DEC default, *and* that the DEC default is 48. These are two statements, and for ARM, they are both false. If you agree, can you please stick AArch64 somewhere in the subject? > This is no longer appropriate now that > KVM has been enhanced to permit any IPA space size permitted by > the architecture. This means the value will revert back to its > default of 48. OK, so just to repeat my general question and documentation request, regarding EmbeddedPkg.dec / PcdPrePiCpuMemorySize, in this specific context: what *remains* affected by PcdPrePiCpuMemorySize, in ArmVirtPkg platforms? > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > ArmVirtPkg/ArmVirtQemu.dsc | 4 ---- > ArmVirtPkg/ArmVirtQemuKernel.dsc | 4 ---- > 2 files changed, 8 deletions(-) > > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc > index cb59c790afcc..42f2adce80e6 100644 > --- a/ArmVirtPkg/ArmVirtQemu.dsc > +++ b/ArmVirtPkg/ArmVirtQemu.dsc > @@ -143,10 +143,6 @@ > gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16 > > [PcdsFixedAtBuild.AARCH64] > - # KVM limits it IPA space to 40 bits (1 TB), so there is no need to > - # support anything bigger, even if the host hardware does > - gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40 > - > # Clearing BIT0 in this PCD prevents installing a 32-bit SMBIOS entry point, > # if the entry point version is >= 3.0. AARCH64 OSes cannot assume the > # presence of the 32-bit entry point anyway (because many AARCH64 systems > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc > index 434d6861a56f..d8fbf14e8f4e 100644 > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > @@ -157,10 +157,6 @@ > # > gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16 > > - # KVM limits it IPA space to 40 bits (1 TB), so there is no need to > - # support anything bigger, even if the host hardware does > - gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40 > - > [PcdsDynamicDefault.common] > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3 > > I'm sorry if my review comments / questions are a mess: I haven't looked at this in ages. Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc index cb59c790afcc..42f2adce80e6 100644 --- a/ArmVirtPkg/ArmVirtQemu.dsc +++ b/ArmVirtPkg/ArmVirtQemu.dsc @@ -143,10 +143,6 @@ gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16 [PcdsFixedAtBuild.AARCH64] - # KVM limits it IPA space to 40 bits (1 TB), so there is no need to - # support anything bigger, even if the host hardware does - gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40 - # Clearing BIT0 in this PCD prevents installing a 32-bit SMBIOS entry point, # if the entry point version is >= 3.0. AARCH64 OSes cannot assume the # presence of the 32-bit entry point anyway (because many AARCH64 systems diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc index 434d6861a56f..d8fbf14e8f4e 100644 --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc @@ -157,10 +157,6 @@ # gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16 - # KVM limits it IPA space to 40 bits (1 TB), so there is no need to - # support anything bigger, even if the host hardware does - gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40 - [PcdsDynamicDefault.common] gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
Drop the PcdPrePiCpuMemorySize definitions that limit it to 40 bits on AArch64 targets. This is no longer appropriate now that KVM has been enhanced to permit any IPA space size permitted by the architecture. This means the value will revert back to its default of 48. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- ArmVirtPkg/ArmVirtQemu.dsc | 4 ---- ArmVirtPkg/ArmVirtQemuKernel.dsc | 4 ---- 2 files changed, 8 deletions(-) -- 2.17.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel