* [PATCH v4 1/3] vhost: add helper to clear device IOTLB
2026-08-14 7:29 [PATCH v4 0/3] vhost/vsock: fix device IOTLB feature lifecycle Jia Jia
@ 2026-08-14 7:29 ` Jia Jia
2026-08-14 7:58 ` Stefano Garzarella
2026-08-15 7:29 ` sashiko-bot
2026-08-14 7:29 ` [PATCH v4 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared Jia Jia
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Jia Jia @ 2026-08-14 7:29 UTC (permalink / raw)
To: Stefan Hajnoczi, Stefano Garzarella, Michael S . Tsirkin,
Jason Wang
Cc: Eugenio Pérez, kvm, virtualization, netdev, linux-kernel
The device IOTLB is shared by vhost backends, but clearing it requires
dropping each virtqueue's IOTLB pointer and metadata cache before the
old table is freed. Add a common helper for this teardown sequence so
backend-specific feature code only needs to decide when the table must
be cleared.
The caller must hold the device mutex. The helper does not update
acked_features; backends continue to update that state in their own
virtqueue loops.
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
drivers/vhost/vhost.c | 24 ++++++++++++++++++++++++
drivers/vhost/vhost.h | 1 +
2 files changed, 25 insertions(+)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 269efad90369..7eaa61d9ceeb 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2283,6 +2283,30 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
}
EXPORT_SYMBOL_GPL(vhost_vring_ioctl);
+/* Caller must hold the device mutex. */
+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);
+
int vhost_init_device_iotlb(struct vhost_dev *d)
{
struct vhost_iotlb *niotlb, *oiotlb;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 0192ade6e749..3c75e8089373 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -277,6 +277,7 @@ ssize_t vhost_chr_read_iter(struct vhost_dev *dev, struct iov_iter *to,
int noblock);
ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
struct iov_iter *from);
+void vhost_clear_device_iotlb(struct vhost_dev *d);
int vhost_init_device_iotlb(struct vhost_dev *d);
void vhost_iotlb_map_free(struct vhost_iotlb *iotlb,
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v4 1/3] vhost: add helper to clear device IOTLB
2026-08-14 7:29 ` [PATCH v4 1/3] vhost: add helper to clear device IOTLB Jia Jia
@ 2026-08-14 7:58 ` Stefano Garzarella
2026-08-15 7:29 ` sashiko-bot
1 sibling, 0 replies; 13+ messages in thread
From: Stefano Garzarella @ 2026-08-14 7:58 UTC (permalink / raw)
To: Jia Jia
Cc: Stefan Hajnoczi, Michael S . Tsirkin, Jason Wang,
Eugenio Pérez, kvm, virtualization, netdev, linux-kernel
On Fri, Aug 14, 2026 at 03:29:01PM +0800, Jia Jia wrote:
>The device IOTLB is shared by vhost backends, but clearing it requires
>dropping each virtqueue's IOTLB pointer and metadata cache before the
>old table is freed. Add a common helper for this teardown sequence so
>backend-specific feature code only needs to decide when the table must
>be cleared.
>
>The caller must hold the device mutex. The helper does not update
>acked_features; backends continue to update that state in their own
>virtqueue loops.
>
>Signed-off-by: Jia Jia <physicalmtea@gmail.com>
>---
> drivers/vhost/vhost.c | 24 ++++++++++++++++++++++++
> drivers/vhost/vhost.h | 1 +
> 2 files changed, 25 insertions(+)
>
>diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>index 269efad90369..7eaa61d9ceeb 100644
>--- a/drivers/vhost/vhost.c
>+++ b/drivers/vhost/vhost.c
>@@ -2283,6 +2283,30 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
> }
> EXPORT_SYMBOL_GPL(vhost_vring_ioctl);
>
>+/* Caller must hold the device mutex. */
>+void vhost_clear_device_iotlb(struct vhost_dev *d)
>+{
>+ struct vhost_iotlb *iotlb;
>+ int i;
Should we move the check of `d->iotlb` here? so the callers doesn't need
to access to an internal details.
Stefano
>+
>+ 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);
>+
> int vhost_init_device_iotlb(struct vhost_dev *d)
> {
> struct vhost_iotlb *niotlb, *oiotlb;
>diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
>index 0192ade6e749..3c75e8089373 100644
>--- a/drivers/vhost/vhost.h
>+++ b/drivers/vhost/vhost.h
>@@ -277,6 +277,7 @@ ssize_t vhost_chr_read_iter(struct vhost_dev *dev, struct iov_iter *to,
> int noblock);
> ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
> struct iov_iter *from);
>+void vhost_clear_device_iotlb(struct vhost_dev *d);
> int vhost_init_device_iotlb(struct vhost_dev *d);
>
> void vhost_iotlb_map_free(struct vhost_iotlb *iotlb,
>--
>2.34.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v4 1/3] vhost: add helper to clear device IOTLB
2026-08-14 7:29 ` [PATCH v4 1/3] vhost: add helper to clear device IOTLB Jia Jia
2026-08-14 7:58 ` Stefano Garzarella
@ 2026-08-15 7:29 ` sashiko-bot
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-15 7:29 UTC (permalink / raw)
To: Jia Jia; +Cc: kvm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Setting `d->iotlb = NULL` before acquiring `vq->mutex` in `vhost_clear_device_iotlb` creates a race condition with active vhost worker threads. This exposes the host kernel to a NULL pointer dereference or an IOMMU bypass (TOCTOU vulnerability) leading to arbitrary host memory corruption.
--
--- Patch [1]: [PATCH v4 1/3] vhost: add helper to clear device IOTLB ---
commit 721258d5c0d5f3c1aeaefcc8dc997b5bbedcd9c1
Author: Jia Jia <physicalmtea@gmail.com>
vhost: add helper to clear device IOTLB
This patch adds a common helper for tearing down the device IOTLB shared
by vhost backends. It drops each virtqueue's IOTLB pointer and metadata
cache before freeing the old table.
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index a0c1d54019aac..0861406f3246f 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
[ ... ]
> @@ -2298,6 +2298,30 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
> }
> EXPORT_SYMBOL_GPL(vhost_vring_ioctl);
>
> +/* Caller must hold the device mutex. */
> +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) {
[Severity: High]
Does setting d->iotlb = NULL before acquiring vq->mutex create a race
condition with active vhost worker threads?
If a worker thread is concurrently executing vhost_copy_to_user() while
holding vq->mutex, it could call translate_desc(), which locklessly
reads dev->iotlb:
struct vhost_iotlb *umem = dev->iotlb ? dev->iotlb : dev->umem;
Since d->iotlb was already set to NULL by vhost_clear_device_iotlb(),
the worker thread might mistakenly fall back to a potentially NULL
dev->umem.
This could bypass the IOTLB translation and expose the host to memory
corruption via forged GIOVAs or result in a NULL pointer dereference.
> + 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);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260814072903.124991-1-physicalmtea@gmail.com?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared
2026-08-14 7:29 [PATCH v4 0/3] vhost/vsock: fix device IOTLB feature lifecycle Jia Jia
2026-08-14 7:29 ` [PATCH v4 1/3] vhost: add helper to clear device IOTLB Jia Jia
@ 2026-08-14 7:29 ` Jia Jia
2026-08-14 7:58 ` Stefano Garzarella
2026-08-15 7:29 ` sashiko-bot
2026-08-14 7:29 ` [PATCH v4 3/3] vhost/vsock: keep IOTLB across feature updates Jia Jia
2026-08-14 7:52 ` [PATCH v4 0/3] vhost/vsock: fix device IOTLB feature lifecycle Stefano Garzarella
3 siblings, 2 replies; 13+ messages in thread
From: Jia Jia @ 2026-08-14 7:29 UTC (permalink / raw)
To: Stefan Hajnoczi, Stefano Garzarella, Michael S . Tsirkin,
Jason Wang
Cc: Eugenio Pérez, kvm, virtualization, netdev, linux-kernel
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.
Use the common vhost helper to detach the device IOTLB before
acknowledging a feature mask without ACCESS_PLATFORM. The helper clears
each virtqueue's IOTLB pointer and metadata cache under its mutex, then
frees the old IOTLB after all virtqueues have dropped their references.
Fixes: e13a6915a03f ("vhost/vsock: add IOTLB API support")
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
drivers/vhost/vsock.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 9aaab6bb8061..b69c260eaeff 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -863,7 +863,11 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
if ((features & (1 << VHOST_F_LOG_ALL)) &&
!vhost_log_access_ok(&vsock->dev)) {
goto err;
}
+ if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
+ vsock->dev.iotlb)
+ vhost_clear_device_iotlb(&vsock->dev);
+
if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) {
if (vhost_init_device_iotlb(&vsock->dev))
goto err;
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v4 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared
2026-08-14 7:29 ` [PATCH v4 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared Jia Jia
@ 2026-08-14 7:58 ` Stefano Garzarella
2026-08-15 7:29 ` sashiko-bot
1 sibling, 0 replies; 13+ messages in thread
From: Stefano Garzarella @ 2026-08-14 7:58 UTC (permalink / raw)
To: Jia Jia
Cc: Stefan Hajnoczi, Michael S . Tsirkin, Jason Wang,
Eugenio Pérez, kvm, virtualization, netdev, linux-kernel
On Fri, Aug 14, 2026 at 03:29:02PM +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.
>
>Use the common vhost helper to detach the device IOTLB before
>acknowledging a feature mask without ACCESS_PLATFORM. The helper clears
>each virtqueue's IOTLB pointer and metadata cache under its mutex, then
>frees the old IOTLB after all virtqueues have dropped their references.
>
>Fixes: e13a6915a03f ("vhost/vsock: add IOTLB API support")
>Suggested-by: Michael S. Tsirkin <mst@redhat.com>
>Signed-off-by: Jia Jia <physicalmtea@gmail.com>
>---
> drivers/vhost/vsock.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index 9aaab6bb8061..b69c260eaeff 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -863,7 +863,11 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
> if ((features & (1 << VHOST_F_LOG_ALL)) &&
> !vhost_log_access_ok(&vsock->dev)) {
> goto err;
> }
>
>+ if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
>+ vsock->dev.iotlb)
>+ vhost_clear_device_iotlb(&vsock->dev);
>+
Why checking the feature again instead of adding an `else if` in the
already existing check?
I mean this...
> if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) {
> if (vhost_init_device_iotlb(&vsock->dev))
> goto err;
} else if (vsock->dev.iotlb) {
vhost_clear_device_iotlb(&vsock->dev);
}
>-- 2.34.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v4 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared
2026-08-14 7:29 ` [PATCH v4 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared Jia Jia
2026-08-14 7:58 ` Stefano Garzarella
@ 2026-08-15 7:29 ` sashiko-bot
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-15 7:29 UTC (permalink / raw)
To: Jia Jia; +Cc: kvm
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Critical] Arbitrary kernel write and privilege escalation due to bypassed `access_ok()` bounds checking when VIRTIO_F_ACCESS_PLATFORM is cleared dynamically.
- [High] Data race causing NULL pointer dereference in `translate_desc()` due to lockless `dev->iotlb` teardown.
Pre-existing issues:
- [High] The vhost-net subsystem suffers from the exact same bug that this patch fixes in vhost_vsock, but `vhost_net_set_features()` was not updated.
--
--- Patch [2]: [PATCH v4 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared ---
commit e884f4b90201c9e0d904641890e2f9e4ea704c9a
Author: Jia Jia <physicalmtea@gmail.com>
vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared
This commit addresses an issue in vhost_vsock_set_features where the device
iotlb remains attached when userspace clears VIRTIO_F_ACCESS_PLATFORM. It
introduces a call to vhost_clear_device_iotlb to detach the iotlb and clear
the metadata cache for each virtqueue. This prevents descriptors from using
stale translations installed prior to the feature change.
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index 86f25ff80722d..a18fe1335349b 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
[ ... ]
> @@ -882,6 +882,10 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
> goto err;
> }
>
> + if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
> + vsock->dev.iotlb)
> + vhost_clear_device_iotlb(&vsock->dev);
[Severity: Critical]
If userspace clears the VIRTIO_F_ACCESS_PLATFORM feature while the device is
running, can this allow arbitrary writes to host kernel memory?
When VIRTIO_F_ACCESS_PLATFORM is enabled, vq_access_ok() intentionally
returns true without validating addresses, deferring validation to the
iotlb.
If userspace provides arbitrary kernel addresses via VHOST_SET_VRING_ADDR,
starts the device, and then dynamically clears the feature,
vhost_clear_device_iotlb() sets vq->iotlb to NULL on the fly.
Since the existing vring addresses are never re-validated, a concurrent
worker thread calling vhost_put_used() -> vhost_copy_to_user() will
observe !vq->iotlb and fall back to __copy_to_user().
Because __copy_to_user() omits bounds checking, it will write to the
previously configured unvalidated kernel addresses.
Should the vring addresses be re-validated, or the device stopped, before
dropping the iotlb?
[Severity: High]
Could clearing the iotlb here create a data race with concurrent worker
threads?
Looking at vhost_clear_device_iotlb() in drivers/vhost/vhost.c, it
locklessly sets d->iotlb to NULL before acquiring vq->mutex for the
virtqueues:
drivers/vhost/vhost.c:vhost_clear_device_iotlb() {
iotlb = d->iotlb;
d->iotlb = NULL;
for (i = 0; i < d->nvqs; ++i) {
struct vhost_virtqueue *vq = d->vqs[i];
mutex_lock(&vq->mutex);
...
}
If a worker thread is concurrently executing vhost_copy_to_user() ->
translate_desc(), it holds vq->mutex. The ioctl thread will block on
the mutex but has already cleared d->iotlb locklessly.
In translate_desc(), the code evaluates the iotlb pointer:
drivers/vhost/vhost.c:translate_desc() {
...
struct vhost_iotlb *umem = dev->iotlb ? dev->iotlb : dev->umem;
...
}
It observes dev->iotlb as NULL and falls back to dev->umem. Since
dev->umem is NULL when using an iotlb, passing umem to
vhost_iotlb_itree_first() will unconditionally dereference iotlb->root,
crashing the host kernel.
Would it be safer to clear d->iotlb after the virtqueues are quiesced?
[Severity: High]
This is a pre-existing issue, but is vhost_net_set_features() in
drivers/vhost/net.c missing the exact same cleanup introduced here?
When userspace clears VIRTIO_F_ACCESS_PLATFORM via the
VHOST_SET_FEATURES ioctl on a vhost-net device, n->dev.iotlb is left
attached. This completely misses the newly introduced
vhost_clear_device_iotlb() cleanup.
Should the identical fix be applied to vhost-net to prevent memory
isolation bypasses via stale translation entries?
> +
> if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) {
> if (vhost_init_device_iotlb(&vsock->dev))
> goto err;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260814072903.124991-1-physicalmtea@gmail.com?part=2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 3/3] vhost/vsock: keep IOTLB across feature updates
2026-08-14 7:29 [PATCH v4 0/3] vhost/vsock: fix device IOTLB feature lifecycle Jia Jia
2026-08-14 7:29 ` [PATCH v4 1/3] vhost: add helper to clear device IOTLB Jia Jia
2026-08-14 7:29 ` [PATCH v4 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared Jia Jia
@ 2026-08-14 7:29 ` Jia Jia
2026-08-14 8:02 ` Stefano Garzarella
2026-08-15 7:29 ` sashiko-bot
2026-08-14 7:52 ` [PATCH v4 0/3] vhost/vsock: fix device IOTLB feature lifecycle Stefano Garzarella
3 siblings, 2 replies; 13+ messages in thread
From: Jia Jia @ 2026-08-14 7:29 UTC (permalink / raw)
To: Stefan Hajnoczi, Stefano Garzarella, Michael S . Tsirkin,
Jason Wang
Cc: Eugenio Pérez, kvm, virtualization, netdev, linux-kernel
VHOST_SET_FEATURES is also used to update logging while a device is
running. When ACCESS_PLATFORM stays enabled, allocating a new empty
IOTLB on every call drops valid translations and forces avoidable
misses.
Initialize the device IOTLB only when one does not already exist.
Fixes: e13a6915a03f ("vhost/vsock: add IOTLB API support")
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
drivers/vhost/vsock.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index b69c260eaeff..9142fa1143b2 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -869,6 +869,7 @@ 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;
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v4 3/3] vhost/vsock: keep IOTLB across feature updates
2026-08-14 7:29 ` [PATCH v4 3/3] vhost/vsock: keep IOTLB across feature updates Jia Jia
@ 2026-08-14 8:02 ` Stefano Garzarella
2026-08-14 11:56 ` Jia Jia
2026-08-15 7:29 ` sashiko-bot
1 sibling, 1 reply; 13+ messages in thread
From: Stefano Garzarella @ 2026-08-14 8:02 UTC (permalink / raw)
To: Jia Jia
Cc: Stefan Hajnoczi, Michael S . Tsirkin, Jason Wang,
Eugenio Pérez, kvm, virtualization, netdev, linux-kernel
On Fri, Aug 14, 2026 at 03:29:03PM +0800, Jia Jia wrote:
>VHOST_SET_FEATURES is also used to update logging while a device is
>running. When ACCESS_PLATFORM stays enabled, allocating a new empty
>IOTLB on every call drops valid translations and forces avoidable
>misses.
>
>Initialize the device IOTLB only when one does not already exist.
>
>Fixes: e13a6915a03f ("vhost/vsock: add IOTLB API support")
>Signed-off-by: Jia Jia <physicalmtea@gmail.com>
>---
> drivers/vhost/vsock.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index b69c260eaeff..9142fa1143b2 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -869,6 +869,7 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
> vsock->dev.iotlb)
> vhost_clear_device_iotlb(&vsock->dev);
>
Ah, okay, now I see why you did it in the other patch, but if we move
dev.iotlb checks in the functions we can still do that.
>- if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) {
>+ if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) &&
>+ !vsock->dev.iotlb) {
> if (vhost_init_device_iotlb(&vsock->dev))
IIUC vhost_init_device_iotlb() handles the case where dev.iotlb is
already initialized and override it. Here we are preventing that.
Will we do the same in vhost-net? If yes, all the callers will call
vhost_init_device_iotlb() with `dev.iotlb` NULL, so should we remove
that behaviour and add the check we are adding here inside the
vhost_init_device_iotlb()?
Or there is an use case where vhost_init_device_iotlb() is called with
an already initialized dev.iotlb ?
Thanks,
Stefano
> goto err;
>--
>2.34.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v4 3/3] vhost/vsock: keep IOTLB across feature updates
2026-08-14 8:02 ` Stefano Garzarella
@ 2026-08-14 11:56 ` Jia Jia
0 siblings, 0 replies; 13+ messages in thread
From: Jia Jia @ 2026-08-14 11:56 UTC (permalink / raw)
To: sgarzare
Cc: Jia Jia, Stefan Hajnoczi, Michael S . Tsirkin, Jason Wang,
Eugenio Pérez, kvm, virtualization, netdev, linux-kernel
> Will we do the same in vhost-net? If yes, all the callers will call
> vhost_init_device_iotlb() with dev.iotlb NULL, so should we remove
> that behaviour and add the check we are adding here inside the
> vhost_init_device_iotlb()?
>
> Or there is an use case where vhost_init_device_iotlb() is called with
> an already initialized dev.iotlb ?
I will check whether there is any valid use case for calling
vhost_init_device_iotlb() with an already initialized dev->iotlb. If
there is no such use case, I will move the check into
vhost_init_device_iotlb() and apply the same change to vhost-net as
well.
Thanks,
Jia
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/3] vhost/vsock: keep IOTLB across feature updates
2026-08-14 7:29 ` [PATCH v4 3/3] vhost/vsock: keep IOTLB across feature updates Jia Jia
2026-08-14 8:02 ` Stefano Garzarella
@ 2026-08-15 7:29 ` sashiko-bot
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-15 7:29 UTC (permalink / raw)
To: Jia Jia; +Cc: kvm
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 <physicalmtea@gmail.com>
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/20260814072903.124991-1-physicalmtea@gmail.com?part=3
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 0/3] vhost/vsock: fix device IOTLB feature lifecycle
2026-08-14 7:29 [PATCH v4 0/3] vhost/vsock: fix device IOTLB feature lifecycle Jia Jia
` (2 preceding siblings ...)
2026-08-14 7:29 ` [PATCH v4 3/3] vhost/vsock: keep IOTLB across feature updates Jia Jia
@ 2026-08-14 7:52 ` Stefano Garzarella
2026-08-14 11:56 ` Jia Jia
3 siblings, 1 reply; 13+ messages in thread
From: Stefano Garzarella @ 2026-08-14 7:52 UTC (permalink / raw)
To: Jia Jia
Cc: Stefan Hajnoczi, Michael S . Tsirkin, Jason Wang,
Eugenio Pérez, kvm, virtualization, netdev, linux-kernel
On Fri, Aug 14, 2026 at 03:29:00PM +0800, Jia Jia wrote:
>vhost-vsock leaves the device IOTLB attached when userspace clears
>VIRTIO_F_ACCESS_PLATFORM. It also replaces an existing IOTLB with an
>empty one whenever a later feature update keeps ACCESS_PLATFORM enabled.
>
>Patch 1 adds a common vhost helper that detaches the device IOTLB from
>all virtqueues, clears their metadata caches, drops queued IOTLB miss
>messages, and frees the old table after all virtqueues have released
>their pointers.
In the v3 you mentioned that this could be used also in vhost-net and
maybe vhost-scsi. Would it be better to include the other patches in
this series as well, since we're basically fixing the same issues, or
have they already been merged?
Thanks,
Stefano
>
>Patch 2 uses the helper when vhost-vsock clears ACCESS_PLATFORM. The
>acked_features update remains in the vhost-vsock-specific loop.
>
>Patch 3 avoids replacing an existing IOTLB when ACCESS_PLATFORM remains
>enabled across a feature update.
>
>Changes since v3:
>- move the IOTLB teardown helper from vhost-vsock into common vhost code
>- keep acked_features updates in the backend-specific loop
>- add the common helper as a separate first patch
>
>Jia Jia (3):
> vhost: add helper to clear device IOTLB
> vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared
> vhost/vsock: keep IOTLB across feature updates
>
> drivers/vhost/vhost.c | 24 ++++++++++++++++++++++++
> drivers/vhost/vhost.h | 1 +
> drivers/vhost/vsock.c | 7 ++++++-
> 3 files changed, 31 insertions(+), 1 deletion(-)
>
>--
>2.34.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v4 0/3] vhost/vsock: fix device IOTLB feature lifecycle
2026-08-14 7:52 ` [PATCH v4 0/3] vhost/vsock: fix device IOTLB feature lifecycle Stefano Garzarella
@ 2026-08-14 11:56 ` Jia Jia
0 siblings, 0 replies; 13+ messages in thread
From: Jia Jia @ 2026-08-14 11:56 UTC (permalink / raw)
To: sgarzare
Cc: Jia Jia, Stefan Hajnoczi, Michael S . Tsirkin, Jason Wang,
Eugenio Pérez, kvm, virtualization, netdev, linux-kernel
> In the v3 you mentioned that this could be used also in vhost-net and
> maybe vhost-scsi. Would it be better to include the other patches in
> this series as well, since we're basically fixing the same issues, or
> have they already been merged?
I did not explain this clearly before. vhost-scsi itself does not
negotiate VIRTIO_F_ACCESS_PLATFORM; the fix I referred to addressed a
different issue. See:
20260726144314.1652934-1-physicalmtea@gmail.com
The issue addressed by this series also led me to find corresponding
problems in vhost-net and vhost-vsock. It now appears that these are the
relevant backends here, so I will include the vhost-net changes in this
series as well.
Thanks,
Jia
^ permalink raw reply [flat|nested] 13+ messages in thread