From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x231.google.com (mail-it0-x231.google.com [IPv6:2607:f8b0:4001:c0b::231]) (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 4D56821DFA8F2 for ; Thu, 6 Apr 2017 11:31:14 -0700 (PDT) Received: by mail-it0-x231.google.com with SMTP id a140so31239427ita.0 for ; Thu, 06 Apr 2017 11:31:14 -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=MtxeaXKlFSQO2mRY3A61B87ER4EHsm9srIUqBGHc7fE=; b=THK31ocMwFvqU6Ed/PQuA4Vhz0WKyzzqRC2Q43EHqgE4EMcEi9QFmF2BDMGgX2Ntcj niL7ij4ZDWgC4MAFre5HqepV+g02BYtgGXY7oRAXCTBawEXzwhlrF0gQxzxBFEX02CGT VPGV9pSJlMXpFfv5roxDKKJMRCIK89cv+yQQ0= 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=MtxeaXKlFSQO2mRY3A61B87ER4EHsm9srIUqBGHc7fE=; b=cqF7ORPJ4NvmR6tYqT8NxTsyeSKg8rBl7epUquASuAwf8J7oAR/ZQyHPnfX7Ob3dDA FnUst9BN8chgZ8/dSsDs3x+Krz5IwGyS1x+IDZreccYcx9YHALQ6rdDopRc4YKmRRDgt YEeaRDZienPuxT5G4jl19c/RvE9XXE9roa7srTGCxHz8Tpm/igJjvlcnZq1Ep+H0ndrM ZhogZjLsiWYI1WSQf2gbo4QQgbCUBro6idv7ysHBT03J4ySF9kBpl/TZ9E1smEiaj14g llmY8O2M1M4/vCqKqJTbDmelbqbLsKW0MKC/Y8Zwa4t2mLFe9B8TgwqopnShSUjkXZQ7 Nw9w== X-Gm-Message-State: AFeK/H0R7ueK/9o7VhOTocbEhQz4u6iaUvHPr5GAs2ek/N5Iodp4zTy0fTxl6rKhVklR0+Ko3KRpk73mk65diEnz X-Received: by 10.36.46.69 with SMTP id i66mr27099240ita.59.1491503473518; Thu, 06 Apr 2017 11:31:13 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.10.27 with HTTP; Thu, 6 Apr 2017 11:31:13 -0700 (PDT) In-Reply-To: <20170406182648.GK25239@bivouac.eciton.net> References: <20170406131551.3322-1-ard.biesheuvel@linaro.org> <20170406131551.3322-2-ard.biesheuvel@linaro.org> <20170406182648.GK25239@bivouac.eciton.net> From: Ard Biesheuvel Date: Thu, 6 Apr 2017 19:31:13 +0100 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , Ryan Harkin , Evan Lloyd , Jeremy Linton Subject: Re: [PATCH v2 1/5] ArmPlatformPkg/FVP: map motherboard VRAM as uncached memory 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: Thu, 06 Apr 2017 18:31:14 -0000 Content-Type: text/plain; charset=UTF-8 On 6 April 2017 at 19:26, Leif Lindholm wrote: > On Thu, Apr 06, 2017 at 02:15:47PM +0100, Ard Biesheuvel wrote: >> The VRAM of the PL111 on the FVP Base/Foundation models is described as >> device memory rather than uncached memory, which is not an accurate >> description of the nature of the region (i.e., a framebuffer), and may >> result in problems when using accelerated string routines to access the >> region, since this may legally involve unaligned accesses or DC ZVA >> instructions, which are not allowed on device mappings. >> >> So split of the 8 MB VRAM region into a separate region, and map it using >> memory attributes. > > "Normal memory attributes"? > OK >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel >> --- >> ArmPlatformPkg/ArmVExpressPkg/Include/Platform/RTSM/ArmPlatform.h | 10 ++++++---- >> ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c | 8 +++++++- >> 2 files changed, 13 insertions(+), 5 deletions(-) >> >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Include/Platform/RTSM/ArmPlatform.h b/ArmPlatformPkg/ArmVExpressPkg/Include/Platform/RTSM/ArmPlatform.h >> index 06414e6e7208..4e17c800a34f 100644 >> --- a/ArmPlatformPkg/ArmVExpressPkg/Include/Platform/RTSM/ArmPlatform.h >> +++ b/ArmPlatformPkg/ArmVExpressPkg/Include/Platform/RTSM/ArmPlatform.h >> @@ -40,9 +40,11 @@ >> #define ARM_VE_SMB_SRAM_BASE 0x2E000000 >> #define ARM_VE_SMB_SRAM_SZ SIZE_64KB >> // USB, Ethernet, VRAM >> -#define ARM_VE_SMB_PERIPH_BASE 0x18000000 >> -#define PL111_CLCD_VRAM_MOTHERBOARD_BASE ARM_VE_SMB_PERIPH_BASE >> -#define ARM_VE_SMB_PERIPH_SZ SIZE_64MB >> +#define ARM_VE_SMB_PERIPH_BASE 0x18800000 >> +#define ARM_VE_SMB_PERIPH_SZ (SIZE_64MB - SIZE_8MB) >> + >> +#define PL111_CLCD_VRAM_MOTHERBOARD_BASE 0x18000000 >> +#define PL111_CLCD_VRAM_MOTHERBOARD_SIZE SIZE_8MB >> >> // DRAM >> #define ARM_VE_DRAM_BASE PcdGet64 (PcdSystemMemoryBase) >> @@ -75,6 +77,6 @@ >> #define PL111_CLCD_CORE_TILE_VIDEO_MODE_OSC_ID 1 >> >> // VRAM offset for the PL111 Colour LCD Controller on the motherboard >> -#define VRAM_MOTHERBOARD_BASE (ARM_VE_SMB_PERIPH_BASE + 0x00000) >> +#define VRAM_MOTHERBOARD_BASE (PL111_CLCD_VRAM_MOTHERBOARD_BASE) >> >> #endif >> diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c >> index 14c7e8e1d672..70c17ae70478 100644 >> --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c >> +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c >> @@ -21,7 +21,7 @@ >> #include >> >> // Number of Virtual Memory Map Descriptors >> -#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 8 >> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 9 >> >> // DDR attributes >> #define DDR_ATTRIBUTES_CACHED ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK >> @@ -130,6 +130,12 @@ ArmPlatformGetVirtualMemoryMap ( >> VirtualMemoryTable[Index].Length = 2 * ARM_VE_SMB_PERIPH_SZ; >> VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE; >> >> + // VRAM >> + VirtualMemoryTable[++Index].PhysicalBase = PL111_CLCD_VRAM_MOTHERBOARD_BASE; >> + VirtualMemoryTable[Index].VirtualBase = PL111_CLCD_VRAM_MOTHERBOARD_BASE; >> + VirtualMemoryTable[Index].Length = PL111_CLCD_VRAM_MOTHERBOARD_SIZE; >> + VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED; > > Hmm, looking at this made me a bit confused though. Normal uncached > memory is certainly bufferable (that's basically what write-combining > means). > It maps to MAIR attribute encoding 0x44, which translates as Normal Memory, Outer Non-Cacheable, Inner Non-Cacheable > This looks like a naming hangover from ARMv5 translation table format. > Is it about time we clean this up? The whole 'ARM_MEMORY_REGION_xxxx' intermediate namespace should be removed, I think. > Or have I sprained my brain? > > / > Leif > >> + >> // Map sparse memory region if present >> if (HasSparseMemory) { >> VirtualMemoryTable[++Index].PhysicalBase = SparseMemoryBase; >> -- >> 2.9.3 >>