[UBUNTU 21.04] qemu s390x/pci: Honor vfio DMA limiting

Bug #1913395 reported by bugproxy
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ubuntu on IBM z Systems
Fix Released
High
Skipper Bug Screeners
qemu (Ubuntu)
Fix Released
High
Canonical Server
Focal
Fix Released
High
Unassigned
Groovy
Fix Released
High
Unassigned

Bug Description

[Impact]

* In case a vfio-pci device on s390x is under I/O load, vfio-pci device
  may end up in error state.

* However, lazy unmapping in s390x can in fact cause quite a large number
  of outstanding DMA requests to build up prior to being purged -
  potentially the entire guest DMA space.

* This results in unexpected errors seen in qemu such as 'VFIO_MAP_DMA
  failed: No space left on device'.

* The solution requires a change to both kernel and qemu.

* The qemu side of things is addressed by this SRU.

[Fix]

* A patch series that utilizes the recent kernel additions. It will check the limits and refresh mappings before being exceeded

[Test Case]

* IBM Z or LinuxONE hardware with Ubuntu Server 20.10 installed.

* PCIe adapters in place that provide vfio, like RoCE Express 2.

* A KVM host needs to be setup and a KVM guest (use again 20.10) that uses vfio.

* Generate I/O that flows through the vf and watch out for error like 'VFIO_MAP_DMA failed: No space left on device' in the log.

* We don't have all of that in place, IBM (has done on the related bug as well) will do these tests.

[Regression Potential]

* This is split in two.
  - generally the reworks - albeit small - for vfio could affect all
    platforms so there I'd expect issues - if any - in vfio use-cases like
    device pass through
  - on s390x there was more changed, but the regressions we need to look
    out for would still be in the same "vfio used for pass through"
    use-case area

[Other]

* The kernel portion got accepted in bug 1907421

---

Description: s390x/pci: Honor vfio DMA limiting
Symptom: vfio-pci device on s390 enters error state
Problem: Kernel commit 492855939bdb added a limit to the number of
               concurrent DMA requests for a vfio container. However, lazy
               unmapping in s390 can in fact cause quite a large number of
               outstanding DMA requests to build up prior to being purged,
               potentially the entire guest DMA space. This results in
               unexpected errors seen in qemu such as 'VFIO_MAP_DMA failed:
               No space left on device'
Solution: The solution requires a change to both kernel and qemu - For
               qemu, add functionality to get the number of allowable DMA
               DMA requests via the VFIO_IOMMU_GET_INFO ioctl and then ensure
               that the guest is told to refresh mappings before exceeding
               the vfio limit.
Reproduction: Put a vfio-pci device on s390 under I/O load

This QEMU issue is related to the kernel issue in launchpad bug #1907421. Backport patches have been attached for a subset of the required patches for this fix... The backports required boiled down to 3 major reasons:
1) For the header sync, I suspect you only want the minimal set of changes needed
2) There is a missing upstream commit (408b55db8be3) that re-organizes the location of 2 s390-pci header files, causing conflicts
3) Adjustments had to be made due to the QEMU build system change (meson)

I initially performed the backport against 4.2/focal-devel; the same patches and process will also apply cleanly to 5.0/groovy-devel. There should be nothing required for hirsute as everything is already in upstream QEMU 5.2.

In summary:
53ba2eee52bf: Backport as patch 0001. Rather than doing a full header sync, update ONLY the header change needed for the DMA fix. See attached patch 0001.
3ab7a0b40d4b: cherry-pick works
7486a62845b1: cherry-pick works
cd7498d07fbb: Backport as patch 0004. This upstream commit added a new part using meson, which does not exist in 5.0.
37fa32de7073: Backport as patch 0005. This was mainly due to conflicts with a missing patch that relocated some include files.
77280d33bc9c: Backport as patch 0006. This was due to different build system + CONFIG_DEVICES doesn't exist.

As such, I have attached patches 0001, 0004, 0005 and 0006. Please cherry pick for patches 0002 and 0003.

To verify, I applied the patches provided and cherry-picks against both focal-devel and groovy-devel. In each case, for the host system I used the groovy kernel Frank provided in launchpad bug #1907421 which includes the kernel portion of this fix -- using these together, I verified that the DMA limit is being read in and honored appropriately by QEMU, and I can no longer trigger an overrun of the DMA space when a guest pushes heavy data transfer via PCI (no errors in log, no transfer stalls).

Also, as related to the last patch of the set, I further verified that no build errors are encountered when configured with --without-default-devices.

Related branches

CVE References

Revision history for this message
bugproxy (bugproxy) wrote : 0001 - Backport of 53ba2eee52bf

Default Comment by Bridge

tags: added: architecture-s39064 bugnameltc-190223 severity-high targetmilestone-inin---
Revision history for this message
bugproxy (bugproxy) wrote : 0004 - Backport of cd7498d07fbb

Default Comment by Bridge

Revision history for this message
bugproxy (bugproxy) wrote : 0005 - Backport of 37fa32de7073

Default Comment by Bridge

Revision history for this message
bugproxy (bugproxy) wrote : 0006 - Backport of 77280d33bc9c

