Re: [PATCH 1/2] virtio_pci: Add a quirk to force DMA Map API for certain legacy devices

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

On Tue, Sep 01, 2026 at 11:46:49AM +1000, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> Legacy virtio devices only have 32 feature bits and therefore can't
> set the VIRTIO_F_ACCESS_PLATFORM (bit 33) feature. This means the
> vring_use_map_api() function will return false.
> 
> Currently Linux endpoint devices use the legacy virtio interface as
> they aren't able to advertise the Common configuration capability.
> As most PCI endpoint capable PCIe controllers do not allow modifying the
> capability list, and thus are unable to advertise the Common configuration
> capability. This means the device's inbound TLPs fault on the host
> SMMU because the vring descriptors carry raw physical addresses.
> 
> This quirk forces a subset of legacy virtio devices to use the
> DMA Map API (vring_use_map_api() will return true), which fixes this
> issue.
> 
> This doesn't affect existing devices as we are checking for an
> otherwise invalid vendor ID.
> 
> Ideally we would update the endpoint devices (like scsi-pci-epf)
> to not use the legacy virtio interface, but lots of endpoint
> hardware (like the one in the RK3588) doesn't allow us to add
> custom capabilities.
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

I don't much like hacks around DMA API, it is very fragile already.

So, here's an idea:

put all the capabilities simply at a fixed offset in a memory BAR.

it's a small spec extension, but saves a lot of trouble IMHO.

And in fact, people already complained that legacy pci config space
should be avoided.

What do you say?


> ---
>  drivers/virtio/virtio_pci_legacy.c | 27 +++++++++++++++++++++++++++
>  drivers/virtio/virtio_ring.c       |  7 +++++++
>  include/linux/virtio.h             |  5 +++++
>  3 files changed, 39 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
> index d9cbb02b35a1..7b529bd451bb 100644
> --- a/drivers/virtio/virtio_pci_legacy.c
> +++ b/drivers/virtio/virtio_pci_legacy.c
> @@ -16,6 +16,7 @@
>  
>  #include "linux/virtio_pci_legacy.h"
>  #include "virtio_pci_common.h"
> +#include <linux/virtio_ids.h>
>  
>  /* virtio config->get_features() implementation */
>  static u64 vp_get_features(struct virtio_device *vdev)
> @@ -220,6 +221,32 @@ int virtio_pci_legacy_probe(struct virtio_pci_device *vp_dev)
>  
>  	vp_dev->vdev.config = &virtio_pci_config_ops;
>  
> +	/*
> +	 * Legacy virtio devices only have 32 feature bits and therefore can't
> +	 * set the VIRTIO_F_ACCESS_PLATFORM (bit 33) feature. This means the
> +	 * vring_use_map_api() function will return false.
> +	 *
> +	 * Currently Linux endpoint devices use the legacy virtio interface as
> +	 * they aren't able to advertise the Common configuration capability.
> +	 * This means the device's inbound TLPs fault on the host SMMU because
> +	 * the vring descriptors carry raw physical addresses.
> +	 *
> +	 * This quirk forces a subset of legacy virtio devices to use the
> +	 * DMA Map API (vring_use_map_api() will return true), which fixes this
> +	 * issue.
> +	 *
> +	 * This doesn't affect existing devices as we are checking for an
> +	 * otherwise invalid vendor ID.
> +	 *
> +	 * Ideally we would update the endpoint devices (like scsi-pci-epf)
> +	 * to not use the legacy virtio interface, but lots of endpoint
> +	 * hardware (like the one in the RK3588) doesn't allow us to add
> +	 * custom capabilities.
> +	 */
> +	if (pci_dev->subsystem_vendor == 0xFFFF &&
> +	    pci_dev->subsystem_device == VIRTIO_ID_SCSI)
> +		vp_dev->vdev.force_use_map_api = true;
> +
>  	vp_dev->config_vector = vp_config_vector;
>  	vp_dev->setup_vq = setup_vq;
>  	vp_dev->del_vq = del_vq;
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 5c169fbb418a..c8f62180c9d3 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -384,6 +384,13 @@ static bool vring_use_map_api(const struct virtio_device *vdev)
>  	if (!virtio_has_dma_quirk(vdev))
>  		return true;
>  
> +	/*
> +	 * A quirk set by certain legacy devices to force us to
> +	 * pretend the VIRTIO_F_ACCESS_PLATFORM feature is enabled.
> +	 */
> +	if (vdev->force_use_map_api)
> +		return true;
> +
>  	/* Otherwise, we are left to guess. */
>  	/*
>  	 * In theory, it's possible to have a buggy QEMU-supposed
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index f923e42cfd01..305c331f33f1 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -151,6 +151,10 @@ struct virtio_admin_cmd {
>   * @config_driver_disabled: configuration change reporting disabled by
>   *                          a driver
>   * @config_change_pending: configuration change reported while disabled
> + * @force_use_map_api: A quirk set by certain legacy devices to force us
> + *                     to pretend the VIRTIO_F_ACCESS_PLATFORM feature is
> + *                     enabled. Set by transports that have no way to
> + *                     negotiate ACCESS_PLATFORM but sit behind a real IOMMU.
>   * @config_lock: protects configuration change reporting
>   * @vqs_list_lock: protects @vqs.
>   * @dev: underlying device.
> @@ -173,6 +177,7 @@ struct virtio_device {
>  	bool config_core_enabled;
>  	bool config_driver_disabled;
>  	bool config_change_pending;
> +	bool force_use_map_api;
>  	spinlock_t config_lock;
>  	spinlock_t vqs_list_lock;
>  	struct device dev;
> -- 
> 2.55.0