Re: [PATCH v6 02/15] s390/vfio-ap: Data structures for facilitating vfio device migration

On 7/27/26 1:40 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The VFIO migration data structure lacks a magic identifier and version number, and relies on an architectural macro for array sizing, posing a risk to ABI stability.
> - [Low] Typo and grammatical error in the docstring for `resuming_mig_file`.
> --
>
> --- Patch [2]: [PATCH v6 02/15] s390/vfio-ap: Data structures for facilitating vfio device migration ---
> commit 1aa240c81b915a9ce3e59c82c532c41c1cab4d99
> Author: Anthony Krowiak <[email protected]>
>
> s390/vfio-ap: Data structures for facilitating vfio device migration
>
> This commit creates the data structures used to facilitate state transitions
> during vfio device migration.
>
>> diff --git a/drivers/s390/crypto/vfio_ap_migration.c b/drivers/s390/crypto/vfio_ap_migration.c
>> new file mode 100644
>> --- /dev/null
>> +++ b/drivers/s390/crypto/vfio_ap_migration.c
> [ ... ]
>> +/**
>> + * struct vfio_ap_migration_data
>> + *
>> + * Manages the migration state for the VFIO device that maintains the AP
>> + * configuration of the guest being migrated.
>> + *
>> + * @mig_state:		the current migration state
>> + * @resuming_mig_file:	the object used to restore the state of the vfio-ap
>> + *			device the destination host:
> [Severity: Low]
> This isn't a bug, but there appears to be a missing word here. Should this
> say "device on the destination host." instead of ending with a colon?
>
>> + * @stop_copy_mig_file: the object used to store the AP configuration of the
>> + *			source guest for transfer to the destination host.
>> + */
>> +struct vfio_ap_migration_data {
> [ ... ]
>> +/**
>> + * struct vfio_ap_config - the guest's AP configuration
>> + *
>> + * @num_queues:	the number of queues passed through to the guest
>> + * @reserved:	padding to ensure proper alignment of @adm
>> + * @adm:	bitmap specifying the control domains in the AP configuration
>> + * @qinfo:	an array of vfio_ap_queue_info objects, each specifying the
>> + *		queue information for a queue passed through to the guest
>> + */
>> +struct vfio_ap_config {
>> +	u32				num_queues;
>> +	u8				reserved[4];
>> +	u64				adm[DIV_ROUND_UP(AP_DOMAINS, 64)];
> [Severity: Medium]
> Does this data structure need a version number or magic identifier to safely
> handle ABI stability across host migrations?
>
> Since this structure defines the binary layout of the migration data stream,
> could changing the architectural macro AP_DOMAINS or the structure layout in
> future kernel versions silently break live migration compatibility between
> hosts running different kernel versions?

The next version of the patch will incorporate magic and version fields.

>
>> +	struct vfio_ap_queue_info	qinfo[] __counted_by(num_queues);
>> +};