public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Leif Lindholm" <leif.lindholm@linaro.org>
To: Pete Batard <pete@akeo.ie>
Cc: devel@edk2.groups.io, ard.biesheuvel@linaro.org, philmd@redhat.com
Subject: Re: [edk2-platforms][PATCH 5/8] Platform/RPi: Clean up and improve early memory init
Date: Mon, 18 Nov 2019 17:38:50 +0000	[thread overview]
Message-ID: <20191118173850.GW7323@bivouac.eciton.net> (raw)
In-Reply-To: <3f9be1ea-db97-1ca8-ed20-e0b3fa211825@akeo.ie>

On Mon, Nov 18, 2019 at 05:34:25PM +0000, Pete Batard wrote:
> Hi Leif,
> 
> On 2019.11.18 17:20, Leif Lindholm wrote:
> > On Thu, Nov 14, 2019 at 04:07:37PM +0000, Pete Batard wrote:
> > > This patch improves memory initialization for the Raspberry Pi Platform by:
> > > 
> > > Using VideoCore mailbox data to reserve only the regions that are actually
> > > mapped. Especially, besides the base RAM size, which was already set using
> > > VideoCore data, we can set the GPU/VideoCore base address as well as the
> > > extended RAM, which is the memory beyond 1 GB that is available on models
> > > such as the Raspberry Pi 4 (for the 2GB or 4GB versions).
> > > 
> > > Introducing a new PcdExtendedMemoryBase PCD for the base address of the
> > > extended memory region, which currently cannot be retrieved from VideoCore
> > > (MBOX_GET_ARM_MEMSIZE still only returns a single region on Bcm2711).
> > > 
> > > Introducing a new RpiPlatformGetVirtualMemoryInfo() companion call to
> > > ArmPlatformGetVirtualMemoryMap() that allows us greatly simplify the
> > > registration of each segment in MemoryPeim() as well as making it easier
> > > to maintain for future models.
> > > 
> > > We also fix SoC register space that should have been marked as reserved
> > > but wasn't until now and remove the unreferenced mSystemMemoryEnd extern
> > > in MemoryInitPeiLib.c.
> > 
> > Minor style comments and one copyright question below.
> > 
> > > Signed-off-by: Pete Batard <pete@akeo.ie>
> > > ---
> > >   Platform/RaspberryPi/Include/Library/RPiMem.h                      |  26 +++
> > >   Platform/RaspberryPi/Library/MemoryInitPeiLib/MemoryInitPeiLib.c   |  94 ++++----
> > >   Platform/RaspberryPi/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf |   1 +
> > >   Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf           |   3 +
> > >   Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c          | 242 +++++++++++---------
> > >   Platform/RaspberryPi/RaspberryPi.dec                               |   1 +
> > >   6 files changed, 210 insertions(+), 157 deletions(-)
> > > 
> > > diff --git a/Platform/RaspberryPi/Include/Library/RPiMem.h b/Platform/RaspberryPi/Include/Library/RPiMem.h
> > > new file mode 100644
> > > index 000000000000..0bfc697ed094
> > > --- /dev/null
> > > +++ b/Platform/RaspberryPi/Include/Library/RPiMem.h
> > > @@ -0,0 +1,26 @@
> > > +/** @file
> > > + *
> > > + *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
> > > + *
> > > + *  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > + *
> > > + **/
> > > +
> > > +#ifndef __RPI_MEM_H__
> > > +#define __RPI_MEM_H__
> > 
> > As mentioned for earlier patch, please drop leading __.
> 
> Will do. I assume this applies to trailing __ as well.

Thanks!
Amusingly not.
There are some toolchains that complain about leading __, but none I
am aware of that complain about trailing ones. I think they may even
be mandated by the coding style :)

/
    Leif

