From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::234; helo=mail-it0-x234.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x234.google.com (mail-it0-x234.google.com [IPv6:2607:f8b0:4001:c0b::234]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 2394C21A0482A for ; Wed, 11 Oct 2017 11:14:43 -0700 (PDT) Received: by mail-it0-x234.google.com with SMTP id 72so4135303itl.5 for ; Wed, 11 Oct 2017 11:18:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=pdtFkbk9mERvrkSnzdL5Cf5vK70n/mBBQnygCrzVsOU=; b=KvX2RnAuB2ln3a4lDOEftIgmwxxY+iTff+Mp9alPTFametB75wmtfEDjN+/5Rv7T26 lnHfu1/ltDpmoh837PfhQVoWBD4EQsqzTKZUJHlEZnBMtfbV58qo1ad5SpvhMeHS47ov gxLPubR8WqIq4l1IAg+Oq4LZsq2Am5UZaF3EA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=pdtFkbk9mERvrkSnzdL5Cf5vK70n/mBBQnygCrzVsOU=; b=Rn8eC9S5Knkuf0jxtCiCI75ssh+igSkXhmTcVfrnXmUoQ24dfsUkQ0fhcmDV3QrgaL TjkEiYhOpOOEhSUwHyiE3CfqwJcD8ukZ5TtoyG7/2dxy4a2FitZziRVMpupq49vCKTCf Tqw/50NFYnAld8yFog086Cypez8CaZW70boWkpV/lTDBBNU5R9Cw3WBuxkiR+pgt21ys eJoxXdQhWRIylRG06QUGsEvHhoKiTsP8B68Nami02pcUjqhNRXIFGvVn1pIgfYEACF+z qGsrzBrqeGzZVikAMaTJH26hQyr1qHNQwSs+fSljCfeplD4JRb6pMNHmAKdW7bYRdB1z q8bQ== X-Gm-Message-State: AMCzsaXC8dUpZunUSO/qcw/g6QXy0YPy3vBmwRDpGacH9su5f3k3a351 dT0dTyDXuGvSX8xmhWZGTfTHfUpx2haFQFhFmO+Znw== X-Google-Smtp-Source: AOwi7QDZ/Psv8NS0AHrOvVMGGrzkx7uIWrHya7Jfdf9DdRv2AIbh0nY85NMOEDHqdJXmb0DuqAXOqphPbCJ2PHt3DNI= X-Received: by 10.36.41.65 with SMTP id p62mr667643itp.48.1507745892729; Wed, 11 Oct 2017 11:18:12 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.131.167 with HTTP; Wed, 11 Oct 2017 11:18:12 -0700 (PDT) In-Reply-To: <20171011170815.fp2ni7r6eimjcgi6@bivouac.eciton.net> References: <1507736449-6073-1-git-send-email-mw@semihalf.com> <1507736449-6073-5-git-send-email-mw@semihalf.com> <20171011170815.fp2ni7r6eimjcgi6@bivouac.eciton.net> From: Ard Biesheuvel Date: Wed, 11 Oct 2017 19:18:12 +0100 Message-ID: To: Leif Lindholm Cc: Marcin Wojtas , "edk2-devel@lists.01.org" , Nadav Haklai , Neta Zur Hershkovits , Kostya Porotchkin , Hua Jing , =?UTF-8?B?SmFuIETEhWJyb8Wb?= Subject: Re: [platforms: PATCH 4/8] Marvell/Armada: Add support from DRAM remapping X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Oct 2017 18:14:44 -0000 Content-Type: text/plain; charset="UTF-8" On 11 October 2017 at 18:08, Leif Lindholm wrote: > Subject: from->for ? > > On Wed, Oct 11, 2017 at 05:40:45PM +0200, Marcin Wojtas wrote: >> From: Ard Biesheuvel >> >> The Armada 70x0/80x0 DRAM controller allows a single window of DRAM >> to be remapped to another location in the physical address space. This >> allows us to free up some memory in the 32-bit addressable region for >> peripheral MMIO and PCI MMIO32 and CONFIG spaces. This patch adjusts >> memory blocks to the configuration done in ATF. > > The ARM Trusted Firmware developers tend to frown at that > abbreviation. Please use ARM-TF in official context. > > Which configuration is this? Presumably, if this was changed in > ARM-TF, we would need to change it this end too, so does not hurt to > be explicit. (It uses a FixedPcd, so clearly it is not dynamically > detected, which the commit message can be read as.) > >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel >> Signed-off-by: Marcin Wojtas >> --- >> Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S | 15 +++++ >> Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf | 3 + >> Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c | 60 ++++++++++++++++---- >> Platform/Marvell/Marvell.dec | 13 +++++ >> 4 files changed, 81 insertions(+), 10 deletions(-) >> >> diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S >> index 72f8cfc..c5be1a9 100644 >> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S >> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S >> @@ -17,6 +17,21 @@ >> >> ASM_FUNC(ArmPlatformPeiBootAction) >> mov x29, xzr >> + >> + .if FixedPcdGet64 (PcdSystemMemoryBase) != 0 >> + .err PcdSystemMemoryBase should be 0x0 on this platform! >> + .endif >> + >> + .if FixedPcdGet64 (PcdSystemMemorySize) > FixedPcdGet32 (PcdDramRemapTarget) >> + // >> + // Use the low range for UEFI itself. The remaining memory will be mapped >> + // and added to the GCD map later. >> + // >> + adr x0, mSystemMemoryEnd >> + MOV64 (x1, FixedPcdGet32 (PcdDramRemapTarget) - 1) >> + str x1, [x0] >> + .endif >> + >> ret >> >> //UINTN >> diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf >> index 2e198c3..838a670 100644 >> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf >> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0Lib.inf >> @@ -67,5 +67,8 @@ >> gArmTokenSpaceGuid.PcdArmPrimaryCoreMask >> gArmTokenSpaceGuid.PcdArmPrimaryCore >> >> + gMarvellTokenSpaceGuid.PcdDramRemapSize >> + gMarvellTokenSpaceGuid.PcdDramRemapTarget >> + >> [Ppis] >> gArmMpCoreInfoPpiGuid >> diff --git a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c >> index 74c9956..2cb2e15 100644 >> --- a/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c >> +++ b/Platform/Marvell/Armada/Library/Armada70x0Lib/Armada70x0LibMem.c >> @@ -35,6 +35,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >> #include >> #include >> #include >> +#include >> #include >> >> // The total number of descriptors, including the final "end-of-table" descriptor. >> @@ -44,6 +45,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >> #define DDR_ATTRIBUTES_CACHED ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK >> #define DDR_ATTRIBUTES_UNCACHED ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED >> >> +STATIC ARM_MEMORY_REGION_DESCRIPTOR VirtualMemoryTable[MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS]; > > mVirtualMemoryTable? > >> + >> /** >> Return the Virtual Memory Map of your platform >> >> @@ -59,20 +62,41 @@ ArmPlatformGetVirtualMemoryMap ( >> IN ARM_MEMORY_REGION_DESCRIPTOR** VirtualMemoryMap >> ) >> { >> - ARM_MEMORY_REGION_DESCRIPTOR *VirtualMemoryTable; >> UINTN Index = 0; >> + UINT64 MemSize; >> + UINT64 MemLowSize; >> + UINT64 MemHighStart; >> + UINT64 MemHighSize; >> + EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes; >> >> ASSERT (VirtualMemoryMap != NULL); >> >> - VirtualMemoryTable = (ARM_MEMORY_REGION_DESCRIPTOR*)AllocatePages(EFI_SIZE_TO_PAGES (sizeof(ARM_MEMORY_REGION_DESCRIPTOR) * MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS)); >> - if (VirtualMemoryTable == NULL) { >> - return; >> - } >> + MemSize = FixedPcdGet64 (PcdSystemMemorySize); >> + MemLowSize = MIN (FixedPcdGet64 (PcdDramRemapTarget), MemSize); >> + MemHighStart = (UINT64)FixedPcdGet64 (PcdDramRemapTarget) + >> + FixedPcdGet32 (PcdDramRemapSize); >> + MemHighSize = MemSize - MemLowSize; >> + >> + 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, >> + FixedPcdGet64 (PcdSystemMemoryBase), >> + MemLowSize >> + ); >> >> // DDR >> - VirtualMemoryTable[Index].PhysicalBase = PcdGet64 (PcdSystemMemoryBase); >> - VirtualMemoryTable[Index].VirtualBase = PcdGet64 (PcdSystemMemoryBase); >> - VirtualMemoryTable[Index].Length = PcdGet64 (PcdSystemMemorySize); >> + VirtualMemoryTable[Index].PhysicalBase = FixedPcdGet64 (PcdSystemMemoryBase); >> + VirtualMemoryTable[Index].VirtualBase = FixedPcdGet64 (PcdSystemMemoryBase); >> + VirtualMemoryTable[Index].Length = MemLowSize; >> VirtualMemoryTable[Index].Attributes = DDR_ATTRIBUTES_CACHED; >> >> // Configuration space 0xF000_0000 - 0xFFFF_FFFF >> @@ -81,13 +105,29 @@ ArmPlatformGetVirtualMemoryMap ( >> VirtualMemoryTable[Index].Length = 0x10000000; >> VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE; >> >> + if (MemSize > MemLowSize) { >> + // >> + // If we have more than MemLowSize worth of DRAM, the remainder will be >> + // mapped at the top of the remapped window. >> + // >> + VirtualMemoryTable[++Index].PhysicalBase = MemHighStart; >> + VirtualMemoryTable[Index].VirtualBase = MemHighStart; >> + VirtualMemoryTable[Index].Length = MemHighSize; >> + VirtualMemoryTable[Index].Attributes = DDR_ATTRIBUTES_CACHED; >> + >> + BuildResourceDescriptorHob ( >> + EFI_RESOURCE_SYSTEM_MEMORY, >> + ResourceAttributes, >> + MemHighStart, >> + MemHighSize >> + ); >> + } >> + >> // End of Table >> VirtualMemoryTable[++Index].PhysicalBase = 0; >> VirtualMemoryTable[Index].VirtualBase = 0; >> VirtualMemoryTable[Index].Length = 0; >> VirtualMemoryTable[Index].Attributes = 0; >> >> - ASSERT((Index + 1) <= MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS); >> - > > Why delete this assert? > >> *VirtualMemoryMap = VirtualMemoryTable; >> } >> diff --git a/Platform/Marvell/Marvell.dec b/Platform/Marvell/Marvell.dec >> index 434d6cb..db1c7fa 100644 >> --- a/Platform/Marvell/Marvell.dec >> +++ b/Platform/Marvell/Marvell.dec >> @@ -194,6 +194,19 @@ >> #TRNG >> gMarvellTokenSpaceGuid.PcdEip76TrngBaseAddress|0x0|UINT64|0x50000053 >> >> + # >> + # DRAM remapping controls. >> + # On the 70x0/80x0 SOCs, the DRAM is mapped at 0x0, and could be up to >> + # 16 GB in size. To allow for 32-bit addressable MMIO peripherals or PCI >> + # windows, a single window of up to 4 GB in size can be remapped elsewhere. >> + # So let's define a 1 GB window at 0xC000000 by default: this is the minimum >> + # alignment that Linux can map optimally (i.e., it's section shift is 30 bits) >> + # and gives us an additional 768 MB (on top of the 256 MB platform MMIO window >> + # at 0xF0000000) for the PCI MMIO32 and CONFIG spaces. > > If DRAM is mapped at 0, and can be up to 16GB in size, why do we need > to remap some of it below 4GB? > I guess what you're actually doing is mapping it _away_. Where to? > > (And a short-short version of this in the commit message should take > care of that item.) > Indeed. Physical RAM starts at 0x0, and to avoid wasting all 32-bit addressable memory space on RAM, the hardware allows some of it to be remapped elsewhere. So by remapping 1 GB, we make 1 GB of address space available for MMIO, PCI config space and PCI MMIO32 space.