public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Pankaj Bansal" <pankaj.bansal@nxp.com>
To: Leif Lindholm <leif@nuviainc.com>,
	Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [PATCH edk2-platforms 2/3] Silicon/NXP: Add support for reserving a chunk from RAM
Date: Sat, 18 Jul 2020 17:32:01 +0000	[thread overview]
Message-ID: <AM0PR04MB6580A4925DD0A808F84407C3B07D0@AM0PR04MB6580.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20200708051933.8123-3-pankaj.bansal@oss.nxp.com>

Please don't merge this Patch.

This patch needs update. The ReservedRam needs to be reported to UEFI.
I will send v2 for this patch. other patches in this series can be reviewed.

Regards,
Pankaj Bansal

> -----Original Message-----
> From: Pankaj Bansal (OSS) <pankaj.bansal@oss.nxp.com>
> Sent: Wednesday, July 8, 2020 10:50 AM
> To: Leif Lindholm <leif@nuviainc.com>; Meenakshi Aggarwal
> <meenakshi.aggarwal@nxp.com>; devel@edk2.groups.io; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Subject: [PATCH edk2-platforms 2/3] Silicon/NXP: Add support for reserving a
> chunk from RAM
> 
> From: Pankaj Bansal <pankaj.bansal@nxp.com>
> 
> Some NXP SOCs have some specialized IP blocks (like MC), which
> require DDR memory to operate. This DDR memory should not be managed
> by OS or UEFI.
> 
> Moreover to ensure that these IP blocks always get memory, and maximum
> contiguous RAM is available for UEFI and OS to use, add the support for
> reserving a chunk from RAM before reporting available RAM to UEFI.
> 
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> ---
>  Silicon/NXP/NxpQoriqLs.dec                                |  10 ++
>  Silicon/NXP/LX2160A/LX2160A.dsc.inc                       |   4 +
>  Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf |   3 +
>  Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c   | 142
> +++++++++++++++++++-
>  4 files changed, 157 insertions(+), 2 deletions(-)
> 
> diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec
> index 188a9fe1f382..0e762066e547 100644
> --- a/Silicon/NXP/NxpQoriqLs.dec
> +++ b/Silicon/NXP/NxpQoriqLs.dec
> @@ -41,3 +41,13 @@ [PcdsDynamic.common]
> 
> gNxpQoriqLsTokenSpaceGuid.PcdPciCfgShiftEnable|FALSE|BOOLEAN|0x000006
> 00
>    gNxpQoriqLsTokenSpaceGuid.PcdPciLsGen4Ctrl|FALSE|BOOLEAN|0x00000601
> 
> gNxpQoriqLsTokenSpaceGuid.PcdPciHideRootPort|FALSE|BOOLEAN|0x0000060
> 2
> +
> +  # Reserved RAM Base address alignment. This number ought to be Power of
> two
> +  # in case no alignment is needed, this number should be 1.
> +
> gNxpQoriqLsTokenSpaceGuid.PcdReservedMemAlignment|0x1|UINT64|0x0000
> 0603
> +  # Size of the RAM to be reserved. This RAM region is neither reported to UEFI
> +  # nor to OS
> +
> gNxpQoriqLsTokenSpaceGuid.PcdReservedMemSize|0x0|UINT64|0x00000604
> +  # Reserved RAM Base address which is calculated based on
> PcdReservedMemSize
> +  # and PcdReservedMemAlignment
> +
> gNxpQoriqLsTokenSpaceGuid.PcdReservedMemBase|0x0|UINT64|0x00000605
> diff --git a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> index 43e361464c8e..755ca169f213 100644
> --- a/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> +++ b/Silicon/NXP/LX2160A/LX2160A.dsc.inc
> @@ -29,6 +29,10 @@ [PcdsDynamicDefault.common]
>    gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x6200000
>    gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0xC0C0000
> 
> +[PcdsDynamicHii]
> +
> gNxpQoriqLsTokenSpaceGuid.PcdReservedMemAlignment|L"ReservedMemAlig
> nment"|gEfiGlobalVariableGuid|0x0|0x20000000|NV,BS
> +
> gNxpQoriqLsTokenSpaceGuid.PcdReservedMemSize|L"ReservedMemSize"|gEfi
> GlobalVariableGuid|0x0|0x20000000|NV,BS
> +
>  [PcdsFixedAtBuild.common]
>    gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0x23A0000
>    gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase|0x2390000
> diff --git a/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
> b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
> index a33f8cd3f743..ed23a86b43d9 100644
> --- a/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
> +++ b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
> @@ -49,6 +49,9 @@ [FixedPcd]
>  [Pcd]
>    gArmTokenSpaceGuid.PcdSystemMemoryBase
>    gArmTokenSpaceGuid.PcdSystemMemorySize
> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemAlignment
> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemBase
> +  gNxpQoriqLsTokenSpaceGuid.PcdReservedMemSize
> 
>  [Depex]
>    TRUE
> diff --git a/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> index 11d1f1260b35..b416323a4ced 100644
> --- a/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> +++ b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
> @@ -118,6 +118,127 @@ GetDramRegionsInfo (
>    return EFI_BUFFER_TOO_SMALL;
>  }
> 
> +/**
> +  Calculate the base address of Reserved RAM.
> +  Reserved RAM is not reported to either UEFI or OS.
> +
> +  @param[in, out] DramRegions  Array of type DRAM_REGION_INFO. The size
> of this
> +                               array must be one more (+ 1) than the maximum
> +                               regions supported on platform. This is because,
> +                               if due to Reserved RAM alignment requirements a
> +                               hole is created in any DRAM region, then the RAM
> +                               after hole gets reported to UEFI and then
> +                               subsequently to OS. which is why, the last entry
> +                               of this array will not be parsed while
> +                               calculating Reserved RAM base address. Caller
> +                               must ensure that last entry of this array is zero
> +                               initialized.
> +  @param[in] NumRegions        Size of DramRegions array (including +1 for hole)
> +  @param[in] ReservedMemSize   Size of RAM to be reserved.
> +
> +  @return  if successful Address of the Reserved RAM region, 0 otherwise.
> +**/
> +STATIC
> +UINTN
> +CalculateReservedMemBase (
> +  IN DRAM_REGION_INFO *DramRegions,
> +  IN UINT32           NumRegions,
> +  IN UINTN            ReservedMemSize
> +)
> +{
> +  UINTN                 ReservedMemAlignment;
> +  EFI_PHYSICAL_ADDRESS  AlignmentMask;
> +  UINTN                 RegionBaseAddress;
> +  UINTN                 RegionSize;
> +  UINTN                 ReservedBaseAddress;
> +  INTN                  Index;
> +  INTN                  Index2;
> +
> +  ReservedMemAlignment = PcdGet64 (PcdReservedMemAlignment);
> +  //
> +  // Compute alignment bit mask
> +  //
> +  if (ReservedMemAlignment) {
> +    AlignmentMask = LShiftU64 (1, LowBitSet64(ReservedMemAlignment)) - 1;
> +  } else {
> +    AlignmentMask = 0;
> +  }
> +
> +  // The DRAM region info is sorted based on the RAM address is SOC memory
> map.
> +  // i.e. DramRegions[0] is at lower address, as compared to DramRegions[1].
> +  // The goal to start from last region is to find the topmost RAM region that
> +  // can contain Reserved RAM region i.e. PcdReservedMemSize.
> +  // Since this RAM is not reported to either UEFI or OS, This ensures that
> +  // maximum amount of lower RAM (32 bit addresses) are left
> +  // for OS to allocate to devices that can only work with 32bit physical
> +  // addresses. E.g. legacy devices that need to DMA to 32bit addresses.
> +  for (Index = NumRegions - 2; Index >=0; Index--) {
> +    RegionBaseAddress = DramRegions[Index].BaseAddress;
> +    RegionSize = DramRegions[Index].Size;
> +
> +    if (ReservedMemSize > RegionSize) {
> +      continue;
> +    }
> +
> +    ReservedBaseAddress = (RegionBaseAddress + RegionSize -
> ReservedMemSize);
> +    ReservedBaseAddress &= (~AlignmentMask);
> +    if (ReservedBaseAddress < RegionBaseAddress) {
> +      continue;
> +    }
> +
> +    // found the region from which reserved mem is to be carved out
> +    // Need to modify the region size and create/delete region if need be
> +
> +    RegionSize -= ReservedMemSize;
> +    if (RegionSize == 0) {
> +      // delete the region but maintain the sorted list of regions
> +      for (Index2 = Index; Index2 < NumRegions; Index2++) {
> +        CopyMem (
> +          &DramRegions[Index2],
> +          &DramRegions[Index2 + 1],
> +          sizeof (DRAM_REGION_INFO)
> +        );
> +      }
> +      break;
> +    }
> +
> +    if (ReservedBaseAddress - RegionBaseAddress) {
> +      DramRegions[Index].Size = ReservedBaseAddress - RegionBaseAddress;
> +      RegionSize -= DramRegions[Index].Size;
> +    } else {
> +      DramRegions[Index].BaseAddress = ReservedBaseAddress +
> ReservedMemSize;
> +      DramRegions[Index].Size = RegionSize;
> +      RegionSize = 0;
> +    }
> +
> +    if (RegionSize == 0) {
> +      break;
> +    }
> +
> +    // A hole has been created in DRAM regions due to Reserved RAM alignment
> +    // requirements. create a new DRAM region for DRAM memory after hole.
> +    // Maintain the sorted list of regions
> +    for (Index2 = NumRegions; Index2 > (Index + 1); Index2--) {
> +      CopyMem (
> +        &DramRegions[Index2],
> +        &DramRegions[Index2 - 1],
> +        sizeof (DRAM_REGION_INFO)
> +      );
> +    }
> +    DramRegions[Index2].BaseAddress = ReservedBaseAddress +
> ReservedMemSize;
> +    DramRegions[Index2].Size = RegionSize;
> +    RegionSize = 0;
> +
> +    break;
> +  }
> +
> +  if (Index == -1) {
> +    return 0;
> +  } else {
> +    return ReservedBaseAddress;
> +  }
> +}
> +
>  /**
>    Get the installed RAM information.
>    Initialize Memory HOBs (Resource Descriptor HOBs)
> @@ -135,7 +256,9 @@ MemoryInitPeiLibConstructor (
>    UINTN                         BaseAddress;
>    UINTN                         Size;
>    UINTN                         Top;
> -  DRAM_REGION_INFO              DramRegions[MAX_DRAM_REGIONS];
> +  // Extra region gets created if we want to reserve a memory region and that
> +  // creates a memory hole because of alignment requirements
> +  DRAM_REGION_INFO              DramRegions[MAX_DRAM_REGIONS + 1];
>    EFI_RESOURCE_ATTRIBUTE_TYPE   ResourceAttributes;
>    UINTN                         FdBase;
>    UINTN                         FdTop;
> @@ -155,6 +278,21 @@ MemoryInitPeiLibConstructor (
> 
>    (VOID)GetDramRegionsInfo (DramRegions, ARRAY_SIZE (DramRegions));
> 
> +  // Get the reserved memory size from non volatile storage
> +  Size = PcdGet64 (PcdReservedMemSize);
> +  if (Size) {
> +    BaseAddress = CalculateReservedMemBase (
> +                    DramRegions,
> +                    ARRAY_SIZE (DramRegions),
> +                    Size
> +                  );
> +    if (BaseAddress) {
> +      DEBUG ((DEBUG_INFO, "ReservedMem: start 0x%lx, size 0x%lx\n",
> +        BaseAddress, Size));
> +      PcdSet64S (PcdReservedMemBase, BaseAddress);
> +    }
> +  }
> +
>    FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);
>    FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);
> 
> @@ -168,7 +306,7 @@ MemoryInitPeiLibConstructor (
>    // This ensures that maximum amount of lower RAM (32 bit addresses) are left
>    // for OS to allocate to devices that can only work with 32bit physical
>    // addresses. E.g. legacy devices that need to DMA to 32bit addresses.
> -  for (Index = MAX_DRAM_REGIONS - 1; Index >= 0; Index--) {
> +  for (Index = MAX_DRAM_REGIONS; Index >= 0; Index--) {
>      if (DramRegions[Index].Size == 0) {
>        continue;
>      }
> --
> 2.17.1


  reply	other threads:[~2020-07-18 17:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-08  5:19 [PATCH edk2-platforms 0/3] Add Features to NXP Platforms Pankaj Bansal
2020-07-08  5:19 ` [PATCH edk2-platforms 1/3] Silicon/NXP: Use runtime safe version of DebugLib Pankaj Bansal
2020-08-05 14:11   ` Leif Lindholm
2020-07-08  5:19 ` [PATCH edk2-platforms 2/3] Silicon/NXP: Add support for reserving a chunk from RAM Pankaj Bansal
2020-07-18 17:32   ` Pankaj Bansal [this message]
2020-08-05 15:12   ` Leif Lindholm
2020-08-05 19:16     ` [EXTERNAL] Re: [edk2-devel] " Bret Barkelew
2020-07-08  5:19 ` [PATCH edk2-platforms 3/3] Silicon/NXP: Add Support for git commit info print Pankaj Bansal
2020-08-05 14:44   ` Leif Lindholm
2020-08-08  2:39     ` Pankaj Bansal
2020-08-12 12:45       ` Leif Lindholm

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=AM0PR04MB6580A4925DD0A808F84407C3B07D0@AM0PR04MB6580.eurprd04.prod.outlook.com \
    --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