On Mon, 20 Aug 2018 15:23:35 +0100
Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 17 August 2018 at 18:08, Alex Williamson <alex.william...@redhat.com>
> wrote:
> > If a vfio assigned device makes use of a physical IOMMU, then memory
> > ballooning is necessarily inhibited due to the page pinning, lack of
> > page level granularity at the IOMMU, and sufficient notifiers to both
> > remove the page on balloon inflation and add it back on deflation.
> > However, not all devices are backed by a physical IOMMU. In the case
> > of mediated devices, if a vendor driver is well synchronized with the
> > guest driver, such that only pages actively used by the guest driver
> > are pinned by the host mdev vendor driver, then there should be no
> > overlap between pages available for the balloon driver and pages
> > actively in use by the device. Under these conditions, ballooning
> > should be safe.
> >
> > vfio-ccw devices are always mediated devices and always operate under
> > the constraints above. Therefore we can consider all vfio-ccw devices
> > as balloon compatible.
> >
> > The situation is far from straightforward with vfio-pci. These
> > devices can be physical devices with physical IOMMU backing or
> > mediated devices where it is unknown whether a physical IOMMU is in
> > use or whether the vendor driver is well synchronized to the working
> > set of the guest driver. The safest approach is therefore to assume
> > all vfio-pci devices are incompatible with ballooning, but allow user
> > opt-in should they have further insight into mediated devices.
> >
> > Signed-off-by: Alex Williamson <alex.william...@redhat.com>
>
> > @@ -2869,6 +2870,27 @@ static void vfio_realize(PCIDevice *pdev, Error
> > **errp)
> > }
> > }
> >
> > + /*
> > + * Mediated devices *might* operate compatibly with memory ballooning,
> > but
> > + * we cannot know for certain, it depends on whether the mdev vendor
> > driver
> > + * stays in sync with the active working set of the guest driver.
> > Prevent
> > + * the x-balloon-allowed option unless this is minimally an mdev
> > device.
> > + */
> > + tmp = g_strdup_printf("%s/subsystem", vdev->vbasedev.sysfsdev);
> > + subsys = realpath(tmp, NULL);
> > + g_free(tmp);
> > + is_mdev = (strcmp(subsys, "/sys/bus/mdev") == 0);
> > + free(subsys);
>
> Hi -- Coverity points out that we aren't checking the return
> value from realpath() here. It returns NULL on failure, in
> which case we'll pass NULL into strcmp() and segfault.
> (CID 1395101).
Thanks, patch posted. The fix is trivial, you're welcome to pull it in
directly, otherwise I'll send a pull request if qemu-trivial doesn't
get to it first. Thanks,
Alex
