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