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 <[email protected]> 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 <[email protected]> >>> Reviewed-by: Shahar Shitrit <[email protected]> >> >> Acked-by: Eugenio Pérez <[email protected]> >> >>> --- >>> 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
