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

[Anthony Krowiak <akrowiak@linux.ibm.com>]

On 8/20/26 9:01 AM, Jason J. Herne wrote:
>
>
> On 8/7/26 6:18 PM, Anthony Krowiak wrote:
>> Add the 'migratable' feature to the features provided by the sysfs
>> /sys/devices/vfio_ap/matrix/feature attribute to indicate that migration
>> of vfio devices is supported. Since live guest migration is not 
>> supported
>> for SE guests, the 'migratable' feature will not be included when the
>> feature attribute is displayed.
>>
>> Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com>
>> ---
>>   drivers/s390/crypto/vfio_ap_drv.c | 14 +++++++++++++-
>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/s390/crypto/vfio_ap_drv.c 
>> b/drivers/s390/crypto/vfio_ap_drv.c
>> index 8e69ed286bb9..abfb34421d74 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_DRV_FEATURES    "guest_matrix hotplug ap_config 
>> migratable"
>> +#define SE_GUEST_FEATURES    "guest_matrix hotplug ap_config"
>> +
>>   static ssize_t features_show(struct device *dev, struct 
>> device_attribute *attr, char *buf)
>>   {
>> -    return sysfs_emit(buf, "guest_matrix hotplug ap_config\n");
>> +    /*
>> +     * If the system running is an SE guest, it will support a 
>> different
>> +     * set of features; for example, it will not support live guest
>> +     * migration
>> +     */
>> +    if (ap_is_se_guest())
>> +        return sysfs_emit(buf, "%s\n", SE_GUEST_FEATURES);
>
> ap_is_se_guest() is defined like this:
> is_prot_virt_guest() && ap_sb_available();
>
> is_prot_virt_guest() makes sense, this tells is this is actually a 
> secured guest.
>
> But ap_sb_available() "Returns 1 if secure binding facility is 
> available".
>
> What do we want to happen here if is_prot_virt_guest() = TRUE while 
> ap_sb_available() = FALSE? Have you considered this scenario?
>
> Also, the code would be a bit more extendible if you emitted the 
> static string of always supported features, and then if(se) emit the 
> string of se support features prepended with a space. Then new feature 
> updates only have a single string to modify, so no risk of 
> accidentally missing one.

Is this what you had in mind?

#define VFIO_AP_BASE_FEATURES "guest_matrix hotplug ap_config"
#define VFIO_AP_SE_EXCLUDED_FEATURES "migration"

if (ap_is_se_guest)
     sysfs_emit(buf, "%s\n", VFIO_AP_BASE_FEATURES);
else
     sysfs_emit(buf, "%s %s\n", VFIO_AP_SE_EXCLUDED_FEATURES);


>
> Also, I'd consider using "migrate" or "migration" as they match 
> existing feature designations better. We did not use hotpluggable :)
>
>> +
>> +    /* Return all features supported by the vfio_ap device driver */
>> +    return sysfs_emit(buf, "%s\n", VFIO_AP_DRV_FEATURES);
>>   }
>>   static DEVICE_ATTR_RO(features);
>
>