[PATCH v2] virtio_pci_modern: skip trailing zero feature dwords

[PATCH v2] virtio_pci_modern: skip trailing zero feature dwords

[Peng Hao]

Since commit 69b9461512246 ("virtio_pci_modern: allow configuring
extended features"), vp_modern_set_extended_features() writes all four
feature dwords on every device, even when the upper ones are zero.

Feature negotiation follows a device reset, which clears the device-side
driver features, so trailing zero dwords need not be written at all.
finalize_features() can be called again without an intervening reset,
though, when a driver's validate callback narrows the features, so also
write any dword written since the last reset, to clear what the previous
call had enabled.

Devices negotiating nothing above bit 63 save four MMIO writes; those
using the 64..95 range (e.g. the UDP tunnel GSO features) save two.
Counting dwords rather than 64-bit words is what makes the latter work:
with VIRTIO_F_VERSION_1 at bit 32 the second dword is set on every modern
device, so a qword count never drops below two.

Signed-off-by: Peng Hao <flyingpeng@tencent.com>
---
 drivers/virtio/virtio_pci_modern_dev.c | 32 ++++++++++++++++++++++----
 include/linux/virtio_pci_modern.h      |  3 +++
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
index 413a8c353463..7b4da37b6ae5 100644
--- a/drivers/virtio/virtio_pci_modern_dev.c
+++ b/drivers/virtio/virtio_pci_modern_dev.c
@@ -230,6 +230,8 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev)
 
 	check_offsets();
 
+	mdev->driver_features_dwords = 1;
+
 	if (mdev->device_id_check) {
 		devid = mdev->device_id_check(pci_dev);
 		if (devid < 0)
@@ -437,6 +439,11 @@ vp_modern_get_driver_extended_features(struct virtio_pci_modern_device *mdev,
 }
 EXPORT_SYMBOL_GPL(vp_modern_get_driver_extended_features);
 
+static u32 vp_modern_features_dword(const u64 *features, int dword)
+{
+	return features[dword / 2] >> (32 * (dword % 2));
+}
+
 /*
  * vp_modern_set_extended_features - set features to device
  * @mdev: the modern virtio-pci device
@@ -446,14 +453,27 @@ void vp_modern_set_extended_features(struct virtio_pci_modern_device *mdev,
 				     const u64 *features)
 {
 	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
-	int i;
+	int dwords = VIRTIO_FEATURES_BITS / 32;
+	int i, write_dwords;
 
-	for (i = 0; i < VIRTIO_FEATURES_BITS / 32; i++) {
-		u32 cur = features[i >> 1] >> (32 * (i & 1));
+	/*
+	 * A device reset clears the driver features, so trailing all-zero
+	 * dwords need not be written out.  Include any dword written since
+	 * that reset, though, so that a repeated finalization can clear
+	 * features which were enabled by the previous one.
+	 */
+	while (dwords > 1 && !vp_modern_features_dword(features, dwords - 1))
+		dwords--;
+
+	write_dwords = max_t(int, dwords, mdev->driver_features_dwords);
 
+	for (i = 0; i < write_dwords; i++) {
 		vp_iowrite32(i, &cfg->guest_feature_select);
-		vp_iowrite32(cur, &cfg->guest_feature);
+		vp_iowrite32(vp_modern_features_dword(features, i),
+			     &cfg->guest_feature);
 	}
+
+	mdev->driver_features_dwords = dwords;
 }
 EXPORT_SYMBOL_GPL(vp_modern_set_extended_features);
 
