Re: [PATCH] vhost-vdpa: Expose ASID group change after DRIVER_OK via backend feature

[Dragos Tatulea <dtatulea@nvidia.com>]

On 29.06.26 11:39, Dragos Tatulea wrote:
> 
> 
> On 21.05.26 10:26, Eugenio Perez Martin wrote:
>> On Mon, May 11, 2026 at 11:46 AM Dragos Tatulea <dtatulea@nvidia.com> wrote:
>>>
>>> The commit in the fixes tag blocked VHOST_VDPA_SET_GROUP_ASID operations
>>> once DRIVER_OK is set. That is too strict for devices which can safely
>>> handle this during live migration flows.
>>>
>>> Bring back this behavior under a new vhost backend feature flag. The
>>> feature is supported by mlx5 and vdpa_sim devices.
>>>
>>> Fixes: 3543b04a4ea3 ("vhost: forbid change vq groups ASID if DRIVER_OK is set")
>>> Signed-off-by: Dragos Tatulea <dtatulea@nvidia.com>
>>> Reviewed-by: Shahar Shitrit <shshitrit@nvidia.com>
>>
>> Acked-by: Eugenio Pérez <eperezma@redhat.com>
>>
>>> ---
>>>  drivers/vdpa/mlx5/net/mlx5_vnet.c |  3 ++-
>>>  drivers/vdpa/vdpa_sim/vdpa_sim.c  |  3 ++-
>>>  drivers/vhost/vdpa.c              | 13 +++++++++++--
>>>  include/uapi/linux/vhost_types.h  |  4 ++++
>>>  4 files changed, 19 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> index ad0d5fbbbca8..f89177957c76 100644
>>> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>>> @@ -2906,7 +2906,8 @@ static void unregister_link_notifier(struct mlx5_vdpa_net *ndev)
>>>
>>>  static u64 mlx5_vdpa_get_backend_features(const struct vdpa_device *vdpa)
>>>  {
>>> -       return BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK);
>>> +       return BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK) |
>>> +              BIT_ULL(VHOST_BACKEND_F_GROUP_ASID_AFTER_DRIVER_OK);
>>>  }
>>>
>>>  static int mlx5_vdpa_set_driver_features(struct vdpa_device *vdev, u64 features)
>>> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>> index 8cb1cc2ea139..253c7fb35ea0 100644
>>> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>>> @@ -428,7 +428,8 @@ static u64 vdpasim_get_device_features(struct vdpa_device *vdpa)
>>>
>>>  static u64 vdpasim_get_backend_features(const struct vdpa_device *vdpa)
>>>  {
>>> -       return BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK);
>>> +       return BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK) |
>>> +              BIT_ULL(VHOST_BACKEND_F_GROUP_ASID_AFTER_DRIVER_OK);
>>>  }
>>>
>>>  static int vdpasim_set_driver_features(struct vdpa_device *vdpa, u64 features)
>>> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
>>> index 692564b1bcbb..67b3f49fa709 100644
>>> --- a/drivers/vhost/vdpa.c
>>> +++ b/drivers/vhost/vdpa.c
>>> @@ -682,7 +682,8 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
>>>                         return -EFAULT;
>>>                 if (idx >= vdpa->ngroups || s.num >= vdpa->nas)
>>>                         return -EINVAL;
>>> -               if (ops->get_status(vdpa) & VIRTIO_CONFIG_S_DRIVER_OK)
>>> +               if ((ops->get_status(vdpa) & VIRTIO_CONFIG_S_DRIVER_OK) &&
>>> +                   !vhost_backend_has_feature(vq, VHOST_BACKEND_F_GROUP_ASID_AFTER_DRIVER_OK))
>>>                         return -EBUSY;
>>>                 if (!ops->set_group_asid)
>>>                         return -EOPNOTSUPP;
>>> @@ -791,7 +792,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
>>>                                  BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST) |
>>>                                  BIT_ULL(VHOST_BACKEND_F_SUSPEND) |
>>>                                  BIT_ULL(VHOST_BACKEND_F_RESUME) |
>>> -                                BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK)))
>>> +                                BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK) |
>>> +                                BIT_ULL(VHOST_BACKEND_F_GROUP_ASID_AFTER_DRIVER_OK)))
>>>                         return -EOPNOTSUPP;
>>>                 if ((features & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) &&
>>>                      !vhost_vdpa_can_suspend(v))
>>> @@ -805,6 +807,13 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
>>>                 if ((features & BIT_ULL(VHOST_BACKEND_F_DESC_ASID)) &&
>>>                      !vhost_vdpa_has_desc_group(v))
>>>                         return -EOPNOTSUPP;
>>
>> Ouch to me here. By reading the errno manual:
>>
>> Nit: By reading errno(3):
>> ENOTSUP - Operation not supported (POSIX.1-2001).
>> EOPNOTSUPP - Operation not supported on socket (POSIX.1-2001).
>>
>> I picked the wrong constant even if they share the same errno value
>> (by the same page of the manual). MST, is it worth changing it?
>>
>>> +               if (features & BIT_ULL(VHOST_BACKEND_F_GROUP_ASID_AFTER_DRIVER_OK)) {
>>> +                       if (!(features & BIT_ULL(VHOST_BACKEND_F_IOTLB_ASID)))
>>> +                               return -EINVAL;
>>> +                       if (!(vhost_vdpa_get_backend_features(v) &
>>> +                               BIT_ULL(VHOST_BACKEND_F_GROUP_ASID_AFTER_DRIVER_OK)))
>>> +                               return -EOPNOTSUPP;
>>> +               }
>>>                 if ((features & BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST)) &&
>>>                      !vhost_vdpa_has_persistent_map(v))
>>>                         return -EOPNOTSUPP;
>>> diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h
>>> index 1c39cc5f5a31..ec1ff8a2e260 100644
>>> --- a/include/uapi/linux/vhost_types.h
>>> +++ b/include/uapi/linux/vhost_types.h
>>> @@ -197,5 +197,9 @@ struct vhost_vdpa_iova_range {
>>>  #define VHOST_BACKEND_F_DESC_ASID    0x7
>>>  /* IOTLB don't flush memory mapping across device reset */
>>>  #define VHOST_BACKEND_F_IOTLB_PERSIST  0x8
>>> +/* Device supports changing the group ASID after DRIVER_OK.
>>> + * Requires VHOST_BACKEND_F_IOTLB_ASID.
>>> + */
>>> +#define VHOST_BACKEND_F_GROUP_ASID_AFTER_DRIVER_OK  0x9
>>>
>>>  #endif
>>> --
>>> 2.54.0
>>>
>>
> 
> Gentle ping. Is this patch missing anything?
> 
One more ping.

Thanks,
Dragos