From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 87B8B21CFA5E8 for ; Fri, 18 Aug 2017 12:34:18 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga104.jf.intel.com with ESMTP; 18 Aug 2017 12:36:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,393,1498546800"; d="scan'208";a="125267622" Received: from pcastin-mobl1.amr.corp.intel.com (HELO localhost) ([10.254.176.77]) by orsmga002.jf.intel.com with ESMTP; 18 Aug 2017 12:36:46 -0700 MIME-Version: 1.0 To: Leif Lindholm Message-ID: <150308500574.19101.8338041212043662212@jljusten-skl> From: Jordan Justen In-Reply-To: <20170818180823.b24fsxtcy63zndq2@bivouac.eciton.net> Cc: Ard Biesheuvel , "edk2-devel@lists.01.org" , Laszlo Ersek References: <20170818130243.4645-1-ard.biesheuvel@linaro.org> <150307684325.17751.1587324408306927566@jljusten-skl> <150307857518.18247.16857169627688155787@jljusten-skl> <20170818180823.b24fsxtcy63zndq2@bivouac.eciton.net> User-Agent: alot/0.6.0dev Date: Fri, 18 Aug 2017 12:36:45 -0700 Subject: Re: [PATCH] OvmfPkg/QemuVideoDxe: map framebuffer as write-combining/non-executable 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: Fri, 18 Aug 2017 19:34:18 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2017-08-18 11:08:23, Leif Lindholm wrote: > On Fri, Aug 18, 2017 at 10:49:35AM -0700, Jordan Justen wrote: > > On 2017-08-18 10:25:14, Ard Biesheuvel wrote: > > > On 18 August 2017 at 18:20, Jordan Justen = wrote: > > > > On 2017-08-18 06:04:01, Ard Biesheuvel wrote: > > > >> On 18 August 2017 at 14:02, Ard Biesheuvel wrote: > > > >> > When QemuVideoDxe takes control of the framebuffer, it is already > > > >> > mapped EFI_MEMORY_UC by core code, and QemuVideoDxe simply recor= ds > > > >> > the base and size from the PCI BAR. > > > >> > > > > >> > On x86 systems, this is sufficient, but on ARM systems, the sema= ntics > > > >> > of EFI_MEMORY_UC regions are quite different from EFI_MEMORY_WC = regions, > > > >> > and treating a region like memory (i.e., dereferencing pointers = into it > > > >> > or using ordinary CopyMem()/SetMem() functions on it) requires t= hat it > > > >> > be mapped with memory semantics, i.e., EFI_MEMORY_WC, EFI_MEMORY= _WT or > > > >> > EFI_MEMORY_WB. > > > >> > > > > >> > Since caching is not appropriate for regions where we rely on si= de > > > >> > effects, remap the frame buffer EFI_MEMORY_WT. > > > >> > > > >> EFI_MEMORY_WC not WT > > > > > > > > If a single pixel is written, then WC may not write it through > > > > immediately. Would WT be more appropriate? > > > = > > > For ARM, that applies equally to WT AFAIK. > > = > > Write-through will not actually write-*through*? > = > Not immediately, no. Hmm, looking the SDM, it looks like WT is similar to WC for writes, but it may service reads from cache, whereas WC will not. Since I assume reading the FB is somewhat rare, it sounds like WC is probably better. > = > Conversely, does IA32/X64 WC not guarantee writing out within a > finite period? I'm not sure, but the SDM does say: "If the WC buffer is partially filled, the writes may be delayed until the next occurrence of a serializing event; such as, an SFENCE or MFENCE instruction, CPUID execution, a read or write to uncached memory, an interrupt occurrence, or a LOCK instruction execution." And, even more arguing in favor of your change: "This type of cache-control is appropriate for video frame buffers, where the order of writes is unimportant as long as the writes update memory so they can be seen on the graphics display." So, it sound like there is no strict guarantee, but practically speaking it will be flushed often enough that a user will never notice the difference. Based on this, if you tweak the comment, and test on X64, then: Reviewed-by: Jordan Justen