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

[Alistair Francis <Alistair.Francis@wdc.com>]

On Tue, 2026-09-01 at 04:43 -0400, Michael S. Tsirkin wrote:
> 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?

I forgot to CC you, but patch sent:

https://lore.kernel.org/virtio-comment/20261002010456.2053759-1-alistair.francis@wdc.com/T/#u

Alistair

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