Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH] virtio_pci_modern: fall back to 64 bits features for devices without an extended features space
@ 2026-09-11  8:11 Peng Hao
  2026-09-11  8:21 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Peng Hao @ 2026-09-11  8:11 UTC (permalink / raw)
  To: mst, jasowangio; +Cc: virtualization, Peng Hao

From: Peng Hao <flyingingpeng@tencent.com>

Since commit 69b9461512246 ("virtio_pci_modern: allow configuring
extended features") the modern virtio-pci driver unconditionally
accesses the whole 128 bits features space, i.e. it drives
device_feature_select / guest_feature_select with the values 0..3.

Devices predating the extended features space only implement the
legacy 64 bits one, and what they report for the selectors above it is
not a valid features space.  Negotiating it makes the driver and the
device end up with different features sets: on a smart NIC exposing a
virtio_net device the link comes up but carries no traffic, while the
same device works with a kernel that only accesses the low 64 bits.

Reading the features space has no side effect, so keep reading all of
it and use the extended part to tell whether the device implements it:
report the legacy 64 bits only when the extended words read back as
all-ones or as an alias of the low words, and latch the device down for
good.  As the features negotiation ANDs the device and driver features,
no feature above bit 63 can be negotiated afterwards.

Writing a selector the device does not implement cannot be relied upon
the same way, so never drive one above the highest word that actually
carries a bit.  The reset preceding the features negotiation zeroes the
device side features, hence the words left unwritten stay cleared.

Also dump the raw device_feature dwords, and add a max_features_u64s
module parameter to force the legacy 64 bits space on devices whose
quirk the detection does not catch.

Conforming devices are unaffected: their extended words are neither
all-ones nor an alias of the low ones, so the detection does not
trigger.

Signed-off-by: Peng Hao <flyingingpeng@tencent.com>
---
diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
index 413a8c353463..cfb10c9f32dd 100644
--- a/drivers/virtio/virtio_pci_modern_dev.c
+++ b/drivers/virtio/virtio_pci_modern_dev.c
@@ -5,6 +5,21 @@
 #include <linux/pci.h>
 #include <linux/delay.h>
 
+static int max_features_u64s = -1;
+module_param(max_features_u64s, int, 0444);
+MODULE_PARM_DESC(max_features_u64s,
+		 "Max number of 64 bit words of the virtio features space to access (1 = legacy 64 bits only, -1 = auto-detect)");
+
+static u8 vp_modern_features_u64s(const struct virtio_pci_modern_device *mdev)
+{
+	u8 u64s = mdev->features_u64s ?: VIRTIO_FEATURES_U64S;
+
+	if (max_features_u64s > 0 && u64s > max_features_u64s)
+		u64s = max_features_u64s;
+
+	return u64s;
+}
+
 /*
  * vp_modern_map_capability - map a part of virtio pci capability
  * @mdev: the modern virtio-pci device
@@ -230,6 +245,8 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev)
 
 	check_offsets();
 
+	mdev->features_u64s = VIRTIO_FEATURES_U64S;
+
 	if (mdev->device_id_check) {
 		devid = mdev->device_id_check(pci_dev);
 		if (devid < 0)
@@ -398,15 +415,36 @@ void vp_modern_get_extended_features(struct virtio_pci_modern_device *mdev,
 				     u64 *features)
 {
 	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+	u32 raw[VIRTIO_FEATURES_BITS / 32];
+	u8 u64s = vp_modern_features_u64s(mdev);
 	int i;
 
-	virtio_features_zero(features);
 	for (i = 0; i < VIRTIO_FEATURES_BITS / 32; i++) {
-		u64 cur;
-
 		vp_iowrite32(i, &cfg->device_feature_select);
-		cur = vp_ioread32(&cfg->device_feature);
-		features[i >> 1] |= cur << (32 * (i & 1));
+		raw[i] = vp_ioread32(&cfg->device_feature);
+	}
+
+	dev_info(&mdev->pci_dev->dev,
+		 "virtio_pci: device_feature[0..3] = 0x%08x 0x%08x 0x%08x 0x%08x\n",
+		 raw[0], raw[1], raw[2], raw[3]);
+
+	virtio_features_zero(features);
+	for (i = 0; i < u64s * 2; i++)
+		features[i >> 1] |= (u64)raw[i] << (32 * (i & 1));
+
+	for (i = 1; i < u64s; i++) {
+		int j;
+
+		if (features[i] != U64_MAX &&
+		    !(features[0] && features[i] == features[0]))
+			continue;
+
+		dev_info(&mdev->pci_dev->dev,
+			 "virtio_pci: no extended features space, using 64 bits features only\n");
+		mdev->features_u64s = 1;
+		for (j = 1; j < VIRTIO_FEATURES_U64S; j++)
+			features[j] = 0;
+		break;
 	}
 }
 EXPORT_SYMBOL_GPL(vp_modern_get_extended_features);
@@ -424,10 +462,11 @@ vp_modern_get_driver_extended_features(struct virtio_pci_modern_device *mdev,
 				       u64 *features)
 {
 	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+	u8 u64s = vp_modern_features_u64s(mdev);
 	int i;
 
 	virtio_features_zero(features);
-	for (i = 0; i < VIRTIO_FEATURES_BITS / 32; i++) {
+	for (i = 0; i < u64s * 2; i++) {
 		u64 cur;
 
 		vp_iowrite32(i, &cfg->guest_feature_select);
@@ -446,9 +485,19 @@ void vp_modern_set_extended_features(struct virtio_pci_modern_device *mdev,
 				     const u64 *features)
 {
 	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
+	u8 u64s = vp_modern_features_u64s(mdev);
 	int i;
 
-	for (i = 0; i < VIRTIO_FEATURES_BITS / 32; i++) {
+	/*
+	 * Never drive a selector the device may not implement: stop at the
+	 * highest word that carries a bit.  The device side features are
+	 * zeroed by the reset that precedes the features negotiation, so the
+	 * words left unwritten keep the value the driver wants for them.
+	 */
+	while (u64s > 1 && !features[u64s - 1])
+		u64s--;
+
+	for (i = 0; i < u64s * 2; i++) {
 		u32 cur = features[i >> 1] >> (32 * (i & 1));
 
 		vp_iowrite32(i, &cfg->guest_feature_select);
diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_modern.h
index 9a3f2fc53bd6..7dc76afa671e 100644
--- a/include/linux/virtio_pci_modern.h
+++ b/include/linux/virtio_pci_modern.h
@@ -27,6 +27,9 @@
  *		    Returns the found device id or ERRNO
  * @dma_mask:	    Optional mask instead of the traditional DMA_BIT_MASK(64),
  *		    for vendor devices with DMA space address limitations
+ * @features_u64s:  Number of 64 bit words of the features space that can be
+ *		    accessed on this device; 1 for devices not implementing
+ *		    the extended (128 bits) features space
  */
 struct virtio_pci_modern_device {
 	struct pci_dev *pci_dev;
@@ -49,6 +52,7 @@ struct virtio_pci_modern_device {
 
 	int (*device_id_check)(struct pci_dev *pdev);
 	u64 dma_mask;
+	u8 features_u64s;
 };
 
 /*
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-15 14:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-11  8:11 [PATCH] virtio_pci_modern: fall back to 64 bits features for devices without an extended features space Peng Hao
2026-09-11  8:21 ` sashiko-bot
2026-09-11  8:28 ` Michael S. Tsirkin
2026-09-15 11:17   ` Hao Peng
2026-09-13 19:20 ` Michael S. Tsirkin
2026-09-15 12:32   ` Hao Peng
2026-09-15 14:31     ` Michael S. Tsirkin

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