Re: [PATCH v2 2/2] ArmVirtPkg/QemuVirtMemInfoLib: remove 1:1 mapping of top of PA range

[Laszlo Ersek <lersek@redhat.com>]

On 11/27/18 18:52, Ard Biesheuvel wrote:
> On Tue, 27 Nov 2018 at 18:26, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> 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 <ard.biesheuvel@linaro.org>
>>> ---
>>>  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?
>>
> 
> Not quite. It complements it, in the sense that is should fix the
> issue reported by Eric when mapping the entire address 48-bit address
> space.

Oh, you meant this one *on top* of that? In particular, on top of:

[edk2] [PATCH v2 11/13] ArmVirtPkg/QemuVirtMemInfoLib: ignore
                        PcdPrePiCpuMemorySize

That wasn't clear to me, sorry.

If this one comes on top of the v2 13-part series, do you ultimately
need v2 11/13 as a separate patch -- in that form anyway? It seems that
you could squash this patch into v2 11/13, and eliminate the dependency
on PcdPrePiCpuMemorySize *by* killing the entry that maps the Peripheral
space after DRAM.

Thanks,
Laszlo