From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by ml01.01.org (Postfix) with ESMTP id 601B020080934 for ; Wed, 5 Apr 2017 14:28:10 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 346FE2B; Wed, 5 Apr 2017 14:28:10 -0700 (PDT) Received: from [192.168.229.136] (u201426.usa.arm.com [10.118.28.78]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C73BF3F23B; Wed, 5 Apr 2017 14:28:09 -0700 (PDT) To: Ard Biesheuvel , Leif Lindholm References: <1473429644-13480-1-git-send-email-ard.biesheuvel@linaro.org> <1473429644-13480-5-git-send-email-ard.biesheuvel@linaro.org> Cc: "edk2-devel@lists.01.org" , "Gao, Liming" , "Kinney, Michael D" From: Jeremy Linton Message-ID: <8871a794-80f8-049f-5abc-ca1d4a8fb3a3@arm.com> Date: Wed, 5 Apr 2017 16:28:09 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: Subject: Re: [PATCH v5 4/4] MdePkg/BaseMemoryLibOptDxe ARM|AARCH64: disallow use in SEC & PEI phases 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, 05 Apr 2017 21:28:10 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Hi, On 04/05/2017 03:34 PM, Ard Biesheuvel wrote: > On 5 April 2017 at 21:12, Jeremy Linton wrote: >> Hi, >> >> On 09/09/2016 09:00 AM, Ard Biesheuvel wrote: >>> >>> The new accelerated ARM and AARCH64 implementations take advantage of >>> features that are only available when the MMU and Dcache are on. So >>> restrict the use of this library to the DXE phase or later. >> >> >> I don't think this is sufficient because DC ZVA doesn't work against device >> memory/etc. That means that users have to somehow know the page/etc >> attributes of memory regions before they call SetMemXX() on them. >> > > Yes. I literally found this out myself yesterday. Note that this > applies equally to unaligned accesses. > > >> I think this is a problem because nowhere in the UEFI specs do I see such >> restrictions on those memory operations. >> > > Using device attributes for memory is something we should ban for > AArch64 in the spec. > >> For a specific problematic example, the LcdGraphicsOutputBlt.c uses it for >> BltVideoFill() and the target of that is likely not regular cached video >> memory. >> > > Those drivers should be using EFI_MEMORY_WC not EFI_MEMORY_UC for the > VRAM mapping. Note that EFI_MEMORY_UC is nGnRnE which is unnecessarily > restrictive. > > I agree there is a general issue here which we should address by > tightening the spec. I don't see a lot of value in avoiding DC ZVA and > unaligned accesses altogether, I'd rather fix the code instead. While I agree with the general sentiment, I find the result brittle. If it were used as a DEBUG build way to locate sub-optmimal code I would be more on board. But shipping it like this, puts it into situations where the user inadvertently changes something (say making the background black and therefore triggering the DC) or some obscure option ROM (we will get there right??!!) triggers it in a place where it can't be debugged. Particularly since we are talking boot, where the few percent perf improvement on this operation is likely completely undetectable. The one place where I can think it might even be measurable is in routines to clear system memory, and those routines could be a special case anyway.