On Thu, Jul 30, 2026 at 09:55:08AM -0400, Stefan Hajnoczi wrote:
> On Thu, Jul 30, 2026 at 12:09:38PM +0800, Jia Jia wrote:
> > vhost_vsock_set_features() initializes dev->iotlb when
> > VIRTIO_F_ACCESS_PLATFORM is enabled. It does not remove that IOTLB
> > when the feature is later cleared. The virtqueue still points at the
> > old IOTLB, and vhost_vsock_handle_tx_kick() passes descriptors to
> > vhost_get_vq_desc(), which translates them through that mapping.
> >
> > A userspace backend can enable ACCESS_PLATFORM, install an IOTLB entry
> > for a payload GPA, start the device, clear ACCESS_PLATFORM, replace the
> > memory table, and reuse the old HVA before submitting the same GPA
> > again. The feature state then says direct memory access is in use
> > while the TX path still uses the old IOTLB HVA.
> >
> > Reject clearing ACCESS_PLATFORM while the device IOTLB exists. Also
> > keep the existing IOTLB when a feature update leaves ACCESS_PLATFORM
> > enabled; VHOST_SET_FEATURES is used for runtime log updates and must
> > not discard the current translations by allocating an empty IOTLB.
> >
> > Fixes: e13a6915a03f ("vhost/vsock: add IOTLB API support")
> > Signed-off-by: Jia Jia <[email protected]>
> > ---
> > drivers/vhost/vsock.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > index ae01457ea2cd..57e8fd1eb670 100644
> > --- a/drivers/vhost/vsock.c
> > +++ b/drivers/vhost/vsock.c
> > @@ -798,6 +798,7 @@ static int vhost_vsock_set_cid(struct vhost_vsock
> > *vsock, u64 guest_cid)
> > static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64
> > features)
> > {
> > struct vhost_virtqueue *vq;
> > + int ret = -EFAULT;
> > int i;
> >
> > if (features & ~VHOST_VSOCK_FEATURES)
> > @@ -809,7 +810,14 @@ static int vhost_vsock_set_features(struct vhost_vsock
> > *vsock, u64 features)
> > goto err;
> > }
> >
> > - if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) {
> > + if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
> > + vsock->dev.iotlb) {
> > + ret = -EBUSY;
> > + goto err;
> > + }
>
> This prevents one problem but there are still other issues with how
> feature bit negotiation and the IOTLB are implemented:
>
> 1. VIRTIO_F_ACCESS_PLATFORM is defined by the VIRTIO spec and must not
> be change after feature bit negotiation. Please reject all feature
> bit updates except VHOST_F_LOG_ALL to comply with the VIRTIO spec and
> eliminate potential bugs in drivers/vhost/vsock.c.
Unfortunately, vhost does not expose the feature negotiation to the
kernel.
> 2. When the device is reset, the iotlb cannot be left initialized
> because there is no guarantee that VIRTIO_F_ACCESS_PLATFORM will be
> negotiated again.
>
> > + if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
> > + !vsock->dev.iotlb) {
> > if (vhost_init_device_iotlb(&vsock->dev))
> > goto err;
> > }
> > @@ -827,7 +835,7 @@ static int vhost_vsock_set_features(struct vhost_vsock
> > *vsock, u64 features)
> >
> > err:
> > mutex_unlock(&vsock->dev.mutex);
> > - return -EFAULT;
> > + return ret;
> > }
> >
> > static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
> > --
> > 2.34.1
> >
