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,
};
