thr3ads.net - Virtualization - [PATCH v3 0/6] Add virtio-iommu built-in topology [Sep 2020]

If this information is useful, please help other people find it:
Share via:

Jean-Philippe Brucker

2020-Aug-21 13:15 UTC

[PATCH v3 0/6] Add virtio-iommu built-in topology

Add a topology description to the virtio-iommu driver and enable x86
platforms.

Since [v2] we have made some progress on adding ACPI support for
virtio-iommu, which is the preferred boot method on x86. It will be a
new vendor-agnostic table describing para-virtual topologies in a
minimal format. However some platforms don't use either ACPI or DT for
booting (for example microvm), and will need the alternative topology
description method proposed here. In addition, since the process to get
a new ACPI table will take a long time, this provides a boot method even
to ACPI-based platforms, if only temporarily for testing and
development.

v3:
* Add patch 1 that moves virtio-iommu to a subfolder.
* Split the rest:
  * Patch 2 adds topology-helper.c, which will be shared with the ACPI
    support.
  * Patch 4 adds definitions.
  * Patch 5 adds parser in topology.c.
* Address other comments.

Linux and QEMU patches available at:
https://jpbrucker.net/git/linux virtio-iommu/devel
https://jpbrucker.net/git/qemu virtio-iommu/devel

[spec] https://lists.oasis-open.org/archives/virtio-dev/202008/msg00067.html
[v2] https://lore.kernel.org/linux-iommu/20200228172537.377327-1-jean-philippe
at linaro.org/
[v1] https://lore.kernel.org/linux-iommu/20200214160413.1475396-1-jean-philippe
at linaro.org/
[rfc] https://lore.kernel.org/linux-iommu/20191122105000.800410-1-jean-philippe
at linaro.org/

Jean-Philippe Brucker (6):
  iommu/virtio: Move to drivers/iommu/virtio/
  iommu/virtio: Add topology helpers
  PCI: Add DMA configuration for virtual platforms
  iommu/virtio: Add topology definitions
  iommu/virtio: Support topology description in config space
  iommu/virtio: Enable x86 support

 drivers/iommu/Kconfig                     |  18 +-
 drivers/iommu/Makefile                    |   3 +-
 drivers/iommu/virtio/Makefile             |   4 +
 drivers/iommu/virtio/topology-helpers.h   |  50 +++++
 include/linux/virt_iommu.h                |  15 ++
 include/uapi/linux/virtio_iommu.h         |  44 ++++
 drivers/iommu/virtio/topology-helpers.c   | 196 ++++++++++++++++
 drivers/iommu/virtio/topology.c           | 259 ++++++++++++++++++++++
 drivers/iommu/{ => virtio}/virtio-iommu.c |   4 +
 drivers/pci/pci-driver.c                  |   5 +
 MAINTAINERS                               |   3 +-
 11 files changed, 597 insertions(+), 4 deletions(-)
 create mode 100644 drivers/iommu/virtio/Makefile
 create mode 100644 drivers/iommu/virtio/topology-helpers.h
 create mode 100644 include/linux/virt_iommu.h
 create mode 100644 drivers/iommu/virtio/topology-helpers.c
 create mode 100644 drivers/iommu/virtio/topology.c
 rename drivers/iommu/{ => virtio}/virtio-iommu.c (99%)

-- 
2.28.0

Jean-Philippe Brucker

2020-Aug-21 13:15 UTC

head link

[PATCH v3 1/6] iommu/virtio: Move to drivers/iommu/virtio/

Before adding new files to the virtio-iommu driver, move it to its own
subfolder, similarly to other IOMMU drivers.

Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org>
---
 drivers/iommu/Makefile                    | 3 +--
 drivers/iommu/virtio/Makefile             | 2 ++
 drivers/iommu/{ => virtio}/virtio-iommu.c | 0
 MAINTAINERS                               | 2 +-
 4 files changed, 4 insertions(+), 3 deletions(-)
 create mode 100644 drivers/iommu/virtio/Makefile
 rename drivers/iommu/{ => virtio}/virtio-iommu.c (100%)

diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 11f1771104f3..fc7523042512 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
-obj-y += amd/ intel/ arm/
+obj-y += amd/ intel/ arm/ virtio/
 obj-$(CONFIG_IOMMU_API) += iommu.o
 obj-$(CONFIG_IOMMU_API) += iommu-traces.o
 obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
@@ -26,4 +26,3 @@ obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
 obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
 obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
 obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o
-obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
diff --git a/drivers/iommu/virtio/Makefile b/drivers/iommu/virtio/Makefile
new file mode 100644
index 000000000000..279368fcc074
--- /dev/null
+++ b/drivers/iommu/virtio/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio/virtio-iommu.c
similarity index 100%
rename from drivers/iommu/virtio-iommu.c
rename to drivers/iommu/virtio/virtio-iommu.c
diff --git a/MAINTAINERS b/MAINTAINERS
index deaafb617361..3602b223c9b2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18451,7 +18451,7 @@ VIRTIO IOMMU DRIVER
 M:	Jean-Philippe Brucker <jean-philippe at linaro.org>
 L:	virtualization at lists.linux-foundation.org
 S:	Maintained
-F:	drivers/iommu/virtio-iommu.c
+F:	drivers/iommu/virtio/
 F:	include/uapi/linux/virtio_iommu.h
 
 VIRTIO MEM DRIVER
-- 
2.28.0

Jean-Philippe Brucker

2020-Aug-21 13:15 UTC

head link

[PATCH v3 2/6] iommu/virtio: Add topology helpers

To support topology description from ACPI and from the builtin
description, add helpers to keep track of I/O topology descriptors.

To ease re-use of the helpers by other drivers and future ACPI
extensions, use the "virt_" prefix rather than "virtio_"
when naming
structs and functions.

Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org>
---
 drivers/iommu/Kconfig                   |   3 +
 drivers/iommu/virtio/Makefile           |   1 +
 drivers/iommu/virtio/topology-helpers.h |  50 ++++++
 include/linux/virt_iommu.h              |  15 ++
 drivers/iommu/virtio/topology-helpers.c | 196 ++++++++++++++++++++++++
 drivers/iommu/virtio/virtio-iommu.c     |   4 +
 MAINTAINERS                             |   1 +
 7 files changed, 270 insertions(+)
 create mode 100644 drivers/iommu/virtio/topology-helpers.h
 create mode 100644 include/linux/virt_iommu.h
 create mode 100644 drivers/iommu/virtio/topology-helpers.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index bef5d75e306b..e29ae50f7100 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -391,4 +391,7 @@ config VIRTIO_IOMMU
 
 	  Say Y here if you intend to run this kernel as a guest.
 
+config VIRTIO_IOMMU_TOPOLOGY_HELPERS
+	bool
+
 endif # IOMMU_SUPPORT
