Re: [PATCH v4 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:
- [Low] Kernel-doc comments for the new migration data structures contain mismatched and missing fields.
- [Low] Use of the architecture-dependent type `size_t` in a struct intended for a migration data payload.
--

--- Patch [2]: [PATCH v4 02/15] s390/vfio-ap: Data structures for facilitating vfio device migration ---
commit c51eed4d1a56608445d8a10292fff2170e3cf604
Author: Anthony Krowiak <akrowiak@linux.ibm.com>

s390/vfio-ap: Data structures for facilitating vfio device migration

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
> index 0000000000000..239168c4b7fff
> --- /dev/null
> +++ b/drivers/s390/crypto/vfio_ap_migration.c
[ ... ]
> +/**
> + * vfio_ap_migration_data - the data needed to migrate a guest with pass-through
> + *			    access to AP devices
> + *
> + * @mig_state:		the current migration state
> + * @resuming_migf:	the object used to resume the target guest
> + * @saving_migf:	the object used to save the state of the source guest
> + */
> +struct vfio_ap_migration_data {
> +	enum vfio_device_mig_state	mig_state;
> +
> +	struct {
> +		struct file *filp;
> +		struct vfio_ap_config	*ap_config;
> +		size_t			config_sz;
> +	} resuming_mig_state;
> +
> +	struct file			*stop_copy_mig_file;
> +};

[Severity: Low]
This isn't a bug, but the kernel-doc block documents fields (@resuming_migf
and @saving_migf) that do not match the actual struct members
(resuming_mig_state and stop_copy_mig_file). Should the documentation be
updated to reflect the actual member names?

[ ... ]
> +/**
> + * 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 qinfo array
> + * @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];
> +	size_t				config_sz;

[Severity: Low]
This isn't a bug, but the kernel-doc block above is missing documentation
for the config_sz member.

Also, since this struct is intended for a migration data payload, does the use
of size_t present an issue for serialization? While s390x is strictly 64-bit,
using an architecture-dependent type in a migration stream is generally an
anti-pattern. Would a fixed-width integer type like u64 be more appropriate
here to ensure consistent structure size?

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

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