On 6 April 2017 at 20:06, Leif Lindholm <leif.lindh...@linaro.org> wrote: > On Thu, Apr 06, 2017 at 07:46:50PM +0100, Ard Biesheuvel wrote: >> >> >> + // 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 >> > >> > Exactly - which is definitely "buffered". >> > >> >> > 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. >> > >> > That sounds like a good idea to me. >> > There's also _NONSECURE crud in there. >> > >> >> Yes. But I hope you're not saying you want that to be done first >> before this patch can go in? > > No, but it may mean it makes sense to add a comment regarding the > Attributes line, since it looks like it's doing the opposite of what > is actually being done. >
Something like --- a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c +++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c @@ -134,6 +134,12 @@ ArmPlatformGetVirtualMemoryMap ( VirtualMemoryTable[++Index].PhysicalBase = PL111_CLCD_VRAM_MOTHERBOARD_BASE; VirtualMemoryTable[Index].VirtualBase = PL111_CLCD_VRAM_MOTHERBOARD_BASE; VirtualMemoryTable[Index].Length = PL111_CLCD_VRAM_MOTHERBOARD_SIZE; + // + // Map the VRAM region as Normal Non-Cacheable memory and not device memory, + // so that we can use the accelerated string routines that may use unaligned + // accesses or DC ZVA instructions. The enum identifier is slightly awkward + // here, but it maps to a memory type that allows buffering and reordering. + // VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED; // Map sparse memory region if present perhaps? _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
