Re: [PATCH v10 14/15] s390/vfio-ap: Add 'migration' feature to sysfs 'features' attribute

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] s390/vfio-ap: features_show() uses guest-side ap_is_se_guest() on KVM host
--

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

s390/vfio-ap: Add 'migration' feature to sysfs 'features' attribute

This commit adds the migration feature string to the vfio_ap matrix sysfs
features attribute unless ap_is_se_guest() returns true.

> diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c
> index 8e69ed286bb9c..c63a050a23569 100644
> --- a/drivers/s390/crypto/vfio_ap_drv.c
> +++ b/drivers/s390/crypto/vfio_ap_drv.c
> @@ -26,9 +26,21 @@ MODULE_LICENSE("GPL v2");
>  struct ap_matrix_dev *matrix_dev;
>  debug_info_t *vfio_ap_dbf_info;
>  
> +#define VFIO_AP_BASE_FEATURES	"guest_matrix hotplug ap_config"
> +#define VFIO_AP_SE_EXCLUDED_FEATURES "migration"
> +
>  static ssize_t features_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
> -	return sysfs_emit(buf, "guest_matrix hotplug ap_config\n");
> +	/*
> +	 * SE guests do not support live guest migration; emit only the base
> +	 * feature set for them.
> +	 */
> +	if (ap_is_se_guest())
> +		return sysfs_emit(buf, "%s\n", VFIO_AP_BASE_FEATURES);
> +
> +	/* Non-SE: base features plus those not available on SE guests */
> +	return sysfs_emit(buf, "%s %s\n", VFIO_AP_BASE_FEATURES,
> +			  VFIO_AP_SE_EXCLUDED_FEATURES);

[Severity: Medium]
Can ap_is_se_guest() ever evaluate to true here on a KVM host?

Looking at drivers/s390/crypto/ap_bus.c:

bool ap_is_se_guest(void)
{
	return is_prot_virt_guest() && ap_sb_available();
}

and arch/s390/boot/uv.c:uv_query_info():

	if (test_bit_inv(BIT_UVC_CMD_SET_SHARED_ACCESS, (unsigned long *)uvcb.inst_calls_list) &&
	    test_bit_inv(BIT_UVC_CMD_REMOVE_SHARED_ACCESS, (unsigned long *)uvcb.inst_calls_list))
		prot_virt_guest = 1;

is_prot_virt_guest() checks whether the currently running kernel is itself a
protected virtualization guest, which is mutually exclusive with running as a
KVM host in is_prot_virt_host_capable().

Because vfio_ap runs on the KVM host, ap_is_se_guest() will always return
false in features_show(), as well as in the earlier patches in this series in
drivers/s390/crypto/vfio_ap_migration.c:

vfio_ap_mdev_probe() -> vfio_ap_setup_vfio_device_migration():
	if (ap_is_se_guest())
		return;

	matrix_mdev->vdev.migration_flags = VFIO_MIGRATION_STOP_COPY;
	matrix_mdev->vdev.mig_ops = &vfio_ap_migration_ops;

and vfio_ap_mdev_open_device() -> vfio_ap_init_migration_data():
	if (ap_is_se_guest())
		return 0;

Additionally, /sys/devices/vfio_ap/matrix/features is a host-wide attribute on
matrix_dev, whereas whether a KVM guest uses Secure Execution is a per-VM
runtime property (such as kvm_s390_pv_is_protected(matrix_mdev->kvm) after the
guest transitions into protected mode via KVM_PV_ENABLE), and a single host
can run both SE and non-SE guests simultaneously.

Should the SE guest migration restriction be enforced per guest or mdev using
host-side KVM protected virtualization state rather than ap_is_se_guest()?

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