Please enable opengl for acceleration and vfio-MDEV support

Bug #1804766 reported by Christian Ehrhardt 
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
libvirt (Ubuntu)
Fix Released
High
Unassigned
Bionic
Won't Fix
Wishlist
Unassigned
Cosmic
Won't Fix
Wishlist
Unassigned
qemu (Ubuntu)
Fix Released
Undecided
Unassigned
Bionic
Won't Fix
Wishlist
Unassigned
Cosmic
Won't Fix
Wishlist
Unassigned

Bug Description

[Impact]

 * Newer GPU hardware of Nvidia (4.15) and Intel (4.16) can be split into
   virtual GPUs by their host device driver. It is customers demand to be
   able to utilize that sooner than waiting for 20.04 to happen. While the
   usability prior to very recent libvirt versions (not backporting all of
   that as it is not SRUable) isn't perfect it works well enough as-is.

 * The fix is enabling a build option and some build depends, the code is
   already there.

 * This applies to the safe SRU exception of "For Long Term Support
   releases we sometimes want to introduce new features. They must not
   change the behaviour on existing installations ..." due to the fact
   that gl is disabled by default. So users need to opt-in to use it after
   the change here.

[Test Case]

 * There are various ways to test this but they all depend on certain HW
   to be available as the feature starts with the Hosts GPU device driver
   breaking the card into shareable mediated devices.

 * In theory some Nvidia GPU cards [1] can work as well, but I didn't have
   one of those to test and most others verifying this might not have one
   either. Therefore I'll outline the use of the much more widespread
   Intel i915 chips. Note in Bionic i915 vGPU needs the HWE kernel to
   work.

 * Prepare Host to be able to split the GPU
   # loading modules
   $ printf "kvmgt\nvfio-iommu-type1\nvfio-mdev" | sudo tee /etc/initramfs-tools/modules
   # in /etc/default/grub add kernel opts
       i915.enable_gvt=1 intel_iommu=on drm.debug=0
   # Refresh boot config and reboot
   $ sudo update-initramfs -u
   $ sudo update-grub

 * Create vGPUs in the host, the PCI path depends on your system.
   # Check lspci if you have a different slot than 0000:00:02.0
   $ cd /sys/bus/pci/devices/0000:00:02.0/i915-GVTg_V4_4/
   # you need new UUIDs, you can e.g. run
   $ uuid -n 2
   # and then take these UUIDs into the driver to create the vGPUs
   $ echo 4dd50f26-ec08-11e8-b838-4bc3356865b6 | sudo tee create
   $ echo 4dd511f6-ec08-11e8-b839-2f163ddee3b3 | sudo tee create
   # you should then see soemthing like:
   $ ls -laF /sys/bus/mdev/devices/
lrwxrwxrwx 1 root root 0 Nov 19 15:38 4dd50f26-ec08-11e8-b838-4bc3356865b6 -> ../../../devices/pci0000:00/0000:00:02.0/4dd50f26-ec08-11e8-b838-4bc3356865b6/
lrwxrwxrwx 1 root root 0 Nov 19 15:38 4dd511f6-ec08-11e8-b839-2f163ddee3b3 -> ../../../devices/pci0000:00/0000:00:02.0/4dd511f6-ec08-11e8-b839-2f163ddee3b3/

 * you can start qemu "as usual" with the tweak of adding
   - gl=on to the display statement
   - a vfio-pci device referencing the MDEV uuid path
   # Example qemu:
   /usr/bin/qemu-system-x86_64 \
   -m 2048 \
   -nodefaults \
   -enable-kvm \
   -M graphics=off \
   -serial stdio \
   -cpu host \
   -display gtk,gl=on \
   -usb -usbdevice tablet \
   -net nic -net user \
   -device vfio-pci,sysfsdev=/sys/bus/pci/devices/0000:00:02.0/4dd511f6-ec08-11e8-b839-2f163ddee3b3,display=on,rombar=0 \
   -hda /var/lib/uvtool/libvirt/images/bionic-vgpu.qcow

  This qemu will boot (you could as well use a Ubuntu ISO, I used a image
  I created with uvtool in this case) and when you log in you will see in
  the guest the Intel HD graphics card in e.g. lspci.

[Regression Potential]

 * by being default off the regressions to current users should be near
   zero. We are not changing actual code, but "only" enable a config.
   If the config would enable code that runs in cases that are default
   enabled (like by an ifdef in C code that makes code outside of gl=on
   run the extra code) there could be a regression that we don't see yet.

