On Tue, 2026-09-01 at 04:43 -0400, Michael S. Tsirkin wrote: > On Tue, Sep 01, 2026 at 11:46:49AM +1000, [email protected] wrote: > > From: Alistair Francis <[email protected]> > > > > 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 <[email protected]> > > 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/[email protected]/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
