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 54B35208F7AC7 for ; Fri, 18 Aug 2017 11:14:04 -0700 (PDT) Received: by mail-it0-x231.google.com with SMTP id o72so9416913ita.1 for ; Fri, 18 Aug 2017 11:16:32 -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=3NGo+MVVhleOkCWDoBFHyd3uzm4Hz97jitCrl8y8Kas=; b=IikkXS9A7FXE6e/v6VnUl4WVibR/Fa+MUVabiXHrCowXWGiMriWyATykI0LdceveI6 +HHEgXgJn7RPxtoqzuztUrrfX8HGujz/Cfp4sSKiyVGh6ss3l1gUXefT28OaHoi6RsRz MEe0ole6bawBWTp5ErZUCty0LrDRxtsl0x6BY= 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=3NGo+MVVhleOkCWDoBFHyd3uzm4Hz97jitCrl8y8Kas=; b=WDogfJcvC880Ek4u7aRJ7otk5+oBa/UjvyWB1GR/g0d90o9hWZ3DREfaYbd4TM/TIZ kP0vAq4lD64wBamEoYVtlBg9QzpOxCQrtTUvSlZiirVmFwEVORIZr5zqwHxaYg02ELFJ 8PpZAzqaFHR5iqYnEhiGexsywf6krN7XkwCx3i22eJTWWzOqPTUE1kvzRcJHWobO2BXz CoeouRGBys6o6u7+azEDBi/HDyqV2vEMgb17D0TW6NpSyPdRSfZfUayymFV6lFWDLura aDVsRuz6+Bh4f0shYQP/iY9mmyF0PkDE5q4/xUB9EW9YXoUwZKs7+0BV634dhWok//N1 F24w== X-Gm-Message-State: AHYfb5iuzlII8pcb7jNsNTb++OdiY/xTL7FHORQgtwE4rXrpFyj9WRxf e4XnSmHsFljkDSJaqLAtC2Fj3woWX2vSOYvqvw== X-Received: by 10.36.41.143 with SMTP id p137mr2457986itp.98.1503080192251; Fri, 18 Aug 2017 11:16:32 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.162.1 with HTTP; Fri, 18 Aug 2017 11:16:31 -0700 (PDT) In-Reply-To: <150307857518.18247.16857169627688155787@jljusten-skl> References: <20170818130243.4645-1-ard.biesheuvel@linaro.org> <150307684325.17751.1587324408306927566@jljusten-skl> <150307857518.18247.16857169627688155787@jljusten-skl> From: Ard Biesheuvel Date: Fri, 18 Aug 2017 19:16:31 +0100 Message-ID: To: Jordan Justen Cc: "edk2-devel@lists.01.org" , Laszlo Ersek , Leif Lindholm 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 18:14:04 -0000 Content-Type: text/plain; charset="UTF-8" 'On 18 August 2017 at 18:49, 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 records >> >> > the base and size from the PCI BAR. >> >> > >> >> > On x86 systems, this is sufficient, but on ARM systems, the semantics >> >> > 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 that 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 side >> >> > 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*? > Both WC and WT go through a write buffer which may reorder writes and does not offer any timeliness guarantees AFAIK. > I'm not sure how well QEMU/KVM models this for x86 or ARM. > On ARM, we have coherency issues with framebuffers that are backed by host memory that is mapped cacheable, because writes via the uncached/write-through mapping of the guest do not invalidate cachelines that cover it. Hence the 'the VGA driver does not operate correctly in the first place' in the commit log. But crashing on an alignment fault is unfriendly, to say the least, and I would prefer to adhere to the architectural requirements in any case. >> >> > >> >> > [Protocols] >> >> > + gEfiCpuArchProtocolGuid # PROTOCOL ALWAYS_CONSUMED >> > >> > I don't think a 'Driver Model' driver needs to add arch protocols into >> > the depex. >> > >> >> To be pedantic: this is not the depex. You can't rely on the protocol >> header to declare gEfiCpuArchProtocolGuid, and declaring it here makes >> the build tools add its declaration to AutoGen.h (I think this has to >> do with the exact .dsc version. Perhaps Laszlo has a better >> recollection of the details.) > > Whoops. You are correct. We want this change... > > -Jordan