Default Comment by Bridge

Changed in ubuntu:
assignee: nobody → Skipper Bug Screeners (skipper-screen-team)
affects: ubuntu → linux (Ubuntu)
Frank Heimes (fheimes)
Changed in ubuntu-z-systems:
assignee: nobody → Skipper Bug Screeners (skipper-screen-team)
Changed in linux (Ubuntu):
assignee: Skipper Bug Screeners (skipper-screen-team) → nobody
Changed in ubuntu-z-systems:
importance: Undecided → High
Revision history for this message
Frank Heimes (fheimes) wrote :

Removing kernel again as affected component, since the kernel side of this issue is already handled in LP 1907421 - https://bugs.launchpad.net/bugs/1907421

no longer affects: linux (Ubuntu)
Changed in qemu (Ubuntu):
assignee: nobody → Canonical Server Team (canonical-server)
tags: added: qemu-21.04
Changed in qemu (Ubuntu):
status: New → Fix Released
Changed in qemu (Ubuntu Focal):
status: New → Triaged
Changed in qemu (Ubuntu Groovy):
status: New → Triaged
Frank Heimes (fheimes)
Changed in ubuntu-z-systems:
status: New → Triaged
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thank you for all the prep work Matthew!
I've:
- imported the patches successfully to Focal.
- updated the headers/filenames/... to match the usual packaging style and SRU prereqs.
- reviewed the changes (LGTM, #5 is quite complex thou)
- all changes are upstream
- backport changes LGTM
- For Groovy they had a bit of fuzz, but applied after minimal changes
- Kicked off PPA builds of those at https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/4409/+packages
- opened MPs for those packaging changes
  - https://code.launchpad.net/~paelzer/ubuntu/+source/qemu/+git/qemu/+merge/397012
  - https://code.launchpad.net/~paelzer/ubuntu/+source/qemu/+git/qemu/+merge/397013

Let this pass build and some test and we will see to kick this to an SRU.
As you know from the past we will also need an SRU template [1] for that I'm good with everything except test steps - if you could help out with those that would be great.

[1]: https://wiki.ubuntu.com/StableReleaseUpdates#SRU_Bug_Template

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

Frank suggested (and was right) that we can mostly take the bug 1907421 SRu content and copy it over. I've done so and think we are good no that.

description: updated
Revision history for this message
bugproxy (bugproxy) wrote : Comment bridged from LTC Bugzilla

------- Comment From <email address hidden> 2021-01-29 08:44 EDT-------
Hi, I see a failure to build on armhf for groovy... I think it might have been an out of disk space error? Should I still go ahead and test with the successfully built s390x?

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

Yeah, you can ignore armhf@groovy for now and continue checking the s390x builds of it.

Revision history for this message
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2021-02-01 11:04 EDT-------
Thanks Christian.

I ran tests on both focal and groovy using the linked qemu PPA builds on s390x with a vfio-pci passthrough mlx adapter. Since the kernel fix hasn't hit focal yet, for both scenarios I used the patched groovy kernel that Frank provided in 1907421 to test the full fix (kernel advertisement of the DMA limit, which the modified QEMU can then exploit to ensure the limit isn't overrun).

For testing, I forced data-transfer workloads that could, prior to this fix, quickly and reliably overrun the DMA space - I left these workloads run for about 30 minutes each, verifying that were no transfer stalls / no errors in the QEMU log. Looks good!

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

Ready, but for an SRU to be combined with bug 1913266 - due to that some extra delay.

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

Ok that other bug was filed wrong (against qemu), it is for libvirt and therefore will not be bundled. But there are a few other things that might (depending on completion) join this SRU (or not). Also at least the former groovy SRU that is ongoing needs some extra time to complete as various cross arch toolsets had some issues with the qemu-static changes.

tags: added: server-next
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Also there will be another round of security updates before these, so I beg your pardon for some delay.

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

Uploaded to Focal/Groovy unapproved fro consideration by the SRU Team.

Changed in qemu (Ubuntu Focal):
status: Triaged → In Progress
Changed in qemu (Ubuntu Groovy):
status: Triaged → In Progress
Frank Heimes (fheimes)
Changed in ubuntu-z-systems:
status: Triaged → In Progress
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

This is about to be accepted into -proposed following the SRU process.
That is good and we want to do it that way, especially for joint test and verification.
But we have a follow on upload that will go to -security afterwards, so we will "release" only one to save users one extra upgrade.
Therefore I'm adding a block-proposed tag to this bug.

tags: added: block-proposed block-proposed-focal block-proposed-groovy
Revision history for this message
Robie Basak (racb) wrote : Please test proposed package

Hello bugproxy, or anyone else affected,

Accepted qemu into groovy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/qemu/1:5.0-5ubuntu9.5 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-groovy to verification-done-groovy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-groovy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in qemu (Ubuntu Groovy):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-groovy
Changed in qemu (Ubuntu Focal):
status: In Progress → Fix Committed
tags: added: verification-needed-focal
Revision history for this message
Robie Basak (racb) wrote :

Hello bugproxy, or anyone else affected,

