Re: [PATCH v4 3/3] vhost/vsock: keep IOTLB across feature updates

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch fails to apply its fixes consistently across the vhost subsystem. The patch correctly fixes the issue of dropped IOTLB translations and missing cleanup on VHOST_SET_FEATURES for vhost-vsock, but completely misses identical bugs in vhost-net. Because `vhost_net_set_features()` lacks the `!n->dev.iotlb` existence check, it continues to unconditionally wipe and reallocate the IOTLB on every feature update when `VIRTIO_F_ACCESS_PLATFORM` is set. Furthermore, it lacks the teardown logic to clear the IOTLB when the feature is dynamically disabled, leaving stale translations active.
--

--- Patch [3]: [PATCH v4 3/3] vhost/vsock: keep IOTLB across feature updates ---
commit 43e7ccb915735bc2e366b5df4fab08c7d9c89e1e
Author: Jia Jia <[email protected]>

vhost/vsock: keep IOTLB across feature updates

This commit updates vhost-vsock to only initialize the device IOTLB when one
does not already exist. This prevents dropping valid translations and forcing
avoidable misses when features like logging are updated while the device is
running.

> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index a18fe1335349b..b5536075fc397 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -886,7 +886,8 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
>  	    vsock->dev.iotlb)
>  		vhost_clear_device_iotlb(&vsock->dev);
>  
> -	if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) {
> +	if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
> +	    !vsock->dev.iotlb) {
>  		if (vhost_init_device_iotlb(&vsock->dev))
>  			goto err;
>  	}

[Severity: High]
This is a pre-existing issue, but should this same fix be applied consistently
to the rest of the vhost subsystem?

Looking at vhost_net_set_features() in drivers/vhost/net.c, it lacks this
existence check and continues to unconditionally wipe and reallocate the IOTLB
on every feature update when VIRTIO_F_ACCESS_PLATFORM is set:

drivers/vhost/net.c:vhost_net_set_features() {
...
	if (virtio_features_test_bit(features, VIRTIO_F_ACCESS_PLATFORM)) {
		if (vhost_init_device_iotlb(&n->dev))
			goto out_unlock;
	}
...
}

This unconditionally reallocates the IOTLB and drops all valid translations,
which could cause a storm of IOTLB misses and latency spikes during events like
live migration when VHOST_F_LOG_ALL is toggled. 

Additionally, vhost-net appears to be missing the teardown logic to clear the
IOTLB when the feature is dynamically disabled, which could leave stale
translations active.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3