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

[Anthony Krowiak <akrowiak@linux.ibm.com>]

On 8/11/26 9:57 AM, Jason J. Herne wrote:
> 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

I will fix that

>
>> + *
>> + * @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.

That is true, will fix.

>
>> + 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
>> + */