diff --git a/drivers/iommu/virtio/Makefile b/drivers/iommu/virtio/Makefile
index 279368fcc074..b42ad47eac7e 100644
--- a/drivers/iommu/virtio/Makefile
+++ b/drivers/iommu/virtio/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
+obj-$(CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS) += topology-helpers.o
diff --git a/drivers/iommu/virtio/topology-helpers.h
b/drivers/iommu/virtio/topology-helpers.h
new file mode 100644
index 000000000000..436ca6a900c5
--- /dev/null
+++ b/drivers/iommu/virtio/topology-helpers.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef TOPOLOGY_HELPERS_H_
+#define TOPOLOGY_HELPERS_H_
+
+#ifdef CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS
+
+/* Identify a device node in the topology */
+struct virt_topo_dev_id {
+	unsigned int			type;
+#define VIRT_TOPO_DEV_TYPE_PCI		1
+#define VIRT_TOPO_DEV_TYPE_MMIO		2
+	union {
+		/* PCI endpoint or range */
+		struct {
+			u16		segment;
+			u16		bdf_start;
+			u16		bdf_end;
+		};
+		/* MMIO region */
+		u64			base;
+	};
+};
+
+/* Specification of an IOMMU */
+struct virt_topo_iommu {
+	struct virt_topo_dev_id		dev_id;
+	struct device			*dev; /* transport device */
+	struct fwnode_handle		*fwnode;
+	struct iommu_ops		*ops;
+	struct list_head		list;
+};
+
+/* Specification of an endpoint */
+struct virt_topo_endpoint {
+	struct virt_topo_dev_id		dev_id;
+	u32				endpoint_id;
+	struct virt_topo_iommu		*viommu;
+	struct list_head		list;
+};
+
+void virt_topo_add_endpoint(struct virt_topo_endpoint *ep);
+void virt_topo_add_iommu(struct virt_topo_iommu *viommu);
+
+void virt_topo_set_iommu_ops(struct device *dev, struct iommu_ops *ops);
+
+#else /* !CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS */
+static inline void virt_topo_set_iommu_ops(struct device *dev, struct iommu_ops
*ops)
+{ }
+#endif /* !CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS */
+#endif /* TOPOLOGY_HELPERS_H_ */
diff --git a/include/linux/virt_iommu.h b/include/linux/virt_iommu.h
new file mode 100644
index 000000000000..17d2bd4732e0
--- /dev/null
+++ b/include/linux/virt_iommu.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef VIRT_IOMMU_H_
+#define VIRT_IOMMU_H_
+
+#ifdef CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS
+int virt_dma_configure(struct device *dev);
+
+#else /* !CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS */
+static inline int virt_dma_configure(struct device *dev)
+{
+	/* Don't disturb the normal DMA configuration methods */
+	return 0;
+}
+#endif /* !CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS */
+#endif /* VIRT_IOMMU_H_ */
diff --git a/drivers/iommu/virtio/topology-helpers.c
b/drivers/iommu/virtio/topology-helpers.c
new file mode 100644
index 000000000000..8815e3a5d431
--- /dev/null
+++ b/drivers/iommu/virtio/topology-helpers.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/dma-iommu.h>
+#include <linux/list.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/virt_iommu.h>
+
+#include "topology-helpers.h"
+
+static LIST_HEAD(viommus);
+static LIST_HEAD(pci_endpoints);
+static LIST_HEAD(mmio_endpoints);
+static DEFINE_MUTEX(viommus_lock);
+
+static bool virt_topo_device_match(struct device *dev,
+				   struct virt_topo_dev_id *id)
+{
+	if (id->type == VIRT_TOPO_DEV_TYPE_PCI && dev_is_pci(dev)) {
+		struct pci_dev *pdev = to_pci_dev(dev);
+		u16 dev_id = pci_dev_id(pdev);
+
+		return pci_domain_nr(pdev->bus) == id->segment &&
+			dev_id >= id->bdf_start &&
+			dev_id <= id->bdf_end;
+	} else if (id->type == VIRT_TOPO_DEV_TYPE_MMIO &&
+		   dev_is_platform(dev)) {
+		struct platform_device *plat_dev = to_platform_device(dev);
+		struct resource *mem;
+
+		mem = platform_get_resource(plat_dev, IORESOURCE_MEM, 0);
+		if (!mem)
+			return false;
+		return mem->start == id->base;
+	}
+	return false;
+}
+
+static const struct iommu_ops *virt_iommu_setup(struct device *dev)
+{
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+	struct virt_topo_iommu *viommu = NULL;
+	struct virt_topo_endpoint *ep;
+	struct pci_dev *pci_dev = NULL;
+	u32 epid;
+	int ret;
+
+	/* Already translated? */
+	if (fwspec && fwspec->ops)
+		return NULL;
+
+	mutex_lock(&viommus_lock);
+	if (dev_is_pci(dev)) {
+		pci_dev = to_pci_dev(dev);
+		list_for_each_entry(ep, &pci_endpoints, list) {
+			if (virt_topo_device_match(dev, &ep->dev_id)) {
+				epid = pci_dev_id(pci_dev) -
+					ep->dev_id.bdf_start +
+					ep->endpoint_id;
+				viommu = ep->viommu;
+				break;
+			}
+		}
+	} else if (dev_is_platform(dev)) {
+		list_for_each_entry(ep, &mmio_endpoints, list) {
+			if (virt_topo_device_match(dev, &ep->dev_id)) {
+				epid = ep->endpoint_id;
+				viommu = ep->viommu;
+				break;
+			}
+		}
+	}
+	mutex_unlock(&viommus_lock);
+	if (!viommu)
+		return NULL;
+
+	/* We're not translating ourselves. */
+	if (virt_topo_device_match(dev, &viommu->dev_id) ||
+	    dev == viommu->dev)
+		return NULL;
+
+	/*
+	 * If we found a PCI range managed by the viommu, we're the one that has
+	 * to request ACS.
+	 */
+	if (pci_dev)
+		pci_request_acs();
+
+	if (!viommu->ops)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	ret = iommu_fwspec_init(dev, viommu->fwnode, viommu->ops);
+	if (ret)
+		return ERR_PTR(ret);
+
+	iommu_fwspec_add_ids(dev, &epid, 1);
+
+	return viommu->ops;
+}
+
+/**
+ * virt_topo_add_endpoint - Register endpoint specification
+ * @ep: the endpoint specification
+ */
+void virt_topo_add_endpoint(struct virt_topo_endpoint *ep)
+{
+	mutex_lock(&viommus_lock);
+	list_add(&ep->list,
+		 ep->dev_id.type == VIRT_TOPO_DEV_TYPE_MMIO ?
+		 &mmio_endpoints : &pci_endpoints);
+	mutex_unlock(&viommus_lock);
+}
+
+/**
+ * virt_topo_add_iommu - Register IOMMU specification
+ * @viommu: the IOMMU specification
+ */
+void virt_topo_add_iommu(struct virt_topo_iommu *viommu)
+{
+	mutex_lock(&viommus_lock);
+	list_add(&viommu->list, &viommus);
+	mutex_unlock(&viommus_lock);
+}
+
+/**
+ * virt_dma_configure - Configure DMA of virtualized devices
+ * @dev: the endpoint
+ *
+ * Setup the DMA and IOMMU ops of a virtual device, for platforms without DT or
+ * ACPI.
+ *
+ * Return: -EPROBE_DEFER if the device is managed by an IOMMU that hasn't
been
+ *   probed yet, 0 otherwise
+ */
+int virt_dma_configure(struct device *dev)
+{
+	const struct iommu_ops *iommu_ops;
+
+	iommu_ops = virt_iommu_setup(dev);
+	if (IS_ERR_OR_NULL(iommu_ops)) {
+		int ret = PTR_ERR(iommu_ops);
+
+		if (ret == -EPROBE_DEFER || ret == 0)
+			return ret;
+		dev_err(dev, "error %d while setting up virt IOMMU\n", ret);
+		return 0;
+	}
+
+	/*
+	 * If we have reason to believe the IOMMU driver missed the initial
+	 * add_device callback for dev, replay it to get things in order.
+	 */
+	if (dev->bus && !device_iommu_mapped(dev))
+		iommu_probe_device(dev);
+
+	/* Assume coherent, as well as full 64-bit addresses. */
+#ifdef CONFIG_ARCH_HAS_SETUP_DMA_OPS
+	arch_setup_dma_ops(dev, 0, ~0ULL, iommu_ops, true);
+#else
+	iommu_setup_dma_ops(dev, 0, ~0ULL);
+#endif
+	return 0;
+}
+
+/**
+ * virt_topo_set_iommu_ops - Set the IOMMU ops of a virtual IOMMU device
+ * @dev: the IOMMU device (transport)
+ * @ops: the new IOMMU ops or NULL
+ *
+ * Setup the iommu_ops associated to an IOMMU, once the driver is loaded
+ * and the device probed.
+ */
+void virt_topo_set_iommu_ops(struct device *dev, struct iommu_ops *ops)
+{
+	struct virt_topo_iommu *viommu;
+
+	mutex_lock(&viommus_lock);
+	list_for_each_entry(viommu, &viommus, list) {
+		/*
+		 * In case the topology driver didn't have a dev handle when
+		 * registering the topology, add it now.
+		 */
+		if (!viommu->dev &&
+		    virt_topo_device_match(dev, &viommu->dev_id))
+			viommu->dev = dev;
+
+		if (viommu->dev == dev) {
+			viommu->ops = ops;
+			viommu->fwnode = ops ? dev->fwnode : NULL;
+			break;
+		}
+	}
+	mutex_unlock(&viommus_lock);
+}
+EXPORT_SYMBOL_GPL(virt_topo_set_iommu_ops);
diff --git a/drivers/iommu/virtio/virtio-iommu.c
b/drivers/iommu/virtio/virtio-iommu.c
index b4da396cce60..b371d15f837f 100644
--- a/drivers/iommu/virtio/virtio-iommu.c
+++ b/drivers/iommu/virtio/virtio-iommu.c
@@ -25,6 +25,8 @@
 
 #include <uapi/linux/virtio_iommu.h>
 
+#include "topology-helpers.h"
+
 #define MSI_IOVA_BASE			0x8000000
 #define MSI_IOVA_LENGTH			0x100000
 
@@ -1065,6 +1067,7 @@ static int viommu_probe(struct virtio_device *vdev)
 	if (ret)
 		goto err_free_vqs;
 
+	virt_topo_set_iommu_ops(dev->parent, &viommu_ops);
 	iommu_device_set_ops(&viommu->iommu, &viommu_ops);
 	iommu_device_set_fwnode(&viommu->iommu, parent_dev->fwnode);
 
@@ -1111,6 +1114,7 @@ static void viommu_remove(struct virtio_device *vdev)
 {
 	struct viommu_dev *viommu = vdev->priv;
 
+	virt_topo_set_iommu_ops(vdev->dev.parent, NULL);
 	iommu_device_sysfs_remove(&viommu->iommu);
 	iommu_device_unregister(&viommu->iommu);
 
diff --git a/MAINTAINERS b/MAINTAINERS
index 3602b223c9b2..8fd53c22a0ab 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18452,6 +18452,7 @@ M:	Jean-Philippe Brucker <jean-philippe at
linaro.org>
 L:	virtualization at lists.linux-foundation.org
 S:	Maintained
 F:	drivers/iommu/virtio/
+F:	include/linux/virt_iommu.h
 F:	include/uapi/linux/virtio_iommu.h
 
 VIRTIO MEM DRIVER
-- 
2.28.0

Jean-Philippe Brucker

2020-Aug-21 13:15 UTC

head link

[PATCH v3 3/6] PCI: Add DMA configuration for virtual platforms

Hardware platforms usually describe the IOMMU topology using either
device-tree pointers or vendor-specific ACPI tables.  For virtual
platforms that don't provide a device-tree, the virtio-iommu device
contains a description of the endpoints it manages.  That information
allows us to probe endpoints after the IOMMU is probed (possibly as late
as userspace modprobe), provided it is discovered early enough.

Add a hook to pci_dma_configure(), which returns -EPROBE_DEFER if the
endpoint is managed by a vIOMMU that will be loaded later, or 0 in any
other case to avoid disturbing the normal DMA configuration methods.
When CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS isn't selected, the call to
virt_dma_configure() is compiled out.

As long as the information is consistent, platforms can provide both a
device-tree and a built-in topology, and the IOMMU infrastructure is
able to deal with multiple DMA configuration methods.

Acked-by: Bjorn Helgaas <bhelgaas at google.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org>
---
 drivers/pci/pci-driver.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 449466f71040..dbe9d33606b0 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -19,6 +19,7 @@
 #include <linux/kexec.h>
 #include <linux/of_device.h>
 #include <linux/acpi.h>
+#include <linux/virt_iommu.h>
 #include "pci.h"
 #include "pcie/portdrv.h"
 
@@ -1605,6 +1606,10 @@ static int pci_dma_configure(struct device *dev)
 	struct device *bridge;
 	int ret = 0;
 
+	ret = virt_dma_configure(dev);
+	if (ret)
+		return ret;
+
 	bridge = pci_get_host_bridge_device(to_pci_dev(dev));
 
 	if (IS_ENABLED(CONFIG_OF) && bridge->parent &&
-- 
2.28.0

Jean-Philippe Brucker

2020-Aug-21 13:15 UTC

head link

[PATCH v3 4/6] iommu/virtio: Add topology definitions

Add struct definitions for describing endpoints managed by the
virtio-iommu. When VIRTIO_IOMMU_F_TOPOLOGY is offered, an array of
virtio_iommu_topo_* structures in config space describes the endpoints,
identified either by their PCI BDF or their physical MMIO address.

Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org>
---
 include/uapi/linux/virtio_iommu.h | 44 +++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/include/uapi/linux/virtio_iommu.h
b/include/uapi/linux/virtio_iommu.h
index 237e36a280cb..70cba30644d5 100644
--- a/include/uapi/linux/virtio_iommu.h
+++ b/include/uapi/linux/virtio_iommu.h
@@ -16,6 +16,7 @@
 #define VIRTIO_IOMMU_F_BYPASS			3
 #define VIRTIO_IOMMU_F_PROBE			4
 #define VIRTIO_IOMMU_F_MMIO			5
+#define VIRTIO_IOMMU_F_TOPOLOGY			6
 
 struct virtio_iommu_range_64 {
 	__le64					start;
@@ -27,6 +28,17 @@ struct virtio_iommu_range_32 {
 	__le32					end;
 };
 
+struct virtio_iommu_topo_config {
+	/* Number of topology description structures */
+	__le16					count;
+	/*
+	 * Offset to the first topology description structure
+	 * (virtio_iommu_topo_*) from the start of the virtio_iommu config
+	 * space. Aligned on 8 bytes.
+	 */
+	__le16					offset;
+};
+
 struct virtio_iommu_config {
 	/* Supported page sizes */
 	__le64					page_size_mask;
@@ -36,6 +48,38 @@ struct virtio_iommu_config {
 	struct virtio_iommu_range_32		domain_range;
 	/* Probe buffer size */
 	__le32					probe_size;
+	struct virtio_iommu_topo_config		topo_config;
+};
+
+#define VIRTIO_IOMMU_TOPO_PCI_RANGE		0x1
+#define VIRTIO_IOMMU_TOPO_MMIO			0x2
+
+struct virtio_iommu_topo_pci_range {
+	/* VIRTIO_IOMMU_TOPO_PCI_RANGE */
+	__u8					type;
+	__u8					reserved;
+	/* Length of this structure */
+	__le16					length;
+	/* First endpoint ID in the range */
+	__le32					endpoint_start;
+	/* PCI domain number */
+	__le16					segment;
+	/* PCI Bus:Device.Function range */
+	__le16					bdf_start;
+	__le16					bdf_end;
+	__le16					padding;
+};
+
+struct virtio_iommu_topo_mmio {
+	/* VIRTIO_IOMMU_TOPO_MMIO */
+	__u8					type;
+	__u8					reserved;
+	/* Length of this structure */
+	__le16					length;
+	/* Endpoint ID */
+	__le32					endpoint;
+	/* Address of the first MMIO region */
+	__le64					address;
 };
 
 /* Request types */
-- 
2.28.0

Jean-Philippe Brucker

2020-Aug-21 13:15 UTC

head link

[PATCH v3 5/6] iommu/virtio: Support topology description in config space

Platforms without device-tree nor ACPI can provide a topology
description embedded into the virtio config space. Parse it.

Use PCI FIXUP to probe the config space early, because we need to
discover the topology before any DMA configuration takes place, and the
virtio driver may be loaded much later. Since we discover the topology
description when probing the PCI hierarchy, the virtual IOMMU cannot
manage other platform devices discovered earlier.

Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org>
---
 drivers/iommu/Kconfig           |  12 ++
 drivers/iommu/virtio/Makefile   |   1 +
 drivers/iommu/virtio/topology.c | 259 ++++++++++++++++++++++++++++++++
 3 files changed, 272 insertions(+)
 create mode 100644 drivers/iommu/virtio/topology.c

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index e29ae50f7100..98d28fdbc19a 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -394,4 +394,16 @@ config VIRTIO_IOMMU
 config VIRTIO_IOMMU_TOPOLOGY_HELPERS
 	bool
 
+config VIRTIO_IOMMU_TOPOLOGY
+	bool "Handle topology properties from the virtio-iommu"
+	depends on VIRTIO_IOMMU
+	depends on PCI
+	default y
+	select VIRTIO_IOMMU_TOPOLOGY_HELPERS
+	help
+	  Enable early probing of virtio-iommu devices to detect the built-in
+	  topology description.
+
+	  Say Y here if you intend to run this kernel as a guest.
+
 endif # IOMMU_SUPPORT
diff --git a/drivers/iommu/virtio/Makefile b/drivers/iommu/virtio/Makefile
index b42ad47eac7e..1eda8ca1cbbf 100644
--- a/drivers/iommu/virtio/Makefile
+++ b/drivers/iommu/virtio/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
+obj-$(CONFIG_VIRTIO_IOMMU_TOPOLOGY) += topology.o
 obj-$(CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS) += topology-helpers.o
diff --git a/drivers/iommu/virtio/topology.c b/drivers/iommu/virtio/topology.c
new file mode 100644
index 000000000000..4923eec618b9
--- /dev/null
+++ b/drivers/iommu/virtio/topology.c
@@ -0,0 +1,259 @@
+// SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/io-64-nonatomic-hi-lo.h>
+#include <linux/iopoll.h>
+#include <linux/list.h>
+#include <linux/pci.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_pci.h>
+#include <uapi/linux/virtio_config.h>
+#include <uapi/linux/virtio_iommu.h>
+
+#include "topology-helpers.h"
+
+struct viommu_cap_config {
+	u8 bar;
+	u32 length; /* structure size */
+	u32 offset; /* structure offset within the bar */
+};
+
+struct viommu_topo_header {
+	u8 type;
+	u8 reserved;
+	u16 length;
+};
+
+static struct virt_topo_endpoint *
+viommu_parse_node(void __iomem *buf, size_t len)
+{
+	int ret = -EINVAL;
+	union {
+		struct viommu_topo_header hdr;
+		struct virtio_iommu_topo_pci_range pci;
+		struct virtio_iommu_topo_mmio mmio;
+	} __iomem *cfg = buf;
+	struct virt_topo_endpoint *spec;
+
+	spec = kzalloc(sizeof(*spec), GFP_KERNEL);
+	if (!spec)
+		return ERR_PTR(-ENOMEM);
+
+	switch (ioread8(&cfg->hdr.type)) {
+	case VIRTIO_IOMMU_TOPO_PCI_RANGE:
+		if (len < sizeof(cfg->pci))
+			goto err_free;
+
+		spec->dev_id.type = VIRT_TOPO_DEV_TYPE_PCI;
+		spec->dev_id.segment = ioread16(&cfg->pci.segment);
+		spec->dev_id.bdf_start = ioread16(&cfg->pci.bdf_start);
+		spec->dev_id.bdf_end = ioread16(&cfg->pci.bdf_end);
+		spec->endpoint_id = ioread32(&cfg->pci.endpoint_start);
+		break;
+	case VIRTIO_IOMMU_TOPO_MMIO:
+		if (len < sizeof(cfg->mmio))
+			goto err_free;
+
+		spec->dev_id.type = VIRT_TOPO_DEV_TYPE_MMIO;
+		spec->dev_id.base = ioread64(&cfg->mmio.address);
+		spec->endpoint_id = ioread32(&cfg->mmio.endpoint);
+		break;
+	default:
+		pr_warn("unhandled format 0x%x\n", ioread8(&cfg->hdr.type));
+		ret = 0;
+		goto err_free;
+	}
+	return spec;
+
+err_free:
+	kfree(spec);
+	return ERR_PTR(ret);
+}
+
+static int viommu_parse_topology(struct device *dev,
+				 struct virtio_iommu_config __iomem *cfg,
+				 size_t max_len)
+{
+	int ret;
+	u16 len;
+	size_t i;
+	LIST_HEAD(endpoints);
+	size_t offset, count;
+	struct virt_topo_iommu *viommu;
+	struct virt_topo_endpoint *ep, *next;
+	struct viommu_topo_header __iomem *cur;
+
+	offset = ioread16(&cfg->topo_config.offset);
+	count = ioread16(&cfg->topo_config.count);
+	if (!offset || !count)
+		return 0;
+
+	viommu = kzalloc(sizeof(*viommu), GFP_KERNEL);
+	if (!viommu)
+		return -ENOMEM;
+
+	viommu->dev = dev;
+
+	for (i = 0; i < count; i++, offset += len) {
+		if (offset + sizeof(*cur) > max_len) {
+			ret = -EOVERFLOW;
+			goto err_free;
+		}
+
+		cur = (void __iomem *)cfg + offset;
+		len = ioread16(&cur->length);
+		if (offset + len > max_len) {
+			ret = -EOVERFLOW;
+			goto err_free;
+		}
+
+		ep = viommu_parse_node((void __iomem *)cur, len);
+		if (!ep) {
+			continue;
+		} else if (IS_ERR(ep)) {
+			ret = PTR_ERR(ep);
+			goto err_free;
+		}
+
+		ep->viommu = viommu;
+		list_add(&ep->list, &endpoints);
+	}
+
+	list_for_each_entry_safe(ep, next, &endpoints, list)
+		/* Moves ep to the helpers list */
+		virt_topo_add_endpoint(ep);
+	virt_topo_add_iommu(viommu);
+
+	return 0;
+err_free:
+	list_for_each_entry_safe(ep, next, &endpoints, list)
+		kfree(ep);
+	kfree(viommu);
+	return ret;
+}
+
+#define VPCI_FIELD(field) offsetof(struct virtio_pci_cap, field)
+
+static inline int viommu_pci_find_capability(struct pci_dev *dev, u8 cfg_type,
+					     struct viommu_cap_config *cap)
+{
+	int pos;
+	u8 bar;
+
+	for (pos = pci_find_capability(dev, PCI_CAP_ID_VNDR);
+	     pos > 0;
+	     pos = pci_find_next_capability(dev, pos, PCI_CAP_ID_VNDR)) {
+		u8 type;
+
+		pci_read_config_byte(dev, pos + VPCI_FIELD(cfg_type), &type);
+		if (type != cfg_type)
+			continue;
+
+		pci_read_config_byte(dev, pos + VPCI_FIELD(bar), &bar);
+
+		/* Ignore structures with reserved BAR values */
+		if (type != VIRTIO_PCI_CAP_PCI_CFG && bar > 0x5)
+			continue;
+
+		cap->bar = bar;
+		pci_read_config_dword(dev, pos + VPCI_FIELD(length),
+				      &cap->length);
+		pci_read_config_dword(dev, pos + VPCI_FIELD(offset),
+				      &cap->offset);
+
+		return pos;
+	}
+	return 0;
+}
+
+static int viommu_pci_reset(struct virtio_pci_common_cfg __iomem *cfg)
+{
+	u8 status;
+	ktime_t timeout = ktime_add_ms(ktime_get(), 100);
+
+	iowrite8(0, &cfg->device_status);
+	while ((status = ioread8(&cfg->device_status)) != 0 &&
+	       ktime_before(ktime_get(), timeout))
+		msleep(1);
+
+	return status ? -ETIMEDOUT : 0;
+}
+
+static void viommu_pci_parse_topology(struct pci_dev *dev)
+{
+	int ret;
+	u32 features;
+	void __iomem *regs, *common_regs;
+	struct viommu_cap_config cap = {0};
+	struct virtio_pci_common_cfg __iomem *common_cfg;
+
+	/*
+	 * The virtio infrastructure might not be loaded at this point. We need
+	 * to access the BARs ourselves.
+	 */
+	ret = viommu_pci_find_capability(dev, VIRTIO_PCI_CAP_COMMON_CFG, &cap);
+	if (!ret) {
+		pci_warn(dev, "common capability not found\n");
+		return;
+	}
+
+	if (pci_enable_device_mem(dev))
+		return;
+
+	common_regs = pci_iomap(dev, cap.bar, 0);
+	if (!common_regs)
+		return;
+
+	common_cfg = common_regs + cap.offset;
+
+	/* Perform the init sequence before we can read the config */
+	ret = viommu_pci_reset(common_cfg);
+	if (ret < 0) {
+		pci_warn(dev, "unable to reset device\n");
+		goto out_unmap_common;
+	}
+
+	iowrite8(VIRTIO_CONFIG_S_ACKNOWLEDGE, &common_cfg->device_status);
+	iowrite8(VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER,
+		 &common_cfg->device_status);
+
+	/* Find out if the device supports topology description */
+	iowrite32(0, &common_cfg->device_feature_select);
+	features = ioread32(&common_cfg->device_feature);
+
+	if (!(features & BIT(VIRTIO_IOMMU_F_TOPOLOGY))) {
+		pci_dbg(dev, "device doesn't have topology description");
+		goto out_reset;
+	}
+
+	ret = viommu_pci_find_capability(dev, VIRTIO_PCI_CAP_DEVICE_CFG, &cap);
+	if (!ret) {
+		pci_warn(dev, "device config capability not found\n");
+		goto out_reset;
+	}
+
+	regs = pci_iomap(dev, cap.bar, 0);
+	if (!regs)
+		goto out_reset;
+
+	pci_info(dev, "parsing virtio-iommu topology\n");
+	ret = viommu_parse_topology(&dev->dev, regs + cap.offset,
+				    pci_resource_len(dev, 0) - cap.offset);
+	if (ret)
+		pci_warn(dev, "failed to parse topology: %d\n", ret);
+
+	pci_iounmap(dev, regs);
+out_reset:
+	ret = viommu_pci_reset(common_cfg);
+	if (ret)
+		pci_warn(dev, "unable to reset device\n");
+out_unmap_common:
+	pci_iounmap(dev, common_regs);
+}
+
+/*
+ * Catch a PCI virtio-iommu implementation early to get the topology
description
+ * before we start probing other endpoints.
+ */
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REDHAT_QUMRANET, 0x1040 +
VIRTIO_ID_IOMMU,
+			viommu_pci_parse_topology);
-- 
2.28.0

Jean-Philippe Brucker

2020-Aug-21 13:15 UTC

head link

[PATCH v3 6/6] iommu/virtio: Enable x86 support

With the built-in topology description in place, x86 platforms can now
use the virtio-iommu.

Architectures that use the generic iommu_dma_ops should normally select
CONFIG_IOMMU_DMA themselves (from arch/*/Kconfig). Since not all x86
drivers have been converted yet, it's currently up to the IOMMU Kconfig
to select it.

Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org>
---
 drivers/iommu/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 98d28fdbc19a..d7cf158745eb 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -383,8 +383,9 @@ config HYPERV_IOMMU
 config VIRTIO_IOMMU
 	tristate "Virtio IOMMU driver"
 	depends on VIRTIO
-	depends on ARM64
+	depends on (ARM64 || X86)
 	select IOMMU_API
+	select IOMMU_DMA if X86
 	select INTERVAL_TREE
 	help
 	  Para-virtualised IOMMU driver with virtio.
-- 
2.28.0

Michael S. Tsirkin

2020-Aug-26 13:26 UTC

head link

[PATCH v3 0/6] Add virtio-iommu built-in topology

On Fri, Aug 21, 2020 at 03:15:34PM +0200, Jean-Philippe Brucker
wrote:> Add a topology description to the virtio-iommu driver and enable x86
> platforms.
> 
> Since [v2] we have made some progress on adding ACPI support for
> virtio-iommu, which is the preferred boot method on x86. It will be a
> new vendor-agnostic table describing para-virtual topologies in a
> minimal format. However some platforms don't use either ACPI or DT for
> booting (for example microvm), and will need the alternative topology
> description method proposed here. In addition, since the process to get
> a new ACPI table will take a long time, this provides a boot method even
> to ACPI-based platforms, if only temporarily for testing and
> development.
OK should I park this in next now? Seems appropriate ...
> v3:
> * Add patch 1 that moves virtio-iommu to a subfolder.
> * Split the rest:
>   * Patch 2 adds topology-helper.c, which will be shared with the ACPI
>     support.
>   * Patch 4 adds definitions.
>   * Patch 5 adds parser in topology.c.
> * Address other comments.
> 
> Linux and QEMU patches available at:
> https://jpbrucker.net/git/linux virtio-iommu/devel
> https://jpbrucker.net/git/qemu virtio-iommu/devel
> 
> [spec]
https://lists.oasis-open.org/archives/virtio-dev/202008/msg00067.html
> [v2]
https://lore.kernel.org/linux-iommu/20200228172537.377327-1-jean-philippe at
linaro.org/
> [v1]
https://lore.kernel.org/linux-iommu/20200214160413.1475396-1-jean-philippe at
linaro.org/
> [rfc]
https://lore.kernel.org/linux-iommu/20191122105000.800410-1-jean-philippe at
linaro.org/
> 
> Jean-Philippe Brucker (6):
>   iommu/virtio: Move to drivers/iommu/virtio/
>   iommu/virtio: Add topology helpers
>   PCI: Add DMA configuration for virtual platforms
>   iommu/virtio: Add topology definitions
>   iommu/virtio: Support topology description in config space
>   iommu/virtio: Enable x86 support
> 
>  drivers/iommu/Kconfig                     |  18 +-
>  drivers/iommu/Makefile                    |   3 +-
>  drivers/iommu/virtio/Makefile             |   4 +
>  drivers/iommu/virtio/topology-helpers.h   |  50 +++++
>  include/linux/virt_iommu.h                |  15 ++
>  include/uapi/linux/virtio_iommu.h         |  44 ++++
>  drivers/iommu/virtio/topology-helpers.c   | 196 ++++++++++++++++
>  drivers/iommu/virtio/topology.c           | 259 ++++++++++++++++++++++
>  drivers/iommu/{ => virtio}/virtio-iommu.c |   4 +
>  drivers/pci/pci-driver.c                  |   5 +
>  MAINTAINERS                               |   3 +-
>  11 files changed, 597 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/iommu/virtio/Makefile
>  create mode 100644 drivers/iommu/virtio/topology-helpers.h
>  create mode 100644 include/linux/virt_iommu.h
>  create mode 100644 drivers/iommu/virtio/topology-helpers.c
>  create mode 100644 drivers/iommu/virtio/topology.c
>  rename drivers/iommu/{ => virtio}/virtio-iommu.c (99%)
> 
> -- 
> 2.28.0
> 
> _______________________________________________
> iommu mailing list
> iommu at lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

Jean-Philippe Brucker

2020-Aug-27 08:01 UTC

head link

[PATCH v3 0/6] Add virtio-iommu built-in topology

On Wed, Aug 26, 2020 at 09:26:02AM -0400, Michael S. Tsirkin
wrote:> On Fri, Aug 21, 2020 at 03:15:34PM +0200, Jean-Philippe Brucker wrote:
> > Add a topology description to the virtio-iommu driver and enable x86
> > platforms.
> > 
> > Since [v2] we have made some progress on adding ACPI support for
> > virtio-iommu, which is the preferred boot method on x86. It will be a
> > new vendor-agnostic table describing para-virtual topologies in a
> > minimal format. However some platforms don't use either ACPI or DT
for
> > booting (for example microvm), and will need the alternative topology
> > description method proposed here. In addition, since the process to
get
> > a new ACPI table will take a long time, this provides a boot method
even
> > to ACPI-based platforms, if only temporarily for testing and
> > development.
> 
> OK should I park this in next now? Seems appropriate ...
Yes that sounds like a good idea. It could uncover new bugs since there is
more automated testing happening for x86.

Thanks,
Jean

Auger Eric

2020-Sep-04 15:29 UTC

head link

[PATCH v3 1/6] iommu/virtio: Move to drivers/iommu/virtio/

Hi Jean,

On 8/21/20 3:15 PM, Jean-Philippe Brucker wrote:> Before adding new files to the virtio-iommu driver, move it to its own
> subfolder, similarly to other IOMMU drivers.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org>
> ---
>  drivers/iommu/Makefile                    | 3 +--
>  drivers/iommu/virtio/Makefile             | 2 ++
>  drivers/iommu/{ => virtio}/virtio-iommu.c | 0
>  MAINTAINERS                               | 2 +-
>  4 files changed, 4 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/iommu/virtio/Makefile
>  rename drivers/iommu/{ => virtio}/virtio-iommu.c (100%)
> 
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 11f1771104f3..fc7523042512 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -1,5 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> -obj-y += amd/ intel/ arm/
> +obj-y += amd/ intel/ arm/ virtio/
>  obj-$(CONFIG_IOMMU_API) += iommu.o
>  obj-$(CONFIG_IOMMU_API) += iommu-traces.o
>  obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
> @@ -26,4 +26,3 @@ obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
>  obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
>  obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
>  obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o
> -obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
> diff --git a/drivers/iommu/virtio/Makefile b/drivers/iommu/virtio/Makefile
> new file mode 100644
> index 000000000000..279368fcc074
> --- /dev/null
> +++ b/drivers/iommu/virtio/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
> diff --git a/drivers/iommu/virtio-iommu.c
b/drivers/iommu/virtio/virtio-iommu.c
> similarity index 100%
> rename from drivers/iommu/virtio-iommu.c
> rename to drivers/iommu/virtio/virtio-iommu.c
> diff --git a/MAINTAINERS b/MAINTAINERS
> index deaafb617361..3602b223c9b2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18451,7 +18451,7 @@ VIRTIO IOMMU DRIVER
>  M:	Jean-Philippe Brucker <jean-philippe at linaro.org>
>  L:	virtualization at lists.linux-foundation.org
>  S:	Maintained
> -F:	drivers/iommu/virtio-iommu.c
> +F:	drivers/iommu/virtio/
>  F:	include/uapi/linux/virtio_iommu.hnot related to this patch but you may add an entry for
Documentation/devicetree/bindings/virtio/iommu.txt

Reviewed-by: Eric Auger <eric.auger at redhat.com>

Thanks

Eric>  
>  VIRTIO MEM DRIVER
>

Auger Eric

2020-Sep-04 15:30 UTC

head link

[PATCH v3 4/6] iommu/virtio: Add topology definitions

Hi Jean,

On 8/21/20 3:15 PM, Jean-Philippe Brucker wrote:> Add struct definitions for describing endpoints managed by the
> virtio-iommu. When VIRTIO_IOMMU_F_TOPOLOGY is offered, an array of
> virtio_iommu_topo_* structures in config space describes the endpoints,
> identified either by their PCI BDF or their physical MMIO address.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org>Reviewed-by: Eric Auger <eric.auger at redhat.com>

Thanks

Eric
> ---
>  include/uapi/linux/virtio_iommu.h | 44 +++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/include/uapi/linux/virtio_iommu.h
b/include/uapi/linux/virtio_iommu.h
> index 237e36a280cb..70cba30644d5 100644
> --- a/include/uapi/linux/virtio_iommu.h
> +++ b/include/uapi/linux/virtio_iommu.h
> @@ -16,6 +16,7 @@
>  #define VIRTIO_IOMMU_F_BYPASS			3
>  #define VIRTIO_IOMMU_F_PROBE			4
>  #define VIRTIO_IOMMU_F_MMIO			5
> +#define VIRTIO_IOMMU_F_TOPOLOGY			6
>  
>  struct virtio_iommu_range_64 {
>  	__le64					start;
> @@ -27,6 +28,17 @@ struct virtio_iommu_range_32 {
>  	__le32					end;
>  };
>  
> +struct virtio_iommu_topo_config {
> +	/* Number of topology description structures */
> +	__le16					count;
> +	/*
> +	 * Offset to the first topology description structure
> +	 * (virtio_iommu_topo_*) from the start of the virtio_iommu config
> +	 * space. Aligned on 8 bytes.
> +	 */
> +	__le16					offset;
> +};
> +
>  struct virtio_iommu_config {
>  	/* Supported page sizes */
>  	__le64					page_size_mask;
> @@ -36,6 +48,38 @@ struct virtio_iommu_config {
>  	struct virtio_iommu_range_32		domain_range;
>  	/* Probe buffer size */
>  	__le32					probe_size;
> +	struct virtio_iommu_topo_config		topo_config;
> +};
> +
> +#define VIRTIO_IOMMU_TOPO_PCI_RANGE		0x1
> +#define VIRTIO_IOMMU_TOPO_MMIO			0x2
> +
> +struct virtio_iommu_topo_pci_range {
> +	/* VIRTIO_IOMMU_TOPO_PCI_RANGE */
> +	__u8					type;
> +	__u8					reserved;
> +	/* Length of this structure */
> +	__le16					length;
> +	/* First endpoint ID in the range */
> +	__le32					endpoint_start;
> +	/* PCI domain number */
> +	__le16					segment;
> +	/* PCI Bus:Device.Function range */
> +	__le16					bdf_start;
> +	__le16					bdf_end;
> +	__le16					padding;
> +};
> +
> +struct virtio_iommu_topo_mmio {
> +	/* VIRTIO_IOMMU_TOPO_MMIO */
> +	__u8					type;
> +	__u8					reserved;
> +	/* Length of this structure */
> +	__le16					length;
> +	/* Endpoint ID */
> +	__le32					endpoint;
> +	/* Address of the first MMIO region */
> +	__le64					address;
>  };
>  
>  /* Request types */
>

Auger Eric

2020-Sep-04 16:05 UTC

head link

[PATCH v3 5/6] iommu/virtio: Support topology description in config space

Hi Jean,

On 8/21/20 3:15 PM, Jean-Philippe Brucker wrote:> Platforms without device-tree nor ACPI can provide a topology
> description embedded into the virtio config space. Parse it.
> 
> Use PCI FIXUP to probe the config space early, because we need to
> discover the topology before any DMA configuration takes place, and the
> virtio driver may be loaded much later. Since we discover the topology
> description when probing the PCI hierarchy, the virtual IOMMU cannot
> manage other platform devices discovered earlier.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org>
> ---
>  drivers/iommu/Kconfig           |  12 ++
>  drivers/iommu/virtio/Makefile   |   1 +
>  drivers/iommu/virtio/topology.c | 259 ++++++++++++++++++++++++++++++++
>  3 files changed, 272 insertions(+)
>  create mode 100644 drivers/iommu/virtio/topology.c
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index e29ae50f7100..98d28fdbc19a 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -394,4 +394,16 @@ config VIRTIO_IOMMU
>  config VIRTIO_IOMMU_TOPOLOGY_HELPERS
>  	bool
>  
> +config VIRTIO_IOMMU_TOPOLOGY
> +	bool "Handle topology properties from the virtio-iommu"
> +	depends on VIRTIO_IOMMU
> +	depends on PCI
> +	default y
> +	select VIRTIO_IOMMU_TOPOLOGY_HELPERS
> +	help
> +	  Enable early probing of virtio-iommu devices to detect the built-in
> +	  topology description.
> +
> +	  Say Y here if you intend to run this kernel as a guest.
> +
>  endif # IOMMU_SUPPORT
> diff --git a/drivers/iommu/virtio/Makefile b/drivers/iommu/virtio/Makefile
> index b42ad47eac7e..1eda8ca1cbbf 100644
> --- a/drivers/iommu/virtio/Makefile
> +++ b/drivers/iommu/virtio/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
> +obj-$(CONFIG_VIRTIO_IOMMU_TOPOLOGY) += topology.o
>  obj-$(CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS) += topology-helpers.o
> diff --git a/drivers/iommu/virtio/topology.c
b/drivers/iommu/virtio/topology.c
> new file mode 100644
> index 000000000000..4923eec618b9
> --- /dev/null
> +++ b/drivers/iommu/virtio/topology.c
> @@ -0,0 +1,259 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/io-64-nonatomic-hi-lo.h>
> +#include <linux/iopoll.h>
> +#include <linux/list.h>
> +#include <linux/pci.h>
> +#include <linux/virtio_ids.h>
> +#include <linux/virtio_pci.h>
> +#include <uapi/linux/virtio_config.h>
> +#include <uapi/linux/virtio_iommu.h>
> +
> +#include "topology-helpers.h"
> +
> +struct viommu_cap_config {
> +	u8 bar;
> +	u32 length; /* structure size */
> +	u32 offset; /* structure offset within the bar */
> +};
> +
> +struct viommu_topo_header {
> +	u8 type;
> +	u8 reserved;
> +	u16 length;
> +};
> +
> +static struct virt_topo_endpoint *
> +viommu_parse_node(void __iomem *buf, size_t len)
> +{
> +	int ret = -EINVAL;
> +	union {
> +		struct viommu_topo_header hdr;
> +		struct virtio_iommu_topo_pci_range pci;
> +		struct virtio_iommu_topo_mmio mmio;
> +	} __iomem *cfg = buf;
> +	struct virt_topo_endpoint *spec;
> +
> +	spec = kzalloc(sizeof(*spec), GFP_KERNEL);
> +	if (!spec)
> +		return ERR_PTR(-ENOMEM);
> +
> +	switch (ioread8(&cfg->hdr.type)) {
> +	case VIRTIO_IOMMU_TOPO_PCI_RANGE:
> +		if (len < sizeof(cfg->pci))
> +			goto err_free;
> +
> +		spec->dev_id.type = VIRT_TOPO_DEV_TYPE_PCI;
> +		spec->dev_id.segment = ioread16(&cfg->pci.segment);
> +		spec->dev_id.bdf_start = ioread16(&cfg->pci.bdf_start);
> +		spec->dev_id.bdf_end = ioread16(&cfg->pci.bdf_end);
> +		spec->endpoint_id = ioread32(&cfg->pci.endpoint_start);
> +		break;
> +	case VIRTIO_IOMMU_TOPO_MMIO:
> +		if (len < sizeof(cfg->mmio))
> +			goto err_free;
> +
> +		spec->dev_id.type = VIRT_TOPO_DEV_TYPE_MMIO;
> +		spec->dev_id.base = ioread64(&cfg->mmio.address);
> +		spec->endpoint_id = ioread32(&cfg->mmio.endpoint);
> +		break;
> +	default:
> +		pr_warn("unhandled format 0x%x\n",
ioread8(&cfg->hdr.type));
> +		ret = 0;
> +		goto err_free;
> +	}
> +	return spec;
> +
> +err_free:
> +	kfree(spec);
> +	return ERR_PTR(ret);
> +}
> +
> +static int viommu_parse_topology(struct device *dev,
> +				 struct virtio_iommu_config __iomem *cfg,
> +				 size_t max_len)
> +{
> +	int ret;
> +	u16 len;
> +	size_t i;
> +	LIST_HEAD(endpoints);
> +	size_t offset, count;
> +	struct virt_topo_iommu *viommu;
> +	struct virt_topo_endpoint *ep, *next;
> +	struct viommu_topo_header __iomem *cur;
> +
> +	offset = ioread16(&cfg->topo_config.offset);
> +	count = ioread16(&cfg->topo_config.count);
> +	if (!offset || !count)
> +		return 0;
> +
> +	viommu = kzalloc(sizeof(*viommu), GFP_KERNEL);
> +	if (!viommu)
> +		return -ENOMEM;
> +
> +	viommu->dev = dev;
> +
> +	for (i = 0; i < count; i++, offset += len) {
> +		if (offset + sizeof(*cur) > max_len) {
> +			ret = -EOVERFLOW;
> +			goto err_free;
> +		}
> +
> +		cur = (void __iomem *)cfg + offset;
> +		len = ioread16(&cur->length);
> +		if (offset + len > max_len) {
> +			ret = -EOVERFLOW;
> +			goto err_free;
> +		}
> +
> +		ep = viommu_parse_node((void __iomem *)cur, len);
> +		if (!ep) {
> +			continue;
> +		} else if (IS_ERR(ep)) {
> +			ret = PTR_ERR(ep);
> +			goto err_free;
> +		}
> +
> +		ep->viommu = viommu;
> +		list_add(&ep->list, &endpoints);
> +	}
> +
> +	list_for_each_entry_safe(ep, next, &endpoints, list)
> +		/* Moves ep to the helpers list */
> +		virt_topo_add_endpoint(ep);
> +	virt_topo_add_iommu(viommu);
> +
> +	return 0;
> +err_free:
> +	list_for_each_entry_safe(ep, next, &endpoints, list)
> +		kfree(ep);
> +	kfree(viommu);
> +	return ret;
> +}
> +
> +#define VPCI_FIELD(field) offsetof(struct virtio_pci_cap, field)
> +
> +static inline int viommu_pci_find_capability(struct pci_dev *dev, u8
cfg_type,
> +					     struct viommu_cap_config *cap)
not sure the inline is useful here> +{
> +	int pos;
> +	u8 bar;
> +
> +	for (pos = pci_find_capability(dev, PCI_CAP_ID_VNDR);
> +	     pos > 0;
> +	     pos = pci_find_next_capability(dev, pos, PCI_CAP_ID_VNDR)) {
> +		u8 type;
> +
> +		pci_read_config_byte(dev, pos + VPCI_FIELD(cfg_type), &type);
> +		if (type != cfg_type)
> +			continue;
> +
> +		pci_read_config_byte(dev, pos + VPCI_FIELD(bar), &bar);
> +
> +		/* Ignore structures with reserved BAR values */
> +		if (type != VIRTIO_PCI_CAP_PCI_CFG && bar > 0x5)
> +			continue;
> +
> +		cap->bar = bar;
> +		pci_read_config_dword(dev, pos + VPCI_FIELD(length),
> +				      &cap->length);
> +		pci_read_config_dword(dev, pos + VPCI_FIELD(offset),
> +				      &cap->offset);
> +
> +		return pos;
> +	}
> +	return 0;
> +}
> +
> +static int viommu_pci_reset(struct virtio_pci_common_cfg __iomem *cfg)
> +{
> +	u8 status;
> +	ktime_t timeout = ktime_add_ms(ktime_get(), 100);
> +
> +	iowrite8(0, &cfg->device_status);
> +	while ((status = ioread8(&cfg->device_status)) != 0 &&
> +	       ktime_before(ktime_get(), timeout))
> +		msleep(1);
> +
> +	return status ? -ETIMEDOUT : 0;
> +}
> +
> +static void viommu_pci_parse_topology(struct pci_dev *dev)
> +{
> +	int ret;
> +	u32 features;
> +	void __iomem *regs, *common_regs;
> +	struct viommu_cap_config cap = {0};
> +	struct virtio_pci_common_cfg __iomem *common_cfg;
> +
> +	/*
> +	 * The virtio infrastructure might not be loaded at this point. We need
> +	 * to access the BARs ourselves.
> +	 */
> +	ret = viommu_pci_find_capability(dev, VIRTIO_PCI_CAP_COMMON_CFG,
&cap);
> +	if (!ret) {
> +		pci_warn(dev, "common capability not found\n");
> +		return;
> +	}
> +
> +	if (pci_enable_device_mem(dev))
> +		return;
> +
> +	common_regs = pci_iomap(dev, cap.bar, 0);
> +	if (!common_regs)
> +		return;
> +
> +	common_cfg = common_regs + cap.offset;
> +
> +	/* Perform the init sequence before we can read the config */
> +	ret = viommu_pci_reset(common_cfg);
> +	if (ret < 0) {
> +		pci_warn(dev, "unable to reset device\n");
> +		goto out_unmap_common;
> +	}
> +
> +	iowrite8(VIRTIO_CONFIG_S_ACKNOWLEDGE, &common_cfg->device_status);
> +	iowrite8(VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER,
> +		 &common_cfg->device_status);
> +
> +	/* Find out if the device supports topology description */
> +	iowrite32(0, &common_cfg->device_feature_select);
> +	features = ioread32(&common_cfg->device_feature);
> +
> +	if (!(features & BIT(VIRTIO_IOMMU_F_TOPOLOGY))) {
> +		pci_dbg(dev, "device doesn't have topology description");
> +		goto out_reset;
> +	}
> +
> +	ret = viommu_pci_find_capability(dev, VIRTIO_PCI_CAP_DEVICE_CFG,
&cap);
> +	if (!ret) {
> +		pci_warn(dev, "device config capability not found\n");
> +		goto out_reset;
> +	}
> +
> +	regs = pci_iomap(dev, cap.bar, 0);
> +	if (!regs)
> +		goto out_reset;
> +
> +	pci_info(dev, "parsing virtio-iommu topology\n");
> +	ret = viommu_parse_topology(&dev->dev, regs + cap.offset,
> +				    pci_resource_len(dev, 0) - cap.offset);
> +	if (ret)
> +		pci_warn(dev, "failed to parse topology: %d\n", ret);
> +
> +	pci_iounmap(dev, regs);
> +out_reset:
> +	ret = viommu_pci_reset(common_cfg);
> +	if (ret)
> +		pci_warn(dev, "unable to reset device\n");
> +out_unmap_common:
> +	pci_iounmap(dev, common_regs);
> +}
> +
> +/*
> + * Catch a PCI virtio-iommu implementation early to get the topology
description
> + * before we start probing other endpoints.
> + */
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REDHAT_QUMRANET, 0x1040 +
VIRTIO_ID_IOMMU,
> +			viommu_pci_parse_topology);
> Reviewed-by: Eric Auger <eric.auger at redhat.com>

Eric

Auger Eric

2020-Sep-04 16:22 UTC

head link

[PATCH v3 2/6] iommu/virtio: Add topology helpers

Hi Jean,

On 8/21/20 3:15 PM, Jean-Philippe Brucker wrote:> To support topology description from ACPI and from the builtin
> description, add helpers to keep track of I/O topology descriptors.
> 
> To ease re-use of the helpers by other drivers and future ACPI
> extensions, use the "virt_" prefix rather than
"virtio_" when naming
> structs and functions.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe at linaro.org>
> ---
>  drivers/iommu/Kconfig                   |   3 +
>  drivers/iommu/virtio/Makefile           |   1 +
>  drivers/iommu/virtio/topology-helpers.h |  50 ++++++
>  include/linux/virt_iommu.h              |  15 ++
>  drivers/iommu/virtio/topology-helpers.c | 196 ++++++++++++++++++++++++
>  drivers/iommu/virtio/virtio-iommu.c     |   4 +
>  MAINTAINERS                             |   1 +
>  7 files changed, 270 insertions(+)
>  create mode 100644 drivers/iommu/virtio/topology-helpers.h
>  create mode 100644 include/linux/virt_iommu.h
>  create mode 100644 drivers/iommu/virtio/topology-helpers.c
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index bef5d75e306b..e29ae50f7100 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -391,4 +391,7 @@ config VIRTIO_IOMMU
>  
>  	  Say Y here if you intend to run this kernel as a guest.
>  
> +config VIRTIO_IOMMU_TOPOLOGY_HELPERS
> +	bool
> +
>  endif # IOMMU_SUPPORT
> diff --git a/drivers/iommu/virtio/Makefile b/drivers/iommu/virtio/Makefile
> index 279368fcc074..b42ad47eac7e 100644
> --- a/drivers/iommu/virtio/Makefile
> +++ b/drivers/iommu/virtio/Makefile
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
> +obj-$(CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS) += topology-helpers.o
> diff --git a/drivers/iommu/virtio/topology-helpers.h
b/drivers/iommu/virtio/topology-helpers.h
> new file mode 100644
> index 000000000000..436ca6a900c5
> --- /dev/null
> +++ b/drivers/iommu/virtio/topology-helpers.h
> @@ -0,0 +1,50 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef TOPOLOGY_HELPERS_H_
> +#define TOPOLOGY_HELPERS_H_
> +
> +#ifdef CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS
> +
> +/* Identify a device node in the topology */
> +struct virt_topo_dev_id {
> +	unsigned int			type;
> +#define VIRT_TOPO_DEV_TYPE_PCI		1
> +#define VIRT_TOPO_DEV_TYPE_MMIO		2
> +	union {
> +		/* PCI endpoint or range */
> +		struct {
> +			u16		segment;
> +			u16		bdf_start;
> +			u16		bdf_end;
> +		};
> +		/* MMIO region */
> +		u64			base;
> +	};
> +};
> +
> +/* Specification of an IOMMU */
> +struct virt_topo_iommu {
> +	struct virt_topo_dev_id		dev_id;
> +	struct device			*dev; /* transport device */
> +	struct fwnode_handle		*fwnode;
> +	struct iommu_ops		*ops;
> +	struct list_head		list;
> +};
> +
> +/* Specification of an endpoint */
> +struct virt_topo_endpoint {
> +	struct virt_topo_dev_id		dev_id;
> +	u32				endpoint_id;
> +	struct virt_topo_iommu		*viommu;
> +	struct list_head		list;
> +};
> +
> +void virt_topo_add_endpoint(struct virt_topo_endpoint *ep);
> +void virt_topo_add_iommu(struct virt_topo_iommu *viommu);
> +
> +void virt_topo_set_iommu_ops(struct device *dev, struct iommu_ops *ops);
> +
> +#else /* !CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS */
> +static inline void virt_topo_set_iommu_ops(struct device *dev, struct
iommu_ops *ops)
> +{ }
> +#endif /* !CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS */
> +#endif /* TOPOLOGY_HELPERS_H_ */
> diff --git a/include/linux/virt_iommu.h b/include/linux/virt_iommu.h
> new file mode 100644
> index 000000000000..17d2bd4732e0
> --- /dev/null
> +++ b/include/linux/virt_iommu.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef VIRT_IOMMU_H_
> +#define VIRT_IOMMU_H_
> +
> +#ifdef CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS
> +int virt_dma_configure(struct device *dev);
> +
> +#else /* !CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS */
> +static inline int virt_dma_configure(struct device *dev)
> +{
> +	/* Don't disturb the normal DMA configuration methods */
> +	return 0;
> +}
> +#endif /* !CONFIG_VIRTIO_IOMMU_TOPOLOGY_HELPERS */
> +#endif /* VIRT_IOMMU_H_ */
> diff --git a/drivers/iommu/virtio/topology-helpers.c
b/drivers/iommu/virtio/topology-helpers.c
> new file mode 100644
> index 000000000000..8815e3a5d431
> --- /dev/null
> +++ b/drivers/iommu/virtio/topology-helpers.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/dma-iommu.h>
> +#include <linux/list.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/virt_iommu.h>
> +
> +#include "topology-helpers.h"
> +
> +static LIST_HEAD(viommus);
> +static LIST_HEAD(pci_endpoints);
> +static LIST_HEAD(mmio_endpoints);
> +static DEFINE_MUTEX(viommus_lock);
> +
> +static bool virt_topo_device_match(struct device *dev,
> +				   struct virt_topo_dev_id *id)
> +{
> +	if (id->type == VIRT_TOPO_DEV_TYPE_PCI && dev_is_pci(dev)) {
> +		struct pci_dev *pdev = to_pci_dev(dev);
> +		u16 dev_id = pci_dev_id(pdev);
> +
> +		return pci_domain_nr(pdev->bus) == id->segment &&
> +			dev_id >= id->bdf_start &&
> +			dev_id <= id->bdf_end;
> +	} else if (id->type == VIRT_TOPO_DEV_TYPE_MMIO &&
> +		   dev_is_platform(dev)) {
> +		struct platform_device *plat_dev = to_platform_device(dev);
> +		struct resource *mem;
> +
> +		mem = platform_get_resource(plat_dev, IORESOURCE_MEM, 0);
> +		if (!mem)
> +			return false;
> +		return mem->start == id->base;
> +	}
> +	return false;
> +}
> +
> +static const struct iommu_ops *virt_iommu_setup(struct device *dev)
> +{
> +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +	struct virt_topo_iommu *viommu = NULL;
> +	struct virt_topo_endpoint *ep;
> +	struct pci_dev *pci_dev = NULL;
> +	u32 epid;
> +	int ret;
> +
> +	/* Already translated? */
> +	if (fwspec && fwspec->ops)
> +		return NULL;
> +
> +	mutex_lock(&viommus_lock);
> +	if (dev_is_pci(dev)) {
> +		pci_dev = to_pci_dev(dev);
> +		list_for_each_entry(ep, &pci_endpoints, list) {
> +			if (virt_topo_device_match(dev, &ep->dev_id)) {
> +				epid = pci_dev_id(pci_dev) -
> +					ep->dev_id.bdf_start +
> +					ep->endpoint_id;
> +				viommu = ep->viommu;
> +				break;
> +			}
> +		}
> +	} else if (dev_is_platform(dev)) {
> +		list_for_each_entry(ep, &mmio_endpoints, list) {
> +			if (virt_topo_device_match(dev, &ep->dev_id)) {
> +				epid = ep->endpoint_id;
> +				viommu = ep->viommu;
> +				break;
> +			}
> +		}
> +	}
> +	mutex_unlock(&viommus_lock);
> +	if (!viommu)
> +		return NULL;
> +
> +	/* We're not translating ourselves. */
> +	if (virt_topo_device_match(dev, &viommu->dev_id) ||
> +	    dev == viommu->dev)
> +		return NULL;
> +
> +	/*
> +	 * If we found a PCI range managed by the viommu, we're the one that
has
> +	 * to request ACS.
> +	 */
> +	if (pci_dev)
> +		pci_request_acs();
> +
> +	if (!viommu->ops)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	ret = iommu_fwspec_init(dev, viommu->fwnode, viommu->ops);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	iommu_fwspec_add_ids(dev, &epid, 1);
> +
> +	return viommu->ops;
> +}
> +
> +/**
> + * virt_topo_add_endpoint - Register endpoint specification
> + * @ep: the endpoint specification
> + */
> +void virt_topo_add_endpoint(struct virt_topo_endpoint *ep)
> +{
> +	mutex_lock(&viommus_lock);
> +	list_add(&ep->list,
> +		 ep->dev_id.type == VIRT_TOPO_DEV_TYPE_MMIO ?
> +		 &mmio_endpoints : &pci_endpoints);
> +	mutex_unlock(&viommus_lock);
> +}
> +
> +/**
> + * virt_topo_add_iommu - Register IOMMU specification
> + * @viommu: the IOMMU specification
> + */
> +void virt_topo_add_iommu(struct virt_topo_iommu *viommu)
> +{
> +	mutex_lock(&viommus_lock);
> +	list_add(&viommu->list, &viommus);
> +	mutex_unlock(&viommus_lock);
> +}
> +
> +/**
> + * virt_dma_configure - Configure DMA of virtualized devices
> + * @dev: the endpoint
> + *
> + * Setup the DMA and IOMMU ops of a virtual device, for platforms without
DT or
> + * ACPI.
> + *
> + * Return: -EPROBE_DEFER if the device is managed by an IOMMU that
hasn't been
> + *   probed yet, 0 otherwise
> + */
> +int virt_dma_configure(struct device *dev)
> +{
> +	const struct iommu_ops *iommu_ops;
> +
> +	iommu_ops = virt_iommu_setup(dev);
> +	if (IS_ERR_OR_NULL(iommu_ops)) {
> +		int ret = PTR_ERR(iommu_ops);
> +
> +		if (ret == -EPROBE_DEFER || ret == 0)
> +			return ret;
> +		dev_err(dev, "error %d while setting up virt IOMMU\n", ret);
> +		return 0;
why do we return 0 here?> +	}
> +
> +	/*
> +	 * If we have reason to believe the IOMMU driver missed the initial
> +	 * add_device callback for dev, replay it to get things in order.
> +	 */
> +	if (dev->bus && !device_iommu_mapped(dev))
> +		iommu_probe_device(dev);
> +
> +	/* Assume coherent, as well as full 64-bit addresses. */
> +#ifdef CONFIG_ARCH_HAS_SETUP_DMA_OPS
> +	arch_setup_dma_ops(dev, 0, ~0ULL, iommu_ops, true);
> +#else
> +	iommu_setup_dma_ops(dev, 0, ~0ULL);
> +#endif
> +	return 0;
> +}
> +
> +/**
> + * virt_topo_set_iommu_ops - Set the IOMMU ops of a virtual IOMMU device
> + * @dev: the IOMMU device (transport)
> + * @ops: the new IOMMU ops or NULL
> + *
> + * Setup the iommu_ops associated to an IOMMU, once the driver is loaded
> + * and the device probed.
> + */
> +void virt_topo_set_iommu_ops(struct device *dev, struct iommu_ops *ops)
> +{
> +	struct virt_topo_iommu *viommu;
> +
> +	mutex_lock(&viommus_lock);
> +	list_for_each_entry(viommu, &viommus, list) {
> +		/*
> +		 * In case the topology driver didn't have a dev handle when
> +		 * registering the topology, add it now.
> +		 */
> +		if (!viommu->dev &&
> +		    virt_topo_device_match(dev, &viommu->dev_id))
> +			viommu->dev = dev;
> +
> +		if (viommu->dev == dev) {
> +			viommu->ops = ops;
> +			viommu->fwnode = ops ? dev->fwnode : NULL;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&viommus_lock);
> +}
> +EXPORT_SYMBOL_GPL(virt_topo_set_iommu_ops);
> diff --git a/drivers/iommu/virtio/virtio-iommu.c
b/drivers/iommu/virtio/virtio-iommu.c
> index b4da396cce60..b371d15f837f 100644
> --- a/drivers/iommu/virtio/virtio-iommu.c
> +++ b/drivers/iommu/virtio/virtio-iommu.c
> @@ -25,6 +25,8 @@
>  
>  #include <uapi/linux/virtio_iommu.h>
>  
> +#include "topology-helpers.h"
> +
>  #define MSI_IOVA_BASE			0x8000000
>  #define MSI_IOVA_LENGTH			0x100000
>  
> @@ -1065,6 +1067,7 @@ static int viommu_probe(struct virtio_device *vdev)
>  	if (ret)
>  		goto err_free_vqs;
>  
> +	virt_topo_set_iommu_ops(dev->parent, &viommu_ops);
>  	iommu_device_set_ops(&viommu->iommu, &viommu_ops);
>  	iommu_device_set_fwnode(&viommu->iommu, parent_dev->fwnode);
>  
> @@ -1111,6 +1114,7 @@ static void viommu_remove(struct virtio_device *vdev)
>  {
>  	struct viommu_dev *viommu = vdev->priv;
>  
> +	virt_topo_set_iommu_ops(vdev->dev.parent, NULL);
>  	iommu_device_sysfs_remove(&viommu->iommu);
>  	iommu_device_unregister(&viommu->iommu);
>  
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3602b223c9b2..8fd53c22a0ab 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18452,6 +18452,7 @@ M:	Jean-Philippe Brucker <jean-philippe at
linaro.org>
>  L:	virtualization at lists.linux-foundation.org
>  S:	Maintained
>  F:	drivers/iommu/virtio/
> +F:	include/linux/virt_iommu.h
>  F:	include/uapi/linux/virtio_iommu.h
>  
>  VIRTIO MEM DRIVER
> Besides

Reviewed-by: Eric Auger <eric.auger at redhat.com>

Thanks

Eric

Auger Eric

2020-Sep-04 16:24 UTC

head link

[PATCH v3 0/6] Add virtio-iommu built-in topology

Hi,

On 8/21/20 3:15 PM, Jean-Philippe Brucker wrote:> Add a topology description to the virtio-iommu driver and enable x86
> platforms.
> 
> Since [v2] we have made some progress on adding ACPI support for
> virtio-iommu, which is the preferred boot method on x86. It will be a
> new vendor-agnostic table describing para-virtual topologies in a
> minimal format. However some platforms don't use either ACPI or DT for
> booting (for example microvm), and will need the alternative topology
> description method proposed here. In addition, since the process to get
> a new ACPI table will take a long time, this provides a boot method even
> to ACPI-based platforms, if only temporarily for testing and
> development.
> 
> v3:
> * Add patch 1 that moves virtio-iommu to a subfolder.
> * Split the rest:
>   * Patch 2 adds topology-helper.c, which will be shared with the ACPI
>     support.
>   * Patch 4 adds definitions.
>   * Patch 5 adds parser in topology.c.
> * Address other comments.
> 
> Linux and QEMU patches available at:
> https://jpbrucker.net/git/linux virtio-iommu/devel
> https://jpbrucker.net/git/qemu virtio-iommu/develI have tested that series with above QEMU branch on ARM with virtio-net
and virtio-blk translated devices in non DT mode.

It works for me:
Tested-by: Eric Auger <eric.auger at redhat.com>

Thanks

Eric
> 
> [spec]
https://lists.oasis-open.org/archives/virtio-dev/202008/msg00067.html
> [v2]
https://lore.kernel.org/linux-iommu/20200228172537.377327-1-jean-philippe at
linaro.org/
> [v1]
https://lore.kernel.org/linux-iommu/20200214160413.1475396-1-jean-philippe at
linaro.org/
> [rfc]
https://lore.kernel.org/linux-iommu/20191122105000.800410-1-jean-philippe at
linaro.org/
> 
> Jean-Philippe Brucker (6):
>   iommu/virtio: Move to drivers/iommu/virtio/
>   iommu/virtio: Add topology helpers
>   PCI: Add DMA configuration for virtual platforms
>   iommu/virtio: Add topology definitions
>   iommu/virtio: Support topology description in config space
>   iommu/virtio: Enable x86 support
> 
>  drivers/iommu/Kconfig                     |  18 +-
>  drivers/iommu/Makefile                    |   3 +-
>  drivers/iommu/virtio/Makefile             |   4 +
>  drivers/iommu/virtio/topology-helpers.h   |  50 +++++
>  include/linux/virt_iommu.h                |  15 ++
>  include/uapi/linux/virtio_iommu.h         |  44 ++++
>  drivers/iommu/virtio/topology-helpers.c   | 196 ++++++++++++++++
>  drivers/iommu/virtio/topology.c           | 259 ++++++++++++++++++++++
>  drivers/iommu/{ => virtio}/virtio-iommu.c |   4 +
>  drivers/pci/pci-driver.c                  |   5 +
>  MAINTAINERS                               |   3 +-
>  11 files changed, 597 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/iommu/virtio/Makefile
>  create mode 100644 drivers/iommu/virtio/topology-helpers.h
>  create mode 100644 include/linux/virt_iommu.h
>  create mode 100644 drivers/iommu/virtio/topology-helpers.c
>  create mode 100644 drivers/iommu/virtio/topology.c
>  rename drivers/iommu/{ => virtio}/virtio-iommu.c (99%)
>

Michael S. Tsirkin

2020-Sep-24 09:00 UTC

head link

[PATCH v3 0/6] Add virtio-iommu built-in topology

On Fri, Sep 04, 2020 at 06:24:12PM +0200, Auger Eric
wrote:> Hi,
> 
> On 8/21/20 3:15 PM, Jean-Philippe Brucker wrote:
> > Add a topology description to the virtio-iommu driver and enable x86
> > platforms.
> > 
> > Since [v2] we have made some progress on adding ACPI support for
> > virtio-iommu, which is the preferred boot method on x86. It will be a
> > new vendor-agnostic table describing para-virtual topologies in a
> > minimal format. However some platforms don't use either ACPI or DT
for
> > booting (for example microvm), and will need the alternative topology
> > description method proposed here. In addition, since the process to
get
> > a new ACPI table will take a long time, this provides a boot method
even
> > to ACPI-based platforms, if only temporarily for testing and
> > development.
> > 
> > v3:
> > * Add patch 1 that moves virtio-iommu to a subfolder.
> > * Split the rest:
> >   * Patch 2 adds topology-helper.c, which will be shared with the ACPI
> >     support.
> >   * Patch 4 adds definitions.
> >   * Patch 5 adds parser in topology.c.
> > * Address other comments.
> > 
> > Linux and QEMU patches available at:
> > https://jpbrucker.net/git/linux virtio-iommu/devel
> > https://jpbrucker.net/git/qemu virtio-iommu/devel
> I have tested that series with above QEMU branch on ARM with virtio-net
> and virtio-blk translated devices in non DT mode.
> 
> It works for me:
> Tested-by: Eric Auger <eric.auger at redhat.com>
> 
> Thanks
> 
> Eric
OK so this looks good. Can you pls repost with the minor tweak
suggested and all acks included, and I will queue this?

Thanks!
> > 
> > [spec]
https://lists.oasis-open.org/archives/virtio-dev/202008/msg00067.html
> > [v2]
https://lore.kernel.org/linux-iommu/20200228172537.377327-1-jean-philippe at
linaro.org/
> > [v1]
https://lore.kernel.org/linux-iommu/20200214160413.1475396-1-jean-philippe at
linaro.org/
> > [rfc]
https://lore.kernel.org/linux-iommu/20191122105000.800410-1-jean-philippe at
linaro.org/
> > 
> > Jean-Philippe Brucker (6):
> >   iommu/virtio: Move to drivers/iommu/virtio/
> >   iommu/virtio: Add topology helpers
> >   PCI: Add DMA configuration for virtual platforms
> >   iommu/virtio: Add topology definitions
> >   iommu/virtio: Support topology description in config space
> >   iommu/virtio: Enable x86 support
> > 
> >  drivers/iommu/Kconfig                     |  18 +-
> >  drivers/iommu/Makefile                    |   3 +-
> >  drivers/iommu/virtio/Makefile             |   4 +
> >  drivers/iommu/virtio/topology-helpers.h   |  50 +++++
> >  include/linux/virt_iommu.h                |  15 ++
> >  include/uapi/linux/virtio_iommu.h         |  44 ++++
> >  drivers/iommu/virtio/topology-helpers.c   | 196 ++++++++++++++++
> >  drivers/iommu/virtio/topology.c           | 259
++++++++++++++++++++++
> >  drivers/iommu/{ => virtio}/virtio-iommu.c |   4 +
> >  drivers/pci/pci-driver.c                  |   5 +
> >  MAINTAINERS                               |   3 +-
> >  11 files changed, 597 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/iommu/virtio/Makefile
> >  create mode 100644 drivers/iommu/virtio/topology-helpers.h
> >  create mode 100644 include/linux/virt_iommu.h
> >  create mode 100644 drivers/iommu/virtio/topology-helpers.c
> >  create mode 100644 drivers/iommu/virtio/topology.c
> >  rename drivers/iommu/{ => virtio}/virtio-iommu.c (99%)
> >

Bjorn Helgaas

2020-Sep-24 15:22 UTC

head link

[PATCH v3 5/6] iommu/virtio: Support topology description in config space

On Fri, Aug 21, 2020 at 03:15:39PM +0200, Jean-Philippe Brucker
wrote:> Platforms without device-tree nor ACPI can provide a topology
> description embedded into the virtio config space. Parse it.
> 
> Use PCI FIXUP to probe the config space early, because we need to
> discover the topology before any DMA configuration takes place, and the
> virtio driver may be loaded much later. Since we discover the topology
> description when probing the PCI hierarchy, the virtual IOMMU cannot
> manage other platform devices discovered earlier.
> +struct viommu_cap_config {
> +	u8 bar;
> +	u32 length; /* structure size */
> +	u32 offset; /* structure offset within the bar */
s/the bar/the BAR/ (to match comment below).
> +static void viommu_pci_parse_topology(struct pci_dev *dev)
> +{
> +	int ret;
> +	u32 features;
> +	void __iomem *regs, *common_regs;
> +	struct viommu_cap_config cap = {0};
> +	struct virtio_pci_common_cfg __iomem *common_cfg;
> +
> +	/*
> +	 * The virtio infrastructure might not be loaded at this point. We need
> +	 * to access the BARs ourselves.
> +	 */
> +	ret = viommu_pci_find_capability(dev, VIRTIO_PCI_CAP_COMMON_CFG,
&cap);
> +	if (!ret) {
> +		pci_warn(dev, "common capability not found\n");
Is the lack of this capability really an error, i.e., is this
pci_warn() or pci_info()?  The "device doesn't have topology
description" below is only pci_dbg(), which suggests that we can live
without this.

Maybe a hint about what "common capability" means?
> +		return;
> +	}
> +
> +	if (pci_enable_device_mem(dev))
> +		return;
> +
> +	common_regs = pci_iomap(dev, cap.bar, 0);
> +	if (!common_regs)
> +		return;
> +
> +	common_cfg = common_regs + cap.offset;
> +
> +	/* Perform the init sequence before we can read the config */
> +	ret = viommu_pci_reset(common_cfg);
I guess this is some special device-specific reset, not any kind of
standard PCI reset?
> +	if (ret < 0) {
> +		pci_warn(dev, "unable to reset device\n");
> +		goto out_unmap_common;
> +	}
> +
> +	iowrite8(VIRTIO_CONFIG_S_ACKNOWLEDGE, &common_cfg->device_status);
> +	iowrite8(VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER,
> +		 &common_cfg->device_status);
> +
> +	/* Find out if the device supports topology description */
> +	iowrite32(0, &common_cfg->device_feature_select);
> +	features = ioread32(&common_cfg->device_feature);
> +
> +	if (!(features & BIT(VIRTIO_IOMMU_F_TOPOLOGY))) {
> +		pci_dbg(dev, "device doesn't have topology description");
> +		goto out_reset;
> +	}
> +
> +	ret = viommu_pci_find_capability(dev, VIRTIO_PCI_CAP_DEVICE_CFG,
&cap);
> +	if (!ret) {
> +		pci_warn(dev, "device config capability not found\n");
> +		goto out_reset;
> +	}
> +
> +	regs = pci_iomap(dev, cap.bar, 0);
> +	if (!regs)
> +		goto out_reset;
> +
> +	pci_info(dev, "parsing virtio-iommu topology\n");
> +	ret = viommu_parse_topology(&dev->dev, regs + cap.offset,
> +				    pci_resource_len(dev, 0) - cap.offset);
> +	if (ret)
> +		pci_warn(dev, "failed to parse topology: %d\n", ret);
> +
> +	pci_iounmap(dev, regs);
> +out_reset:
> +	ret = viommu_pci_reset(common_cfg);
> +	if (ret)
> +		pci_warn(dev, "unable to reset device\n");
> +out_unmap_common:
> +	pci_iounmap(dev, common_regs);
> +}
> +
> +/*
> + * Catch a PCI virtio-iommu implementation early to get the topology
description
> + * before we start probing other endpoints.
> + */
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REDHAT_QUMRANET, 0x1040 +
VIRTIO_ID_IOMMU,
> +			viommu_pci_parse_topology);
> -- 
> 2.28.0
>

Jean-Philippe Brucker

2020-Sep-25 08:48 UTC

head link

[PATCH v3 0/6] Add virtio-iommu built-in topology

On Fri, Aug 21, 2020 at 03:15:34PM +0200, Jean-Philippe Brucker
wrote:> Add a topology description to the virtio-iommu driver and enable x86
> platforms.
> 
> Since [v2] we have made some progress on adding ACPI support for
> virtio-iommu, which is the preferred boot method on x86. It will be a
> new vendor-agnostic table describing para-virtual topologies in a
> minimal format. However some platforms don't use either ACPI or DT for
> booting (for example microvm), and will need the alternative topology
> description method proposed here. In addition, since the process to get
> a new ACPI table will take a long time, this provides a boot method even
> to ACPI-based platforms, if only temporarily for testing and
> development.
> 
> v3:
> * Add patch 1 that moves virtio-iommu to a subfolder.
> * Split the rest:
>   * Patch 2 adds topology-helper.c, which will be shared with the ACPI
>     support.
>   * Patch 4 adds definitions.
>   * Patch 5 adds parser in topology.c.
> * Address other comments.
> 
> Linux and QEMU patches available at:
> https://jpbrucker.net/git/linux virtio-iommu/devel
> https://jpbrucker.net/git/qemu virtio-iommu/devel
I'm parking this work again, until we make progress on the ACPI table, or
until a platform without ACPI and DT needs it. Until then, I've pushed v4
to my virtio-iommu/topo branch and will keep it rebased on master.

Thanks,
Jean

Michael S. Tsirkin

2020-Sep-25 10:22 UTC

head link

[PATCH v3 0/6] Add virtio-iommu built-in topology

On Fri, Sep 25, 2020 at 10:48:06AM +0200, Jean-Philippe Brucker
wrote:> On Fri, Aug 21, 2020 at 03:15:34PM +0200, Jean-Philippe Brucker wrote:
> > Add a topology description to the virtio-iommu driver and enable x86
> > platforms.
> > 
> > Since [v2] we have made some progress on adding ACPI support for
> > virtio-iommu, which is the preferred boot method on x86. It will be a
> > new vendor-agnostic table describing para-virtual topologies in a
> > minimal format. However some platforms don't use either ACPI or DT
for
> > booting (for example microvm), and will need the alternative topology
> > description method proposed here. In addition, since the process to
get
> > a new ACPI table will take a long time, this provides a boot method
even
> > to ACPI-based platforms, if only temporarily for testing and
> > development.
> > 
> > v3:
> > * Add patch 1 that moves virtio-iommu to a subfolder.
> > * Split the rest:
> >   * Patch 2 adds topology-helper.c, which will be shared with the ACPI
> >     support.
> >   * Patch 4 adds definitions.
> >   * Patch 5 adds parser in topology.c.
> > * Address other comments.
> > 
> > Linux and QEMU patches available at:
> > https://jpbrucker.net/git/linux virtio-iommu/devel
> > https://jpbrucker.net/git/qemu virtio-iommu/devel
> 
> I'm parking this work again, until we make progress on the ACPI table,
or
> until a platform without ACPI and DT needs it. Until then, I've pushed
v4
> to my virtio-iommu/topo branch and will keep it rebased on master.
> 
> Thanks,
> Jean
I think you guys need to work on virtio spec too, not too much left to
do there ...

-- 
MST

Possibly Parallel Threads

Search for more apparently analagous threads

Virtualization - Sep 2020 - [PATCH v3 0/6] Add virtio-iommu built-in topology

[PATCH v3 0/6] Add virtio-iommu built-in topology

[PATCH v3 1/6] iommu/virtio: Move to drivers/iommu/virtio/

[PATCH v3 2/6] iommu/virtio: Add topology helpers

[PATCH v3 3/6] PCI: Add DMA configuration for virtual platforms

[PATCH v3 4/6] iommu/virtio: Add topology definitions

[PATCH v3 5/6] iommu/virtio: Support topology description in config space

[PATCH v3 6/6] iommu/virtio: Enable x86 support

[PATCH v3 0/6] Add virtio-iommu built-in topology

[PATCH v3 0/6] Add virtio-iommu built-in topology

[PATCH v3 1/6] iommu/virtio: Move to drivers/iommu/virtio/

[PATCH v3 4/6] iommu/virtio: Add topology definitions

[PATCH v3 5/6] iommu/virtio: Support topology description in config space

[PATCH v3 2/6] iommu/virtio: Add topology helpers

[PATCH v3 0/6] Add virtio-iommu built-in topology

[PATCH v3 0/6] Add virtio-iommu built-in topology

[PATCH v3 5/6] iommu/virtio: Support topology description in config space

[PATCH v3 0/6] Add virtio-iommu built-in topology

[PATCH v3 0/6] Add virtio-iommu built-in topology

Possibly Parallel Threads