ACK: [SRU][G][PATCH 0/2] vfio: pass DMA availability information to userspace (LP: 1907421)

[Ian May]

Patches look good, apply clean and build tested

Acked-by: Ian May <ian.may at canonical.com>

On 2021-01-20 21:04:36 , frank.heimes at canonical.com wrote:
> BugLink: https://bugs.launchpad.net/bugs/1907421
> 
> SRU Justification:
> 
> [Impact]
> 
> * In case a vfio-pci device on s390x is under I/O load, vfio-pci device may end up in error state.
> 
> * The commit 492855939bdb added a limit to the number of concurrent DMA requests for a vfio container.
> 
> * 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 kernel side of things is addressed by this SRU.
> 
> * The fix adds the ability to provide the number of allowable DMA requests via VFIO_IOMMU_GET_INFO ioctl.
> 
> * The actual fix comes with commit 7d6e1329652e, but another fix ccd59dce1a21 is needed to get it cleanly applied.
> 
> [Fix]
> 
> * ccd59dce1a21f473518bf273bdf5b182bab955b3 ccd59dce1a21 "vfio/type1: Refactor vfio_iommu_type1_ioctl()"
> 
> * 7d6e1329652ed971d1b6e0e7bea66fba5044e271 7d6e1329652e "vfio iommu: Add dma available capability"
> 
> [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.
> 
> [Regression Potential]
> 
> * The first patch ccd59dce1a21 modifies the common code file drivers/vfio/vfio_iommu_type1.c quity significantly.
> 
> * But the reason is not that it introduces a lot of new things, it's a refactoring patch.
> 
> * Nevertheless if done in a bad way it can significantly harm the IO memory management of virtual function adapters.
> 
> * In worst case it may break them entirely, instead of 'just' exeeding the entire DMA space.
> 
> * Things could also go wrong while doing the mapping and unmapping of DMA, that may even have an impact beyond vf adapters - harming other DMA devices.
> 
> * The handling of dirty pages is also touched and the ioctl itself - which is important to keep the control of the devices.
> 
> * But as said before, it re-factoring work, it's upstream accepted since 5.9 and the provenance shows that many engineers had an eye on these changes.
> 
> * The second patch 7d6e1329652e - that inclides the needed fix - comes with far less modifications.
> 
> * It also tounches drivers/vfio/vfio_iommu_type1.c but in a way that it mainly
>   adds one new function (vfio_iommu_dma_avail_build_caps) and an add. if statement in vfio_iommu_type1_get_info.
> 
> * That allows to provide the current allowable number of DMA mappings to the userspace via the IOMMU info chain,
>   so that the userspace can take appropriate mitigation.
> 
> * Potential problems can be that the current allowable number of DMA mappings are wrong
>   and in best case just mappings are wasted
>   and in worst case there are more reported than available in reality, which could have a severe impact.
> 
> * What happens in such a case is a bit depending on the userspace.
> 
> * This patch got upstream accepted with kernel 5.10 and pre-tested by IBM.
> 
> * In addition a PPA was created with a patched groovy kernel that was shared for further testing.
> 
> [Other]
> 
> * The patch got upstream accepted with kernel v5.10, hence it will land in Hirsute once the target kernel 5.10 is in.
> 
> * For 5.4 this will come as upstream stable update 5.4.90/91.
> 
> * For 5.8 (and 5.9) upstream stable support already ended, hence this SRU is only needed for groovy.
> 
> Liu Yi L (1):
>   vfio/type1: Refactor vfio_iommu_type1_ioctl()
> 
> Matthew Rosato (1):
>   vfio iommu: Add dma available capability
> 
>  drivers/vfio/vfio_iommu_type1.c | 409 ++++++++++++++++++--------------
>  include/uapi/linux/vfio.h       |  15 ++
>  2 files changed, 244 insertions(+), 180 deletions(-)
> 
> -- 
> 2.25.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team