Re: [Qemu-devel] [PULL 4/4] vfio/ccw/pci: Allow devices to opt-in for ballooning

[Alex Williamson]

On Mon, 20 Aug 2018 15:23:35 +0100
Peter Maydell <address@hidden> wrote:

> On 17 August 2018 at 18:08, Alex Williamson <address@hidden> 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 <address@hidden>  
> 
> > @@ -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