On Mon, Aug 10, 2026 at 09:40:17PM +0800, Jia Jia wrote:
vhost_vsock_set_features() leaves the device IOTLB attached when
userspace clears VIRTIO_F_ACCESS_PLATFORM. Descriptors can therefore
continue to use translations installed before the feature change,
including HVAs made stale by a later memory table update.
Detach the device IOTLB before acknowledging a feature mask without
ACCESS_PLATFORM. Serialize each virtqueue handoff with its own mutex
while clearing its IOTLB pointer, resetting its metadata cache, and
updating its acknowledged features. Keep the old IOTLB alive until all
virtqueues have dropped their references, then free it.
Also drop queued IOTLB miss messages and wake readers now that the
device no longer accepts IOTLB updates.
Fixes: e13a6915a03f ("vhost/vsock: add IOTLB API support")
Suggested-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Jia Jia <[email protected]>
---
drivers/vhost/vsock.c | 39 ++++++++++++++++++++++++++++++++++-----
1 file changed, 34 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 9aaab6bb8061..7372c22691de 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -851,6 +851,30 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock,
u64 guest_cid)
return 0;
}
+/* Caller must hold the device mutex. */
+static void vhost_vsock_clear_iotlb(struct vhost_vsock *vsock, u64 features)
+{
+ struct vhost_iotlb *iotlb;
+ struct vhost_virtqueue *vq;
+ int i;
+
+ iotlb = vsock->dev.iotlb;
+ vsock->dev.iotlb = NULL;
+
+ for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
+ mutex_lock(&vsock->vqs[i].mutex);
+ vq = &vsock->vqs[i];
You can assing vq before the mutex_lock() and use it there too (and in
mutex_unlock()), as we do in all other places in this file.
+ vq->iotlb = NULL;
+ memset(vq->meta_iotlb, 0, sizeof(vq->meta_iotlb));
+ vq->acked_features = features;
+ mutex_unlock(&vsock->vqs[i].mutex);
+ }
+
+ vhost_clear_msg(&vsock->dev);
+ vhost_iotlb_free(iotlb);
+ wake_up_interruptible_poll(&vsock->dev.wait, EPOLLIN | EPOLLRDNORM);
+}
I don't see anything vsock specific here. Would it be better to move
this to vhost.c and reuse some of the functions we have there?
I mean something like this (untested and may be incomplete):
void vhost_clear_device_iotlb(struct vhost_dev *d)
{
struct vhost_iotlb *iotlb;
int i;
iotlb = d->iotlb;
d->iotlb = NULL;
for (i = 0; i < d->nvqs; ++i) {
struct vhost_virtqueue *vq = d->vqs[i];
mutex_lock(&vq->mutex);
vq->iotlb = NULL;
__vhost_vq_meta_reset(vq);
mutex_unlock(&vq->mutex);
}
vhost_clear_msg(d);
vhost_iotlb_free(iotlb);
wake_up_interruptible_poll(&d->wait, EPOLLIN | EPOLLRDNORM);
}
EXPORT_SYMBOL_GPL(vhost_clear_device_iotlb);
+
static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
{
struct vhost_virtqueue *vq;
@@ -872,11 +896,16 @@ static int vhost_vsock_set_features(struct vhost_vsock
*vsock, u64 features)
vsock->seqpacket_allow = features & (1ULL << VIRTIO_VSOCK_F_SEQPACKET);
- for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
- vq = &vsock->vqs[i];
- mutex_lock(&vq->mutex);
- vq->acked_features = features;
- mutex_unlock(&vq->mutex);
+ if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
+ vsock->dev.iotlb) {
+ vhost_vsock_clear_iotlb(vsock, features);
+ } else {
+ for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
+ vq = &vsock->vqs[i];
+ mutex_lock(&vq->mutex);
+ vq->acked_features = features;
+ mutex_unlock(&vq->mutex);
+ }
TBH I don't like this mix.
Why assigning acked_features inside vhost_vsock_clear_iotlb()?
IMO it's confusing. I see that you're saving another loop around the
VQs, but this code is not easy to understand IMO.
I think we have 2 options:
1. leave the loop for acked_features and don't set it in
vhost_vsock_clear_iotlb() (less code touched by this patch).
This is also what to do if we move the clear_iotlb() function
in vhost.c.
2. change the code to have a single for loop with if block inside to
reset IOTLB stuff if needed. In this case maybe better to avoid the
function and move the entire code here.
I prefer 1 with clear_iotlb() in vhost.c, but I'm not fully against 2.
Thanks,
Stefano
}
mutex_unlock(&vsock->dev.mutex);
return 0;
--
2.34.1
