Re: [PATCH v10 15/15] s390/vfio-ap: Add live guest migration chapter to vfio-ap.rst

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] s390/vfio-ap: outdated migration error logs and return codes in vfio-ap.rst
--

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

s390/vfio-ap: Add live guest migration chapter to vfio-ap.rst

This commit adds a Live Guest Migration chapter to
Documentation/arch/s390/vfio-ap.rst describing how to configure and
troubleshoot live guest migration for KVM guests with passed-through AP
devices, along with minor comment updates in the vfio_ap driver.

> diff --git a/Documentation/arch/s390/vfio-ap.rst b/Documentation/arch/s390/vfio-ap.rst
> index ac0c07f76ddd7..f5d9e32875035 100644
> --- a/Documentation/arch/s390/vfio-ap.rst
> +++ b/Documentation/arch/s390/vfio-ap.rst

[ ... ]

> +3. Assign adapters, domains and control domains to the destination mediated device
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +Assign the same adapters, domains and control domains to the destination
> +mediated device as are assigned to the source guest's mediated device. For
> +example, using the same adapters, domains and control domains from the
> +previous step:
> +
> +.. code-block:: bash
> +
> +   echo 0x0 > /sys/devices/vfio_ap/matrix/62177883-f1bb-47f0-914d-32a22e3a8804/assign_adapter
> +   echo 0x14 > /sys/devices/vfio_ap/matrix/62177883-f1bb-47f0-914d-32a22e3a8804/assign_adapter
> +   echo 0x16 > /sys/devices/vfio_ap/matrix/62177883-f1bb-47f0-914d-32a22e3a8804/assign_adapter
> +   echo 0x33 > /sys/devices/vfio_ap/matrix/62177883-f1bb-47f0-914d-32a22e3a8804/assign_domain
> +   echo 0x3a > /sys/devices/vfio_ap/matrix/62177883-f1bb-47f0-914d-32a22e3a8804/assign_domain

[Severity: Low]
This isn't a bug, but since this example states that it assigns the adapters,
domains, and control domains from the previous steps, should it also include
the assign_control_domain commands for 0x33 and 0x3a?

[ ... ]

> +One or more queues are not bound to the vfio_ap driver on the destination host
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +The queue devices passed through to the source guest are not installed in the
> +destination host or are not bound to its vfio_ap device driver.
> +
> +**Source host**
> +
> ++--------------+----------------------------------------------------------------+
> +| Log          | Message                                                        |
> ++==============+================================================================+
> +| Console log: | error: operation failed: migration failed. Message from the    |
> +|              | source host: operation failed: job 'migration out' failed:     |
> +|              | Sibling indicated error 1. Message from the destination host:  |
> +|              | operation failed: job 'migration in' failed: load of migration |
> +|              | failed: No such device: Failed to load vmstate version_id: 1,  |
> +|              | ret: -19                                                       |
> ++--------------+----------------------------------------------------------------+
> +
> +**Destination host**
> +
> ++--------------+----------------------------------------------------------------+
> +| Kernel log:  | vfio_ap_mdev $UUID: Queue $APID.$APQI not available to         |
> +|              | vfio_ap driver on target host                                  |
> ++--------------+----------------------------------------------------------------+
> +| QEMU log:    | shutting down, reason=failed                                   |
> +|              | qemu-system-s390x: terminating on signal $SIGNAL_NUM from      |
> +|              | pid $PID $DAEMON                                               |
> ++--------------+----------------------------------------------------------------+
> +
> +
> +One or more control domains are not available on the destination host
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +The control domains passed through to the source guest are not installed on the
> +destination host.
> +
> +**Source host**
> +
> ++--------------+----------------------------------------------------------------+
> +| Log          | Message                                                        |
> ++==============+================================================================+
> +| Console log: | error: operation failed: migration failed. Message from the    |
> +|              | source host: operation failed: job 'migration out' failed:     |
> +|              | Sibling indicated error 1. Message from the destination host:  |
> +|              | operation failed: job 'migration in' failed: load of migration |
> +|              | failed: No such device: Failed to load vmstate version_id: 1,  |
> +|              | ret: -19                                                       |
> ++--------------+----------------------------------------------------------------+
> +
> +**Destination host**
> +
> ++--------------+----------------------------------------------------------------+
> +| Kernel log:  | vfio_ap_mdev $UUID: Control domain $DOMAIN_NUM not available   |
> +|              | on the destination host                                        |
> ++--------------+----------------------------------------------------------------+

[Severity: Low]
Are the documented kernel log messages and return codes in these two sections
out of sync with the current implementation in
drivers/s390/crypto/vfio_ap_migration.c?

When a queue or control domain from the source guest is missing on the
destination host, do_post_copy_validation() calls
ap_config_is_subset_of_shadow_apcb():

drivers/s390/crypto/vfio_ap_migration.c:ap_config_is_subset_of_shadow_apcb() {
	for (int i = 0; i < source_config->num_queues; i++) {
		...
		if (!test_bit_inv(apid, matrix_mdev->shadow_apcb.apm) ||
		    !test_bit_inv(apqi, matrix_mdev->shadow_apcb.aqm)) {
			pr_err_ratelimited("vfio_ap_mdev %s: Queue %02lx.%04lx not in destination mdev's AP configuration\n",
					   mdev_name, apid, apqi);
			ret = false;
		}
	}

	for_each_set_bit_inv(domain_num, (unsigned long *)source_config->adm,
			     AP_DOMAINS) {
		if (!test_bit_inv(domain_num, matrix_mdev->shadow_apcb.adm)) {
			pr_err_ratelimited("vfio_ap_mdev %s: Control domain %04lx not in destination mdev's AP configuration\n",
					   mdev_name, domain_num);
			ret = false;
		}
	}
}

And do_post_copy_validation() then returns -EINVAL (-22, Invalid argument)
rather than -ENODEV (-19, No such device):

	if (!ap_config_is_subset_of_shadow_apcb(mdev_name, matrix_mdev,
						source_config))
		return -EINVAL;

Should the documentation be updated to reflect the "not in destination mdev's
AP configuration" messages and -EINVAL (-22) return code?

[ ... ]

> diff --git a/drivers/s390/crypto/vfio_ap_migration.c b/drivers/s390/crypto/vfio_ap_migration.c
> index 660b5a21fe9e8..ac135872da2fa 100644
> --- a/drivers/s390/crypto/vfio_ap_migration.c
> +++ b/drivers/s390/crypto/vfio_ap_migration.c
> @@ -217,8 +217,9 @@ static const struct vfio_migration_ops vfio_ap_migration_ops;
>  /**
>   * vfio_ap_setup_vfio_device_migration:
>   *
> + * Enable
>   * Plug the object containing the function pointers to the migration callbacks
> - * required to facilitate VFIO AP migration into the mdev
> + * used to facilitate migration of a vfio-ap device.

[Severity: Low]
This isn't a bug, but was the stray "Enable" line in the comment above
vfio_ap_setup_vfio_device_migration() left in by mistake?

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