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

On Mon, Aug 03, 2026 at 11:18:50PM -0400, Michael S. Tsirkin wrote:
> 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?

Thanks for the review.

My understanding is as follows. The proposed sequence protects the
lifetime of the old IOTLB, but it does not keep the translation state
consistent during the transition.

dev->iotlb is shared by all VQs, while vq->iotlb, meta_iotlb, and
acked_features are per-VQ state. A kick handler only holds its own VQ
mutex. If dev->iotlb is cleared first, a handler that already holds a VQ
mutex can continue using the old vq->iotlb and metadata cache, while
translate_desc() sees dev->iotlb == NULL and falls back to dev->umem. The
same handler could therefore observe both the IOVA/IOTLB and GPA/umem
views.

Locking each VQ in turn before freeing the old IOTLB prevents a lifetime
issue, but it does not remove this mixed-state window. Taking all VQ
mutexes before changing dev->iotlb lets active handlers finish and
prevents new handlers from running until the shared and per-VQ state has
been updated consistently.

If VHOST_SET_FEATURES is guaranteed to run only while all VQs are stopped
or otherwise quiesced, then the shorter sequence should be sufficient.
Since the ioctl itself does not enforce that, I thought this transition
also needed to be safe while a VQ may still be active.

Please correct me if I have misunderstood anything. Thank you very much.