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

[sashiko-bot@kernel.org]

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 <akrowiak@linux.ibm.com>

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?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727173239.2420754-1-akrowiak@linux.ibm.com?part=2