From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x233.google.com (mail-io0-x233.google.com [IPv6:2607:f8b0:4001:c06::233]) (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 421C721E49BB0 for ; Tue, 22 Aug 2017 10:14:25 -0700 (PDT) Received: by mail-io0-x233.google.com with SMTP id o196so18148196ioe.0 for ; Tue, 22 Aug 2017 10:16:58 -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=B1YyCPTogFymnL5XcePtkOitD7hv2BeM3/YcdK1X65k=; b=IKtPFucQZOVOGUKqVFMVAgx6JYgCHUEWK8TOHwjVcILCSwH9Ax0hVVh8hkKEMAyj17 IHAmtY+ewSW5meGfRUFslMgsevgSa8EpqtlmVXm1Kcqq+WvcHqFlMh6a0MOkeyGRDRKi 34Qp24uReg8Q9N4YFQd3jplAJl/7iHMmDrmOw= 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=B1YyCPTogFymnL5XcePtkOitD7hv2BeM3/YcdK1X65k=; b=CWuVffUYdGky8LXGdWb6/bYxJhM1qXmlqp/8v9Sa5d32vcpil5rHyeHgEAOPD6+VgT W/mLgcj8rdRlyNkWFf81ZOeQ+4fmlpiCirUS8IY64SBqvGzG0d2+pmUAgPNGKgTkZvvJ oZWAqMDSMCyElFdDuuEd5mM+zIHxc2Z3YclMgA00XbQPXPlC9vo+Eye9Ze9flK7NnZ7b 1kYBNkMN5wqVn3jMNfa4+oDqNu6uu/9a6RGOxGloqwt23zgW7Pv6bSihTONyXVCB4K+M o+P1g1bwTVFbxPBJrMfQ3hnxeZ6JCriGJeazK/1j4sKZClTPNwfETXdKb2A7k15x8w1O 8b8g== X-Gm-Message-State: AHYfb5g42AZvrQobMdjTRufPyrG+EpzzurVBf7G1hcWweGbK82PqFV0u 0tEU3LhCwwayAsm4uRZsoT2H5krLMuRJ X-Received: by 10.107.132.96 with SMTP id g93mr1201552iod.149.1503422217369; Tue, 22 Aug 2017 10:16:57 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.162.1 with HTTP; Tue, 22 Aug 2017 10:16:56 -0700 (PDT) In-Reply-To: References: <20170822163013.12233-1-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Tue, 22 Aug 2017 18:16:56 +0100 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Jordan Justen Subject: Re: [PATCH] ArmVirtPkg: remove QemuVideoDxe from ArmVirtQemu and ArmVirtQemuKernel 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: Tue, 22 Aug 2017 17:14:25 -0000 Content-Type: text/plain; charset="UTF-8" On 22 August 2017 at 18:02, Laszlo Ersek wrote: > On 08/22/17 18:30, Ard Biesheuvel wrote: >> One of the reasons for introducing virtio-gpu support to OvmfPkg and >> ArmVirtpkg was the fact that under KVM virtualization on ARM, the >> legacy VGA cannot be used reliably. This is due to an implementation >> detail of QEMU+KVM, which remaps cached host memory into the guest >> address space as a framebuffer behind a PCI BAR. Given that the purpose >> of a memory mapped framebuffer is its side effects, such BARs should >> never be mapped cacheable in the guest, and the mismatched attributes >> between host and guest result in a loss of coherency, visible as >> corruption in the framebuffer image. >> >> This issue does not occur under TCG emulation, nor did we expect it to >> actually bring down the guest under KVM, and so it was deemed harmless >> to keep support for the VGA device as well. However, as it turns out, >> the fact that the framebuffer BAR is mapped using device semantics by >> default may result in unalignment faults when we use the ordinary string >> copy routines on the contents. In theory, we could work around this by >> remapping the BAR as write combining, but it appears the generic PCI >> bus driver does not actually implement this. >> >> So let's remove the QemuVideoDxe driver altogether. This may result >> in loss of functionality for use cases that rely on the framebuffer >> to be directly addressable (such as EFIFB), but given that this never >> worked reliably under KVM in the first place, let's not let that stop >> us from dropping support for it. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel >> --- >> ArmVirtPkg/ArmVirtQemu.dsc | 2 -- >> ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 1 - >> ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 -- >> 3 files changed, 5 deletions(-) >> >> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc >> index e23a6d17bc44..2e6e76224987 100644 >> --- a/ArmVirtPkg/ArmVirtQemu.dsc >> +++ b/ArmVirtPkg/ArmVirtQemu.dsc >> @@ -57,7 +57,6 @@ >> BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf >> PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf >> CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf >> - FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf >> QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf >> FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf >> PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf >> @@ -357,7 +356,6 @@ >> # >> # Video support >> # >> - OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf >> OvmfPkg/VirtioGpuDxe/VirtioGpu.inf >> OvmfPkg/PlatformDxe/Platform.inf >> >> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc >> index 237b2d03a714..3194aa3edc8e 100644 >> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc >> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc >> @@ -167,7 +167,6 @@ READ_LOCK_STATUS = TRUE >> # >> # Video support >> # >> - INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf >> INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf >> INF OvmfPkg/PlatformDxe/Platform.inf >> >> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc >> index aa01debfda69..69de887277cb 100644 >> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc >> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc >> @@ -57,7 +57,6 @@ >> BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf >> PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf >> CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf >> - FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf >> QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf >> FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf >> PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf >> @@ -348,7 +347,6 @@ >> # >> # Video support >> # >> - OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf >> OvmfPkg/VirtioGpuDxe/VirtioGpu.inf >> OvmfPkg/PlatformDxe/Platform.inf >> >> > > (My R-b stands; these are comments for a possible followup patch.) > > Please see: > > - commit 84a75f70e903 ("OvmfPkg/QemuVideoDxe: enable ARM builds", > 2015-02-23), > > - commit 05a537945872 ("OvmfPkg/QemuVideoDxe: Helper functions for > unaligned port I/O.", 2017-04-07)? > > In my opinion, we should now revert parts of these commits, in one > followup patch: > > - from the first commit, we should revert only the "VALID_ARCHITECTURES" > comment change (the rest is built upon by the second commit, and should > be preserved) > > - from the second commit, we should revert the addition of [Sources.ARM, > Sources.AARCH64]. > > This boils down to removing ARM and AARCH64 references from the > QemuVideoDxe.inf file. If you agree, could you please submit such a > followup patch? > Sure, but pending the graphical GRUB discussion.