mbox series

[edk2,v2,00/13] ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit

Message ID 20181126223801.17121-1-ard.biesheuvel@linaro.org
Headers show
Series ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit | expand

Message

Ard Biesheuvel Nov. 26, 2018, 10:37 p.m. UTC
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 refactor the way we deal with this limit, both for bare metal and for
virtual targets, so that
a) the range of the GCD memory map is based directly on the CPU's PA range
b) the range of the 1:1 mapping in the page tables is based on the CPU's PA
   range (unless it exceeds what the architecture permits for 4k pages)
c) PcdPrePiCpuMemorySize is no longer needed, and can be removed.

Patch #1 introduces ARM_MMU_IDMAP_RANGE and ArmGetPhysicalAddressBits ()
in ArmLib.

Patch #2 updates the virt targets to replace a local definition of
ArmGetPhysicalAddressBits () with the new ArmLib implementation.

Patch #3 updates the ARM MMU code so that the maximum size of the
virtual address space is based on ArmGetPhysicalAddressBits () or
ARM_MMU_IDMAP_RANGE (whichever produces the smallest result), and
no longer on PcdPrePiCpuMemorySize

Patch #4 - #8 update all invocations of BuildCpuHob() so that the size
of the GCD memory map is based on the CPU's capabilities (and no longer
on PcdPrePiCpuMemorySize)

Patch #9 and #10 drop some unused PCD references from .inf files.

Patch #11 updates QemuVirtMemInfoLib to no longer take PcdPrePiCpuMemorySize
into account.

Patch #12 removes all overrides of PcdPrePiCpuMemorySize

Patch #13 (which should only be merged after edk2-platforms has been updated
as well) removes PcdPrePiCpuMemorySize entirely.

Changes since v1:
- add a define ARM_MMU_IDMAP_RANGE that describes the maximum size of
  the virtual address space when running with 4k pages, which may
  deviate from the maximum size when running under the OS
- to avoid having to reason in complicated ways between ARM_MMU_IDMAP_RANGE,
  the number of physical address bits supported by the CPU and the size
  of the GCD memory map, just drop PcdPrePiCpuMemorySize entirely

Note that I dropped a couple of acks (from Laszlo and Philippe) due to the
fact that the code looks rather different now.

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 (13):
  ArmPkg/ArmLib: add support for reading the max physical address space
    size
  ArmVirtPkg: refactor reading of the physical address space size
  ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account
  ArmPkg/CpuPei: base GCD memory space size on CPU's PA range
  ArmPlatformPkg/PrePi: base GCD memory space size on CPU's PA range
  ArmVirtPkg/PrePi: base GCD memory space size on CPU's PA range
  BeagleBoardPkg/PrePi: base GCD memory space size on CPU's PA range
  EmbeddedPkg/PrePiHobLib: base GCD memory space size on CPU's PA range
  ArmPlatformPkg/PlatformPei: drop unused PCD references
  EmbeddedPkg/PrePiLib: drop unused PCD reference
  ArmVirtPkg/QemuVirtMemInfoLib: ignore PcdPrePiCpuMemorySize
  ArmVirtPkg: drop PcdPrePiCpuMemorySize assignments from all platforms
  EmbeddedPkg/EmbeddedPkg.dec: drop PcdPrePiCpuMemorySize declarations

 EmbeddedPkg/EmbeddedPkg.dec                   |  4 --
 ArmVirtPkg/ArmVirt.dsc.inc                    |  3 --
 ArmVirtPkg/ArmVirtQemu.dsc                    |  4 --
 ArmVirtPkg/ArmVirtQemuKernel.dsc              |  4 --
 ArmPkg/Drivers/CpuPei/CpuPei.inf              |  1 -
 ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf    |  3 --
 ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf     |  3 --
 ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf |  3 --
 ArmPlatformPkg/PlatformPei/PlatformPeim.inf   |  3 --
 ArmPlatformPkg/PrePi/PeiMPCore.inf            |  1 -
 ArmPlatformPkg/PrePi/PeiUniCore.inf           |  1 -
 .../QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf |  7 ----
 .../QemuVirtMemInfoPeiLib.inf                 |  7 ----
 .../XenVirtMemInfoLib/XenVirtMemInfoLib.inf   |  6 ---
 .../PrePi/ArmVirtPrePiUniCoreRelocatable.inf  |  1 -
 BeagleBoardPkg/PrePi/PeiUniCore.inf           |  1 -
 .../Library/PrePiHobLib/PrePiHobLib.inf       |  1 -
 EmbeddedPkg/Library/PrePiLib/PrePiLib.inf     |  1 -
 ArmPkg/Include/Library/ArmLib.h               | 17 ++++++++
 .../Include/Library/ArmVirtMemInfoLib.h       |  1 +
 ArmPkg/Drivers/CpuPei/CpuPei.c                |  2 +-
 .../Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 11 +++++-
 ArmPlatformPkg/PrePi/PrePi.c                  |  2 +-
 .../ArmVirtMemoryInitPeiLib.c                 |  5 ++-
 .../QemuVirtMemInfoLib/QemuVirtMemInfoLib.c   | 11 +-----
 .../XenVirtMemInfoLib/XenVirtMemInfoLib.c     |  8 +---
 ArmVirtPkg/PrePi/PrePi.c                      |  2 +-
 BeagleBoardPkg/PrePi/PrePi.c                  |  2 +-
 EmbeddedPkg/Library/PrePiHobLib/Hob.c         |  2 +-
 ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 16 ++++++++
 ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S     |  8 ++++
 .../QemuVirtMemInfoLib/AArch64/PhysAddrTop.S  | 39 -------------------
 .../QemuVirtMemInfoLib/Arm/PhysAddrTop.S      | 24 ------------
 .../XenVirtMemInfoLib/AArch64/PhysAddrTop.S   | 39 -------------------
 .../XenVirtMemInfoLib/Arm/PhysAddrTop.S       | 24 ------------
 35 files changed, 64 insertions(+), 203 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.19.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek Nov. 27, 2018, 12:11 p.m. UTC | #1
