Re: [PATCH v7 04/15] s390/vfio-ap: Reset migration state in VFIO_DEVICE_RESET ioctl handler

On 8/7/26 6:18 PM, Anthony Krowiak wrote:
> According to the VFIO migration specifications, userspace must issue a
> VFIO_DEVICE_RESET ioctl if a vfio device enters an error state or fails a
> state transition to recover the device and return its migration state to
> RUNNING. This patch adds a function that is called when a VFIO_DEVICE_RESET
> is issued that releases the migration files and frees the storage allocated
> for maintaining the migration state.
> 
> Signed-off-by: Anthony Krowiak <[email protected]>
> ---
>   drivers/s390/crypto/vfio_ap_migration.c | 20 ++++++++++++++++++++
>   drivers/s390/crypto/vfio_ap_ops.c       |  4 ++++
>   drivers/s390/crypto/vfio_ap_private.h   |  1 +
>   3 files changed, 25 insertions(+)
> 
> diff --git a/drivers/s390/crypto/vfio_ap_migration.c b/drivers/s390/crypto/vfio_ap_migration.c
> index cf303d228a0a..1613813124b6 100644
> --- a/drivers/s390/crypto/vfio_ap_migration.c
> +++ b/drivers/s390/crypto/vfio_ap_migration.c
> @@ -238,3 +238,23 @@ void vfio_ap_release_migration_data(struct ap_matrix_mdev *matrix_mdev)
>   	kfree(matrix_mdev->mig_data);
>   	matrix_mdev->mig_data = NULL;
>   }
> +
> +/**
> + * vfio_ap_reset_migration_state - Reset the vfio-ap migration state
> + *
> + * @matrix_mdev: pointer to the object maintaining the vfio-ap device state
> + *
> + * Called during VFIO_DEVICE_RESET to clean up any active migration
> + * state and reset the device to RUNNING state as required by the VFIO
> + * migration specification.
> + */
> +void vfio_ap_reset_migration_state(struct ap_matrix_mdev *matrix_mdev)
> +{
> +	lockdep_assert_held(&matrix_dev->mdevs_lock);
> +
> +	if (!matrix_mdev->mig_data)
> +		return;
> +
> +	vfio_ap_release_mig_files(matrix_mdev);
> +	matrix_mdev->mig_data->mig_state = VFIO_DEVICE_STATE_RUNNING;
> +}
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 90b0fce0123b..d05372b50d2f 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -2330,6 +2330,10 @@ static ssize_t vfio_ap_mdev_ioctl(struct vfio_device *vdev,
>   		break;
>   	case VFIO_DEVICE_RESET:
>   		ret = vfio_ap_mdev_reset_queues(matrix_mdev);
> +		if (ret == 0) {
> +			/* Reset migration state per VFIO migration spec */
> +			vfio_ap_reset_migration_state(matrix_mdev);
> +		}
>   		break;

Why do we only reset migration state when vfio_ap_mdev() returns 0? Even 
if this call fails, we are in the VFIO_DEVICE_RESET path and we need to 
clean up the migration data, right? Or did I miss something?


>   	case VFIO_DEVICE_GET_IRQ_INFO:
>   		ret = vfio_ap_get_irq_info(arg);
> diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h
> index a2a713f93674..1fbdfcce5a11 100644
> --- a/drivers/s390/crypto/vfio_ap_private.h
> +++ b/drivers/s390/crypto/vfio_ap_private.h
> @@ -175,5 +175,6 @@ void vfio_ap_on_scan_complete(struct ap_config_info *new_config_info,
>   void vfio_ap_init_migration_capabilities(struct ap_matrix_mdev *matrix_mdev);
>   int vfio_ap_init_migration_data(struct ap_matrix_mdev *matrix_mdev);
>   void vfio_ap_release_migration_data(struct ap_matrix_mdev *matrix_mdev);
> +void vfio_ap_reset_migration_state(struct ap_matrix_mdev *matrix_mdev);
>   
>   #endif /* _VFIO_AP_PRIVATE_H_ */