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

On 7/27/26 2:45 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The 'migratable' feature is unconditionally advertised in sysfs, even when migration is unsupported and explicitly disabled.
> --
>
> --- Patch [14]: [PATCH v6 14/15] s390/vfio-ap: Add 'migratable' feature to sysfs 'features' attribute ---
> commit 4ed92108ce1d93cfce3f8740cc391bfad091179c
> Author: Anthony Krowiak <[email protected]>
>
> s390/vfio-ap: Add 'migratable' feature to sysfs 'features' attribute
>
> This commit adds the 'migratable' feature to the sysfs features attribute
> to indicate that migration of vfio devices is supported.
>
>> diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c
>> index 8e69ed286bb9c..4e973736522bc 100644
>> --- a/drivers/s390/crypto/vfio_ap_drv.c
>> +++ b/drivers/s390/crypto/vfio_ap_drv.c
>> @@ -26,9 +26,11 @@ MODULE_LICENSE("GPL v2");
>>   struct ap_matrix_dev *matrix_dev;
>>   debug_info_t *vfio_ap_dbf_info;
>>   
>> +#define FEATURES "guest_matrix hotplug ap_config migratable"
>> +
>>   static ssize_t features_show(struct device *dev, struct device_attribute *attr, char *buf)
>>   {
>> -	return sysfs_emit(buf, "guest_matrix hotplug ap_config\n");
>> +	return sysfs_emit(buf, "%s\n", FEATURES);
> [Severity: Medium]
> Will this unconditionally advertise the migratable feature even when the
> environment does not support it?
>
> If the system is running as a Secure Execution (SE) guest, migration is
> typically disabled. By hardcoding "migratable" into the FEATURES string,
> management tools like libvirt querying the features sysfs attribute might
> incorrectly assume migration is supported.
>
> This could lead orchestrators to make incorrect VM placement decisions,
> followed by unexpected migration failures when QEMU rejects the migration
> because the VFIO mdev lacks the capability.
>
> Should features_show() check environmental constraints, such as
> ap_is_se_guest(), before including the migratable flag in the output?

This is a legitimate concern; the ap_is_se_guest() check will be 
incorporated
to eliminate returning the 'migratable' if it returns true.
>
>>   }
>>   static DEVICE_ATTR_RO(features);