Re: [edk2] [PATCH v2 10/14] ArmVirtPkg/ArmVirtXen: add ArmVirtMemInfoLib implementation

[Julien Grall]

Hi Ard,

On 11/22/2017 10:07 AM, Ard Biesheuvel wrote:

Clone the existing ArmPlatformGetVirtualMemoryMap () for this platform,
clean it up slightly (by using a static buffer rather than a heap
allocation, and removing the support for uncached DRAM mappings), and
turn it into a new ArmVirtMemInfoLib implementation.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Acked-by: Laszlo Ersek 
Cc: Julien Grall 


I have tested the series on both Arm32 and Arm64 guests:

Tested-by: Julien Grall 

[...]


+VOID
+EFIAPI
+ArmVirtGetMemoryMap (
+  OUT ARM_MEMORY_REGION_DESCRIPTOR   **VirtualMemoryMap
+  )
+{
+  ASSERT (VirtualMemoryMap != NULL);
+
+  //
+  // Map the entire physical memory space as cached. The only device
+  // we care about is the GIC, which will be stage 2 mapped as a device
+  // by the hypervisor, overriding the cached mapping we install here.
+  //


I was about to complain about the fact you rely on the hypervisor 
setting correct stage-2 memory attribute. But I see this is a copy of 
the current code :).


We did relax the attribute for MMIO in the case of the Hardware Domain. 
I will keep the UEFI implementation in mind if we ever decide to relax 
for guests too.




+  mVirtualMemoryTable[0].PhysicalBase = 0x0;
+  mVirtualMemoryTable[0].VirtualBase  = 0x0;
+  mVirtualMemoryTable[0].Length   = ArmGetPhysAddrTop ();
+  mVirtualMemoryTable[0].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+
+  mVirtualMemoryTable[1].PhysicalBase = 0x0;
+  mVirtualMemoryTable[1].VirtualBase  = 0x0;
+  mVirtualMemoryTable[1].Length   = 0x0;
+  mVirtualMemoryTable[1].Attributes   = 0x0;
+
+  *VirtualMemoryMap = mVirtualMemoryTable;
+}


Cheers,

--
Julien Grall
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v2 10/14] ArmVirtPkg/ArmVirtXen: add ArmVirtMemInfoLib implementation

[Ard Biesheuvel]

Clone the existing ArmPlatformGetVirtualMemoryMap () for this platform,
clean it up slightly (by using a static buffer rather than a heap
allocation, and removing the support for uncached DRAM mappings), and
turn it into a new ArmVirtMemInfoLib implementation.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Acked-by: Laszlo Ersek 
Cc: Julien Grall 
---
 ArmVirtPkg/ArmVirtXen.dsc  |  1 +
 ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S | 39 
 ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S | 24 
 ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c   | 63 

 ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf | 41 +
 5 files changed, 168 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
index 7a443483d1ac..3df684d13cb0 100644
--- a/ArmVirtPkg/ArmVirtXen.dsc
+++ b/ArmVirtPkg/ArmVirtXen.dsc
@@ -44,6 +44,7 @@ [LibraryClasses]
   
VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
 
   
ArmPlatformLib|ArmVirtPkg/Library/ArmXenRelocatablePlatformLib/ArmXenRelocatablePlatformLib.inf
+  ArmVirtMemInfoLib|ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf
 
   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
 
diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S 
b/ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S
new file mode 100644
index ..a1f6a194d59b
--- /dev/null
+++ b/ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S
@@ -0,0 +1,39 @@
+#
+#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
+#  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD 
License
+#  which accompanies this distribution.  The full text of the license may be 
found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+#
+
+#include 
+
+//EFI_PHYSICAL_ADDRESS
+//GetPhysAddrTop (
+//  VOID
+//  );
+ASM_FUNC(ArmGetPhysAddrTop)
+  mrs   x0, id_aa64mmfr0_el1
+  adr   x1, .LPARanges
+  and   x0, x0, #7
+  ldrb  w1, [x1, x0]
+  mov   x0, #1
+  lsl   x0, x0, x1
+  ret
+
+//
+// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the
+// physical address space support on this CPU:
+// 0 == 32 bits, 1 == 36 bits, etc etc
+// 6 and 7 are reserved
+//
+.LPARanges:
+  .byte 32, 36, 40, 42, 44, 48, -1, -1
+
+ASM_FUNCTION_REMOVE_IF_UNREFERENCED
diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S 
b/ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S
new file mode 100644
index ..9cd81529fb3d
--- /dev/null
+++ b/ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S
@@ -0,0 +1,24 @@
+#
+#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
+#  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD 
License
+#  which accompanies this distribution.  The full text of the license may be 
found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+#
+
+#include 
+
+//EFI_PHYSICAL_ADDRESS
+//GetPhysAddrTop (
+//  VOID
+//  );
+ASM_FUNC(ArmGetPhysAddrTop)
+  mov   r0, #0x
+  mov   r1, #0x1
+  bxlr
diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c 
b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c
new file mode 100644
index ..88ff3167cbfd
--- /dev/null
+++ b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c
@@ -0,0 +1,63 @@
+/** @file
+
+  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+
+STATIC ARM_MEMORY_REGION_DESCRIPTOR  mVirtualMemoryTable[2];
+
+EFI_PHYSICAL_ADDRESS
+ArmGetPhysAddrTop (
+  VOID
+  );
+
+/**
+  Return the Virtual Memory Map of your platform
+
+  This Virtual Memory Map is used by MemoryInitPei Module to initialize the MMU
+  on your platform.
+
+  @param[out]   VirtualMemoryMapArray of