On Sat, Oct 21, 2023 at 5:28 PM Si-Wei Liu <si-wei.liu@xxxxxxxxxx> wrote: > > Using .compat_reset op from the previous patch, the buggy .reset > behaviour can be kept as-is on older userspace apps, which don't ack the > IOTLB_PERSIST backend feature. As this compatibility quirk is limited to > those drivers that used to be buggy in the past, it won't affect change > the behaviour or affect ABI on the setups with API compliant driver. > > The separation of .compat_reset from the regular .reset allows > vhost-vdpa able to know which driver had broken behaviour before, so it > can apply the corresponding compatibility quirk to the individual driver > whenever needed. Compared to overloading the existing .reset with > flags, .compat_reset won't cause any extra burden to the implementation > of every compliant driver. > > Signed-off-by: Si-Wei Liu <si-wei.liu@xxxxxxxxxx> > --- > drivers/vhost/vdpa.c | 17 +++++++++++++---- > drivers/virtio/virtio_vdpa.c | 2 +- > include/linux/vdpa.h | 7 +++++-- > 3 files changed, 19 insertions(+), 7 deletions(-) > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > index acc7c74ba7d6..9ce40003793b 100644 > --- a/drivers/vhost/vdpa.c > +++ b/drivers/vhost/vdpa.c > @@ -227,13 +227,22 @@ static void vhost_vdpa_unsetup_vq_irq(struct vhost_vdpa *v, u16 qid) > irq_bypass_unregister_producer(&vq->call_ctx.producer); > } > > -static int vhost_vdpa_reset(struct vhost_vdpa *v) > +static int _compat_vdpa_reset(struct vhost_vdpa *v) > { > struct vdpa_device *vdpa = v->vdpa; > + u32 flags = 0; > > - v->in_batch = 0; > + flags |= !vhost_backend_has_feature(v->vdev.vqs[0], > + VHOST_BACKEND_F_IOTLB_PERSIST) ? > + VDPA_RESET_F_CLEAN_MAP : 0; > + > + return vdpa_reset(vdpa, flags); > +} > > - return vdpa_reset(vdpa); > +static int vhost_vdpa_reset(struct vhost_vdpa *v) > +{ > + v->in_batch = 0; > + return _compat_vdpa_reset(v); > } > > static long vhost_vdpa_bind_mm(struct vhost_vdpa *v) > @@ -312,7 +321,7 @@ static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp) > vhost_vdpa_unsetup_vq_irq(v, i); > > if (status == 0) { > - ret = vdpa_reset(vdpa); > + ret = _compat_vdpa_reset(v); > if (ret) > return ret; > } else > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c > index 06ce6d8c2e00..8d63e5923d24 100644 > --- a/drivers/virtio/virtio_vdpa.c > +++ b/drivers/virtio/virtio_vdpa.c > @@ -100,7 +100,7 @@ static void virtio_vdpa_reset(struct virtio_device *vdev) > { > struct vdpa_device *vdpa = vd_get_vdpa(vdev); > > - vdpa_reset(vdpa); > + vdpa_reset(vdpa, 0); > } > > static bool virtio_vdpa_notify(struct virtqueue *vq) > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h > index 6b8cbf75712d..db15ac07f8a6 100644 > --- a/include/linux/vdpa.h > +++ b/include/linux/vdpa.h > @@ -519,14 +519,17 @@ static inline struct device *vdpa_get_dma_dev(struct vdpa_device *vdev) > return vdev->dma_dev; > } > > -static inline int vdpa_reset(struct vdpa_device *vdev) > +static inline int vdpa_reset(struct vdpa_device *vdev, u32 flags) > { > const struct vdpa_config_ops *ops = vdev->config; > int ret; > > down_write(&vdev->cf_lock); > vdev->features_valid = false; > - ret = ops->reset(vdev); > + if (ops->compat_reset && flags) > + ret = ops->compat_reset(vdev, flags); > + else > + ret = ops->reset(vdev); Instead of inventing a new API that carries the flags. Tweak the existing one seems to be simpler and better? As compat_reset(vdev, 0) == reset(vdev) Then you don't need the switch in the parent as well +static int vdpasim_reset(struct vdpa_device *vdpa) +{ + return vdpasim_compat_reset(vdpa, 0); +} Thanks > up_write(&vdev->cf_lock); > return ret; > } > -- > 2.39.3 > _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization