Re: [PATCH v8 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:
- [High] vfio-ap: migration ABI layout depends on internal kernel macro AP_DOMAINS
- [High] s390/vfio-ap: missing interrupt state in migration ABI
--

commit a77ac9d23bc32798a481b033c179b8862ae321e5
Author: Anthony Krowiak <akrowiak@linux.ibm.com>

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

This commit introduces the data structures used to facilitate state
transitions during vfio device migration for the s390 crypto subsystem.
It adds the foundational structures for saving and restoring AP configuration.

> diff --git a/drivers/s390/crypto/vfio_ap_migration.c b/drivers/s390/crypto/vfio_ap_migration.c
> new file mode 100644
> index 0000000000000..f76d151a5ccec
> --- /dev/null
> +++ b/drivers/s390/crypto/vfio_ap_migration.c

[ ... ]

> +struct vfio_ap_queue_info {
> +	u64 data;
> +	u16 apqn;
> +	u8  reserved[6];
> +};

[Severity: High]
Does this structure capture the necessary fields for interrupt configuration
state?

During live migration, the guest's saved_iova and saved_isc appear to be
dropped as they are omitted here. After live migration, will AP queues on the
destination host have interrupts disabled because the Notification Indicator
Byte (NIB) address is not restored, potentially causing guests waiting for
crypto operations to hang?

Furthermore, since the 6-byte reserved padding is too small to hold an 8-byte
saved_iova and 1-byte saved_isc, could this prevent non-destructively
extending the ABI later without breaking compatibility?

[ ... ]

> +struct vfio_ap_config {
> +	u32				magic;
> +	u32				version;
> +	u32				num_queues;
> +	u8				reserved[4];
> +	u64				adm[DIV_ROUND_UP(AP_DOMAINS, 64)];
> +	struct vfio_ap_queue_info	qinfo[] __counted_by(num_queues);
> +};

[Severity: High]
Does relying on the internal AP_DOMAINS macro here tie the migration ABI
layout to potentially changing architecture limits?

Since this struct is directly read from and written to the migration file
stream as raw bytes, if the AP_DOMAINS macro increases in the future
to support new hardware limits, wouldn't the size of the adm array grow
and shift the start offset of the qinfo[] array?

This could silently break migration compatibility between older and newer
kernels without a protocol version bump, as the destination would read qinfo
elements from the wrong offset.

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