Re: [PATCH v7 03/15] s390/vfio-ap: Functions to initialize/release vfio device migration data

["Jason J. Herne" <jjherne@linux.ibm.com>]

On 8/7/26 6:18 PM, Anthony Krowiak wrote:
> Provides the functions that initialize and release the data structures
> used during live guest migration:
> 
> ...
> 
> Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> ---
>   drivers/s390/crypto/vfio_ap_migration.c | 127 ++++++++++++++++++++++++
>   drivers/s390/crypto/vfio_ap_ops.c       |  64 ++++++++++--
>   drivers/s390/crypto/vfio_ap_private.h   |   4 +
>   3 files changed, 187 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/s390/crypto/vfio_ap_migration.c b/drivers/s390/crypto/vfio_ap_migration.c
> index 374d3a67cb21..cf303d228a0a 100644
> --- a/drivers/s390/crypto/vfio_ap_migration.c
> +++ b/drivers/s390/crypto/vfio_ap_migration.c
> @@ -4,6 +4,7 @@
>    *
>    * Copyright IBM Corp. 2025
>    */
> +#include <linux/file.h>
>   #include "vfio_ap_private.h"
>   
>   /* Magic number and version for the vfio_ap_config migration blob */
> @@ -111,3 +112,129 @@ struct vfio_ap_config {
>   	u64				adm[DIV_ROUND_UP(AP_DOMAINS, 64)];
>   	struct vfio_ap_queue_info	qinfo[] __counted_by(num_queues);
>   };
> +
> +static void
> +vfio_ap_release_stop_copy_file(struct vfio_ap_migration_data *mig_data)
> +{
> +	/* Stub to be implemented when the mig_data->stop_copy_mig_file.ap_config
> +	 * object is allocated.
> +	 */
> +}
> +
> +static void vfio_ap_release_resuming_file(struct vfio_ap_migration_data *mig_data)
> +{
> +	/* Stub to be implemented when the mig_data->resuming_mig_file.ap_config
> +	 * object is allocated.
> +	 */
> +}
> +
> +static struct file *vfio_ap_set_state(struct vfio_device *vdev,
> +				      enum vfio_device_mig_state  new_state)
> +{
> +	return NULL;
> +}
> +
> +static int vfio_ap_get_state(struct vfio_device *vdev,
> +			     enum vfio_device_mig_state  *current_state)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static int vfio_ap_get_data_size(struct vfio_device *vdev,
> +				 unsigned long *stop_copy_length)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static const struct vfio_migration_ops vfio_ap_migration_ops = {
> +	.migration_set_state = vfio_ap_set_state,
> +	.migration_get_state = vfio_ap_get_state,
> +	.migration_get_data_size = vfio_ap_get_data_size,
> +};
> +
> +/**
> + * vfio_ap_init_migrations_capabilities - initialize migration capabilities

typo: vfio_ap_init_migration s _capabilities

> + *
> + * @matrix_mdev: pointer to object containing the mdev state
> + */
> +void vfio_ap_init_migration_capabilities(struct ap_matrix_mdev *matrix_mdev)
> +{
> +	if (ap_is_se_guest())
> +		return;
> +
> +	matrix_mdev->vdev.migration_flags = VFIO_MIGRATION_STOP_COPY;
> +	matrix_mdev->vdev.mig_ops = &vfio_ap_migration_ops;
> +}
> +
> +/**
> + * vfio_ap_init_migration_data - initialize migration data and functions
> + *
> + * @matrix_mdev: pointer to object containing the mdev state
> + *
> + * Return: zero if initialization is successful; otherwise, returns a error.
> + */
> +int vfio_ap_init_migration_data(struct ap_matrix_mdev *matrix_mdev)
> +{
> +	struct vfio_ap_migration_data *mig_data;
> +
> +	lockdep_assert_held(&matrix_dev->mdevs_lock);
> +
> +	mig_data = kzalloc_obj(struct vfio_ap_migration_data, GFP_KERNEL);
> +	if (!mig_data)
> +		return -ENOMEM;
> +
> +	mig_data->mig_state = VFIO_DEVICE_STATE_RUNNING;
> +	matrix_mdev->mig_data = mig_data;
> +
> +	return 0;
> +}
> +
> +/**
> + * vfio_ap_release_mig_files:
> + *
> + * Free the ap_config buffers for any open migration FDs. Although a
> + * migration FD may still be held open by userspace, it is safe to free
> + * mig_data here because:
> + *
> + *   1. matrix_mdev remains valid for the lifetime of any open migration
> + *      FD via the vfio_device registration reference taken in
> + *      vfio_ap_open_file_stream() and dropped in
> + *      vfio_ap_release_mig_file().
> + *
> + *   2. mig_data is only accessed by the migration file ops
> + *      (vfio_ap_stop_copy_read, vfio_ap_resuming_write) under
> + *      mdevs_lock. Once mig_data is set to NULL by the caller, those
> + *      paths will see NULL and return -ENODEV before dereferencing it.
> + *
> + * @matrix_mdev: The object used to maintain the state for a mediated device
> + */
> +static void vfio_ap_release_mig_files(struct ap_matrix_mdev *matrix_mdev)
> +{
> +	struct vfio_ap_migration_data *mig_data;
> +
> +	lockdep_assert_held(&matrix_dev->mdevs_lock);
> +
> +	mig_data = matrix_mdev->mig_data;
> +	if (!mig_data)
> +		return;

Both callers already perform the above check under lock. This one seems 
redundant.

> +	vfio_ap_release_stop_copy_file(mig_data);
> +	vfio_ap_release_resuming_file(mig_data);
> +}
> +
> +/**
> + * vfio_ap_release_migration_data: reclaim private migration data
> + *
> + * @vdev: pointer to the mdev
> + */