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; >> }