Tian, Kevin
2022-Apr-06 07:02 UTC
[PATCH 2/5] vfio: Require that devices support DMA cache coherence
> From: Jason Gunthorpe <jgg at nvidia.com> > Sent: Wednesday, April 6, 2022 3:29 AM > > On Tue, Apr 05, 2022 at 01:10:44PM -0600, Alex Williamson wrote: > > On Tue, 5 Apr 2022 13:16:01 -0300 > > Jason Gunthorpe <jgg at nvidia.com> wrote: > > > > > dev_is_dma_coherent() is the control to determine if IOMMU_CACHE can > be > > > supported. > > > > > > IOMMU_CACHE means that normal DMAs do not require any additional > coherency > > > mechanism and is the basic uAPI that VFIO exposes to userspace. For > > > instance VFIO applications like DPDK will not work if additional coherency > > > operations are required. > > > > > > Therefore check dev_is_dma_coherent() before allowing a device to join > a > > > domain. This will block device/platform/iommu combinations from using > VFIO > > > that do not support cache coherent DMA. > > > > > > Signed-off-by: Jason Gunthorpe <jgg at nvidia.com> > > > drivers/vfio/vfio.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c > > > index a4555014bd1e72..2a3aa3e742d943 100644 > > > +++ b/drivers/vfio/vfio.c > > > @@ -32,6 +32,7 @@ > > > #include <linux/vfio.h> > > > #include <linux/wait.h> > > > #include <linux/sched/signal.h> > > > +#include <linux/dma-map-ops.h> > > > #include "vfio.h" > > > > > > #define DRIVER_VERSION "0.3" > > > @@ -1348,6 +1349,11 @@ static int vfio_group_get_device_fd(struct > vfio_group *group, char *buf) > > > if (IS_ERR(device)) > > > return PTR_ERR(device); > > > > > > + if (group->type == VFIO_IOMMU && !dev_is_dma_coherent(device- > >dev)) { > > > + ret = -ENODEV; > > > + goto err_device_put; > > > + } > > > + > > > > Failing at the point where the user is trying to gain access to the > > device seems a little late in the process and opaque, wouldn't we > > rather have vfio bus drivers fail to probe such devices? I'd expect > > this to occur in the vfio_register_group_dev() path. Thanks, > > Yes, that is a good point. > > So like this: > > int vfio_register_group_dev(struct vfio_device *device) > { > + if (!dev_is_dma_coherent(device->dev)) > + return -EINVAL; > + > return __vfio_register_dev(device, > vfio_group_find_or_alloc(device->dev)); > } > > I fixed it up. >if that is the case should it also apply to usnic and vdpa in the first patch (i.e. fail the probe)?