On 11/27/18 15:54, Ard Biesheuvel wrote:
> Currently, we map DRAM as EFI_MEMORY_WB, and the remainder of the
> entire virtual address space is mapped with EFI_MEMORY_UC attributes,
> regardless of whether any devices actually reside there.
>
> Now that we are relaxing the address space limit to more than 40 bits,
> mapping all that address space actually takes up more space in page
> tables than we have so far made available as temporary RAM. So let's
> get rid of the mapping rather than increasing the available RAM, given
> that the mapping is not particularly useful anyway.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c | 17
> +++++------------
> 1 file changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> index 815ca145b644..70863abb2e7b 100644
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> @@ -73,21 +73,14 @@ ArmVirtGetMemoryMap (
> VirtualMemoryTable[1].Length = VirtualMemoryTable[0].PhysicalBase;
> VirtualMemoryTable[1].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>
> - // Peripheral space after DRAM
> - VirtualMemoryTable[2].PhysicalBase = VirtualMemoryTable[0].Length +
> VirtualMemoryTable[1].Length;
> - VirtualMemoryTable[2].VirtualBase = VirtualMemoryTable[2].PhysicalBase;
> - VirtualMemoryTable[2].Length = TopOfAddressSpace -
> - VirtualMemoryTable[2].PhysicalBase;
> - VirtualMemoryTable[2].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> -
> // Remap the FD region as normal executable memory
> - VirtualMemoryTable[3].PhysicalBase = PcdGet64 (PcdFdBaseAddress);
> - VirtualMemoryTable[3].VirtualBase = VirtualMemoryTable[3].PhysicalBase;
> - VirtualMemoryTable[3].Length = FixedPcdGet32 (PcdFdSize);
> - VirtualMemoryTable[3].Attributes =
> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> + VirtualMemoryTable[2].PhysicalBase = PcdGet64 (PcdFdBaseAddress);
> + VirtualMemoryTable[2].VirtualBase = VirtualMemoryTable[2].PhysicalBase;
> + VirtualMemoryTable[2].Length = FixedPcdGet32 (PcdFdSize);
> + VirtualMemoryTable[2].Attributes =
> ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
>
> // End of Table
> - ZeroMem (&VirtualMemoryTable[4], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
> + ZeroMem (&VirtualMemoryTable[3], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
>
> *VirtualMemoryMap = VirtualMemoryTable;
> }
>
(1) This supplants your other series "[PATCH v2 00/13] ArmPkg, ArmVirtPkg: lift
40-bit IPA space limit" minimally due to a contextual conflict; is that right?
(2) Regarding the patch itself. Currently we have:
- VirtualMemoryTable[0]: "System DRAM"
- VirtualMemoryTable[1]: "Peripheral space before DRAM"
- VirtualMemoryTable[2]: "Peripheral space after DRAM"
- VirtualMemoryTable[3]: "Remap the FD region as normal executable
memory"
Let's see what is affected, from the physical map in QEMU's "hw/arm/virt.c", if
we evict VirtualMemoryTable[2]:
/* Additional 64 MB redist region (can contain up to 512 redistributors) */
[VIRT_GIC_REDIST2] = { 0x4000000000ULL, 0x4000000 },
[VIRT_PCIE_ECAM_HIGH] = { 0x4010000000ULL, 0x10000000 },
/* Second PCIe window, 512GB wide at the 512GB boundary */
[VIRT_PCIE_MMIO_HIGH] = { 0x8000000000ULL, 0x8000000000ULL },
I have no idea about VIRT_GIC_REDIST2, but, given that in ArmVirtQemu we do
uniprocessor only, it doesn't seem worrisome.
VIRT_PCIE_ECAM_HIGH should be handled by patch #1. (VIRT_PCIE_ECAM_HIGH
*replaces* [VIRT_PCIE_ECAM] = { 0x3f000000, 0x01000000 }, if memory serves.)
VIRT_PCIE_MMIO_HIGH is *in addition* to [VIRT_PCIE_MMIO] = { 0x10000000,
0x2eff0000 }, but we need not do anything about that specifically, because we
advertize it to PciHostBridgeDxe via our FdtPciHostBridgeLib instance, and
PciHostBridgeDxe handles the GCD aspects for the range automatically.
So, together with patch #1, I think this is safe. If we catch a data abort
anyway, we'll have to clean up the GCD handling in other drivers.
Reviewed-by: Laszlo Ersek <[email protected]>
Thanks
LAszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
