Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH v7 0/3] vhost: fix device IOTLB feature lifecycle
@ 2026-08-20  8:03 Jia Jia
  2026-08-20  8:03 ` [PATCH v7 1/3] vhost: invalidate vring access on IOTLB transitions Jia Jia
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Jia Jia @ 2026-08-20  8:03 UTC (permalink / raw)
  To: stefanha, sgarzare, mst, jasowangio
  Cc: eperezma, weiyj.lk, kvm, virtualization, netdev, linux-kernel

Both vhost-vsock and vhost-net can leave the device IOTLB attached when
userspace clears VIRTIO_F_ACCESS_PLATFORM. They can also replace an
existing IOTLB with a new empty table when a later feature update keeps
ACCESS_PLATFORM enabled, for example when updating logging.

When the IOTLB mode changes, the vring addresses previously supplied by
userspace no longer have the same address-space meaning. Leaving those
addresses installed would allow an old IOVA to be used as a direct
userspace address after the IOTLB is detached.

This series invalidates the vring access state during IOTLB transitions,
makes IOTLB initialization idempotent, and uses a common teardown helper
for vhost-vsock and vhost-net. IOTLB mode changes are applied even while a
virtqueue backend is attached. The device-wide IOTLB is dropped first,
each virtqueue then clears its IOTLB pointer and cached ring access under
its own mutex, and the old table is freed only after every virtqueue has
completed the handoff.

A successful live mode change leaves the backend attached but invalidates
the cached vring addresses. Userspace must configure the vring addresses
for the new address mode before data processing can resume. When
ACCESS_PLATFORM is enabled, the usual IOTLB miss/update protocol
repopulates the new table.

Changes since v6:
- rebase on the current vhost tree;
- remove blank lines between commit trailers;
- drop unrelated error propagation changes from the backend patches.

Jia Jia (3):
  vhost: invalidate vring access on IOTLB transitions
  vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared
  vhost/net: discard IOTLB when ACCESS_PLATFORM is cleared

 drivers/vhost/net.c   |  2 ++
 drivers/vhost/vhost.c | 53 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/vhost/vhost.h |  1 +
 drivers/vhost/vsock.c |  4 +++-
 4 files changed, 58 insertions(+), 2 deletions(-)


base-commit: b282418bc366194677eafd1dad180d92254586ac
-- 
2.34.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v7 1/3] vhost: invalidate vring access on IOTLB transitions
  2026-08-20  8:03 [PATCH v7 0/3] vhost: fix device IOTLB feature lifecycle Jia Jia
@ 2026-08-20  8:03 ` Jia Jia
  2026-08-20  9:11   ` Stefano Garzarella
  2026-08-21  8:05   ` sashiko-bot
  2026-08-20  8:03 ` [PATCH v7 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared Jia Jia
  2026-08-20  8:03 ` [PATCH v7 3/3] vhost/net: " Jia Jia
  2 siblings, 2 replies; 11+ messages in thread
From: Jia Jia @ 2026-08-20  8:03 UTC (permalink / raw)
  To: stefanha, sgarzare, mst, jasowangio
  Cc: eperezma, weiyj.lk, kvm, virtualization, netdev, linux-kernel

When ACCESS_PLATFORM changes, the addresses cached in desc, avail, and
used change meaning with the address space. Clear the cached vring access
state when the device IOTLB is installed or removed so stale IOVAs cannot
be reused as direct userspace addresses.

Keep device IOTLB initialization idempotent and apply the mode change even
when a virtqueue backend is attached. Drop the device-wide IOTLB first,
then clear each VQ state under its own mutex, and keep the old table alive
until every VQ has completed the handoff.

A successful live mode change leaves the backend attached but invalidates
the cached vring addresses. Userspace must configure the vring addresses
for the new address mode before data processing can resume.

Fixes: 6b1e6cc7855b ("vhost: new device IOTLB API")
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
 drivers/vhost/vhost.c | 53 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/vhost/vhost.h |  1 +
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 14637cff0bd4..31fff9800045 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -344,6 +344,17 @@ static void __vhost_vq_meta_reset(struct vhost_virtqueue *vq)
 		vq->meta_iotlb[j] = NULL;
 }
 