@@ -495,6 +515,10 @@ void vp_modern_set_status(struct virtio_pci_modern_device *mdev,
 {
 	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
 
+	/* A reset clears the device's copy of the driver features. */
+	if (!status)
+		mdev->driver_features_dwords = 1;
+
 	/*
 	 * Per memory-barriers.txt, wmb() is not needed to guarantee
 	 * that the cache coherent memory writes have completed
diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_modern.h
index 9a3f2fc53bd6..b9f4783f0e02 100644
--- a/include/linux/virtio_pci_modern.h
+++ b/include/linux/virtio_pci_modern.h
@@ -27,6 +27,8 @@
  *		    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
+ * @driver_features_dwords: Number of 32-bit driver feature words written
+ *		    to the device since the last reset
  */
 struct virtio_pci_modern_device {
 	struct pci_dev *pci_dev;
@@ -49,6 +51,7 @@ struct virtio_pci_modern_device {
 
 	int (*device_id_check)(struct pci_dev *pdev);
 	u64 dma_mask;
+	u8 driver_features_dwords;
 };
 
 /*
-- 
2.43.7

Re: [PATCH v2] virtio_pci_modern: skip trailing zero feature dwords

[sashiko-bot]

> Since commit 69b9461512246 ("virtio_pci_modern: allow configuring
> extended features"), vp_modern_set_extended_features() writes all four
> feature dwords on every device, even when the upper ones are zero.
> 
> Feature negotiation follows a device reset, which clears the device-side
> driver features, so trailing zero dwords need not be written at all.
> finalize_features() can be called again without an intervening reset,
> though, when a driver's validate callback narrows the features, so also
> write any dword written since the last reset, to clear what the previous
> call had enabled.
> 
> Devices negotiating nothing above bit 63 save four MMIO writes; those
> using the 64..95 range (e.g. the UDP tunnel GSO features) save two.
> Counting dwords rather than 64-bit words is what makes the latter work:
> with VIRTIO_F_VERSION_1 at bit 32 the second dword is set on every modern
> device, so a qword count never drops below two.
> 
> Signed-off-by: Peng Hao <flyingpeng@tencent.com>

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

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

Re: [PATCH v2] virtio_pci_modern: skip trailing zero feature dwords

[Michael S. Tsirkin]

On Thu, Sep 24, 2026 at 03:14:12PM +0800, Peng Hao wrote:
> Since commit 69b9461512246 ("virtio_pci_modern: allow configuring
> extended features"), vp_modern_set_extended_features() writes all four
> feature dwords on every device, even when the upper ones are zero.
> 
> Feature negotiation follows a device reset, which clears the device-side
> driver features, so trailing zero dwords need not be written at all.
> finalize_features() can be called again without an intervening reset,
> though, when a driver's validate callback narrows the features, so also
> write any dword written since the last reset, to clear what the previous
> call had enabled.
> 
> Devices negotiating nothing above bit 63 save four MMIO writes; those
> using the 64..95 range (e.g. the UDP tunnel GSO features) save two.
> Counting dwords rather than 64-bit words is what makes the latter work:
> with VIRTIO_F_VERSION_1 at bit 32 the second dword is set on every modern
> device, so a qword count never drops below two.
> 
> Signed-off-by: Peng Hao <flyingpeng@tencent.com>

So .. why does all this matter? how many exits do you save
during a guest boot? is it worth the complexity?

> ---
>  drivers/virtio/virtio_pci_modern_dev.c | 32 ++++++++++++++++++++++----
>  include/linux/virtio_pci_modern.h      |  3 +++
>  2 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
> index 413a8c353463..7b4da37b6ae5 100644
> --- a/drivers/virtio/virtio_pci_modern_dev.c
> +++ b/drivers/virtio/virtio_pci_modern_dev.c
> @@ -230,6 +230,8 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev)
>  
>  	check_offsets();
>  
> +	mdev->driver_features_dwords = 1;
> +
>  	if (mdev->device_id_check) {
>  		devid = mdev->device_id_check(pci_dev);
>  		if (devid < 0)
> @@ -437,6 +439,11 @@ vp_modern_get_driver_extended_features(struct virtio_pci_modern_device *mdev,
>  }
>  EXPORT_SYMBOL_GPL(vp_modern_get_driver_extended_features);
>  
> +static u32 vp_modern_features_dword(const u64 *features, int dword)
> +{
> +	return features[dword / 2] >> (32 * (dword % 2));
> +}
> +
>  /*
>   * vp_modern_set_extended_features - set features to device
>   * @mdev: the modern virtio-pci device
> @@ -446,14 +453,27 @@ void vp_modern_set_extended_features(struct virtio_pci_modern_device *mdev,
>  				     const u64 *features)
>  {
>  	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
> -	int i;
> +	int dwords = VIRTIO_FEATURES_BITS / 32;
> +	int i, write_dwords;
>  
> -	for (i = 0; i < VIRTIO_FEATURES_BITS / 32; i++) {
> -		u32 cur = features[i >> 1] >> (32 * (i & 1));


below is arguing with previous version of code
instead of straight explaining what is this code doing.
pls rewrite this comment.

> +	/*
> +	 * A device reset

what "a device reset"? who did reset and when?

> clears the driver features, so trailing all-zero

trailing?

> +	 * dwords need not be written out.  Include any dword written since
> +	 * that reset,

what "that" reset?

> though, so that a repeated finalization can clear
> +	 * features which were enabled by the previous one.
> +	 */

> +	while (dwords > 1 && !vp_modern_features_dword(features, dwords - 1))
> +		dwords--;

what's all this > 1, - 1?

> +
> +	write_dwords = max_t(int, dwords, mdev->driver_features_dwords);

and what is this. i have a vague idea but needs a comment.

>  
> +	for (i = 0; i < write_dwords; i++) {
>  		vp_iowrite32(i, &cfg->guest_feature_select);
> -		vp_iowrite32(cur, &cfg->guest_feature);
> +		vp_iowrite32(vp_modern_features_dword(features, i),
> +			     &cfg->guest_feature);
>  	}
> +
> +	mdev->driver_features_dwords = dwords;


contradicts the comment near driver_features_dwords.

>  }
>  EXPORT_SYMBOL_GPL(vp_modern_set_extended_features);
>  
> @@ -495,6 +515,10 @@ void vp_modern_set_status(struct virtio_pci_modern_device *mdev,
>  {
>  	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
>  
> +	/* A reset clears the device's copy of the driver features. */
> +	if (!status)
> +		mdev->driver_features_dwords = 1;

so why 1 not 0?

> +
>  	/*
>  	 * Per memory-barriers.txt, wmb() is not needed to guarantee
>  	 * that the cache coherent memory writes have completed
> diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_modern.h
> index 9a3f2fc53bd6..b9f4783f0e02 100644
> --- a/include/linux/virtio_pci_modern.h
> +++ b/include/linux/virtio_pci_modern.h
> @@ -27,6 +27,8 @@
>   *		    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
> + * @driver_features_dwords: Number of 32-bit driver feature words written
> + *		    to the device since the last reset
>   */
>  struct virtio_pci_modern_device {
>  	struct pci_dev *pci_dev;
> @@ -49,6 +51,7 @@ struct virtio_pci_modern_device {
>  
>  	int (*device_id_check)(struct pci_dev *pdev);
>  	u64 dma_mask;
> +	u8 driver_features_dwords;
>  };
>  
>  /*
> -- 
> 2.43.7

Re: [PATCH v2] virtio_pci_modern: skip trailing zero feature dwords

[Hao Peng]

On Fri, Sep 25, 2026 at 12:05 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Sep 24, 2026 at 03:14:12PM +0800, Peng Hao wrote:
> > Since commit 69b9461512246 ("virtio_pci_modern: allow configuring
> > extended features"), vp_modern_set_extended_features() writes all four
> > feature dwords on every device, even when the upper ones are zero.
> >
> > Feature negotiation follows a device reset, which clears the device-side
> > driver features, so trailing zero dwords need not be written at all.
> > finalize_features() can be called again without an intervening reset,
> > though, when a driver's validate callback narrows the features, so also
> > write any dword written since the last reset, to clear what the previous
> > call had enabled.
> >
> > Devices negotiating nothing above bit 63 save four MMIO writes; those
> > using the 64..95 range (e.g. the UDP tunnel GSO features) save two.
> > Counting dwords rather than 64-bit words is what makes the latter work:
> > with VIRTIO_F_VERSION_1 at bit 32 the second dword is set on every modern
> > device, so a qword count never drops below two.
> >
> > Signed-off-by: Peng Hao <flyingpeng@tencent.com>
>
> So .. why does all this matter? how many exits do you save
> during a guest boot? is it worth the complexity?
>
it depends on the number of modern virtio-pci devices:
  - A device using only feature bits 0..63 previously required eight
MMIO writes and now requires four, saving four
    MMIO writes, normally four VM-exits.
  - A device using bits 64..95 saves two MMIO writes/exits.
  - Thus, for example, a guest with five ordinary modern virtio-pci
devices saves about 20 exits during their
    initial feature negotiation.


> > ---
> >  drivers/virtio/virtio_pci_modern_dev.c | 32 ++++++++++++++++++++++----
> >  include/linux/virtio_pci_modern.h      |  3 +++
> >  2 files changed, 31 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
> > index 413a8c353463..7b4da37b6ae5 100644
> > --- a/drivers/virtio/virtio_pci_modern_dev.c
> > +++ b/drivers/virtio/virtio_pci_modern_dev.c
> > @@ -230,6 +230,8 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev)
> >
> >       check_offsets();
> >
> > +     mdev->driver_features_dwords = 1;
> > +
> >       if (mdev->device_id_check) {
> >               devid = mdev->device_id_check(pci_dev);
> >               if (devid < 0)
> > @@ -437,6 +439,11 @@ vp_modern_get_driver_extended_features(struct virtio_pci_modern_device *mdev,
> >  }
> >  EXPORT_SYMBOL_GPL(vp_modern_get_driver_extended_features);
> >
> > +static u32 vp_modern_features_dword(const u64 *features, int dword)
> > +{
> > +     return features[dword / 2] >> (32 * (dword % 2));
> > +}
> > +
> >  /*
> >   * vp_modern_set_extended_features - set features to device
> >   * @mdev: the modern virtio-pci device
> > @@ -446,14 +453,27 @@ void vp_modern_set_extended_features(struct virtio_pci_modern_device *mdev,
> >                                    const u64 *features)
> >  {
> >       struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
> > -     int i;
> > +     int dwords = VIRTIO_FEATURES_BITS / 32;
> > +     int i, write_dwords;
> >
> > -     for (i = 0; i < VIRTIO_FEATURES_BITS / 32; i++) {
> > -             u32 cur = features[i >> 1] >> (32 * (i & 1));
>
>
> below is arguing with previous version of code
> instead of straight explaining what is this code doing.
> pls rewrite this comment.
>
> > +     /*
> > +      * A device reset
>
> what "a device reset"? who did reset and when?
>
> > clears the driver features, so trailing all-zero
>
> trailing?
>
> > +      * dwords need not be written out.  Include any dword written since
> > +      * that reset,
>
> what "that" reset?
>
The reset in question is performed by the virtio core.
register_virtio_device() calls virtio_reset_device() before
  driver matching and feature negotiation. For modern virtio-pci,
vp_reset() writes zero to device_status and waits
  for the device to report zero. That reset clears all driver feature registers.
> > though, so that a repeated finalization can clear
> > +      * features which were enabled by the previous one.
> > +      */
>
> > +     while (dwords > 1 && !vp_modern_features_dword(features, dwords - 1))
> > +             dwords--;
>
dwords is the number of registers needed to represent the new feature
set. The loop examines
 register dwords - 1 and reduces the count while the highest register
is zero. Register 0
 remains part of the range.
> what's all this > 1, - 1?
>
> > +
> > +     write_dwords = max_t(int, dwords, mdev->driver_features_dwords);
>
> and what is this. i have a vague idea but needs a comment.
>
> >
> > +     for (i = 0; i < write_dwords; i++) {
> >               vp_iowrite32(i, &cfg->guest_feature_select);
> > -             vp_iowrite32(cur, &cfg->guest_feature);
> > +             vp_iowrite32(vp_modern_features_dword(features, i),
> > +                          &cfg->guest_feature);
> >       }
> > +
> > +     mdev->driver_features_dwords = dwords;
>
>
> contradicts the comment near driver_features_dwords.
>
> >  }
> >  EXPORT_SYMBOL_GPL(vp_modern_set_extended_features);
> >
> > @@ -495,6 +515,10 @@ void vp_modern_set_status(struct virtio_pci_modern_device *mdev,
> >  {
> >       struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
> >
> > +     /* A reset clears the device's copy of the driver features. */
> > +     if (!status)
> > +             mdev->driver_features_dwords = 1;
>
> so why 1 not 0?
>
Initializing driver_features_dwords to 1 after reset was unnecessarily
confusing. The reset leaves no
previously programmed range to clear, so the new version sets it to 0.
> > +
> >       /*
> >        * Per memory-barriers.txt, wmb() is not needed to guarantee
> >        * that the cache coherent memory writes have completed
> > diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_modern.h
> > index 9a3f2fc53bd6..b9f4783f0e02 100644
> > --- a/include/linux/virtio_pci_modern.h
> > +++ b/include/linux/virtio_pci_modern.h
> > @@ -27,6 +27,8 @@
> >   *               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
> > + * @driver_features_dwords: Number of 32-bit driver feature words written
> > + *               to the device since the last reset
> >   */
> >  struct virtio_pci_modern_device {
> >       struct pci_dev *pci_dev;
> > @@ -49,6 +51,7 @@ struct virtio_pci_modern_device {
> >
> >       int (*device_id_check)(struct pci_dev *pdev);
> >       u64 dma_mask;
> > +     u8 driver_features_dwords;
> >  };
> >
> >  /*
> > --
> > 2.43.7
>

Re: [PATCH v2] virtio_pci_modern: skip trailing zero feature dwords

[Michael S. Tsirkin]

On Mon, Sep 28, 2026 at 08:31:15PM +0800, Hao Peng wrote:
> On Fri, Sep 25, 2026 at 12:05 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Thu, Sep 24, 2026 at 03:14:12PM +0800, Peng Hao wrote:
> > > Since commit 69b9461512246 ("virtio_pci_modern: allow configuring
> > > extended features"), vp_modern_set_extended_features() writes all four
> > > feature dwords on every device, even when the upper ones are zero.
> > >
> > > Feature negotiation follows a device reset, which clears the device-side
> > > driver features, so trailing zero dwords need not be written at all.
> > > finalize_features() can be called again without an intervening reset,
> > > though, when a driver's validate callback narrows the features, so also
> > > write any dword written since the last reset, to clear what the previous
> > > call had enabled.
> > >
> > > Devices negotiating nothing above bit 63 save four MMIO writes; those
> > > using the 64..95 range (e.g. the UDP tunnel GSO features) save two.
> > > Counting dwords rather than 64-bit words is what makes the latter work:
> > > with VIRTIO_F_VERSION_1 at bit 32 the second dword is set on every modern
> > > device, so a qword count never drops below two.
> > >
> > > Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> >
> > So .. why does all this matter? how many exits do you save
> > during a guest boot? is it worth the complexity?
> >
> it depends on the number of modern virtio-pci devices:
>   - A device using only feature bits 0..63 previously required eight
> MMIO writes and now requires four, saving four
>     MMIO writes, normally four VM-exits.
>   - A device using bits 64..95 saves two MMIO writes/exits.
>   - Thus, for example, a guest with five ordinary modern virtio-pci
> devices saves about 20 exits during their
>     initial feature negotiation.

Out of some 10000-100000 that boot takes? Why does this matter?

> 
> > > ---
> > >  drivers/virtio/virtio_pci_modern_dev.c | 32 ++++++++++++++++++++++----
> > >  include/linux/virtio_pci_modern.h      |  3 +++
> > >  2 files changed, 31 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
> > > index 413a8c353463..7b4da37b6ae5 100644
> > > --- a/drivers/virtio/virtio_pci_modern_dev.c
> > > +++ b/drivers/virtio/virtio_pci_modern_dev.c
> > > @@ -230,6 +230,8 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev)
> > >
> > >       check_offsets();
> > >
> > > +     mdev->driver_features_dwords = 1;
> > > +
> > >       if (mdev->device_id_check) {
> > >               devid = mdev->device_id_check(pci_dev);
> > >               if (devid < 0)
> > > @@ -437,6 +439,11 @@ vp_modern_get_driver_extended_features(struct virtio_pci_modern_device *mdev,
> > >  }
> > >  EXPORT_SYMBOL_GPL(vp_modern_get_driver_extended_features);
> > >
> > > +static u32 vp_modern_features_dword(const u64 *features, int dword)
> > > +{
> > > +     return features[dword / 2] >> (32 * (dword % 2));
> > > +}
> > > +
> > >  /*
> > >   * vp_modern_set_extended_features - set features to device
> > >   * @mdev: the modern virtio-pci device
> > > @@ -446,14 +453,27 @@ void vp_modern_set_extended_features(struct virtio_pci_modern_device *mdev,
> > >                                    const u64 *features)
> > >  {
> > >       struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
> > > -     int i;
> > > +     int dwords = VIRTIO_FEATURES_BITS / 32;
> > > +     int i, write_dwords;
> > >
> > > -     for (i = 0; i < VIRTIO_FEATURES_BITS / 32; i++) {
> > > -             u32 cur = features[i >> 1] >> (32 * (i & 1));
> >
> >
> > below is arguing with previous version of code
> > instead of straight explaining what is this code doing.
> > pls rewrite this comment.
> >
> > > +     /*
> > > +      * A device reset
> >
> > what "a device reset"? who did reset and when?
> >
> > > clears the driver features, so trailing all-zero
> >
> > trailing?
> >
> > > +      * dwords need not be written out.  Include any dword written since
> > > +      * that reset,
> >
> > what "that" reset?
> >
> The reset in question is performed by the virtio core.
> register_virtio_device() calls virtio_reset_device() before
>   driver matching and feature negotiation. For modern virtio-pci,
> vp_reset() writes zero to device_status and waits
>   for the device to report zero. That reset clears all driver feature registers.
> > > though, so that a repeated finalization can clear
> > > +      * features which were enabled by the previous one.
> > > +      */
> >
> > > +     while (dwords > 1 && !vp_modern_features_dword(features, dwords - 1))
> > > +             dwords--;
> >
> dwords is the number of registers needed to represent the new feature
> set. The loop examines
>  register dwords - 1 and reduces the count while the highest register
> is zero. Register 0
>  remains part of the range.
> > what's all this > 1, - 1?
> >
> > > +
> > > +     write_dwords = max_t(int, dwords, mdev->driver_features_dwords);
> >
> > and what is this. i have a vague idea but needs a comment.
> >
> > >
> > > +     for (i = 0; i < write_dwords; i++) {
> > >               vp_iowrite32(i, &cfg->guest_feature_select);
> > > -             vp_iowrite32(cur, &cfg->guest_feature);
> > > +             vp_iowrite32(vp_modern_features_dword(features, i),
> > > +                          &cfg->guest_feature);
> > >       }
> > > +
> > > +     mdev->driver_features_dwords = dwords;
> >
> >
> > contradicts the comment near driver_features_dwords.
> >
> > >  }
> > >  EXPORT_SYMBOL_GPL(vp_modern_set_extended_features);
> > >
> > > @@ -495,6 +515,10 @@ void vp_modern_set_status(struct virtio_pci_modern_device *mdev,
> > >  {
> > >       struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
> > >
> > > +     /* A reset clears the device's copy of the driver features. */
> > > +     if (!status)
> > > +             mdev->driver_features_dwords = 1;
> >
> > so why 1 not 0?
> >
> Initializing driver_features_dwords to 1 after reset was unnecessarily
> confusing. The reset leaves no
> previously programmed range to clear, so the new version sets it to 0.
> > > +
> > >       /*
> > >        * Per memory-barriers.txt, wmb() is not needed to guarantee
> > >        * that the cache coherent memory writes have completed
> > > diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_modern.h
> > > index 9a3f2fc53bd6..b9f4783f0e02 100644
> > > --- a/include/linux/virtio_pci_modern.h
> > > +++ b/include/linux/virtio_pci_modern.h
> > > @@ -27,6 +27,8 @@
> > >   *               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
> > > + * @driver_features_dwords: Number of 32-bit driver feature words written
> > > + *               to the device since the last reset
> > >   */
> > >  struct virtio_pci_modern_device {
> > >       struct pci_dev *pci_dev;
> > > @@ -49,6 +51,7 @@ struct virtio_pci_modern_device {
> > >
> > >       int (*device_id_check)(struct pci_dev *pdev);
> > >       u64 dma_mask;
> > > +     u8 driver_features_dwords;
> > >  };
> > >
> > >  /*
> > > --
> > > 2.43.7
> >