diff mbox

qemu: map "virtio" video model to "virt" machtype correctly (arm/aarch64)

Message ID 20160916040446.7556-1-lersek@redhat.com
State Superseded
Headers show

Commit Message

Laszlo Ersek Sept. 16, 2016, 4:04 a.m. UTC 
Most of QEMU's PCI display device models, such as:

  libvirt video/model/@type  QEMU -device
  -------------------------  ------------
  cirrus                     cirrus-vga
  vga                        VGA
  qxl                        qxl-vga
  virtio                     virtio-vga

come with a linear framebuffer (sometimes called "VGA compatibility
framebuffer"). This linear framebuffer lives in one of the PCI device's
MMIO BARs, and allows guest code (primarily: firmware drivers, and
non-accelerated OS drivers) to display graphics with direct memory access.

Due to architectural reasons on aarch64/KVM hosts, this kind of
framebuffer doesn't / can't work in

  qemu-system-(arm|aarch64) -M virt

machines. Cache coherency issues guarantee a corrupted / unusable display.
The problem has been researched by several people, including kvm-arm
maintainers, and it's been decided that the best way (practically the only
way) to have boot time graphics for such guests is to consolidate on
QEMU's "virtio-gpu-pci" device.

>From <https://bugzilla.redhat.com/show_bug.cgi?id=1195176>, libvirt
supports

  <devices>
    <video>
      <model type='virtio'/>
    </video>
  </devices>

but libvirt unconditionally maps @type='virtio' to QEMU's "virtio-vga"
device model. (See the qemuBuildDeviceVideoStr() function and the
"qemuDeviceVideo" enum impl.)

According to the above, this is not right for the "virt" machine type; the
qemu-system-(arm|aarch64) binaries don't even recognize the "virtio-vga"
device model (justifiedly). Whereas "virtio-gpu-pci", which is a pure
virtio device without a compatibility framebuffer, is available, and works
fine.

(The ArmVirtQemu ("AAVMF") platform of edk2 -- that is, the UEFI firmware
for "virt" -- supports "virtio-gpu-pci", as of upstream commit
3ef3209d3028. See
<https://tianocore.acgmultimedia.com/show_bug.cgi?id=66>.)

Override the default mapping of "virtio", from "virtio-vga" to
"virtio-gpu-pci", if qemuDomainMachineIsVirt() evaluates to true.

Cc: Andrea Bolognani <abologna@redhat.com>
Cc: Drew Jones <drjones@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1372901
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 src/qemu/qemu_command.c                                                  |  4 ++
 tests/qemuxml2argvtest.c                                                 |  5 +++
 tests/qemuxml2xmltest.c                                                  |  5 +++
 tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.args    | 26 +++++++++++
 tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.xml     | 36 ++++++++++++++++
 tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-video-virtio-gpu-pci.xml | 45 ++++++++++++++++++++
 6 files changed, 121 insertions(+)

Comments

Laszlo Ersek Sept. 16, 2016, 7:23 a.m. UTC | #1 
On 09/16/16 08:28, Martin Kletzander wrote:
> On Fri, Sep 16, 2016 at 06:04:46AM +0200, Laszlo Ersek wrote:

>> Most of QEMU's PCI display device models, such as:

>>

>>  libvirt video/model/@type  QEMU -device

>>  -------------------------  ------------

>>  cirrus                     cirrus-vga

>>  vga                        VGA

>>  qxl                        qxl-vga

>>  virtio                     virtio-vga

>>

>> come with a linear framebuffer (sometimes called "VGA compatibility

>> framebuffer"). This linear framebuffer lives in one of the PCI device's

>> MMIO BARs, and allows guest code (primarily: firmware drivers, and

>> non-accelerated OS drivers) to display graphics with direct memory

>> access.

>>

>> Due to architectural reasons on aarch64/KVM hosts, this kind of

>> framebuffer doesn't / can't work in

>>

>>  qemu-system-(arm|aarch64) -M virt

>>

>> machines. Cache coherency issues guarantee a corrupted / unusable

>> display.

>> The problem has been researched by several people, including kvm-arm

>> maintainers, and it's been decided that the best way (practically the

>> only

>> way) to have boot time graphics for such guests is to consolidate on

>> QEMU's "virtio-gpu-pci" device.

>>

>>> From <https://bugzilla.redhat.com/show_bug.cgi?id=1195176>, libvirt

>> supports

>>

>>  <devices>

>>    <video>

>>      <model type='virtio'/>

>>    </video>

>>  </devices>

>>

>> but libvirt unconditionally maps @type='virtio' to QEMU's "virtio-vga"

>> device model. (See the qemuBuildDeviceVideoStr() function and the

>> "qemuDeviceVideo" enum impl.)

>>

>> According to the above, this is not right for the "virt" machine type;

>> the

>> qemu-system-(arm|aarch64) binaries don't even recognize the "virtio-vga"

>> device model (justifiedly). Whereas "virtio-gpu-pci", which is a pure

>> virtio device without a compatibility framebuffer, is available, and

>> works

>> fine.

>>

>> (The ArmVirtQemu ("AAVMF") platform of edk2 -- that is, the UEFI firmware

>> for "virt" -- supports "virtio-gpu-pci", as of upstream commit

>> 3ef3209d3028. See

>> <https://tianocore.acgmultimedia.com/show_bug.cgi?id=66>.)

>>

>> Override the default mapping of "virtio", from "virtio-vga" to

>> "virtio-gpu-pci", if qemuDomainMachineIsVirt() evaluates to true.

>>

>> Cc: Andrea Bolognani <abologna@redhat.com>

>> Cc: Drew Jones <drjones@redhat.com>

>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>

>> Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>

>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1372901

>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

> 

> Very nicely written, makes sense and since virtio-vga didn't work for

> virt machines, there is no problem with changing it.


Thank you!

> One small nit though, syntax-check will complain that one of the lines

> in the .args file is longer, you can either use 'tests/test-wrap-argv.pl

> --in-place

> tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.args'

> or just squash this diff in, whatever you find easier:


While working on the test data, I consulted other files, and I noticed
that they were carefully wrapped. I tried to do the same -- did I fail?

$ wc -L \
  tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.args
74 tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.args

and "tests/test-wrap-argv.pl" has

    my $max_len = 78;

Hmmm... After running the script, it looks like I was too cautious. The
problem is actually that I broke one of those lines too early, and now
it's too short. :)

I'll send a v2.

Thanks!
Laszlo

> diff --git

> i/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.args

> w/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.args

> index 56dbdfb66fa2..76ee977a3ca2 100644

> --- i/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.args

> +++ w/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.args

> @@ -20,7 +20,7 @@ QEMU_AUDIO_DRV=none \

> addr=0x1 \

> -device ioh3420,port=0x9,chassis=2,id=pci.2,bus=pcie.0,multifunction=on,\

> addr=0x1.0x1 \

> --device virtio-net-pci,vlan=0,id=net0,mac=52:54:00:73:34:53,bus=pci.1,\

> -addr=0x0,bootindex=1 \

> +-device

> virtio-net-pci,vlan=0,id=net0,mac=52:54:00:73:34:53,bus=pci.1,addr=0x0,\

> +bootindex=1 \

> -net user,vlan=0,name=hostnet0 \

> -device virtio-gpu-pci,id=video0,bus=pci.2,addr=0x0

> -- 

> 

> Other than that, with that small thing fixed:

> 

> Acked-by: Martin Kletzander <mkletzan@redhat.com>


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
diff mbox

Patch

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3a61863b9abb..038c38f2217c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4325,6 +4325,10 @@  qemuBuildDeviceVideoStr(const virDomainDef *def,
                            virDomainVideoTypeToString(video->type));
             goto error;
         }
+        if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
+            qemuDomainMachineIsVirt(def)) {
+            model = "virtio-gpu-pci";
+        }
     } else {
         if (video->type != VIR_DOMAIN_VIDEO_TYPE_QXL) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index dbb0e4d56142..e8540779a4b5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1872,6 +1872,11 @@  mymain(void)
             QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM,
             QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE,
             QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_VIRTIO_SCSI);
+    DO_TEST("aarch64-video-virtio-gpu-pci",
+            QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_OBJECT_GPEX,
+            QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_IOH3420,
+            QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
+            QEMU_CAPS_DEVICE_VIRTIO_GPU, QEMU_CAPS_BOOTINDEX);
     DO_TEST("aarch64-aavmf-virtio-mmio",
             QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB,
             QEMU_CAPS_DEVICE_VIRTIO_MMIO,
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 14c2b0ccf2ce..fb05c8571411 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -831,6 +831,11 @@  mymain(void)
             QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM,
             QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_DEVICE_PCI_BRIDGE,
             QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_VIRTIO_SCSI);
