Re: [PATCH v2 1/2] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Jul 31, 2026 at 06:34:13PM +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 IOTLB before acknowledging a feature mask without
> ACCESS_PLATFORM. Hold all virtqueue mutexes in index order while clearing
> the device and virtqueue IOTLB pointers, resetting metadata caches, and
> updating the acknowledged features. This prevents a kick handler from
> observing a mixed translation state.
> 
> Free the old IOTLB after releasing the virtqueue mutexes. Also drop
> the old IOTLB miss messages and wake readers now that the device no
> longer accepts IOTLB updates.
> 
> Fixes: e13a6915a03f ("vhost/vsock: add IOTLB API support")
> Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> ---
>  drivers/vhost/vsock.c | 43 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 38 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index 9aaab6bb8061..562b9e139a76 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -851,6 +851,34 @@ 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;
> +
> +	for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++)
> +		mutex_lock_nested(&vsock->vqs[i].mutex, i);
> +
> +	iotlb = vsock->dev.iotlb;
> +	vsock->dev.iotlb = NULL;
> +
> +	for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
> +		vq = &vsock->vqs[i];
> +		vq->iotlb = NULL;
> +		memset(vq->meta_iotlb, 0, sizeof(vq->meta_iotlb));
> +		vq->acked_features = features;
> +	}
> +
> +	for (i = ARRAY_SIZE(vsock->vqs); i-- > 0;)
> +		mutex_unlock(&vsock->vqs[i].mutex);

Why lock down all vqs like this?  Would this work just as well instead?

	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];
		vq->iotlb = NULL;
		memset(vq->meta_iotlb, 0, sizeof(vq->meta_iotlb));
		vq->acked_features = features;
		mutex_unlock(&vsock->vqs[i].mutex);
	}

and if no why not?


> +	vhost_clear_msg(&vsock->dev);
> +	vhost_iotlb_free(iotlb);
> +	wake_up_interruptible_poll(&vsock->dev.wait, EPOLLIN | EPOLLRDNORM);
> +}
> +
>  static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
>  {
>  	struct vhost_virtqueue *vq;
> @@ -872,11 +900,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);
> +		}
>  	}
>  	mutex_unlock(&vsock->dev.mutex);
>  	return 0;
> -- 
> 2.34.1