> 
> > > +
> > > +#define RPI_MEM_BASIC_REGION    0
> > > +#define RPI_MEM_RUNTIME_REGION  1
> > > +#define RPI_MEM_RESERVED_REGION 2
> > > +
> > > +typedef struct {
> > > +  CONST CHAR16*                 Name;
> > > +  UINTN                         Type;
> > > +} RPI_MEMORY_REGION_INFO;
> > > +
> > > +VOID
> > > +RpiPlatformGetVirtualMemoryInfo (
> > > +  IN RPI_MEMORY_REGION_INFO** MemoryInfo
> > > +  );
> > > +
> > > +#endif /* __RPI_MEM_H__ */
> > > diff --git a/Platform/RaspberryPi/Library/MemoryInitPeiLib/MemoryInitPeiLib.c b/Platform/RaspberryPi/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> > > index 60cf397f8baa..3a0f7e19e993 100644
> > > --- a/Platform/RaspberryPi/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> > > +++ b/Platform/RaspberryPi/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> > > @@ -15,8 +15,7 @@
> > >   #include <Library/HobLib.h>
> > >   #include <Library/MemoryAllocationLib.h>
> > >   #include <Library/PcdLib.h>
> > > -
> > > -extern UINT64 mSystemMemoryEnd;
> > > +#include <Library/RPiMem.h>
> > >   VOID
> > >   BuildMemoryTypeInformationHob (
> > > @@ -39,23 +38,32 @@ InitMmu (
> > >     }
> > >   }
> > > +STATIC
> > > +VOID
> > > +AddBasicMemoryRegion (
> > > +  IN ARM_MEMORY_REGION_DESCRIPTOR *Desc
> > > +)
> > > +{
> > > +  BuildResourceDescriptorHob (
> > > +    EFI_RESOURCE_SYSTEM_MEMORY,
> > > +    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,
> > > +    Desc->PhysicalBase,
> > > +    Desc->Length
> > > +  );
> > > +}
> > > +
> > >   STATIC
> > >   VOID
> > >   AddRuntimeServicesRegion (
> > >     IN ARM_MEMORY_REGION_DESCRIPTOR *Desc
> > >   )
> > >   {
> > > -  BuildResourceDescriptorHob (
> > > -    EFI_RESOURCE_SYSTEM_MEMORY,
> > > -    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,
> > > -    Desc->PhysicalBase,
> > > -    Desc->Length
> > > -  );
> > > +  AddBasicMemoryRegion (Desc);
> > >     BuildMemoryAllocationHob (
> > >       Desc->PhysicalBase,
> > > @@ -70,17 +78,7 @@ AddReservedMemoryRegion (
> > >     IN ARM_MEMORY_REGION_DESCRIPTOR *Desc
> > >     )
> > >   {
> > > -  BuildResourceDescriptorHob (
> > > -    EFI_RESOURCE_SYSTEM_MEMORY,
> > > -    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,
> > > -    Desc->PhysicalBase,
> > > -    Desc->Length
> > > -  );
> > > +  AddBasicMemoryRegion (Desc);
> > >     BuildMemoryAllocationHob (
> > >       Desc->PhysicalBase,
> > > @@ -89,6 +87,12 @@ AddReservedMemoryRegion (
> > >     );
> > >   }
> > > +void (*AddRegion[]) (IN ARM_MEMORY_REGION_DESCRIPTOR *Desc) = {
> > > +  AddBasicMemoryRegion,
> > > +  AddRuntimeServicesRegion,
> > > +  AddReservedMemoryRegion,
> > > +  };
> > > +
> > >   /*++
> > >   Routine Description:
> > > @@ -113,36 +117,28 @@ MemoryPeim (
> > >     )
> > >   {
> > >     ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
> > > +  RPI_MEMORY_REGION_INFO       *MemoryInfo;
> > > +  UINTN                        Index;
> > >     // Get Virtual Memory Map from the Platform Library
> > >     ArmPlatformGetVirtualMemoryMap (&MemoryTable);
> > > -  // Ensure PcdSystemMemorySize has been set
> > > -  ASSERT (PcdGet64 (PcdSystemMemorySize) != 0);
> > > +  // Get additional info not provided by MemoryTable
> > > +  RpiPlatformGetVirtualMemoryInfo (&MemoryInfo);
> > > -  // FD without variable store
> > > -  AddReservedMemoryRegion (&MemoryTable[0]);
> > > -
> > > -  // Variable store.
> > > -  AddRuntimeServicesRegion (&MemoryTable[1]);
> > > -
> > > -  // Trusted Firmware region
> > > -  AddReservedMemoryRegion (&MemoryTable[2]);
> > > -
> > > -  // Usable memory.
> > > -  BuildResourceDescriptorHob (
> > > -    EFI_RESOURCE_SYSTEM_MEMORY,
> > > -    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,
> > > -    MemoryTable[3].PhysicalBase,
> > > -    MemoryTable[3].Length
> > > -  );
> > > -
> > > -  AddReservedMemoryRegion (&MemoryTable[4]);
> > > +  // Register each memory region
> > > +  for (Index = 0; MemoryTable[Index].Length != 0; Index++) {
> > > +    ASSERT (MemoryInfo[Index].Type < ARRAY_SIZE (AddRegion));
> > > +    DEBUG ((DEBUG_INFO, "%s:\n"
> > > +      "\tPhysicalBase: 0x%lX\n"
> > > +      "\tVirtualBase: 0x%lX\n"
> > > +      "\tLength: 0x%lX\n",
> > > +      MemoryInfo[Index].Name,
> > > +      MemoryTable[Index].PhysicalBase,
> > > +      MemoryTable[Index].VirtualBase,
> > > +      MemoryTable[Index].Length));
> > > +    AddRegion[MemoryInfo[Index].Type] (&MemoryTable[Index]);
> > > +  }
> > >     // Build Memory Allocation Hob
> > >     InitMmu (MemoryTable);
> > > diff --git a/Platform/RaspberryPi/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf b/Platform/RaspberryPi/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
> > > index 0084c010936d..d39210c7a4f1 100644
> > > --- a/Platform/RaspberryPi/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
> > > +++ b/Platform/RaspberryPi/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
> > > @@ -24,6 +24,7 @@ [Packages]
> > >     EmbeddedPkg/EmbeddedPkg.dec
> > >     ArmPkg/ArmPkg.dec
> > >     ArmPlatformPkg/ArmPlatformPkg.dec
> > > +  Platform/RaspberryPi/RaspberryPi.dec
> > >   [LibraryClasses]
> > >     DebugLib
> > > diff --git a/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf b/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
> > > index 85462febdd8d..c0e2a75451c3 100644
> > > --- a/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
> > > +++ b/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
> > > @@ -22,6 +22,7 @@ [Packages]
> > >     EmbeddedPkg/EmbeddedPkg.dec
> > >     ArmPkg/ArmPkg.dec
> > >     ArmPlatformPkg/ArmPlatformPkg.dec
> > > +  Silicon/Broadcom/Bcm27xx/Bcm27xx.dec
> > >     Silicon/Broadcom/Bcm283x/Bcm283x.dec
> > >     Platform/RaspberryPi/RaspberryPi.dec
> > > @@ -50,10 +51,12 @@ [FixedPcd]
> > >     gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset
> > >     gArmTokenSpaceGuid.PcdSystemMemoryBase
> > >     gArmTokenSpaceGuid.PcdSystemMemorySize
> > > +  gRaspberryPiTokenSpaceGuid.PcdExtendedMemoryBase
> > >     gRaspberryPiTokenSpaceGuid.PcdNvStorageEventLogSize
> > >     gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> > >     gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
> > >     gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
> > > +  gBcm27xxTokenSpaceGuid.PcdBcm27xxRegistersAddress
> > >     gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
> > >   [Ppis]
> > > diff --git a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c
> > > index 2bfd3f020a6e..1a6a13b9591b 100644
> > > --- a/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c
> > > +++ b/Platform/RaspberryPi/Library/PlatformLib/RaspberryPiMem.c
> > > @@ -1,7 +1,9 @@
> > >   /** @file
> > >    *
> > > + *  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
> > >    *  Copyright (c) 2017-2018, Andrey Warkentin <andrey.warkentin@gmail.com>
> > >    *  Copyright (c) 2014, Linaro Limited. All rights reserved.
> > > + *  Copyright (c) 2013-2018, ARM Limited. All rights reserved.
> > 
> > This confuses me slightly - is it an indication that some code has
> > been copied from a file including this copyright?
> 
> It is. I copy/pasted some code from ArmJunoMem.c that had this copyright,
> which I then proceeded to modify. Hence the reason why you find the
> copyright notice.

Sure, that's fine. Could you add a line about that in commit message
though? (Literally just "incorporates some code from ArmJunoMem.c"
would suffice.)

> > 
> > >    *
> > >    *  SPDX-License-Identifier: BSD-2-Clause-Patent
> > >    *
> > > @@ -9,8 +11,11 @@
> > >   #include <Library/ArmPlatformLib.h>
> > >   #include <Library/DebugLib.h>
> > > -#include <IndustryStandard/Bcm2836.h>
> > >   #include <Library/PcdLib.h>
> > > +#include <Library/RPiMem.h>
> > > +#include <Library/MemoryAllocationLib.h>
> > 
> > Could you move MemoryAllocationLib.h above PcdLib.h?
> 
> Will do.
> 
> I will also take into account comments for the other patches wrt to
> alphabetical order for the PCDs as well as the additional comment for x1 in
> the assembly before submitting a v2.

Thanks!

/
   LEif

> Regards,
> 
> /Pete
> 
> > 
> > /
> >      Leif
> > 
> > > +#include <IndustryStandard/Bcm2711.h>
> > > +#include <IndustryStandard/Bcm2836.h>
> > >   UINT64 mSystemMemoryBase;
> > >   extern UINT64 mSystemMemoryEnd;
> > > @@ -18,6 +23,13 @@ UINT64 mVideoCoreBase;
> > >   UINT64 mVideoCoreSize;
> > >   UINT32 mBoardRevision;
> > > +
> > > +// The total number of descriptors, including the final "end-of-table" descriptor.
> > > +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 10
> > > +
> > > +STATIC BOOLEAN                  VirtualMemoryInfoInitialized = FALSE;
> > > +STATIC RPI_MEMORY_REGION_INFO   VirtualMemoryInfo[MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS];
> > > +
> > >   #define VariablesSize (FixedPcdGet32(PcdFlashNvStorageVariableSize) +   \
> > >                          FixedPcdGet32(PcdFlashNvStorageFtwWorkingSize) + \
> > >                          FixedPcdGet32(PcdFlashNvStorageFtwSpareSize) +  \
> > > @@ -29,49 +41,6 @@ UINT32 mBoardRevision;
> > >   #define ATFBase (FixedPcdGet64(PcdFdBaseAddress) + FixedPcdGet32(PcdFdSize))
> > > -STATIC ARM_MEMORY_REGION_DESCRIPTOR RaspberryPiMemoryRegionDescriptor[] = {
> > > -  {
> > > -    /* Firmware Volume. */
> > > -    FixedPcdGet64 (PcdFdBaseAddress), FixedPcdGet64 (PcdFdBaseAddress),
> > > -    FixedPcdGet32 (PcdFdSize) - VariablesSize,
> > > -    ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
> > > -  },
> > > -  {
> > > -    /* Variables Volume. */
> > > -    VariablesBase, VariablesBase,
> > > -    VariablesSize,
> > > -    ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
> > > -  },
> > > -  {
> > > -    /* ATF reserved RAM. */
> > > -    ATFBase, ATFBase,
> > > -    FixedPcdGet64 (PcdSystemMemoryBase) - ATFBase,
> > > -    ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
> > > -  },
> > > -  {
> > > -    /* System RAM. */
> > > -    FixedPcdGet64 (PcdSystemMemoryBase), FixedPcdGet64 (PcdSystemMemoryBase),
> > > -    0,
> > > -    ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
> > > -  },
> > > -  {
> > > -    /* Reserved GPU RAM. */
> > > -    0,
> > > -    0,
> > > -    0,
> > > -    ARM_MEMORY_REGION_ATTRIBUTE_DEVICE
> > > -  },
> > > -  {
> > > -    /* SOC registers. */
> > > -    BCM2836_SOC_REGISTERS,
> > > -    BCM2836_SOC_REGISTERS,
> > > -    BCM2836_SOC_REGISTER_LENGTH,
> > > -    ARM_MEMORY_REGION_ATTRIBUTE_DEVICE
> > > -  },
> > > -  {
> > > -  }
> > > -};
> > > -
> > >   /**
> > >     Return the Virtual Memory Map of your platform
> > > @@ -89,6 +58,11 @@ ArmPlatformGetVirtualMemoryMap (
> > >     IN ARM_MEMORY_REGION_DESCRIPTOR** VirtualMemoryMap
> > >     )
> > >   {
> > > +  UINTN                         Index = 0;
> > > +  UINTN                         GpuIndex;
> > > +  INT64                         ExtendedMemorySize;
> > > +  ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
> > > +
> > >     // Early output of the info we got from VideoCore can prove valuable.
> > >     DEBUG ((DEBUG_INFO, "Board Rev: 0x%lX\n", mBoardRevision));
> > >     DEBUG ((DEBUG_INFO, "Base RAM : 0x%ll08X (Size 0x%ll08X)\n", mSystemMemoryBase, mSystemMemoryEnd + 1));
> > > @@ -97,68 +71,120 @@ ArmPlatformGetVirtualMemoryMap (
> > >     ASSERT (mSystemMemoryBase == 0);
> > >     ASSERT (VirtualMemoryMap != NULL);
> > > -  RaspberryPiMemoryRegionDescriptor[3].Length = mSystemMemoryEnd + 1 -
> > > -    FixedPcdGet64 (PcdSystemMemoryBase);
> > > -
> > > -  RaspberryPiMemoryRegionDescriptor[4].PhysicalBase =
> > > -    RaspberryPiMemoryRegionDescriptor[3].PhysicalBase +
> > > -    RaspberryPiMemoryRegionDescriptor[3].Length;
> > > -
> > > -  RaspberryPiMemoryRegionDescriptor[4].VirtualBase =
> > > -    RaspberryPiMemoryRegionDescriptor[4].PhysicalBase;
> > > -
> > > -  RaspberryPiMemoryRegionDescriptor[4].Length =
> > > -    RaspberryPiMemoryRegionDescriptor[5].PhysicalBase -
> > > -    RaspberryPiMemoryRegionDescriptor[4].PhysicalBase;
> > > -
> > > -  DEBUG ((DEBUG_INFO, "FD:\n"
> > > -    "\tPhysicalBase: 0x%lX\n"
> > > -    "\tVirtualBase: 0x%lX\n"
> > > -    "\tLength: 0x%lX\n",
> > > -    RaspberryPiMemoryRegionDescriptor[0].PhysicalBase,
> > > -    RaspberryPiMemoryRegionDescriptor[0].VirtualBase,
> > > -    RaspberryPiMemoryRegionDescriptor[0].Length +
> > > -    RaspberryPiMemoryRegionDescriptor[1].Length));
> > > -
> > > -  DEBUG ((DEBUG_INFO, "Variables (part of FD):\n"
> > > -    "\tPhysicalBase: 0x%lX\n"
> > > -    "\tVirtualBase: 0x%lX\n"
> > > -    "\tLength: 0x%lX\n",
> > > -    RaspberryPiMemoryRegionDescriptor[1].PhysicalBase,
> > > -    RaspberryPiMemoryRegionDescriptor[1].VirtualBase,
> > > -    RaspberryPiMemoryRegionDescriptor[1].Length));
> > > -
> > > -  DEBUG ((DEBUG_INFO, "ATF RAM:\n"
> > > -    "\tPhysicalBase: 0x%lX\n"
> > > -    "\tVirtualBase: 0x%lX\n"
> > > -    "\tLength: 0x%lX\n",
> > > -    RaspberryPiMemoryRegionDescriptor[2].PhysicalBase,
> > > -    RaspberryPiMemoryRegionDescriptor[2].VirtualBase,
> > > -    RaspberryPiMemoryRegionDescriptor[2].Length));
> > > -
> > > -  DEBUG ((DEBUG_INFO, "System RAM:\n"
> > > -    "\tPhysicalBase: 0x%lX\n"
> > > -    "\tVirtualBase: 0x%lX\n"
> > > -    "\tLength: 0x%lX\n",
> > > -    RaspberryPiMemoryRegionDescriptor[3].PhysicalBase,
> > > -    RaspberryPiMemoryRegionDescriptor[3].VirtualBase,
> > > -    RaspberryPiMemoryRegionDescriptor[3].Length));
> > > -
> > > -  DEBUG ((DEBUG_INFO, "GPU Reserved:\n"
> > > -    "\tPhysicalBase: 0x%lX\n"
> > > -    "\tVirtualBase: 0x%lX\n"
> > > -    "\tLength: 0x%lX\n",
> > > -    RaspberryPiMemoryRegionDescriptor[4].PhysicalBase,
> > > -    RaspberryPiMemoryRegionDescriptor[4].VirtualBase,
> > > -    RaspberryPiMemoryRegionDescriptor[4].Length));
> > > -
> > > -  DEBUG ((DEBUG_INFO, "SoC reserved:\n"
> > > -    "\tPhysicalBase: 0x%lX\n"
> > > -    "\tVirtualBase: 0x%lX\n"
> > > -    "\tLength: 0x%lX\n",
> > > -    RaspberryPiMemoryRegionDescriptor[5].PhysicalBase,
> > > -    RaspberryPiMemoryRegionDescriptor[5].VirtualBase,
> > > -    RaspberryPiMemoryRegionDescriptor[5].Length));
> > > -
> > > -  *VirtualMemoryMap = RaspberryPiMemoryRegionDescriptor;
> > > +  VirtualMemoryTable = (ARM_MEMORY_REGION_DESCRIPTOR*)AllocatePages
> > > +                       (EFI_SIZE_TO_PAGES (sizeof (ARM_MEMORY_REGION_DESCRIPTOR) *
> > > +                       MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS));
> > > +  if (VirtualMemoryTable == NULL) {
> > > +    return;
> > > +  }
> > > +
> > > +
> > > +  // Firmware Volume
> > > +  VirtualMemoryTable[Index].PhysicalBase    = FixedPcdGet64 (PcdFdBaseAddress);
> > > +  VirtualMemoryTable[Index].VirtualBase     = VirtualMemoryTable[Index].PhysicalBase;
> > > +  VirtualMemoryTable[Index].Length          = FixedPcdGet32 (PcdFdSize) - VariablesSize;
> > > +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> > > +  VirtualMemoryInfo[Index].Type             = RPI_MEM_RESERVED_REGION;
> > > +  VirtualMemoryInfo[Index++].Name           = L"FD";
> > > +
> > > +  // Variable Volume
> > > +  VirtualMemoryTable[Index].PhysicalBase    = VariablesBase;
> > > +  VirtualMemoryTable[Index].VirtualBase     = VirtualMemoryTable[Index].PhysicalBase;
> > > +  VirtualMemoryTable[Index].Length          = VariablesSize;
> > > +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> > > +  VirtualMemoryInfo[Index].Type             = RPI_MEM_RUNTIME_REGION;
> > > +  VirtualMemoryInfo[Index++].Name           = L"FD Variables";
> > > +
> > > +  // TF-A reserved RAM
> > > +  VirtualMemoryTable[Index].PhysicalBase    = ATFBase;
> > > +  VirtualMemoryTable[Index].VirtualBase     = VirtualMemoryTable[Index].PhysicalBase;
> > > +  VirtualMemoryTable[Index].Length          = FixedPcdGet64 (PcdSystemMemoryBase) - ATFBase;
> > > +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> > > +  VirtualMemoryInfo[Index].Type             = RPI_MEM_RESERVED_REGION;
> > > +  VirtualMemoryInfo[Index++].Name           = L"TF-A RAM";
> > > +
> > > +  // Base System RAM
> > > +  VirtualMemoryTable[Index].PhysicalBase    = FixedPcdGet64 (PcdSystemMemoryBase);
> > > +  VirtualMemoryTable[Index].VirtualBase     = VirtualMemoryTable[Index].PhysicalBase;
> > > +  VirtualMemoryTable[Index].Length          = mSystemMemoryEnd + 1 - FixedPcdGet64 (PcdSystemMemoryBase);
> > > +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> > > +  VirtualMemoryInfo[Index].Type             = RPI_MEM_BASIC_REGION;
> > > +  VirtualMemoryInfo[Index++].Name           = L"Base System RAM";
> > > +
> > > +  // GPU Reserved
> > > +  GpuIndex = Index;
> > > +  VirtualMemoryTable[Index].PhysicalBase    = mVideoCoreBase;
> > > +  VirtualMemoryTable[Index].VirtualBase     = VirtualMemoryTable[Index].PhysicalBase;
> > > +  VirtualMemoryTable[Index].Length          = mVideoCoreSize;
> > > +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > > +  VirtualMemoryInfo[Index].Type             = RPI_MEM_RESERVED_REGION;
> > > +  VirtualMemoryInfo[Index++].Name           = L"GPU Reserved";
> > > +
> > > +  // Compute the amount of extended RAM available on this platform
> > > +  ExtendedMemorySize = SIZE_256MB;
> > > +  ExtendedMemorySize <<= (mBoardRevision >> 20) & 0x07;
> > > +  ExtendedMemorySize -= SIZE_1GB;
> > > +  if (ExtendedMemorySize > 0) {
> > > +    VirtualMemoryTable[Index].PhysicalBase  = FixedPcdGet64 (PcdExtendedMemoryBase);
> > > +    VirtualMemoryTable[Index].VirtualBase   = VirtualMemoryTable[Index].PhysicalBase;
> > > +    VirtualMemoryTable[Index].Length        = ExtendedMemorySize;
> > > +    VirtualMemoryTable[Index].Attributes    = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> > > +    VirtualMemoryInfo[Index].Type           = RPI_MEM_BASIC_REGION;
> > > +    VirtualMemoryInfo[Index++].Name         = L"Extended System RAM";
> > > +  }
> > > +
> > > +  // Extended SoC registers (PCIe, genet, ...)
> > > +  if (BCM2711_SOC_REGISTERS > 0) {
> > > +    VirtualMemoryTable[Index].PhysicalBase  = BCM2711_SOC_REGISTERS;
> > > +    VirtualMemoryTable[Index].VirtualBase   = VirtualMemoryTable[Index].PhysicalBase;
> > > +    VirtualMemoryTable[Index].Length        = BCM2711_SOC_REGISTER_LENGTH;
> > > +    VirtualMemoryTable[Index].Attributes    = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > > +    VirtualMemoryInfo[Index].Type           = RPI_MEM_RESERVED_REGION;
> > > +    VirtualMemoryInfo[Index++].Name         = L"SoC Reserved (27xx)";
> > > +  }
> > > +
> > > +  // Base SoC registers
> > > +  VirtualMemoryTable[Index].PhysicalBase    = BCM2836_SOC_REGISTERS;
> > > +  // On the Pi 3 the SoC registers may overlap VideoCore => fix this
> > > +  if (VirtualMemoryTable[GpuIndex].PhysicalBase + VirtualMemoryTable[GpuIndex].Length > VirtualMemoryTable[Index].PhysicalBase) {
> > > +    VirtualMemoryTable[GpuIndex].Length = VirtualMemoryTable[Index].PhysicalBase - VirtualMemoryTable[GpuIndex].PhysicalBase;
> > > +  }
> > > +  VirtualMemoryTable[Index].VirtualBase     = VirtualMemoryTable[Index].PhysicalBase;
> > > +  VirtualMemoryTable[Index].Length          = BCM2836_SOC_REGISTER_LENGTH;
> > > +  VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> > > +  VirtualMemoryInfo[Index].Type             = RPI_MEM_RESERVED_REGION;
> > > +  VirtualMemoryInfo[Index++].Name           = L"SoC Reserved (283x)";
> > > +
> > > +  // End of Table
> > > +  VirtualMemoryTable[Index].PhysicalBase    = 0;
> > > +  VirtualMemoryTable[Index].VirtualBase     = 0;
> > > +  VirtualMemoryTable[Index].Length          = 0;
> > > +  VirtualMemoryTable[Index++].Attributes    = (ARM_MEMORY_REGION_ATTRIBUTES)0;
> > > +
> > > +  ASSERT(Index <= MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS);
> > > +
> > > +  *VirtualMemoryMap = VirtualMemoryTable;
> > > +  VirtualMemoryInfoInitialized = TRUE;
> > > +}
> > > +
> > > +/**
> > > +  Return additional memory info not populated by the above call.
> > > +
> > > +  This call should follow the one to ArmPlatformGetVirtualMemoryMap ().
> > > +
> > > +**/
> > > +VOID
> > > +RpiPlatformGetVirtualMemoryInfo (
> > > +  IN RPI_MEMORY_REGION_INFO** MemoryInfo
> > > +  )
> > > +{
> > > +  ASSERT (VirtualMemoryInfo != NULL);
> > > +
> > > +  if (!VirtualMemoryInfoInitialized) {
> > > +    DEBUG ((DEBUG_ERROR,
> > > +      "ArmPlatformGetVirtualMemoryMap must be called before RpiPlatformGetVirtualMemoryInfo.\n"));
> > > +    return;
> > > +  }
> > > +
> > > +  *MemoryInfo = VirtualMemoryInfo;
> > >   }
> > > diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec
> > > index 3e9171eccb13..c7e17350544a 100644
> > > --- a/Platform/RaspberryPi/RaspberryPi.dec
> > > +++ b/Platform/RaspberryPi/RaspberryPi.dec
> > > @@ -42,6 +42,7 @@ [PcdsFixedAtBuild.common]
> > >     gRaspberryPiTokenSpaceGuid.PcdNvStorageVariableBase|0x0|UINT32|0x00000005
> > >     gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwSpareBase|0x0|UINT32|0x00000006
> > >     gRaspberryPiTokenSpaceGuid.PcdNvStorageFtwWorkingBase|0x0|UINT32|0x00000007
> > > +  gRaspberryPiTokenSpaceGuid.PcdExtendedMemoryBase|0x0|UINT32|0x00000008
> > >   [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> > >     gRaspberryPiTokenSpaceGuid.PcdCpuClock|0|UINT32|0x0000000d
> > > -- 
> > > 2.21.0.windows.1
> > > 
> 

  reply	other threads:[~2019-11-18 17:38 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-14 16:07 [edk2-platforms][PATCH 0/8] Platform/RPi: Early Raspberry Pi 4 groundwork Pete Batard
2019-11-14 16:07 ` [edk2-platforms][PATCH 1/8] Platform/RPi: Add model family detection Pete Batard
2019-11-14 16:36   ` [edk2-devel] " Michael Brown
2019-11-14 16:55     ` Pete Batard
2019-11-18 17:51   ` Leif Lindholm
2019-11-18 17:58     ` Pete Batard
2019-11-18 18:05       ` Leif Lindholm
2019-11-18 18:32         ` Pete Batard
2019-11-19 15:07           ` Ard Biesheuvel
2019-11-19 16:30             ` [edk2-devel] " Pete Batard
2019-11-20 10:27               ` Leif Lindholm
2019-11-20 21:50                 ` Pete Batard
2019-11-21  8:55                   ` Laszlo Ersek
2019-11-21  9:04                     ` Laszlo Ersek
2019-11-21 20:02                       ` Pete Batard
2019-11-14 16:07 ` [edk2-platforms][PATCH 2/8] Platform/RPi: Replace Bcm283x SoC base register address with a PCD Pete Batard
2019-11-18 16:48   ` Leif Lindholm
2019-11-18 17:19     ` [edk2-devel] " samer.el-haj-mahmoud
2019-11-18 17:26       ` Leif Lindholm
2019-11-14 16:07 ` [edk2-platforms][PATCH 3/8] Silicon/Broadcom: Add Bcm2711 header Pete Batard
2019-11-18 16:50   ` Leif Lindholm
2019-11-14 16:07 ` [edk2-platforms][PATCH 4/8] Platform/RPi: Read more variables from VideoCore during early init Pete Batard
2019-11-18 17:11   ` Leif Lindholm
2019-11-14 16:07 ` [edk2-platforms][PATCH 5/8] Platform/RPi: Clean up and improve early memory init Pete Batard
2019-11-18 17:20   ` Leif Lindholm
2019-11-18 17:34     ` Pete Batard
2019-11-18 17:38       ` Leif Lindholm [this message]
2019-11-18 17:40         ` Pete Batard
2019-11-14 16:07 ` [edk2-platforms][PATCH 6/8] Platform/RPi: Replace Mailbox and Watchdog addresses with PCDs Pete Batard
2019-11-18 11:13   ` Philippe Mathieu-Daudé
2019-11-18 13:32     ` Pete Batard
2019-11-14 16:07 ` [edk2-platforms][PATCH 7/8] Platform/RPi: Replace MMCHS1BASE define with a PCD Pete Batard
2019-11-14 16:07 ` [edk2-platforms][PATCH 8/8] Platform/RPi: Replace DW2_USB_BASE_ADDRESS " Pete Batard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191118173850.GW7323@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox