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

[Martin Kletzander]

On Fri, Sep 16, 2016 at 09:23:30AM +0200, Laszlo Ersek wrote:
>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 at redhat.com>
>>> Cc: Drew Jones <drjones at redhat.com>
>>> Cc: Marc-André Lureau <marcandre.lureau at redhat.com>
>>> Suggested-by: Marc-André Lureau <marcandre.lureau at redhat.com>
>>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1372901
>>> Signed-off-by: Laszlo Ersek <lersek at 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. :)
>

It's "prefer wrapping where spaces are, then wrap after commas", the
check is done by running the same script that can fix it in place for
you, so I think there is no need for v2 just to check some whitespaces
for which we have a script =D

>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 at redhat.com>
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160916/a7a29984/attachment-0001.sig>