Accepted qemu into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/qemu/1:4.2-3ubuntu6.13 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Frank Heimes (fheimes)
Changed in ubuntu-z-systems:
status: In Progress → Fix Committed
Revision history for this message
Ubuntu SRU Bot (ubuntu-sru-bot) wrote : Autopkgtest regression report (qemu/1:4.2-3ubuntu6.13)

All autopkgtests for the newly accepted qemu (1:4.2-3ubuntu6.13) for focal have finished running.
The following regressions have been reported in tests triggered by the package:

edk2/0~20191122.bd85bf54-2ubuntu3.1 (amd64)
open-iscsi/2.0.874-7.1ubuntu6.1 (amd64)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/focal/update_excuses.html#qemu

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

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

@IBM please verify those builds on Focal and Groovy within a SE environment and let us know about the results.

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

FYI - autopkgtest issues in Focal resolved

Revision history for this message
bugproxy (bugproxy) wrote : Comment bridged from LTC Bugzilla

------- Comment From <email address hidden> 2021-02-11 08:39 EDT-------
(In reply to comment #30)
> @IBM please verify those builds on Focal and Groovy within a SE environment
> and let us know about the results.

Hi Christian, passthrough is not supported in an SE environment. Therefore I can't test that the reported issue for this bug is fixed within an SE environment, only in a non-SE environment with PCI available.

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

Sorry mjrosato, I was mixing up this with another in flight SRU. Indeed testing in "the normal" environment is what we need here.

Revision history for this message
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2021-02-11 09:37 EDT-------
No problem, I suspected this was the case but wanted to be sure.

So, I've run verification on both focal and groovy using the qemu available in -proposed. With the caveat that for focal I used Frank's kernel build provided in 1907421 as we're still waiting for the backport from upstream stable on that for focal.

On both focal and groovy, I created a guest with a vfio-pci passthrough mlx device on s390x and forced data-transfer workloads that could, prior to this fix, quickly and reliably overrun the DMA space and result in transfer stalls / QEMU error messages. No more errors -- looks good!

tags: added: verification-done-focal verification-done-groovy
removed: verification-needed-focal verification-needed-groovy
bugproxy (bugproxy)
tags: added: targetmilestone-inin2104
removed: targetmilestone-inin---
Mathew Hodson (mhodson)
Changed in qemu (Ubuntu):
importance: Undecided → High
Changed in qemu (Ubuntu Focal):
importance: Undecided → High
Changed in qemu (Ubuntu Groovy):
importance: Undecided → High
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package qemu - 1:5.0-5ubuntu9.6

---------------
qemu (1:5.0-5ubuntu9.6) groovy-security; urgency=medium

  * SECURITY REGRESSION: fix multiple regressions caused by CVE-2020-13754
    security update (LP: #1914883)
    - debian/patches/ubuntu/CVE-2020-13754-3.patch: log invalid memory
      accesses in memory.c.
    - debian/patches/ubuntu/CVE-2020-13754-4.patch: allow 16-bit writes to
      memory region in hw/riscv/sifive_test.c.
    - debian/patches/ubuntu/CVE-2020-13754-5.patch: allow 64-bit accesses
      in hw/timer/slavio_timer.c.
    - debian/patches/ubuntu/CVE-2020-13754-6.patch: allow less than 32-bit
      accesses in hw/char/bcm2835_aux.c.
    - debian/patches/ubuntu/CVE-2020-13754-7.patch: unbreak size mismatch
      memory accesses in hw/display/artist.c.

 -- Marc Deslauriers <email address hidden> Wed, 10 Feb 2021 08:10:20 -0500

Changed in qemu (Ubuntu Groovy):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package qemu - 1:4.2-3ubuntu6.14

---------------
qemu (1:4.2-3ubuntu6.14) focal-security; urgency=medium

  * SECURITY REGRESSION: fix multiple regressions caused by CVE-2020-13754
    security update (LP: #1914883)
    - debian/patches/ubuntu/CVE-2020-13754-3.patch: log invalid memory
      accesses in memory.c.
    - debian/patches/ubuntu/CVE-2020-13754-4.patch: allow 16-bit writes to
      memory region in hw/riscv/sifive_test.c.
    - debian/patches/ubuntu/CVE-2020-13754-5.patch: allow 64-bit accesses
      in hw/timer/slavio_timer.c.
    - debian/patches/ubuntu/CVE-2020-13754-6.patch: allow less than 32-bit
      accesses in hw/char/bcm2835_aux.c.
    - debian/patches/ubuntu/CVE-2020-13754-9.patch: fix
      valid.max_access_size to access address registers in
      hw/usb/hcd-xhci.c.

 -- Marc Deslauriers <email address hidden> Wed, 10 Feb 2021 08:17:08 -0500

Changed in qemu (Ubuntu Focal):
status: Fix Committed → Fix Released
Frank Heimes (fheimes)
Changed in ubuntu-z-systems:
status: Fix Committed → Fix Released
Revision history for this message
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2021-02-23 08:51 EDT-------
IBM Bugzilla status->closed, Fix Released with all requested distros.

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.