Re: [PATCH V6 vfio 05/10] vfio: Introduce the DMA logging feature support

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 5 Sep 2022 13:58:47 +0300
Yishai Hadas <yishaih@nvidia.com> wrote:
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index e05ddc6fe6a5..b17f2f454389 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -108,6 +110,21 @@ struct vfio_migration_ops {
>  				   enum vfio_device_mig_state *curr_state);
>  };
>  
> +/**
> + * @log_start: Optional callback to ask the device start DMA logging.
> + * @log_stop: Optional callback to ask the device stop DMA logging.
> + * @log_read_and_clear: Optional callback to ask the device read
> + *         and clear the dirty DMAs in some given range.

I don't see anywhere in the core that we track the device state
relative to the DEVICE_FEATURE_DMA_LOGGING features, nor do we
explicitly put the responsibility on the driver implementation to
handle invalid user requests.  The mlx5 driver implementation appears
to do this, but maybe we should at least include a requirement here, ex.

   The vfio core implementation of the DEVICE_FEATURE_DMA_LOGGING_ set
   of features does not track logging state relative to the device,
   therefore the device implementation of vfio_log_ops must handle
   arbitrary user requests.  This includes rejecting subsequent calls
   to log_start without an intervening log_stop, as well as graceful
   handling of log_stop and log_read_and_clear from invalid states.

With something like that.

Acked-by: Alex Williamson <alex.williamson@redhat.com>

You can also add my Ack on 3, 4, and (fwiw) 6-10 as I assume this would
be a PR from Leon.  Thanks,

Alex

> + */
> +struct vfio_log_ops {
> +	int (*log_start)(struct vfio_device *device,
> +		struct rb_root_cached *ranges, u32 nnodes, u64 *page_size);
> +	int (*log_stop)(struct vfio_device *device);
> +	int (*log_read_and_clear)(struct vfio_device *device,
> +		unsigned long iova, unsigned long length,
> +		struct iova_bitmap *dirty);
> +};
> +
>  /**
>   * vfio_check_feature - Validate user input for the VFIO_DEVICE_FEATURE ioctl
>   * @flags: Arg from the device_feature op