ArmVirtPkg/ArmVirtQemu.dsc | 2 -- ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 1 - ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 -- 3 files changed, 5 deletions(-)
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 <ard.biesheuvel@linaro.org>
---
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
--
2.11.0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
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 <ard.biesheuvel@linaro.org> > --- > 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 > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, Aug 22, 2017 at 05:30:13PM +0100, 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.
For the record, this would most likely mean we would not be able to
test graphical installers on QEMU. GRUB certainly looks like it's
using FrameBufferBase. Maybe that isn't the most important use-case,
but it's certainly not invalid.
/
Leif
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> 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
>
> --
> 2.11.0
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 22 August 2017 at 17:57, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Tue, Aug 22, 2017 at 05:30:13PM +0100, 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. > > For the record, this would most likely mean we would not be able to > test graphical installers on QEMU. GRUB certainly looks like it's > using FrameBufferBase. Maybe that isn't the most important use-case, > but it's certainly not invalid. > This came up at the time, and I /though/ the conclusion was that GRUB hooks into the stack at a higher level. I will give Debian's GRUB a quick spin tomorrow. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 08/22/17 18:57, Leif Lindholm wrote: > On Tue, Aug 22, 2017 at 05:30:13PM +0100, 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. > > For the record, this would most likely mean we would not be able to > test graphical installers on QEMU. Hmmm, RHEL-7.3+ (and Fedora 2x+, for some "x" I don't recall), for Aarch64, definitely install in GUI mode, on virtio-gpu-pci, *including* a perfectly working grub2. I'd been obsessed with this (i.e., a fully graphical installation in aarch64/KVM guests). I also have GUI-enabled openSUSE Tumbleweed and Ubuntu guests on my Mustang. I've booted them just now, and their grub2's work flawlessly. These guests are not new, the installer ISO names are: - openSUSE-Tumbleweed-DVD-aarch64-Snapshot20161031-Media.iso - ubuntu-16.04.2-server-arm64.iso I remember that for the Ubuntu installation, I had to pick the "HWE kernel" from the grub menu. I've uploaded a few screenshots here: https://people.redhat.com/lersek/aarch64-vgpu-screenshots-7c6bb412-923d-468b-8cfa-5894abd90b40/ > GRUB certainly looks like it's > using FrameBufferBase. Please see this (quite small) grub2 patch: <http://mid.mail-archive.com/1485987247-16230-1-git-send-email-agraf@suse.de>. (See also the responses from phcoder.) > Maybe that isn't the most important use-case, > but it's certainly not invalid. To me personally, the use case you describe is extremely important (see above). My impression has been that this has been sorted out for ages. At least Alex Graf posted the grub2 patch in Feb 2017 (CC'ing him now). ... FWIW, as far as I can see, Fedora and RHEL do *not* carry Alex's grub2 patch. Yet grub2 works just fine with virtio-gpu-pci (BLT only). Not sure about the grub2 details, but I consider this problem (graphical GNU/Linux installers in aarch64 guests, using virtio-gpu-pci) solved; cross-distro. Thanks, Laszlo >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> 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 >> >> -- >> 2.11.0 >> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, Aug 22, 2017 at 07:38:03PM +0200, Laszlo Ersek wrote: > On 08/22/17 18:57, Leif Lindholm wrote: > > On Tue, Aug 22, 2017 at 05:30:13PM +0100, 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. > > > > For the record, this would most likely mean we would not be able to > > test graphical installers on QEMU. > > Hmmm, RHEL-7.3+ (and Fedora 2x+, for some "x" I don't recall), for > Aarch64, definitely install in GUI mode, on virtio-gpu-pci, *including* > a perfectly working grub2. I'd been obsessed with this (i.e., a fully > graphical installation in aarch64/KVM guests). OK, good, then our goals are aligned. > I also have GUI-enabled openSUSE Tumbleweed and Ubuntu guests on my > Mustang. I've booted them just now, and their grub2's work flawlessly. > These guests are not new, the installer ISO names are: > > - openSUSE-Tumbleweed-DVD-aarch64-Snapshot20161031-Media.iso > - ubuntu-16.04.2-server-arm64.iso > > I remember that for the Ubuntu installation, I had to pick the "HWE > kernel" from the grub menu. > > I've uploaded a few screenshots here: > > https://people.redhat.com/lersek/aarch64-vgpu-screenshots-7c6bb412-923d-468b-8cfa-5894abd90b40/ > > > GRUB certainly looks like it's > > using FrameBufferBase. > > Please see this (quite small) grub2 patch: > <http://mid.mail-archive.com/1485987247-16230-1-git-send-email-agraf@suse.de>. > > (See also the responses from phcoder.) > > > Maybe that isn't the most important use-case, > > but it's certainly not invalid. > > To me personally, the use case you describe is extremely important (see > above). My impression has been that this has been sorted out for ages. > At least Alex Graf posted the grub2 patch in Feb 2017 (CC'ing him now). Yes, grub work pretty much seized for anything other than preparing the release until that happened, and then stalled for a while (presumably down to recovery). > ... FWIW, as far as I can see, Fedora and RHEL do *not* carry Alex's > grub2 patch. Yet grub2 works just fine with virtio-gpu-pci (BLT only). "works just fine" is not the same as "are unaffected by the lack of direct framebuffer access". I will have a skim through the code and see what (if any) situations could cause direct accesses to the framebuffer. > Not sure about the grub2 details, but I consider this problem (graphical > GNU/Linux installers in aarch64 guests, using virtio-gpu-pci) solved; > cross-distro. If we have a way to resolve this situation, I do not religiously oppose a temporary breakage. However, if we (for example) end up with "upstream GRUB won't be able to test the graphic installer of Debian Stable for the next two years", that will be inconvenient. Best Regards, Leif > Thanks, > Laszlo > > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> --- > >> 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 > >> > >> -- > >> 2.11.0 > >> > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 22 August 2017 at 20:05, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Tue, Aug 22, 2017 at 07:38:03PM +0200, Laszlo Ersek wrote: >> On 08/22/17 18:57, Leif Lindholm wrote: >> > On Tue, Aug 22, 2017 at 05:30:13PM +0100, 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. >> > >> > For the record, this would most likely mean we would not be able to >> > test graphical installers on QEMU. >> >> Hmmm, RHEL-7.3+ (and Fedora 2x+, for some "x" I don't recall), for >> Aarch64, definitely install in GUI mode, on virtio-gpu-pci, *including* >> a perfectly working grub2. I'd been obsessed with this (i.e., a fully >> graphical installation in aarch64/KVM guests). > > OK, good, then our goals are aligned. > >> I also have GUI-enabled openSUSE Tumbleweed and Ubuntu guests on my >> Mustang. I've booted them just now, and their grub2's work flawlessly. >> These guests are not new, the installer ISO names are: >> >> - openSUSE-Tumbleweed-DVD-aarch64-Snapshot20161031-Media.iso >> - ubuntu-16.04.2-server-arm64.iso >> >> I remember that for the Ubuntu installation, I had to pick the "HWE >> kernel" from the grub menu. >> >> I've uploaded a few screenshots here: >> >> https://people.redhat.com/lersek/aarch64-vgpu-screenshots-7c6bb412-923d-468b-8cfa-5894abd90b40/ >> >> > GRUB certainly looks like it's >> > using FrameBufferBase. >> >> Please see this (quite small) grub2 patch: >> <http://mid.mail-archive.com/1485987247-16230-1-git-send-email-agraf@suse.de>. >> >> (See also the responses from phcoder.) >> >> > Maybe that isn't the most important use-case, >> > but it's certainly not invalid. >> >> To me personally, the use case you describe is extremely important (see >> above). My impression has been that this has been sorted out for ages. >> At least Alex Graf posted the grub2 patch in Feb 2017 (CC'ing him now). > > Yes, grub work pretty much seized for anything other than preparing > the release until that happened, and then stalled for a while > (presumably down to recovery). > >> ... FWIW, as far as I can see, Fedora and RHEL do *not* carry Alex's >> grub2 patch. Yet grub2 works just fine with virtio-gpu-pci (BLT only). > > "works just fine" is not the same as "are unaffected by the lack of > direct framebuffer access". I will have a skim through the code and > see what (if any) situations could cause direct accesses to the > framebuffer. > >> Not sure about the grub2 details, but I consider this problem (graphical >> GNU/Linux installers in aarch64 guests, using virtio-gpu-pci) solved; >> cross-distro. > > If we have a way to resolve this situation, I do not religiously > oppose a temporary breakage. > > However, if we (for example) end up with "upstream GRUB won't be able > to test the graphic installer of Debian Stable for the next two > years", that will be inconvenient. > I *think* there is no disagreement here, only Laszlo and I were under the impression that this was a solved issue. Interestingly, some GRUB versions without the patch also work, including Ubuntu's, so I wonder what the deal is here. I'll dig into this tomorrow. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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 <ard.biesheuvel@linaro.org>
> ---
> 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?
Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 22 August 2017 at 18:02, Laszlo Ersek <lersek@redhat.com> 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 <ard.biesheuvel@linaro.org>
>> ---
>> 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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, Aug 22, 2017 at 06:16:56PM +0100, Ard Biesheuvel wrote:
> On 22 August 2017 at 18:02, Laszlo Ersek <lersek@redhat.com> 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 <ard.biesheuvel@linaro.org>
> >> ---
> >> 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.
So, after looking at the GRUB code, I am leaning towards agreeing that
this is actually not a problem at all ... probably. The efi_gop driver
does a Blt() of the entire screen from an off-screen buffer for all
updates _unless_ it fails to allocate that off-screen buffer.
So, basically, if you run out of memory at that point, it will try to
preserve a way to get messages out about that. I will send a question
out to grub-devel regarding this behaviour.
However, looking at the specification, a question remains over how
software can determine whether direct FB access is possible. I mean, a
value of 0 seems like a decent hint, but the spec says nothing on the
topic.
/
Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 23 August 2017 at 14:15, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Aug 22, 2017 at 06:16:56PM +0100, Ard Biesheuvel wrote:
>> On 22 August 2017 at 18:02, Laszlo Ersek <lersek@redhat.com> 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 <ard.biesheuvel@linaro.org>
>> >> ---
>> >> 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.
>
> So, after looking at the GRUB code, I am leaning towards agreeing that
> this is actually not a problem at all ... probably. The efi_gop driver
> does a Blt() of the entire screen from an off-screen buffer for all
> updates _unless_ it fails to allocate that off-screen buffer.
>
> So, basically, if you run out of memory at that point, it will try to
> preserve a way to get messages out about that. I will send a question
> out to grub-devel regarding this behaviour.
>
> However, looking at the specification, a question remains over how
> software can determine whether direct FB access is possible. I mean, a
> value of 0 seems like a decent hint, but the spec says nothing on the
> topic.
>
It will assume the FB is accessible unless the pixel format is PixelBltOnly
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 08/23/17 15:17, Ard Biesheuvel wrote:
> On 23 August 2017 at 14:15, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> On Tue, Aug 22, 2017 at 06:16:56PM +0100, Ard Biesheuvel wrote:
>>> On 22 August 2017 at 18:02, Laszlo Ersek <lersek@redhat.com> 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 <ard.biesheuvel@linaro.org>
>>>>> ---
>>>>> 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.
>>
>> So, after looking at the GRUB code, I am leaning towards agreeing that
>> this is actually not a problem at all ... probably. The efi_gop driver
>> does a Blt() of the entire screen from an off-screen buffer for all
>> updates _unless_ it fails to allocate that off-screen buffer.
>>
>> So, basically, if you run out of memory at that point, it will try to
>> preserve a way to get messages out about that. I will send a question
>> out to grub-devel regarding this behaviour.
>>
>> However, looking at the specification, a question remains over how
>> software can determine whether direct FB access is possible. I mean, a
>> value of 0 seems like a decent hint, but the spec says nothing on the
>> topic.
>>
>
> It will assume the FB is accessible unless the pixel format is PixelBltOnly
>
Correct, the UEFI spec (v2.7) says in "12.9 Graphics Output Protocol":
PixelBltOnly This mode does not support a physical frame buffer.
and
PixelFormat Enumeration that defines the physical format of the
pixel. A value of PixelBltOnly implies that a linear
frame buffer is not available for this mode.
IIRC simply recognizing and accepting this enum constant was the point
of Alex's patch.
Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, Aug 23, 2017 at 03:36:37PM +0200, Laszlo Ersek wrote:
> On 08/23/17 15:17, Ard Biesheuvel wrote:
> >>>> (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.
> >>
> >> So, after looking at the GRUB code, I am leaning towards agreeing that
> >> this is actually not a problem at all ... probably. The efi_gop driver
> >> does a Blt() of the entire screen from an off-screen buffer for all
> >> updates _unless_ it fails to allocate that off-screen buffer.
> >>
> >> So, basically, if you run out of memory at that point, it will try to
> >> preserve a way to get messages out about that. I will send a question
> >> out to grub-devel regarding this behaviour.
> >>
> >> However, looking at the specification, a question remains over how
> >> software can determine whether direct FB access is possible. I mean, a
> >> value of 0 seems like a decent hint, but the spec says nothing on the
> >> topic.
> >>
> >
> > It will assume the FB is accessible unless the pixel format is PixelBltOnly
> >
>
> Correct, the UEFI spec (v2.7) says in "12.9 Graphics Output Protocol":
>
> PixelBltOnly This mode does not support a physical frame buffer.
>
> and
>
> PixelFormat Enumeration that defines the physical format of the
> pixel. A value of PixelBltOnly implies that a linear
> frame buffer is not available for this mode.
-ETOOMANYLEVELSOFINDIRECTION
Right, so that's a non-issue.
Hopefully, that means other operating systems can also deal with the
lack.
> IIRC simply recognizing and accepting this enum constant was the point
> of Alex's patch.
Ah, yes, that makes sense now.
It's a bit surprising things work without this patch, but GRUBs
fallback seems to match anyway.
/
Leif
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, Aug 23, 2017 at 04:00:57PM +0100, Leif Lindholm wrote:
> On Wed, Aug 23, 2017 at 03:36:37PM +0200, Laszlo Ersek wrote:
> > On 08/23/17 15:17, Ard Biesheuvel wrote:
> > >>>> (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.
> > >>
> > >> So, after looking at the GRUB code, I am leaning towards agreeing that
> > >> this is actually not a problem at all ... probably. The efi_gop driver
> > >> does a Blt() of the entire screen from an off-screen buffer for all
> > >> updates _unless_ it fails to allocate that off-screen buffer.
> > >>
> > >> So, basically, if you run out of memory at that point, it will try to
> > >> preserve a way to get messages out about that. I will send a question
> > >> out to grub-devel regarding this behaviour.
> > >>
> > >> However, looking at the specification, a question remains over how
> > >> software can determine whether direct FB access is possible. I mean, a
> > >> value of 0 seems like a decent hint, but the spec says nothing on the
> > >> topic.
> > >>
> > >
> > > It will assume the FB is accessible unless the pixel format is PixelBltOnly
> > >
> >
> > Correct, the UEFI spec (v2.7) says in "12.9 Graphics Output Protocol":
> >
> > PixelBltOnly This mode does not support a physical frame buffer.
> >
> > and
> >
> > PixelFormat Enumeration that defines the physical format of the
> > pixel. A value of PixelBltOnly implies that a linear
> > frame buffer is not available for this mode.
>
> -ETOOMANYLEVELSOFINDIRECTION
> Right, so that's a non-issue.
>
> Hopefully, that means other operating systems can also deal with the
> lack.
>
> > IIRC simply recognizing and accepting this enum constant was the point
> > of Alex's patch.
>
> Ah, yes, that makes sense now.
> It's a bit surprising things work without this patch, but GRUBs
> fallback seems to match anyway.
To clarify:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 24 August 2017 at 13:02, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Wed, Aug 23, 2017 at 04:00:57PM +0100, Leif Lindholm wrote:
>> On Wed, Aug 23, 2017 at 03:36:37PM +0200, Laszlo Ersek wrote:
>> > On 08/23/17 15:17, Ard Biesheuvel wrote:
>> > >>>> (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.
>> > >>
>> > >> So, after looking at the GRUB code, I am leaning towards agreeing that
>> > >> this is actually not a problem at all ... probably. The efi_gop driver
>> > >> does a Blt() of the entire screen from an off-screen buffer for all
>> > >> updates _unless_ it fails to allocate that off-screen buffer.
>> > >>
>> > >> So, basically, if you run out of memory at that point, it will try to
>> > >> preserve a way to get messages out about that. I will send a question
>> > >> out to grub-devel regarding this behaviour.
>> > >>
>> > >> However, looking at the specification, a question remains over how
>> > >> software can determine whether direct FB access is possible. I mean, a
>> > >> value of 0 seems like a decent hint, but the spec says nothing on the
>> > >> topic.
>> > >>
>> > >
>> > > It will assume the FB is accessible unless the pixel format is PixelBltOnly
>> > >
>> >
>> > Correct, the UEFI spec (v2.7) says in "12.9 Graphics Output Protocol":
>> >
>> > PixelBltOnly This mode does not support a physical frame buffer.
>> >
>> > and
>> >
>> > PixelFormat Enumeration that defines the physical format of the
>> > pixel. A value of PixelBltOnly implies that a linear
>> > frame buffer is not available for this mode.
>>
>> -ETOOMANYLEVELSOFINDIRECTION
>> Right, so that's a non-issue.
>>
>> Hopefully, that means other operating systems can also deal with the
>> lack.
>>
>> > IIRC simply recognizing and accepting this enum constant was the point
>> > of Alex's patch.
>>
>> Ah, yes, that makes sense now.
>> It's a bit surprising things work without this patch, but GRUBs
>> fallback seems to match anyway.
>
> To clarify:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Pushed, thanks.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.