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? Thanks, Dragos
