On 22 August 2017 at 18:02, Laszlo Ersek <[email protected]> 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 <[email protected]> >> --- >> 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. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
