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

[Peter Maydell]

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
-- PMM