On 11/26/18 23:37, 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 refactor the way we deal with this limit, both for bare metal and for

> virtual targets, so that

> a) the range of the GCD memory map is based directly on the CPU's PA range

> b) the range of the 1:1 mapping in the page tables is based on the CPU's PA

>    range (unless it exceeds what the architecture permits for 4k pages)

> c) PcdPrePiCpuMemorySize is no longer needed, and can be removed.

> 

> Patch #1 introduces ARM_MMU_IDMAP_RANGE and ArmGetPhysicalAddressBits ()

> in ArmLib.


OK, so the crucial piece of info I missed under v1 was that given the
fixed 4KB page size under UEFI, we might not be able to identity map all
the memory that the CPU would otherwise be capable of addressing
(assuming the OS set up a larger page size).

However... that seems to leave us with a conundrum. (I'm 100% sure it is
nothing new to you, but it is new to me.)

If we size the GCD memory space map exactly to what we can identity map
under UEFI, then the UEFI memmap will not advertize the rest of the RAM
to the OS, and the memory will be unusable.

On the other hand, if we size the GCD to the exact RAM size (part of
which could be out of reach for the CPU *under UEFI*, using 4KB pages),
then the OS will be happy. But, what happens when gBS->AllocatePages()
is served from such a high (>48bits) address range, and then the client
module tries to access the (not mapped) chunk?

Hm... Actually, once this becomes a practical problem, I think we can
take care of it, in a platform PEIM. Namely, just pre-allocate the
entire inaccessible / unmappable range (beyond 48-bits) as Boot Services
Data type memory. That will keep the rest of the firmware out (no
well-behaving module will read or write memory that it didn't allocate),
and the OS will be happy to release and reuse that memory.

Does this make sense?

I'd like to be sure that I understand this right, before starting my v2
review.

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Nov. 27, 2018, 12:13 p.m. UTC | #2
On Tue, 27 Nov 2018 at 13:11, Laszlo Ersek <lersek@redhat.com> wrote:
>

> On 11/26/18 23:37, 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 refactor the way we deal with this limit, both for bare metal and for

> > virtual targets, so that

> > a) the range of the GCD memory map is based directly on the CPU's PA range

> > b) the range of the 1:1 mapping in the page tables is based on the CPU's PA

> >    range (unless it exceeds what the architecture permits for 4k pages)

> > c) PcdPrePiCpuMemorySize is no longer needed, and can be removed.

