Re: [PATCH v3] virtio_pci_modern: skip unused driver feature registers

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Sep 28, 2026 at 08:34:55PM +0800, Peng Hao wrote:
> vp_modern_set_extended_features() currently writes all four 32-bit
> driver feature registers.  Selecting and writing each register requires
> two MMIO accesses, and each access may cause a VM exit when Linux runs as
> a guest.
> 
> The virtio core resets a device before feature negotiation.  Resetting
> clears all driver feature registers, so the initial finalization only
> needs to write through the highest nonzero register.  A typical modern
> device uses bits 0..63 and therefore saves four MMIO accesses, or up to
> four VM exits, during each feature negotiation.  Devices using a feature
> in bits 64..95 save two.

Why does this matter, in the big scheme of things?

> A driver's validate callback may remove features and cause a second
> finalization without another reset.  Track how many registers the
> previous finalization wrote and cover at least that range on the next
> call, writing zero where needed to clear a removed feature.
> 
> Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> ---
>  drivers/virtio/virtio_pci_modern_dev.c | 38 ++++++++++++++++++++++++--
>  include/linux/virtio_pci_modern.h      |  3 ++
>  2 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
> index 413a8c3..631b60f 100644
> --- a/drivers/virtio/virtio_pci_modern_dev.c
> +++ b/drivers/virtio/virtio_pci_modern_dev.c
> @@ -437,6 +437,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, unsigned int index)
> +{
> +	return features[index / 2] >> (32 * (index % 2));
> +}
> +
>  /*
>   * vp_modern_set_extended_features - set features to device
>   * @mdev: the modern virtio-pci device
> @@ -446,14 +451,38 @@ 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 required_features_dwords = VIRTIO_FEATURES_BITS / 32;
> +	u8 features_dwords_to_write;
>  	int i;
>  
> -	for (i = 0; i < VIRTIO_FEATURES_BITS / 32; i++) {
> -		u32 cur = features[i >> 1] >> (32 * (i & 1));
> +	/*
> +	 * Find the highest 32-bit feature register containing a negotiated
> +	 * feature.  Register 0 is always written, even when no device-specific
> +	 * feature was negotiated.
> +	 */
> +	while (required_features_dwords > 1 &&
> +	       !vp_modern_features_dword(features,
> +					 required_features_dwords - 1))
> +		required_features_dwords--;
>  
> +	/*
> +	 * vp_modern_set_status(mdev, 0) clears every driver feature register
> +	 * and sets driver_features_dwords to 0.  The first finalization
> +	 * therefore only needs to write required_features_dwords.  If
> +	 * finalization is repeated without a reset, also rewrite every register
> +	 * touched by the previous call so that features removed by validation
> +	 * are cleared.
> +	 */
> +	features_dwords_to_write = max(required_features_dwords,
> +				       mdev->driver_features_dwords);
> +
> +	for (i = 0; i < features_dwords_to_write; 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 = required_features_dwords;
>  }
>  EXPORT_SYMBOL_GPL(vp_modern_set_extended_features);
>  
> @@ -495,6 +524,9 @@ void vp_modern_set_status(struct virtio_pci_modern_device *mdev,
>  {
>  	struct virtio_pci_common_cfg __iomem *cfg = mdev->common;
>  
> +	if (!status)
> +		mdev->driver_features_dwords = 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 9a3f2fc..7a5b200 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 registers written
> + *		    by the most recent feature finalization, or 0 after 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;
>  };
>  
>  /*