Re: [edk2] [PATCH edk2-platforms 02/14] Silicon/Synquacer: add MemoryInitPeiLib implementation

2017-09-11 Thread Leif Lindholm 
On Fri, Sep 08, 2017 at 07:23:03PM +0100, Ard Biesheuvel wrote:
> Replace the common MemoryInitPeiLib implementation with one that does
> not remove the primary FV from the memory map. This is a waste of
> memory and TLB entries, given that the OS can no longer use a 1 GB
> block mapping to map this memory.
> 
> Since we have our own implementation now, there is no point in using
> ArmPlatformLib's GetVirtualMemoryMap() implementation, and we can
> simply declare and map the regions directly.

Is there any reason we could not extend this to a better core
implementation, kept in ArmPkg?
(No, that does not need to happen now.)

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  
> Silicon/Socionext/Synquacer/Library/SynquacerMemoryInitPeiLib/SynquacerMemoryInitPeiLib.c
>| 140 
>  
> Silicon/Socionext/Synquacer/Library/SynquacerMemoryInitPeiLib/SynquacerMemoryInitPeiLib.inf
>  |  50 +++
>  2 files changed, 190 insertions(+)
> 
> diff --git 
> a/Silicon/Socionext/Synquacer/Library/SynquacerMemoryInitPeiLib/SynquacerMemoryInitPeiLib.c
>  
> b/Silicon/Socionext/Synquacer/Library/SynquacerMemoryInitPeiLib/SynquacerMemoryInitPeiLib.c
> new file mode 100644
> index ..1d25d63f1b6c
> --- /dev/null
> +++ 
> b/Silicon/Socionext/Synquacer/Library/SynquacerMemoryInitPeiLib/SynquacerMemoryInitPeiLib.c
> @@ -0,0 +1,140 @@
> +/** @file
> +*
> +*  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> +*  Copyright (c) 2017, Linaro, Ltd. 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 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#define ARM_MEMORY_REGION(Base, Size) \
> +  { (Base), (Base), (Size), ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK }
> +
> +#define ARM_DEVICE_REGION(Base, Size) \
> +  { (Base), (Base), (Size), ARM_MEMORY_REGION_ATTRIBUTE_DEVICE }
> +
> +VOID
> +BuildMemoryTypeInformationHob (
> +  VOID
> +  );
> +
> +STATIC ARM_MEMORY_REGION_DESCRIPTOR mVirtualMemoryTable[] = {
> +  // DDR - 2 GB
> +  ARM_MEMORY_REGION (SYNQUACER_SYSTEM_MEMORY_1_BASE,
> + SYNQUACER_SYSTEM_MEMORY_1_SZ),
> +
> +  // DDR - 30 GB
> +  ARM_MEMORY_REGION (SYNQUACER_SYSTEM_MEMORY_2_BASE,
> + SYNQUACER_SYSTEM_MEMORY_2_SZ),
> +
> +  // DDR - 32 GB
> +//  ARM_MEMORY_REGION (SYNQUACER_SYSTEM_MEMORY_3_BASE,
> +// SYNQUACER_SYSTEM_MEMORY_3_SZ),

Could these be kept behind a #define (or FixedPcd) until no longer
needed?

> +
> +  // Synquacer OnChip non-secure ROM
> +  ARM_MEMORY_REGION (SYNQUACER_NON_SECURE_ROM_BASE,
> + SYNQUACER_NON_SECURE_ROM_SZ),
> +
> +  // Synquacer OnChip peripherals
> +  ARM_DEVICE_REGION (SYNQUACER_PERIPHERALS_BASE,
> + SYNQUACER_PERIPHERALS_SZ),
> +
> +  // Synquacer OnChip non-secure SRAM
> +  ARM_MEMORY_REGION (SYNQUACER_NON_SECURE_SRAM_BASE,
> + SYNQUACER_NON_SECURE_SRAM_SZ),
> +
> +  // Synquacer GIC-500
> +  ARM_DEVICE_REGION (SYNQUACER_GIC500_DIST_BASE, SYNQUACER_GIC500_DIST_SIZE),
> +  ARM_DEVICE_REGION (SYNQUACER_GIC500_RDIST_BASE, 
> SYNQUACER_GIC500_RDIST_SIZE),
> +
> +  // Synquacer eMMC(SDH30)
> +  ARM_DEVICE_REGION (SYNQUACER_EMMC_BASE, SYNQUACER_EMMC_BASE_SZ),
> +
> +  // Synquacer EEPROM
> +  ARM_DEVICE_REGION (SYNQUACER_EEPROM_BASE, SYNQUACER_EEPROM_BASE_SZ),
> +
> +  // Synquacer NETSEC
> +  ARM_DEVICE_REGION (SYNQUACER_NETSEC_BASE, SYNQUACER_NETSEC_BASE_SZ),
> +
> +  // PCIe control registers
> +  ARM_DEVICE_REGION (SYNQUACER_PCIE_BASE, SYNQUACER_PCIE_SIZE),
> +
> +  // PCIe config space
> +  ARM_DEVICE_REGION (SYNQUACER_PCI_SEG0_CONFIG_BASE,
> + SYNQUACER_PCI_SEG0_CONFIG_SIZE),
> +  ARM_DEVICE_REGION (SYNQUACER_PCI_SEG1_CONFIG_BASE,
> + SYNQUACER_PCI_SEG1_CONFIG_SIZE),
> +
> +  // PCIe I/O space
> +  ARM_DEVICE_REGION (SYNQUACER_PCI_SEG0_PORTIO_MEMBASE,
> + SYNQUACER_PCI_SEG0_PORTIO_MEMSIZE),
> +  ARM_DEVICE_REGION (SYNQUACER_PCI_SEG1_PORTIO_MEMBASE,
> + SYNQUACER_PCI_SEG1_PORTIO_MEMSIZE),
> +
> +  { }
> +};
> +
> +EFI_STATUS
> +EFIAPI
> +MemoryPeim (
> +  IN EFI_PHYSICAL_ADDRESS   UefiMemoryBase,
> +  IN UINT64 UefiMemorySize
> +  )
> +{
> +  EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
> +  RETURN_STATUS Status;
> +
> +  ResourceAttributes =
> +  EFI_RESOURCE_ATTRIBUTE_PRESENT |
> +  EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> +

[edk2] [PATCH edk2-platforms 02/14] Silicon/Synquacer: add MemoryInitPeiLib implementation

2017-09-08 Thread Ard Biesheuvel 
Replace the common MemoryInitPeiLib implementation with one that does
not remove the primary FV from the memory map. This is a waste of
memory and TLB entries, given that the OS can no longer use a 1 GB
block mapping to map this memory.

Since we have our own implementation now, there is no point in using
ArmPlatformLib's GetVirtualMemoryMap() implementation, and we can
simply declare and map the regions directly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 
Silicon/Socionext/Synquacer/Library/SynquacerMemoryInitPeiLib/SynquacerMemoryInitPeiLib.c
   | 140 
 
Silicon/Socionext/Synquacer/Library/SynquacerMemoryInitPeiLib/SynquacerMemoryInitPeiLib.inf
 |  50 +++
 2 files changed, 190 insertions(+)

diff --git 
a/Silicon/Socionext/Synquacer/Library/SynquacerMemoryInitPeiLib/SynquacerMemoryInitPeiLib.c
 
b/Silicon/Socionext/Synquacer/Library/SynquacerMemoryInitPeiLib/SynquacerMemoryInitPeiLib.c
new file mode 100644
index ..1d25d63f1b6c
--- /dev/null
+++ 
b/Silicon/Socionext/Synquacer/Library/SynquacerMemoryInitPeiLib/SynquacerMemoryInitPeiLib.c
@@ -0,0 +1,140 @@
+/** @file
+*
+*  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+*  Copyright (c) 2017, Linaro, Ltd. 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 
+#include 
+#include 
+
+#include 
+#include 
+
+#define ARM_MEMORY_REGION(Base, Size) \
+  { (Base), (Base), (Size), ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK }
+
+#define ARM_DEVICE_REGION(Base, Size) \
+  { (Base), (Base), (Size), ARM_MEMORY_REGION_ATTRIBUTE_DEVICE }
+
+VOID
+BuildMemoryTypeInformationHob (
+  VOID
+  );
+
+STATIC ARM_MEMORY_REGION_DESCRIPTOR mVirtualMemoryTable[] = {
+  // DDR - 2 GB
+  ARM_MEMORY_REGION (SYNQUACER_SYSTEM_MEMORY_1_BASE,
+ SYNQUACER_SYSTEM_MEMORY_1_SZ),
+
+  // DDR - 30 GB
+  ARM_MEMORY_REGION (SYNQUACER_SYSTEM_MEMORY_2_BASE,
+ SYNQUACER_SYSTEM_MEMORY_2_SZ),
+
+  // DDR - 32 GB
+//  ARM_MEMORY_REGION (SYNQUACER_SYSTEM_MEMORY_3_BASE,
+// SYNQUACER_SYSTEM_MEMORY_3_SZ),
+
+  // Synquacer OnChip non-secure ROM
+  ARM_MEMORY_REGION (SYNQUACER_NON_SECURE_ROM_BASE,
+ SYNQUACER_NON_SECURE_ROM_SZ),
+
+  // Synquacer OnChip peripherals
+  ARM_DEVICE_REGION (SYNQUACER_PERIPHERALS_BASE,
+ SYNQUACER_PERIPHERALS_SZ),
+
+  // Synquacer OnChip non-secure SRAM
+  ARM_MEMORY_REGION (SYNQUACER_NON_SECURE_SRAM_BASE,
+ SYNQUACER_NON_SECURE_SRAM_SZ),
+
+  // Synquacer GIC-500
+  ARM_DEVICE_REGION (SYNQUACER_GIC500_DIST_BASE, SYNQUACER_GIC500_DIST_SIZE),
+  ARM_DEVICE_REGION (SYNQUACER_GIC500_RDIST_BASE, SYNQUACER_GIC500_RDIST_SIZE),
+
+  // Synquacer eMMC(SDH30)
+  ARM_DEVICE_REGION (SYNQUACER_EMMC_BASE, SYNQUACER_EMMC_BASE_SZ),
+
+  // Synquacer EEPROM
+  ARM_DEVICE_REGION (SYNQUACER_EEPROM_BASE, SYNQUACER_EEPROM_BASE_SZ),
+
+  // Synquacer NETSEC
+  ARM_DEVICE_REGION (SYNQUACER_NETSEC_BASE, SYNQUACER_NETSEC_BASE_SZ),
+
+  // PCIe control registers
+  ARM_DEVICE_REGION (SYNQUACER_PCIE_BASE, SYNQUACER_PCIE_SIZE),
+
+  // PCIe config space
+  ARM_DEVICE_REGION (SYNQUACER_PCI_SEG0_CONFIG_BASE,
+ SYNQUACER_PCI_SEG0_CONFIG_SIZE),
+  ARM_DEVICE_REGION (SYNQUACER_PCI_SEG1_CONFIG_BASE,
+ SYNQUACER_PCI_SEG1_CONFIG_SIZE),
+
+  // PCIe I/O space
+  ARM_DEVICE_REGION (SYNQUACER_PCI_SEG0_PORTIO_MEMBASE,
+ SYNQUACER_PCI_SEG0_PORTIO_MEMSIZE),
+  ARM_DEVICE_REGION (SYNQUACER_PCI_SEG1_PORTIO_MEMBASE,
+ SYNQUACER_PCI_SEG1_PORTIO_MEMSIZE),
+
+  { }
+};
+
+EFI_STATUS
+EFIAPI
+MemoryPeim (
+  IN EFI_PHYSICAL_ADDRESS   UefiMemoryBase,
+  IN UINT64 UefiMemorySize
+  )
+{
+  EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
+  RETURN_STATUS Status;
+
+  ResourceAttributes =
+  EFI_RESOURCE_ATTRIBUTE_PRESENT |
+  EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
+  EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
+  EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
+  EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE |
+  EFI_RESOURCE_ATTRIBUTE_TESTED;
+
+  BuildResourceDescriptorHob (
+EFI_RESOURCE_SYSTEM_MEMORY,
+ResourceAttributes,
+SYNQUACER_SYSTEM_MEMORY_1_BASE,
+SYNQUACER_SYSTEM_MEMORY_1_SZ);
+
+  BuildResourceDescriptorHob (
+EFI_RESOURCE_SYSTEM_MEMORY,
+ResourceAttributes,
+SYNQUACER_SYSTEM_MEMORY_2_BASE,
+SYNQUACER_SYSTEM_MEMORY_2_SZ);
+
+//  BuildResourceDescriptorHob (
+//