[RFC PATCH 08/12] virtio_pci: support VIRTIO_F_DMB

Alexander Graf <[email protected]> Sun, 9 Aug 2026 18:20:06 +0000
Newsgroups dev.linux.lists.virtualization,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Let a modern virtio-pci device place its virtqueues and the buffers they
reference in a Device Memory Buffer of its own: accept VIRTIO_F_DMB from
vp_transport_features(), and implement the get_dmb_shm_id config op on
top of vp_modern_get_dmb_shm_id().

Refuse a device whose common configuration is too short to hold
dmb_shm_id, which would put that read outside what vp_modern_probe()
mapped. The accept commits before the id can be read, so a region we
fail to locate afterwards fails virtio_features_ok() and probe sets the
FAILED status bit.

Four conditions gate the accept:

  1) VIRTIO_F_ACCESS_PLATFORM, because the feature is only defined
     together with it.
  2) VIRTIO_F_ORDER_PLATFORM where the device offers it. Without it
     the ring emits the weaker barriers that assume the device sees
     memory the way another CPU does, and a region that is not
     ordinary host memory breaks that assumption.
  3) CONFIG_VIRTIO_DMB, so a device offering the feature to a kernel
     built without it is driven as an ordinary device.
  4) VIRTIO_F_VERSION_1, because virtio_features_ok() returns early
     without it, which would leave the feature negotiated and the
     region never built. This transport refuses such a device anyway.

vp_dmb_ordering_ok() asks the device with vp_modern_get_features()
instead of reading the feature word vp_transport_features() is handed.
That word holds what the driver accepts, so a device offering
VIRTIO_F_ORDER_PLATFORM to a driver that declined it would read there as
a device that never offered it.

Link: https://lore.kernel.org/virtio-comment/[email protected]/
Assisted-by: Kiro:claude-opus-5 checkpatch sparse
Signed-off-by: Alexander Graf <[email protected]>
---
 drivers/virtio/virtio_pci_modern.c | 65 ++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 565d37b630b3..c43c1fc6e843 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -364,6 +364,36 @@ static void vp_modern_avq_cleanup(struct virtio_device *vdev)
 	}
 }
 
+/*
+ * The proposal makes accepting VIRTIO_F_DMB conditional on accepting
+ * VIRTIO_F_ORDER_PLATFORM where the device offers it.  Without it the barriers
+ * the ring emits order accesses only as seen by a device that can be assumed
+ * to run on identical CPUs in an SMP configuration, which a device whose
+ * region is not ordinary host memory is not.  Accepting
+ * VIRTIO_F_ORDER_PLATFORM is otherwise only a SHOULD, so nothing else couples
+ * the two.
+ *
+ * The offer is read back from the device rather than taken from the feature
+ * word vp_transport_features() is given, because that word is what the driver
+ * still wants rather than what the device offered.  The two differ exactly
+ * where this has to hold: virtio_dev_probe() calls finalize_features() a
+ * second time when a driver's validate() changed the set, and a validate()
+ * that declined VIRTIO_F_ORDER_PLATFORM leaves the bit absent from the word
+ * as well, which would read as an offer that never happened.  virtio_balloon
+ * declines VIRTIO_F_ACCESS_PLATFORM from validate() today, so the shape is
+ * not hypothetical.
+ */
+static bool vp_dmb_ordering_ok(struct virtio_device *vdev)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+
+	if (__virtio_test_bit(vdev, VIRTIO_F_ORDER_PLATFORM))
+		return true;
+
+	return !(vp_modern_get_features(&vp_dev->mdev) &
+		 BIT_ULL(VIRTIO_F_ORDER_PLATFORM));
+}
+
 static void vp_transport_features(struct virtio_device *vdev, u64 features)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
@@ -378,6 +408,27 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features)
 
 	if (features & BIT_ULL(VIRTIO_F_ADMIN_VQ))
 		__virtio_set_bit(vdev, VIRTIO_F_ADMIN_VQ);
+
+	/*
+	 * VIRTIO_F_DMB is only defined together with
+	 * VIRTIO_F_ACCESS_PLATFORM, so accept it only when the driver accepts
+	 * that too.  vring_transport_features() has already run, so the bit in
+	 * vdev is the one the driver accepts rather than the one the device
+	 * offered, and the proposal words the requirement against what the
+	 * driver accepts.  VIRTIO_F_ORDER_PLATFORM is required where the device
+	 * offers it, for the reason vp_dmb_ordering_ok() gives.
+	 * VIRTIO_F_VERSION_1 is required because the core locates and releases
+	 * the region from virtio_features_ok(), which returns before it gets
+	 * that far for a device without VERSION_1, so accepting the feature
+	 * without it would leave the feature negotiated and the region never
+	 * built.
+	 */
+	if (IS_ENABLED(CONFIG_VIRTIO_DMB) &&
+	    (features & BIT_ULL(VIRTIO_F_DMB)) &&
+	    __virtio_test_bit(vdev, VIRTIO_F_ACCESS_PLATFORM) &&
+	    (features & BIT_ULL(VIRTIO_F_VERSION_1)) &&
+	    vp_dmb_ordering_ok(vdev))
+		__virtio_set_bit(vdev, VIRTIO_F_DMB);
 }
 
 static int __vp_check_common_size_one_feature(struct virtio_device *vdev, u32 fbit,
@@ -413,6 +464,9 @@ static int vp_check_common_size(struct virtio_device *vdev)
 	if (vp_check_common_size_one_feature(vdev, VIRTIO_F_ADMIN_VQ, admin_queue_num))
 		return -EINVAL;
 
+	if (vp_check_common_size_one_feature(vdev, VIRTIO_F_DMB, dmb_shm_id))
+		return -EINVAL;
+
 	return 0;
 }
 
@@ -878,6 +932,15 @@ static bool vp_get_shm_region(struct virtio_device *vdev,
 	return true;
 }
 
+static int vp_get_dmb_shm_id(struct virtio_device *vdev, u16 *id)
+{
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+
+	*id = vp_modern_get_dmb_shm_id(&vp_dev->mdev);
+
+	return 0;
+}
+
 /*
  * virtio_pci_admin_has_dev_parts - Checks whether the device parts
  * functionality is supported
@@ -1241,6 +1304,7 @@ static const struct virtio_config_ops virtio_pci_config_nodev_ops = {
 	.set_vq_affinity = vp_set_vq_affinity,
 	.get_vq_affinity = vp_get_vq_affinity,
 	.get_shm_region  = vp_get_shm_region,
+	.get_dmb_shm_id = vp_get_dmb_shm_id,
 	.disable_vq_and_reset = vp_modern_disable_vq_and_reset,
 	.enable_vq_after_reset = vp_modern_enable_vq_after_reset,
 };
@@ -1261,6 +1325,7 @@ static const struct virtio_config_ops virtio_pci_config_ops = {
 	.set_vq_affinity = vp_set_vq_affinity,
 	.get_vq_affinity = vp_get_vq_affinity,
 	.get_shm_region  = vp_get_shm_region,
+	.get_dmb_shm_id = vp_get_dmb_shm_id,
 	.disable_vq_and_reset = vp_modern_disable_vq_and_reset,
 	.enable_vq_after_reset = vp_modern_enable_vq_after_reset,
 };