Re: [PATCH v13 05/14] s390/vfio-ap: Callback to get/set vfio device mig state during guest migration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] s390/vfio-ap: wrong SE guest check for migration in
  vfio_ap_set_state()
--

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

s390/vfio-ap: Callback to get/set vfio device mig state during guest migration

This commit adds the migration_set_state and migration_get_state callbacks
for vfio-ap mediated devices to support VFIO_DEVICE_FEATURE_MIGRATION.

> diff --git a/drivers/s390/crypto/vfio_ap_migration.c b/drivers/s390/crypto/vfio_ap_migration.c
> index 94772cab1e855..df012f80ed0d7 100644
> --- a/drivers/s390/crypto/vfio_ap_migration.c
> +++ b/drivers/s390/crypto/vfio_ap_migration.c

[ ... ]

> +static struct file *vfio_ap_set_state(struct vfio_device *vdev,
> +				      enum vfio_device_mig_state  new_state)
> +{
> +	int ret;
> +	struct file *filp = NULL;
> +	struct ap_matrix_mdev *matrix_mdev;
> +	enum vfio_device_mig_state next_state;
> +	struct vfio_ap_migration_data *mig_data;
> +
> +	mutex_lock(&matrix_dev->mdevs_lock);
> +
> +	matrix_mdev = container_of(vdev, struct ap_matrix_mdev, vdev);
> +	mig_data = matrix_mdev->mig_data;
> +
> +	/*
> +	 * The mig_data pointer is set in the vfio_ap_init_migration_data
> +	 * function which is called when the vfio-ap device fd is opened.
> +	 * Since the implicit pre-open state is RUNNING, a request to set
> +	 * RUNNING is a no-op. Any other state transition is invalid before
> +	 * open_device.
> +	 */
> +	if (!mig_data) {
> +		mutex_unlock(&matrix_dev->mdevs_lock);
> +		if (new_state == VFIO_DEVICE_STATE_RUNNING)
> +			return NULL;
> +		return ERR_PTR(-ENODEV);
> +	}

[Severity: Medium]
Does vfio_ap_set_state() reject migration state transitions when the
attached KVM guest is running in Secure Execution (SE) mode?

vfio_ap_init_migration_data() tries to skip allocating mig_data for SE
guests by checking ap_is_se_guest(), which tests is_prot_virt_guest() &&
ap_sb_available().  That checks whether the host kernel itself is running
inside an SE guest, which is always false on a KVM host.  In addition, a
KVM guest enters Protected Virtualization later during boot via
KVM_PV_ENABLE (kvm_s390_pv_init_vm()), after open_device has already
called vfio_ap_init_migration_data().

As a result, matrix_mdev->mig_data is always allocated on a KVM host, and
vfio_ap_set_state() allows migration state transitions for SE KVM guests
instead of checking whether matrix_mdev->kvm is protected (for example via
kvm_s390_pv_is_protected()).

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