[Other Info]

 * This is NOT virgl based vGPUs as in bug 1657409
 * Some related links for the MDEV-GPU topic in general:
   https://bugzilla.redhat.com/show_bug.cgi?id=1460804
   https://bugzilla.redhat.com/show_bug.cgi?id=1337290
   https://docs.nvidia.com/grid/latest/grid-vgpu-release-notes-red-hat-el-kvm/index.html
   http://on-demand.gputechconf.com/gtc/2017/presentation/s7572-suneel-marthi-trevor-grant-extending-mahout-samsara.pdf
   https://www.kraxel.org/blog/2018/04/vgpu-display-support-finally-merged-upstream/
   https://github.com/intel/gvt-linux/wiki/GVTg_Setup_Guide
   https://docs.nvidia.com/grid/latest/grid-vgpu-release-notes-red-hat-el-kvm/index.html
   https://www.linux-kvm.org/images/5/59/02x03-Neo_Jia_and_Kirti_Wankhede-vGPU_on_KVM-A_VFIO_based_Framework.pdf
   http://on-demand.gputechconf.com/gtc/2017/presentation/s7572-suneel-marthi-trevor-grant-extending-mahout-samsara.pdf

[1]: https://docs.nvidia.com/grid/latest/grid-vgpu-user-guide/index.html

----

Hi,
for quite a while we had thought enabling opengl to go along the MIR for virglrenderer (bug 1657409), but missed that it is a prereq for using MDEVs as well.
This now become much more important as newer drivers (recent nvidia drivers, as well as kernel >4.16 for intel) made this much more available to users - and it would work just fine if qemu had opengl enabled.

Other than for virgl we don't need additional libraries to be promoted to MAIN.
Those we need for "just" opengl are already in MAIN for Ubuntu Desktop support.
So the change is as easy as adding a few build-deps and setting a config option.

The code is already in place, qemu 2.11 (Bionic) has all it needs for nvidia and 2.12 >=Cosmic has the code for Intel.
That said flipping this build option (no other change) would allow:
- >= 18.10 use vfio MDEVs for gpu's of nvidia and intel
- 18.04 use vfio MDEVs for gpu's of nvidia
- 18.04 + HWE kernel + later Cloud-Archive with qemu >2.12 use vfio MDEVs for intel gpus

I did a bunch of tests and found no related regressions.
And in terms of SRU terms this falls clearly under "enabling new HW/Feature in LTS" exception as the use of vfio-MDEV for gpus just wasn't as clear back then but becomes more and more important now.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Next steps:
- extra testing
- getting it into Disco
- considering SRUs from there

Changed in qemu (Ubuntu Bionic):
status: New → Triaged
Changed in qemu (Ubuntu Cosmic):
status: New → Triaged
Changed in qemu (Ubuntu):
status: New → In Progress
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

As there are currently so many dev/SRU/CVE things are in flight on these packages tests and builds are combined in https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3520/+packages

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I forgot to mention - gl is by default off, you have to enable it on qemu commandline (or through the management software of your choice if it supports setting that already).
But that makes the enabling rather safe in terms of "SRUs should not affect existing users" as it adds a "new, but default-off" feature.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Working PPA version as part of the planned next SRU cycle (post the currently in flight CVE fixes) at [1].

[1]: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3520/+packages

description: updated
description: updated
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Since enabling new build-deps sometimes causes unexpected deps up to component mismatches I checked the builds.

59 Dependencies are in main
5 Dependencies are in universe, but from this SRC
0 Dependencies are new (likely introduced by this build)
1 Dependencies are in universe (component mismatch)
0 packages are of unknown state

This is disco, but other releases look more or less the same.
The one universe package is from qemu-user-binfmt which is a binary built by src:qemu but not in main itself, therefore not a problem.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package qemu - 1:2.12+dfsg-3ubuntu9

