All of lore.kernel.org
 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; 4+ 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] 4+ messages in thread

* Re: [PATCH] virtio_pci_modern: fall back to 64 bits features for devices without an extended features space
  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-13 19:20 ` Michael S. Tsirkin
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-09-11  8:21 UTC (permalink / raw)
  To: Peng Hao; +Cc: virtualization, Michael S. Tsirkin, Eugenio Perez

> 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
> [ ... ]
> 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>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911081133.16434-1-flyingpeng@tencent.com?part=1


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

* Re: [PATCH] virtio_pci_modern: fall back to 64 bits features for devices without an extended features space
  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-13 19:20 ` Michael S. Tsirkin
  2 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2026-09-11  8:28 UTC (permalink / raw)
  To: Peng Hao; +Cc: jasowangio, virtualization, Peng Hao

On Fri, Sep 11, 2026 at 04:11:33PM +0800, Peng Hao wrote:
> 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.


That's a device bug then? The spec says:

\begin{description}
\item[\field{device_feature_select}]
        The driver uses this to select which feature bits \field{device_feature} shows.
        Value 0x0 selects Feature Bits 0 to 31, 0x1 selects Feature Bits 32 to 63, etc.

that "etc" means any value is valid)

There's no "extended features space". It's a quick hack we did
in the virtio code to avoid changing all drivers, but the spec
treats all features uniformly.


So you have a device that ignores a feature selector? Or some specific
bits from it?

>  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.

So using all bits is illegal, and so is anything that will by luck
mirror low bits?

This is really VIRTIO_F_BAD_FEATURE mess replaying itself.

Really quite a hack :( 



>  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.

This part is ok.

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

dump?

> and add a max_features_u64s
> module parameter to force the legacy 64 bits space on devices whose
> quirk the detection does not catch.

This is even worse.


> 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>

Can you simply fix the device please? Why not?

And please tell us in what way exactly are your devices broken?

I am not merging multiple hacks "just in case" because there's no
way to test them for me.

> ---
> 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	[flat|nested] 4+ messages in thread

* Re: [PATCH] virtio_pci_modern: fall back to 64 bits features for devices without an extended features space
  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-13 19:20 ` Michael S. Tsirkin
  2 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2026-09-13 19:20 UTC (permalink / raw)
  To: Peng Hao; +Cc: jasowangio, virtualization, Peng Hao

On Fri, Sep 11, 2026 at 04:11:33PM +0800, Peng Hao wrote:
> 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>


Also can you dump the pci header of the device please?

> ---
> 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	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-13 19:20 UTC | newest]

Thread overview: 4+ 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-13 19:20 ` Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.