+/* Caller must hold the virtqueue mutex. */
+static void vhost_vq_invalidate_access(struct vhost_virtqueue *vq)
+{
+	vq->desc = NULL;
+	vq->avail = NULL;
+	vq->used = NULL;
+	vq->log_used = false;
+	vq->log_addr = -1ull;
+	__vhost_vq_meta_reset(vq);
+}
+
 static void vhost_vq_meta_reset(struct vhost_dev *d)
 {
 	int i;
@@ -1918,6 +1929,9 @@ int vq_meta_prefetch(struct vhost_virtqueue *vq)
 {
 	unsigned int num = vq->num;
 
+	if (!vq->desc || !vq->avail || !vq->used)
+		return 0;
+
 	if (!vq->iotlb)
 		return 1;
 
@@ -2287,11 +2301,48 @@ 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;
+	if (!iotlb)
+		return;
+
+	/*
+	 * Drop the device-wide view first.  Each VQ then drops its
+	 * per-VQ view and its cached ring access under its own mutex.
+	 * Keep the old table alive until every VQ has completed this
+	 * handoff, since a worker may still be using it while waiting
+	 * for its VQ mutex.
+	 */
+	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_invalidate_access(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;
 	int i;
 
+	if (d->iotlb)
+		return 0;
+
 	if (max_iotlb_entries <= 0)
 		return -EINVAL;
 
@@ -2307,7 +2358,7 @@ int vhost_init_device_iotlb(struct vhost_dev *d)
 
 		mutex_lock(&vq->mutex);
 		vq->iotlb = niotlb;
-		__vhost_vq_meta_reset(vq);
+		vhost_vq_invalidate_access(vq);
 		mutex_unlock(&vq->mutex);
 	}
 
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] 11+ messages in thread

* [PATCH v7 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared
  2026-08-20  8:03 [PATCH v7 0/3] vhost: fix device IOTLB feature lifecycle Jia Jia
  2026-08-20  8:03 ` [PATCH v7 1/3] vhost: invalidate vring access on IOTLB transitions Jia Jia
@ 2026-08-20  8:03 ` Jia Jia
  2026-08-20  9:13   ` Stefano Garzarella
  2026-08-21  8:05   ` sashiko-bot
  2026-08-20  8:03 ` [PATCH v7 3/3] vhost/net: " Jia Jia
  2 siblings, 2 replies; 11+ messages in thread
From: Jia Jia @ 2026-08-20  8:03 UTC (permalink / raw)
  To: stefanha, sgarzare, mst, jasowangio
  Cc: eperezma, weiyj.lk, kvm, virtualization, netdev, linux-kernel

Clear the device IOTLB when userspace clears VIRTIO_F_ACCESS_PLATFORM.
Otherwise descriptor translation can continue to use mappings installed
before the feature change.

The common helper invalidates cached vring access and applies the
transition even while a backend is attached. The backend remains attached,
but userspace must configure the vring addresses for the new address mode
after a successful live transition.

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, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 9aaab6bb8061..e1e9d002d6ae 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -865,9 +865,11 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
 		goto err;
 	}
 
-	if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) {
+	if (features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) {
 		if (vhost_init_device_iotlb(&vsock->dev))
 			goto err;
+	} else {
+		vhost_clear_device_iotlb(&vsock->dev);
 	}
 
 	vsock->seqpacket_allow = features & (1ULL << VIRTIO_VSOCK_F_SEQPACKET);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v7 3/3] vhost/net: discard IOTLB when ACCESS_PLATFORM is cleared
  2026-08-20  8:03 [PATCH v7 0/3] vhost: fix device IOTLB feature lifecycle Jia Jia
  2026-08-20  8:03 ` [PATCH v7 1/3] vhost: invalidate vring access on IOTLB transitions Jia Jia
  2026-08-20  8:03 ` [PATCH v7 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared Jia Jia
@ 2026-08-20  8:03 ` Jia Jia
  2026-08-21  8:05   ` sashiko-bot
  2 siblings, 1 reply; 11+ messages in thread
From: Jia Jia @ 2026-08-20  8:03 UTC (permalink / raw)
  To: stefanha, sgarzare, mst, jasowangio
  Cc: eperezma, weiyj.lk, kvm, virtualization, netdev, linux-kernel

Apply the common device IOTLB teardown when userspace clears
VIRTIO_F_ACCESS_PLATFORM. This drops stale translations and avoids
rebuilding an existing IOTLB during feature updates that keep
ACCESS_PLATFORM enabled.

The transition invalidates cached vring access even with an attached
backend. The backend remains attached, but userspace must configure the
vring addresses for the new address mode after a successful live
transition.

Fixes: 6b1e6cc7855b ("vhost: new device IOTLB API")
Link: https://lore.kernel.org/all/20260726141158.1652386-1-physicalmtea@gmail.com/
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
 drivers/vhost/net.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 38d9c184082d..4d9d7c2216ed 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1696,6 +1696,8 @@ static int vhost_net_set_features(struct vhost_net *n, const u64 *features)
 	if (virtio_features_test_bit(features, VIRTIO_F_ACCESS_PLATFORM)) {
 		if (vhost_init_device_iotlb(&n->dev))
 			goto out_unlock;
+	} else {
+		vhost_clear_device_iotlb(&n->dev);
 	}
 
 	for (i = 0; i < VHOST_NET_VQ_MAX; ++i) {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 1/3] vhost: invalidate vring access on IOTLB transitions
  2026-08-20  8:03 ` [PATCH v7 1/3] vhost: invalidate vring access on IOTLB transitions Jia Jia
@ 2026-08-20  9:11   ` Stefano Garzarella
  2026-08-20 12:38     ` Jia Jia
  2026-08-21  8:05   ` sashiko-bot
  1 sibling, 1 reply; 11+ messages in thread
From: Stefano Garzarella @ 2026-08-20  9:11 UTC (permalink / raw)
  To: Jia Jia
  Cc: stefanha, mst, jasowangio, eperezma, weiyj.lk, kvm,
	virtualization, netdev, linux-kernel

On Thu, Aug 20, 2026 at 04:03:30PM +0800, Jia Jia wrote:
>When ACCESS_PLATFORM changes, the addresses cached in desc, avail, and
>used change meaning with the address space. Clear the cached vring access
>state when the device IOTLB is installed or removed so stale IOVAs cannot
>be reused as direct userspace addresses.
>
>Keep device IOTLB initialization idempotent and apply the mode change even
>when a virtqueue backend is attached. Drop the device-wide IOTLB first,
>then clear each VQ state under its own mutex, and keep the old table alive
>until every VQ has completed the handoff.
>
>A successful live mode change leaves the backend attached but invalidates
>the cached vring addresses. Userspace must configure the vring addresses
>for the new address mode before data processing can resume.
>
>Fixes: 6b1e6cc7855b ("vhost: new device IOTLB API")
>Signed-off-by: Jia Jia <physicalmtea@gmail.com>
>---
> drivers/vhost/vhost.c | 53 ++++++++++++++++++++++++++++++++++++++++++-
> drivers/vhost/vhost.h |  1 +
> 2 files changed, 53 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>index 14637cff0bd4..31fff9800045 100644
>--- a/drivers/vhost/vhost.c
>+++ b/drivers/vhost/vhost.c
>@@ -344,6 +344,17 @@ static void __vhost_vq_meta_reset(struct vhost_virtqueue *vq)
> 		vq->meta_iotlb[j] = NULL;
> }
>
>+/* Caller must hold the virtqueue mutex. */
>+static void vhost_vq_invalidate_access(struct vhost_virtqueue *vq)
>+{
>+	vq->desc = NULL;
>+	vq->avail = NULL;
>+	vq->used = NULL;
>+	vq->log_used = false;
>+	vq->log_addr = -1ull;
>+	__vhost_vq_meta_reset(vq);
>+}
>+
> static void vhost_vq_meta_reset(struct vhost_dev *d)
> {
> 	int i;
>@@ -1918,6 +1929,9 @@ int vq_meta_prefetch(struct vhost_virtqueue *vq)
> {
> 	unsigned int num = vq->num;
>
>+	if (!vq->desc || !vq->avail || !vq->used)
>+		return 0;
>+
> 	if (!vq->iotlb)
> 		return 1;
>
>@@ -2287,11 +2301,48 @@ 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)

Is this the right patch where introduce this function?

IMO should be introduced when we use it.

About that I'm not sure if it is better to squash the other 2 patches 
with this one, otherwise will be this bisectable?
I mean with just this patch applied (and without the other 2) who is 
going to free the old iotlb?

>+{
>+	struct vhost_iotlb *iotlb;
>+	int i;
>+
>+	iotlb = d->iotlb;
>+	if (!iotlb)
>+		return;
>+
>+	/*
>+	 * Drop the device-wide view first.  Each VQ then drops its
>+	 * per-VQ view and its cached ring access under its own mutex.
>+	 * Keep the old table alive until every VQ has completed this
>+	 * handoff, since a worker may still be using it while waiting
>+	 * for its VQ mutex.
>+	 */
>+	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_invalidate_access(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;
> 	int i;
>
>+	if (d->iotlb)
>+		return 0;
>+

IIUC after this patch `oiotlb` is always NULL, can we remove it?

Thanks,
Stefano

> 	if (max_iotlb_entries <= 0)
> 		return -EINVAL;
>
>@@ -2307,7 +2358,7 @@ int vhost_init_device_iotlb(struct vhost_dev *d)
>
> 		mutex_lock(&vq->mutex);
> 		vq->iotlb = niotlb;
>-		__vhost_vq_meta_reset(vq);
>+		vhost_vq_invalidate_access(vq);
> 		mutex_unlock(&vq->mutex);
> 	}
>
>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] 11+ messages in thread

* Re: [PATCH v7 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared
  2026-08-20  8:03 ` [PATCH v7 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared Jia Jia
@ 2026-08-20  9:13   ` Stefano Garzarella
  2026-08-20 12:26     ` Jia Jia
  2026-08-21  8:05   ` sashiko-bot
  1 sibling, 1 reply; 11+ messages in thread
From: Stefano Garzarella @ 2026-08-20  9:13 UTC (permalink / raw)
  To: Jia Jia
  Cc: stefanha, mst, jasowangio, eperezma, weiyj.lk, kvm,
	virtualization, netdev, linux-kernel

On Thu, Aug 20, 2026 at 04:03:31PM +0800, Jia Jia wrote:
>Clear the device IOTLB when userspace clears VIRTIO_F_ACCESS_PLATFORM.
>Otherwise descriptor translation can continue to use mappings installed
>before the feature change.
>
>The common helper invalidates cached vring access and applies the
>transition even while a backend is attached. The backend remains attached,
>but userspace must configure the vring addresses for the new address mode
>after a successful live transition.
>
>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, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index 9aaab6bb8061..e1e9d002d6ae 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -865,9 +865,11 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
> 		goto err;
> 	}
>
>-	if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) {
>+	if (features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) {

Unrelated change...

I think I've already pointed something like this in previous versions: 
for a patch that fixes a specific issue, these unrelated changes should 
be avoided. Please keep this in mind.

Stefano

> 		if (vhost_init_device_iotlb(&vsock->dev))
> 			goto err;
>+	} else {
>+		vhost_clear_device_iotlb(&vsock->dev);
> 	}
>
> 	vsock->seqpacket_allow = features & (1ULL << VIRTIO_VSOCK_F_SEQPACKET);
>-- 
>2.34.1
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared
  2026-08-20  9:13   ` Stefano Garzarella
@ 2026-08-20 12:26     ` Jia Jia
  0 siblings, 0 replies; 11+ messages in thread
From: Jia Jia @ 2026-08-20 12:26 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: stefanha, mst, jasowangio, eperezma, weiyj.lk, kvm,
	virtualization, netdev, linux-kernel

>
> On Thu, Aug 20, 2026 at 04:03:31PM +0800, Jia Jia wrote:
> >Clear the device IOTLB when userspace clears VIRTIO_F_ACCESS_PLATFORM.
> >Otherwise descriptor translation can continue to use mappings installed
> >before the feature change.
> >
> >The common helper invalidates cached vring access and applies the
> >transition even while a backend is attached. The backend remains attached,
> >but userspace must configure the vring addresses for the new address mode
> >after a successful live transition.
> >
> >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, 3 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> >index 9aaab6bb8061..e1e9d002d6ae 100644
> >--- a/drivers/vhost/vsock.c
> >+++ b/drivers/vhost/vsock.c
> >@@ -865,9 +865,11 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
> >               goto err;
> >       }
> >
> >-      if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) {
> >+      if (features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) {
>
> Unrelated change...
>
> I think I've already pointed something like this in previous versions:
> for a patch that fixes a specific issue, these unrelated changes should
> be avoided. Please keep this in mind.
>
> Stefano

You're right, thanks for the reminder. That was my oversight—I should
have cleaned it up more carefully. I'll remember that going forward.

>
> >               if (vhost_init_device_iotlb(&vsock->dev))
> >                       goto err;
> >+      } else {
> >+              vhost_clear_device_iotlb(&vsock->dev);
> >       }
> >
> >       vsock->seqpacket_allow = features & (1ULL << VIRTIO_VSOCK_F_SEQPACKET);
> >--
> >2.34.1
> >
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 1/3] vhost: invalidate vring access on IOTLB transitions
  2026-08-20  9:11   ` Stefano Garzarella
@ 2026-08-20 12:38     ` Jia Jia
  0 siblings, 0 replies; 11+ messages in thread
From: Jia Jia @ 2026-08-20 12:38 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: stefanha, mst, jasowangio, eperezma, weiyj.lk, kvm,
	virtualization, netdev, linux-kernel

>
> On Thu, Aug 20, 2026 at 04:03:30PM +0800, Jia Jia wrote:
> >When ACCESS_PLATFORM changes, the addresses cached in desc, avail, and
> >used change meaning with the address space. Clear the cached vring access
> >state when the device IOTLB is installed or removed so stale IOVAs cannot
> >be reused as direct userspace addresses.
> >
> >Keep device IOTLB initialization idempotent and apply the mode change even
> >when a virtqueue backend is attached. Drop the device-wide IOTLB first,
> >then clear each VQ state under its own mutex, and keep the old table alive
> >until every VQ has completed the handoff.
> >
> >A successful live mode change leaves the backend attached but invalidates
> >the cached vring addresses. Userspace must configure the vring addresses
> >for the new address mode before data processing can resume.
> >
> >Fixes: 6b1e6cc7855b ("vhost: new device IOTLB API")
> >Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> >---
> > drivers/vhost/vhost.c | 53 ++++++++++++++++++++++++++++++++++++++++++-
> > drivers/vhost/vhost.h |  1 +
> > 2 files changed, 53 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >index 14637cff0bd4..31fff9800045 100644
> >--- a/drivers/vhost/vhost.c
> >+++ b/drivers/vhost/vhost.c
> >@@ -344,6 +344,17 @@ static void __vhost_vq_meta_reset(struct vhost_virtqueue *vq)
> >               vq->meta_iotlb[j] = NULL;
> > }
> >
> >+/* Caller must hold the virtqueue mutex. */
> >+static void vhost_vq_invalidate_access(struct vhost_virtqueue *vq)
> >+{
> >+      vq->desc = NULL;
> >+      vq->avail = NULL;
> >+      vq->used = NULL;
> >+      vq->log_used = false;
> >+      vq->log_addr = -1ull;
> >+      __vhost_vq_meta_reset(vq);
> >+}
> >+
> > static void vhost_vq_meta_reset(struct vhost_dev *d)
> > {
> >       int i;
> >@@ -1918,6 +1929,9 @@ int vq_meta_prefetch(struct vhost_virtqueue *vq)
> > {
> >       unsigned int num = vq->num;
> >
> >+      if (!vq->desc || !vq->avail || !vq->used)
> >+              return 0;
> >+
> >       if (!vq->iotlb)
> >               return 1;
> >
> >@@ -2287,11 +2301,48 @@ 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)
>
> Is this the right patch where introduce this function?
>
> IMO should be introduced when we use it.
>
> About that I'm not sure if it is better to squash the other 2 patches
> with this one, otherwise will be this bisectable?
> I mean with just this patch applied (and without the other 2) who is
> going to free the old iotlb?
>

 I'll rework the series so that the helper is introduced together with
 the vhost-net and vhost-vsock callers, most likely by squashing the three
 patches. I'm also reviewing the existing IOTLB replacement semantics
 before preparing the next revision.

> >+{
> >+      struct vhost_iotlb *iotlb;
> >+      int i;
> >+
> >+      iotlb = d->iotlb;
> >+      if (!iotlb)
> >+              return;
> >+
> >+      /*
> >+       * Drop the device-wide view first.  Each VQ then drops its
> >+       * per-VQ view and its cached ring access under its own mutex.
> >+       * Keep the old table alive until every VQ has completed this
> >+       * handoff, since a worker may still be using it while waiting
> >+       * for its VQ mutex.
> >+       */
> >+      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_invalidate_access(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;
> >       int i;
> >
> >+      if (d->iotlb)
> >+              return 0;
> >+
>
> IIUC after this patch `oiotlb` is always NULL, can we remove it?
>
> Thanks,
> Stefano

  Thanks, I took another look. I don't think we should simply remove
  oiotlb while keeping the early return, since the early return itself may
  be too broad.

  One case I need to examine more carefully is vhost stop/start. QEMU
  unregisters the IOMMU listener while vhost is stopped. If an IOVA
  mapping changes during that interval, preserving the existing kernel
  IOTLB on the next VHOST_SET_FEATURES may retain an entry pointing to the
  old HVA. A later lookup could hit that stale entry, so no IOTLB miss
  would be reported and no replacement update would be triggered through
  the miss path for that IOVA.

  The original replacement path, including oiotlb, installs a new empty
  table and safely keeps the old table alive until all VQs have switched.
  I need to review this lifecycle more carefully before deciding whether
  that replacement semantic can be removed. A logging-only feature update
  may want to preserve the IOTLB, while a restart boundary may need a new
  one, and d->iotlb != NULL alone does not distinguish those cases.

>
> >       if (max_iotlb_entries <= 0)
> >               return -EINVAL;
> >
> >@@ -2307,7 +2358,7 @@ int vhost_init_device_iotlb(struct vhost_dev *d)
> >
> >               mutex_lock(&vq->mutex);
> >               vq->iotlb = niotlb;
> >-              __vhost_vq_meta_reset(vq);
> >+              vhost_vq_invalidate_access(vq);
> >               mutex_unlock(&vq->mutex);
> >       }
> >
> >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] 11+ messages in thread

* Re: [PATCH v7 1/3] vhost: invalidate vring access on IOTLB transitions
  2026-08-20  8:03 ` [PATCH v7 1/3] vhost: invalidate vring access on IOTLB transitions Jia Jia
  2026-08-20  9:11   ` Stefano Garzarella
@ 2026-08-21  8:05   ` sashiko-bot
  1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-21  8:05 UTC (permalink / raw)
  To: Jia Jia; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition in vhost_clear_device_iotlb() causes NULL pointer dereference or IOMMU bypass
--

--- Patch [1]: [PATCH v7 1/3] vhost: invalidate vring access on IOTLB transitions ---
commit da0274e257afa43e8d844051613c8dcc1f9d744f
Author: Jia Jia <physicalmtea@gmail.com>

vhost: invalidate vring access on IOTLB transitions

When ACCESS_PLATFORM changes, the addresses cached in desc, avail, and
used change meaning with the address space. Clear the cached vring access
state when the device IOTLB is installed or removed so stale IOVAs cannot
be reused as direct userspace addresses.

> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 14637cff0bd46..31fff98000450 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
[ ... ]
> @@ -2287,11 +2301,48 @@ 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;
> +	if (!iotlb)
> +		return;
> +
> +	/*
> +	 * Drop the device-wide view first.  Each VQ then drops its
> +	 * per-VQ view and its cached ring access under its own mutex.
> +	 * Keep the old table alive until every VQ has completed this
> +	 * handoff, since a worker may still be using it while waiting
> +	 * for its VQ mutex.
> +	 */
> +	d->iotlb = NULL;

[Severity: High]
Does clearing d->iotlb here without holding vq->mutex introduce a race
condition with the vhost worker thread?

If a userspace process dynamically clears the VIRTIO_F_ACCESS_PLATFORM
feature, this function clears d->iotlb without synchronization against
concurrent vhost worker threads.

Meanwhile, an active worker thread holding vq->mutex might be calling
translate_desc():

drivers/vhost/vhost.c:translate_desc() {
    ...
    struct vhost_iotlb *umem = dev->iotlb ? dev->iotlb : dev->umem;
    ...
}

Because there is no READ_ONCE() or other synchronization on dev->iotlb, could
the compiler double-read dev->iotlb, observing a non-NULL value for the
condition but NULL for the assignment, leaving umem = NULL?

Alternatively, if dev->umem was never populated, reading dev->iotlb as NULL
also leaves umem as NULL.  The worker then calls vhost_iotlb_itree_first()
which dereferences umem->root, potentially causing a host kernel panic.

> +
> +	for (i = 0; i < d->nvqs; ++i) {
> +		struct vhost_virtqueue *vq = d->vqs[i];
> +
> +		mutex_lock(&vq->mutex);
> +		vq->iotlb = NULL;
> +		vhost_vq_invalidate_access(vq);
> +		mutex_unlock(&vq->mutex);
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820080332.313933-1-physicalmtea@gmail.com?part=1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 3/3] vhost/net: discard IOTLB when ACCESS_PLATFORM is cleared
  2026-08-20  8:03 ` [PATCH v7 3/3] vhost/net: " Jia Jia
@ 2026-08-21  8:05   ` sashiko-bot
  0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-21  8:05 UTC (permalink / raw)
  To: Jia Jia; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Concurrent execution of vhost_clear_device_iotlb() and translate_desc() causes the vhost worker thread to mistakenly translate IOVAs using the guest physical memory map (GPA map), leading to arbitrary guest memory corruption.
--

--- Patch [3]: [PATCH v7 3/3] vhost/net: discard IOTLB when ACCESS_PLATFORM is cleared ---
commit 4247ba53ba530a29217ab642534cab65e8a1846d
Author: Jia Jia <physicalmtea@gmail.com>
vhost/net: discard IOTLB when ACCESS_PLATFORM is cleared

This commit applies common device IOTLB teardown when userspace clears
VIRTIO_F_ACCESS_PLATFORM. This avoids rebuilding an existing IOTLB during
feature updates that keep it enabled, while requiring userspace to
reconfigure vring addresses for the new address mode.

Link: https://lore.kernel.org/all/20260726141158.1652386-1-physicalmtea@gmail.com/

> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 38d9c184082d0..4d9d7c2216ed5 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -1696,6 +1696,8 @@ static int vhost_net_set_features(struct vhost_net *n, const u64 *features)
>  	if (virtio_features_test_bit(features, VIRTIO_F_ACCESS_PLATFORM)) {
>  		if (vhost_init_device_iotlb(&n->dev))
>  			goto out_unlock;
> +	} else {
> +		vhost_clear_device_iotlb(&n->dev);

[Severity: Critical]
Does this introduce a race condition with the vhost worker thread that
could lead to guest memory corruption?

When userspace clears VIRTIO_F_ACCESS_PLATFORM on a running device, this
code calls vhost_clear_device_iotlb() while the backend remains attached.
Looking at vhost_clear_device_iotlb() in drivers/vhost/vhost.c, it drops
the global device IOTLB pointer before taking the per-VQ mutex:

drivers/vhost/vhost.c:vhost_clear_device_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;
	...
}

Concurrently, if the vhost worker thread is active, it will have acquired
vq->mutex and proceed to translate addresses. However, translate_desc()
evaluates the global dev->iotlb pointer rather than the protected
vq->iotlb pointer:

drivers/vhost/vhost.c:translate_desc() {
	...
	struct vhost_iotlb *umem = dev->iotlb ? dev->iotlb : dev->umem;
	...
}

Since d->iotlb was cleared without the per-VQ mutex, the active worker
thread might observe dev->iotlb as NULL and incorrectly fall back to
dev->umem (the guest physical memory map).

Could this cause IOVAs to be mistakenly translated as GPAs, leading the
kernel to read or write network payloads to the wrong Host Virtual Addresses?

>  	}
>  
>  	for (i = 0; i < VHOST_NET_VQ_MAX; ++i) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820080332.313933-1-physicalmtea@gmail.com?part=3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v7 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared
  2026-08-20  8:03 ` [PATCH v7 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared Jia Jia
  2026-08-20  9:13   ` Stefano Garzarella
@ 2026-08-21  8:05   ` sashiko-bot
  1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-21  8:05 UTC (permalink / raw)
  To: Jia Jia; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Concurrent lockless clearing of dev->iotlb causes a TOCTOU race in translate_desc(), leading to a NULL pointer dereference or out-of-bounds memory corruption.
--

--- Patch [2]: [PATCH v7 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared ---
commit feb57c09c27392564744aed50569fa28daa5a083
Author: Jia Jia <physicalmtea@gmail.com>

vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared

Clear the device IOTLB when userspace clears VIRTIO_F_ACCESS_PLATFORM.
Otherwise descriptor translation can continue to use mappings installed
before the feature change.

> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index 9aaab6bb8061c..e1e9d002d6aeb 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -865,9 +865,11 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
>  		goto err;
>  	}
>  
> -	if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) {
> +	if (features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)) {
>  		if (vhost_init_device_iotlb(&vsock->dev))
>  			goto err;
> +	} else {
> +		vhost_clear_device_iotlb(&vsock->dev);

[Severity: Critical]
Does this concurrent clearing of the IOTLB introduce a regression with
active backend workers?

Looking at vhost_clear_device_iotlb(), it sets d->iotlb to NULL locklessly
before acquiring the vq->mutex:

drivers/vhost/vhost.c:vhost_clear_device_iotlb() {
    ...
    d->iotlb = NULL;

    for (i = 0; i < d->nvqs; ++i) {
        struct vhost_virtqueue *vq = d->vqs[i];
        mutex_lock(&vq->mutex);
    ...
}

If a virtqueue backend worker is actively processing descriptors via
translate_desc() concurrently, it evaluates dev->iotlb without a READ_ONCE():

drivers/vhost/vhost.c:translate_desc() {
    ...
    struct vhost_iotlb *umem = dev->iotlb ? dev->iotlb : dev->umem;
    ...
}

Can the compiler emit multiple fetches here, such that the first fetch is
non-NULL but the second fetch returns NULL? If umem becomes NULL, passing it
to vhost_iotlb_itree_first() could cause a NULL pointer dereference.

Additionally, if the worker sees dev->iotlb as NULL immediately, will it
incorrectly fall back to dev->umem and mistakenly translate the IOVA using
the GPA memory map, leading to memory corruption?

>  	}
>  
>  	vsock->seqpacket_allow = features & (1ULL << VIRTIO_VSOCK_F_SEQPACKET);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820080332.313933-1-physicalmtea@gmail.com?part=2

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-08-21  8:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20  8:03 [PATCH v7 0/3] vhost: fix device IOTLB feature lifecycle Jia Jia
2026-08-20  8:03 ` [PATCH v7 1/3] vhost: invalidate vring access on IOTLB transitions Jia Jia
2026-08-20  9:11   ` Stefano Garzarella
2026-08-20 12:38     ` Jia Jia
2026-08-21  8:05   ` sashiko-bot
2026-08-20  8:03 ` [PATCH v7 2/3] vhost/vsock: discard IOTLB when ACCESS_PLATFORM is cleared Jia Jia
2026-08-20  9:13   ` Stefano Garzarella
2026-08-20 12:26     ` Jia Jia
2026-08-21  8:05   ` sashiko-bot
2026-08-20  8:03 ` [PATCH v7 3/3] vhost/net: " Jia Jia
2026-08-21  8:05   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox