Re: [PATCH v6 13/15] s390/vfio-ap: Callback to get the size of data to be migrated during guest migration

Anthony Krowiak <[email protected]> Thu, 30 Jul 2026 07:26:08 -0400
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.kvm,org.kernel.vger.linux-s390
Message-ID <[email protected]>

On 7/27/26 2:19 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Logic duplication for calculating the configuration size.
> --
>
> --- Patch [13]: [PATCH v6 13/15] s390/vfio-ap: Callback to get the size of data to be migrated during guest migration ---
> commit f0e78a002713b19ca67967cef71fbd73dea59ee7
> Author: Anthony Krowiak <[email protected]>
>
> s390/vfio-ap: Callback to get the size of data to be migrated during guest migration
>
> This commit adds the mandatory migration_get_data_size callback for VFIO
> migration support on the s390 vfio_ap driver. The callback calculates the
> estimated data length required to migrate the device's internal state.
>
>> diff --git a/drivers/s390/crypto/vfio_ap_migration.c b/drivers/s390/crypto/vfio_ap_migration.c
>> index 1c385908fc39b..8379cf8bbce89 100644
>> --- a/drivers/s390/crypto/vfio_ap_migration.c
>> +++ b/drivers/s390/crypto/vfio_ap_migration.c
>> @@ -1392,7 +1392,18 @@ static int vfio_ap_get_state(struct vfio_device *vdev,
>>   static int vfio_ap_get_data_size(struct vfio_device *vdev,
>>   				 unsigned long *stop_copy_length)
>>   {
>> -	return -EOPNOTSUPP;
>> +	struct ap_matrix_mdev *matrix_mdev;
>> +	size_t qinfo_sz;
>> +	int num_queues;
>> +
>> +	mutex_lock(&matrix_dev->mdevs_lock);
>> +	matrix_mdev = container_of(vdev, struct ap_matrix_mdev, vdev);
>> +	num_queues = vfio_ap_mdev_get_num_queues(&matrix_mdev->shadow_apcb);
>> +	qinfo_sz = num_queues * sizeof(struct vfio_ap_queue_info);
>> +	*stop_copy_length = qinfo_sz + sizeof(struct vfio_ap_config);
> [Severity: Low]
> Does this code manually duplicate the size calculation logic already present
> in the existing static helper vfio_ap_config_size() within this same file?

Yes, it does. The vfio_ap_get_data_size function will be modified to 
call the
vfio_ap_config_size function to set *stop_copy_length.

>
> Additionally, since struct vfio_ap_config uses a trailing flexible array
> member for qinfo, would it be better to use the standard struct_size() macro
> to compute the total length instead of manual multiplication and addition?

struct_size(p, member, count) expects a pointer to the structure for which
the size is to be calculated, so this can not be used; however, I think the
struct_size_t

>
>> +	mutex_unlock(&matrix_dev->mdevs_lock);
>> +
>> +	return 0;
>>   }