---------------
qemu (1:2.12+dfsg-3ubuntu9) disco; urgency=medium

  [ Marc Deslauriers ]
  * SECURITY UPDATE: integer overflow in NE2000 NIC emulation
    - debian/patches/CVE-2018-10839.patch: use proper type in
      hw/net/ne2000.c.
    - CVE-2018-10839
  * SECURITY UPDATE: integer overflow via crafted QMP command
    - debian/patches/CVE-2018-12617.patch: check bytes count read by
      guest-file-read in qga/commands-posix.c.
    - CVE-2018-12617
  * SECURITY UPDATE: OOB heap buffer r/w access in NVM Express Controller
    - debian/patches/CVE-2018-16847.patch: check size in hw/block/nvme.c.
    - CVE-2018-16847
  * SECURITY UPDATE: buffer overflow in rtl8139
    - debian/patches/CVE-2018-17958.patch: use proper type in
      hw/net/rtl8139.c.
    - CVE-2018-17958
  * SECURITY UPDATE: buffer overflow in pcnet
    - debian/patches/CVE-2018-17962.patch: use proper type in
      hw/net/pcnet.c.
    - CVE-2018-17962
  * SECURITY UPDATE: DoS via large packet sizes
    - debian/patches/CVE-2018-17963.patch: check size in net/net.c.
    - CVE-2018-17963
  * SECURITY UPDATE: DoS in lsi53c895a
    - debian/patches/CVE-2018-18849.patch: check message length value is
      valid in hw/scsi/lsi53c895a.c.
    - CVE-2018-18849
  * SECURITY UPDATE: Out-of-bounds r/w stack access in ppc64
    - debian/patches/CVE-2018-18954.patch: check size before data buffer
      access in hw/ppc/pnv_lpc.c.
    - CVE-2018-18954
  * SECURITY UPDATE: race condition in 9p
    - debian/patches/CVE-2018-19364-1.patch: use write lock in
      hw/9pfs/cofile.c.
    - debian/patches/CVE-2018-19364-2.patch: use write lock in
      hw/9pfs/9p.c.
    - CVE-2018-19364

  [ Christian Ehrhardt]
  * debian/patches/ubuntu/lp1787405-*: Support guest dedicated Crypto
    Adapters on s390x (LP: #1787405)
  * enable opengl for vfio-MDEV support (LP: #1804766)
    - d/control-in: set --enable-opengl
    - d/control-in: add gl related build-dependencies

 -- Christian Ehrhardt <email address hidden> Wed, 21 Nov 2018 13:17:01 -0500

Changed in qemu (Ubuntu):
status: In Progress → Fix Released
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

After the current CVEs passed I verified the precheck PPA matches what we expected.

I did another test with the proposed SRU in regard to the opengl changes on UI based workflows.
Ensuring that by default nothing changes (speed/acceleration) and if tools like e.g. virt-manager would get into trouble once the option is supported.

At least for all my tests:
- UI behaves unchanged by default.
- opengl can be activated in UI and commandline

I realized from my notes to this bug we also need libvirt:

Switching gl on needs the apparmor rules that I mentioned in the description.
# For opengl based display options (LP: #1804766)
/dev/dri/ r,
/dev/dri/* r,

Along that as mentioned libvirt-qemu needs to become member of the video group.
sudo usermod -a -G video libvirt-qemu

I also want to check with the security Team how safe they consider this or if we want to make this an "admin has to opt in" thing.

Hmm, lets unbundle that from the SRU unless strictly required and only continue once better testable (HW availability)

I should add a libvirt task for that to cover it as well.

Changed in qemu (Ubuntu Bionic):
importance: Undecided → Wishlist
Changed in qemu (Ubuntu Cosmic):
importance: Undecided → Wishlist
Changed in libvirt (Ubuntu Bionic):
status: New → Triaged
importance: Undecided → Wishlist
Changed in libvirt (Ubuntu Cosmic):
importance: Undecided → Wishlist
status: New → Triaged
Changed in libvirt (Ubuntu):
status: New → Triaged
importance: Undecided → High
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

@ubuntu-security:
would you mind checking and letting me know your opinion on adding the following to /etc/apparmor/abstractions/libvirt-qemu
Note: I have not seen a way to detect in virt-aa-helper if gl is enabled to do it dynamically:

# For opengl based display options (LP: #1804766)
/dev/dri/ r,
/dev/dri/* r,

Furthermore I'd want to make the user we run qemu with be part of the video group (if available).
Like this in the postinst after creating the user:
  $ sudo usermod -a -G video libvirt-qemu

Please let me know if you'd consider that ok - needed for sharing vGPUs of cards as MDEV to the guest. Also down the road required for accelerated virtio-vga once we would support virglrender.

tags: added: libvirt-19.04 qemu-19.04
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

For the security question - amurray pointed out this is in abstractions/X already.
Down the road I might need the EGL line as well.
So if you could also discuss if I should instead of the raw rules use that abstraction that would be nice.

Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (11.2 KiB)

This bug was fixed in the package libvirt - 5.0.0-1ubuntu1

---------------
libvirt (5.0.0-1ubuntu1) disco; urgency=medium

  * Merged with Debian unstable
    Among many other new features and fixes this includes fixes for:
    LP: #1754871 - 1799446 zPCI passthrough support for KVM
    LP: #1811198 - remove arbitrary limit on socket_id/core_id
    Remaining changes:
    - Disable libssh2 support (universe dependency)
    - Disable firewalld support (universe dependency)
    - Set qemu-group to kvm (for compat with older ubuntu)
    - Additional apport package-hook
    - Autostart default bridged network (As upstream does, but not Debian).
      In addition to just enabling it our solution provides:
      + do not autostart if subnet is already taken (e.g. in guests).
      + iterate some alternative subnets before giving up
    - d/p/ubuntu/Allow-libvirt-group-to-access-the-socket.patch: This is
      the group based access to libvirt functions as it was used in Ubuntu
      for quite long.
      + d/p/ubuntu/daemon-augeas-fix-expected.patch fix some related tests
        due to the group access change.
      + d/libvirt-daemon-system.postinst: add users in sudo to the libvirt
        group.
    - ubuntu/parallel-shutdown.patch: set parallel shutdown by default.
    - Update Vcs-Git and Vcs-Browser fields to point to launchpad
    - Xen related
      - d/p/ubuntu/ubuntu-libxl-qemu-path.patch: this change was split. The
        section that adapts the path of the emulator to the Debian/Ubuntu
        packaging is kept.
      - d/p/ubuntu/ubuntu-libxl-Fix-up-VRAM-to-minimum-requirements.patch: auto
        set VRAM to minimum requirements
      - d/p/ubuntu/xen-default-uri.patch: set default URI on xen hosts
      - Add libxl log directory
      - libvirt-uri.sh: Automatically switch default libvirt URI for users on
        Xen dom0 via user profile (was missing on changelogs before)
    - d/p/ubuntu/apibuild-skip-libvirt-common.h: drop libvirt-common.h from
      included_files to avoid build failures due to duplicate definitions.
    - Update README.Debian with Ubuntu changes
    - Enable some additional features on ppc64el and s390x (for arch parity)
      + systemtap, zfs, numa and numad on s390x.
      + systemtap on ppc64el.
    - d/t/control, d/t/smoke-qemu-session: fixup smoke-qemu-session by making
      vmlinuz available and accessible (Debian bug 848314)
    - d/t/control, d/t/smoke-lxc: fix up lxc smoke test isolation
    - d/p/ubuntu/ubuntu_machine_type.patch: accept ubuntu types as pci440fx
    - Further upstreamed apparmor Delta, especially any new one
      Our former delta is split into logical pieces and is either Ubuntu only
      or is part of a continuous upstreaming effort.
      Listing related remaining changes in debian/patches/ubuntu-aa/:
      + 0001-apparmor-Allow-pygrub-to-run-on-Debian-Ubuntu.patch: apparmor:
        Allow pygrub to run on Debian/Ubuntu
      + 0003-apparmor-libvirt-qemu-Allow-read-access-to-overcommi.patch:
        apparmor, libvirt-qemu: Allow read access to overcommit_memory
      + 0007-apparmor-libvirt-qemu-Allow-owner-read-access-to-PRO.patch:
        apparmor, libvirt-qemu: Allow owner rea...

Changed in libvirt (Ubuntu):
status: Triaged → Fix Released
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

We already did the base enablement in bionic/cosmic already (built with opengl and it is usable).
Due to the further kernel (i915) and userspace drivers (nvidia) and MIR (virtglrenderer only in main since 19.04) requirements there isn't too much benefit backporting the further bits that we worked on.
Therefore the further actions to make it more easy to consume (apparmor changes, udev changes, ...) and more stable (plenty of upstream fixes) will not be backported.
Especially the stabilization part for gl/mdevs has too much potential regression for the existing 18.04 users - unless we find a super-use-case that pushes this trade-off more towards "worth it".

Therefore I'm setting the SRU tasks for these "further" actions to Won't Fix.
If one wants to work with that but stay at 18.04 the current thought is to use the Ubuntu Cloud Archive [1] for that.

[1]: https://wiki.ubuntu.com/OpenStack/CloudArchive

Changed in qemu (Ubuntu Cosmic):
status: Triaged → Won't Fix
Changed in qemu (Ubuntu Bionic):
status: Triaged → Won't Fix
Changed in libvirt (Ubuntu Cosmic):
status: Triaged → Won't Fix
Changed in libvirt (Ubuntu Bionic):
status: Triaged → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.