+    DO_TEST("aarch64-video-virtio-gpu-pci",
+            QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_OBJECT_GPEX,
+            QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_IOH3420,
+            QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
+            QEMU_CAPS_DEVICE_VIRTIO_GPU, QEMU_CAPS_BOOTINDEX);
 
     DO_TEST_FULL("aarch64-gic-none", WHEN_BOTH, GIC_NONE, NONE);
     DO_TEST_FULL("aarch64-gic-none-v2", WHEN_BOTH, GIC_V2, NONE);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.args
new file mode 100644
index 000000000000..56dbdfb66fa2
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.args
@@ -0,0 +1,26 @@ 
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/opt/qemu-installed/bin/qemu-system-aarch64 \
+-name aarch64-vgpu \
+-S \
+-M virt \
+-cpu cortex-a57 \
+-m 1024 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid f3197c89-6457-44fe-b26d-897090ba6541 \
+-nographic \
+-nodefconfig \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-aarch64-vgpu/monitor.sock,server,nowait \
+-device ioh3420,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,\
+addr=0x1 \
+-device ioh3420,port=0x9,chassis=2,id=pci.2,bus=pcie.0,multifunction=on,\
+addr=0x1.0x1 \
+-device virtio-net-pci,vlan=0,id=net0,mac=52:54:00:73:34:53,bus=pci.1,\
+addr=0x0,bootindex=1 \
+-net user,vlan=0,name=hostnet0 \
+-device virtio-gpu-pci,id=video0,bus=pci.2,addr=0x0
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.xml b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.xml
new file mode 100644
index 000000000000..4b52a731b043
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-video-virtio-gpu-pci.xml
@@ -0,0 +1,36 @@ 
+<domain type='qemu'>
+  <name>aarch64-vgpu</name>
+  <uuid>f3197c89-6457-44fe-b26d-897090ba6541</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='aarch64' machine='virt'>hvm</type>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+  <cpu mode='custom' match='exact'>
+    <model fallback='allow'>cortex-a57</model>
+  </cpu>
+  <devices>
+    <emulator>/opt/qemu-installed/bin/qemu-system-aarch64</emulator>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <controller type='pci' index='1' model='pcie-root-port'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/>
+    </controller>
+    <controller type='pci' index='2' model='pcie-root-port'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1' multifunction='on'/>
+    </controller>
+    <interface type='user'>
+      <mac address='52:54:00:73:34:53'/>
+      <model type='virtio'/>
+      <boot order='1'/>
+      <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
+    </interface>
+    <video>
+      <model type='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
+    </video>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-video-virtio-gpu-pci.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-video-virtio-gpu-pci.xml
new file mode 100644
index 000000000000..26f6a51622ef
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-video-virtio-gpu-pci.xml
@@ -0,0 +1,45 @@ 
+<domain type='qemu'>
+  <name>aarch64-vgpu</name>
+  <uuid>f3197c89-6457-44fe-b26d-897090ba6541</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='aarch64' machine='virt'>hvm</type>
+  </os>
+  <features>
+    <acpi/>
+    <gic version='2'/>
+  </features>
+  <cpu mode='custom' match='exact'>
+    <model fallback='allow'>cortex-a57</model>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/opt/qemu-installed/bin/qemu-system-aarch64</emulator>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <controller type='pci' index='1' model='pcie-root-port'>
+      <model name='ioh3420'/>
+      <target chassis='1' port='0x8'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/>
+    </controller>
+    <controller type='pci' index='2' model='pcie-root-port'>
+      <model name='ioh3420'/>
+      <target chassis='2' port='0x9'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1' multifunction='on'/>
+    </controller>
+    <interface type='user'>
+      <mac address='52:54:00:73:34:53'/>
+      <model type='virtio'/>
+      <boot order='1'/>
+      <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
+    </interface>
+    <video>
+      <model type='virtio' heads='1' primary='yes'/>
+      <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
+    </video>
+  </devices>
+</domain>