> >

> > Patch #1 introduces ARM_MMU_IDMAP_RANGE and ArmGetPhysicalAddressBits ()

> > in ArmLib.

>

> OK, so the crucial piece of info I missed under v1 was that given the

> fixed 4KB page size under UEFI, we might not be able to identity map all

> the memory that the CPU would otherwise be capable of addressing

> (assuming the OS set up a larger page size).

>

> However... that seems to leave us with a conundrum. (I'm 100% sure it is

> nothing new to you, but it is new to me.)

>

> If we size the GCD memory space map exactly to what we can identity map

> under UEFI, then the UEFI memmap will not advertize the rest of the RAM

> to the OS, and the memory will be unusable.

>

> On the other hand, if we size the GCD to the exact RAM size (part of

> which could be out of reach for the CPU *under UEFI*, using 4KB pages),

> then the OS will be happy. But, what happens when gBS->AllocatePages()

> is served from such a high (>48bits) address range, and then the client

> module tries to access the (not mapped) chunk?

>


That is an excellent question, given that IA32 and ARM are in exactly
the same boat with [L]PAE.

> Hm... Actually, once this becomes a practical problem, I think we can

> take care of it, in a platform PEIM. Namely, just pre-allocate the

> entire inaccessible / unmappable range (beyond 48-bits) as Boot Services

> Data type memory. That will keep the rest of the firmware out (no

> well-behaving module will read or write memory that it didn't allocate),

> and the OS will be happy to release and reuse that memory.

>

> Does this make sense?

>


I will look into how IA32 and ARM deal with this at the moment. Well spotted!

> I'd like to be sure that I understand this right, before starting my v2

> review.

>

> Thanks!

> Laszlo

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Nov. 27, 2018, 2:35 p.m. UTC | #3
On 11/27/18 13:13, Ard Biesheuvel wrote:
> On Tue, 27 Nov 2018 at 13:11, Laszlo Ersek <lersek@redhat.com> wrote:

>>

>> On 11/26/18 23:37, 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 refactor the way we deal with this limit, both for bare metal and for

>>> virtual targets, so that

>>> a) the range of the GCD memory map is based directly on the CPU's PA range

>>> b) the range of the 1:1 mapping in the page tables is based on the CPU's PA

>>>    range (unless it exceeds what the architecture permits for 4k pages)

>>> c) PcdPrePiCpuMemorySize is no longer needed, and can be removed.

>>>

>>> Patch #1 introduces ARM_MMU_IDMAP_RANGE and ArmGetPhysicalAddressBits ()

>>> in ArmLib.

>>

>> OK, so the crucial piece of info I missed under v1 was that given the

>> fixed 4KB page size under UEFI, we might not be able to identity map all

>> the memory that the CPU would otherwise be capable of addressing

>> (assuming the OS set up a larger page size).

>>

>> However... that seems to leave us with a conundrum. (I'm 100% sure it is

>> nothing new to you, but it is new to me.)

>>

>> If we size the GCD memory space map exactly to what we can identity map

>> under UEFI, then the UEFI memmap will not advertize the rest of the RAM

>> to the OS, and the memory will be unusable.

>>

>> On the other hand, if we size the GCD to the exact RAM size (part of

>> which could be out of reach for the CPU *under UEFI*, using 4KB pages),

>> then the OS will be happy. But, what happens when gBS->AllocatePages()

>> is served from such a high (>48bits) address range, and then the client

>> module tries to access the (not mapped) chunk?

>>

> 

> That is an excellent question, given that IA32 and ARM are in exactly

> the same boat with [L]PAE.


And that's an excellent observation too; I should have noticed the
parallel with at least IA32 myself!

I remember the article where Brian explains why vendors ship IA32-only
firmware on 64-bit capable Intel processors. Hm... It's here:

https://software.intel.com/en-us/blogs/2015/07/22/why-cheap-systems-run-32-bit-uefi-on-x64-systems

Obviously Linux people were never happy with that, so they worked around
it, and the kernel switches to 64-bit mode itself, AFAIK... But what
about the RAM that the 32-bit DXE phase (and firmware runtime) can't
see, but the OS can? Maybe Linux can provide some pointers here (as you
say).

[...]

Thank you!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel