Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1514870 > unrolled thread

[RFC 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 (Alt II)

Back to article view | Back to linux.kernel

Contents

  [RFC 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 (Alt II) Eric Auger <eric.auger@redhat.com> - 2016-11-03 22:50 +0100
    [RFC 2/8] iommu/iova: fix __alloc_and_insert_iova_range Eric Auger <eric.auger@redhat.com> - 2016-11-03 22:50 +0100
    [RFC 4/8] iommu: Add a list of iommu_reserved_region in iommu_domain Eric Auger <eric.auger@redhat.com> - 2016-11-03 22:50 +0100
    [RFC 5/8] vfio/type1: Introduce RESV_IOVA_RANGE capability Eric Auger <eric.auger@redhat.com> - 2016-11-03 22:50 +0100
    [RFC 3/8] iommu/dma: Allow MSI-only cookies Eric Auger <eric.auger@redhat.com> - 2016-11-03 22:50 +0100
    [RFC 7/8] iommu/vt-d: Implement add_reserved_regions callback Eric Auger <eric.auger@redhat.com> - 2016-11-03 22:50 +0100
    [RFC 1/8] vfio: fix vfio_info_cap_add/shift Eric Auger <eric.auger@redhat.com> - 2016-11-03 22:50 +0100
    [RFC 6/8] iommu: Handle the list of reserved regions Eric Auger <eric.auger@redhat.com> - 2016-11-03 22:50 +0100
    Re: [RFC 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 (Alt II) Alex Williamson <alex.williamson@redhat.com> - 2016-11-04 05:10 +0100
      Summary of LPC guest MSI discussion in Santa Fe (was: Re: [RFC 0/8]  KVM PCIe/MSI passthrough on ARM/ARM64 (Alt II)) Will Deacon <will.deacon@arm.com> - 2016-11-08 03:50 +0100
        Re: Summary of LPC guest MSI discussion in Santa Fe Auger Eric <eric.auger@redhat.com> - 2016-11-08 15:30 +0100
          Re: Summary of LPC guest MSI discussion in Santa Fe Will Deacon <will.deacon@arm.com> - 2016-11-08 19:00 +0100
            Re: Summary of LPC guest MSI discussion in Santa Fe Don Dutile <ddutile@redhat.com> - 2016-11-08 20:10 +0100
              Re: Summary of LPC guest MSI discussion in Santa Fe Will Deacon <will.deacon@arm.com> - 2016-11-08 20:20 +0100
              Re: Summary of LPC guest MSI discussion in Santa Fe Auger Eric <eric.auger@redhat.com> - 2016-11-09 08:50 +0100
        Re: Summary of LPC guest MSI discussion in Santa Fe Don Dutile <ddutile@redhat.com> - 2016-11-08 17:10 +0100
        Re: Summary of LPC guest MSI discussion in Santa Fe (was: Re: [RFC  0/8] KVM PCIe/MSI passthrough on ARM/ARM64 (Alt II)) Christoffer Dall <christoffer.dall@linaro.org> - 2016-11-08 21:30 +0100
          Re: Summary of LPC guest MSI discussion in Santa Fe (was: Re: [RFC  0/8] KVM PCIe/MSI passthrough on ARM/ARM64 (Alt II)) Alex Williamson <alex.williamson@redhat.com> - 2016-11-09 00:40 +0100
            Re: Summary of LPC guest MSI discussion in Santa Fe Don Dutile <ddutile@redhat.com> - 2016-11-09 04:00 +0100
              Re: Summary of LPC guest MSI discussion in Santa Fe Will Deacon <will.deacon@arm.com> - 2016-11-09 18:10 +0100
                Re: Summary of LPC guest MSI discussion in Santa Fe Don Dutile <ddutile@redhat.com> - 2016-11-09 20:00 +0100
                  Re: Summary of LPC guest MSI discussion in Santa Fe Christoffer Dall <christoffer.dall@linaro.org> - 2016-11-09 20:30 +0100
                    Re: Summary of LPC guest MSI discussion in Santa Fe Alex Williamson <alex.williamson@redhat.com> - 2016-11-09 21:10 +0100
                      Re: Summary of LPC guest MSI discussion in Santa Fe Joerg Roedel <joro@8bytes.org> - 2016-11-10 15:50 +0100
                        Re: Summary of LPC guest MSI discussion in Santa Fe Alex Williamson <alex.williamson@redhat.com> - 2016-11-10 18:10 +0100
                    Re: Summary of LPC guest MSI discussion in Santa Fe Will Deacon <will.deacon@arm.com> - 2016-11-09 21:40 +0100
                      Re: Summary of LPC guest MSI discussion in Santa Fe Alex Williamson <alex.williamson@redhat.com> - 2016-11-09 23:20 +0100
                        Re: Summary of LPC guest MSI discussion in Santa Fe Will Deacon <will.deacon@arm.com> - 2016-11-09 23:30 +0100
                          Re: Summary of LPC guest MSI discussion in Santa Fe Alex Williamson <alex.williamson@redhat.com> - 2016-11-10 00:30 +0100
                            Re: Summary of LPC guest MSI discussion in Santa Fe Will Deacon <will.deacon@arm.com> - 2016-11-10 00:40 +0100
                              Re: Summary of LPC guest MSI discussion in Santa Fe Alex Williamson <alex.williamson@redhat.com> - 2016-11-10 01:10 +0100
                                Re: Summary of LPC guest MSI discussion in Santa Fe Auger Eric <eric.auger@redhat.com> - 2016-11-10 01:20 +0100
                                  Re: Summary of LPC guest MSI discussion in Santa Fe Alex Williamson <alex.williamson@redhat.com> - 2016-11-10 02:00 +0100
                                    Re: Summary of LPC guest MSI discussion in Santa Fe Will Deacon <will.deacon@arm.com> - 2016-11-10 03:10 +0100
                                      Re: Summary of LPC guest MSI discussion in Santa Fe Auger Eric <eric.auger@redhat.com> - 2016-11-10 12:20 +0100
                                        Re: Summary of LPC guest MSI discussion in Santa Fe Alex Williamson <alex.williamson@redhat.com> - 2016-11-10 18:50 +0100
                                          Re: Summary of LPC guest MSI discussion in Santa Fe Joerg Roedel <joro@8bytes.org> - 2016-11-11 12:20 +0100
                                            Re: Summary of LPC guest MSI discussion in Santa Fe Alex Williamson <alex.williamson@redhat.com> - 2016-11-11 17:00 +0100
                                              Re: Summary of LPC guest MSI discussion in Santa Fe Alex Williamson <alex.williamson@redhat.com> - 2016-11-11 17:10 +0100
                                              Re: Summary of LPC guest MSI discussion in Santa Fe Don Dutile <ddutile@redhat.com> - 2016-11-11 17:30 +0100
                                            Re: Summary of LPC guest MSI discussion in Santa Fe Don Dutile <ddutile@redhat.com> - 2016-11-11 17:10 +0100
                                  Re: Summary of LPC guest MSI discussion in Santa Fe Joerg Roedel <joro@8bytes.org> - 2016-11-10 16:00 +0100
                  Re: Summary of LPC guest MSI discussion in Santa Fe Robin Murphy <robin.murphy@arm.com> - 2016-11-09 21:20 +0100
                    Re: Summary of LPC guest MSI discussion in Santa Fe Joerg Roedel <joro@8bytes.org> - 2016-11-10 16:30 +0100

Page 1 of 3  [1] 2 3  Next page →

#1514870 — [RFC 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 (Alt II)

Following Will & Robin's suggestions, this series attempts to propose
an alternative to [1] where the host would arbitrarily decide the
location of the IOVA MSI window and would be able to report to the
userspace the list of reserved IOVA regions that cannot be used
along with VFIO_IOMMU_MAP_DMA. This would allow the userspace to react
in case of conflict.

Userspace can retrieve all the reserved regions through the VFIO_IOMMU_GET_INFO
IOCTL by querying the new RESV_IOVA_RANGE chained capability. Each reserved
IOVA range is put in a separate capability.

At IOMMU level, the reserved regions are stored in an iommu_domain list
which is populated on each device attachment. An IOMMU add_reserved_regions
callback specializes the registration of the reserved regions.

On x86, the [FEE0_0000h - FEF0_000h] MSI window is registered (NOT tested).

On ARM, the PCI host bridge windows (ACS check to be added?) + the MSI IOVA
reserved regions are populated by the arm-smmu driver. Currently the MSI
IOVA region is arbitrarily located at 0x8000000 and 1MB sized.  An IOVA domain
is created in add_reserved_regions callback. Then MSIs are transparently
mapped using this IOVA domain.

This series currently does not address some features addressed in [1]:
- MSI IOVA size requirement computation
- IRQ safety assessment

This RFC was just tested on ARM Overdrive with QEMU and is sent to help
potential discussions at LPC. Additionnal development + testing is needed.

2 tentative fixes may be submitted separately:
- vfio: fix vfio_info_cap_add/shift
- iommu/iova: fix __alloc_and_insert_iova_range

Best Regards

Eric

[1] [PATCH v14 00/16] KVM PCIe/MSI passthrough on ARM/ARM64
https://lkml.org/lkml/2016/10/12/347

Git: complete series available at
https://github.com/eauger/linux/tree/v4.9-rc3-reserved-rfc


Eric Auger (7):
  vfio: fix vfio_info_cap_add/shift
  iommu/iova: fix __alloc_and_insert_iova_range
  iommu: Add a list of iommu_reserved_region in iommu_domain
  vfio/type1: Introduce RESV_IOVA_RANGE capability
  iommu: Handle the list of reserved regions
  iommu/vt-d: Implement add_reserved_regions callback
  iommu/arm-smmu: implement add_reserved_regions callback

Robin Murphy (1):
  iommu/dma: Allow MSI-only cookies

 drivers/iommu/arm-smmu.c        | 63 +++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/dma-iommu.c       | 39 +++++++++++++++++++++++++
 drivers/iommu/intel-iommu.c     | 48 ++++++++++++++++++++++---------
 drivers/iommu/iommu.c           | 25 ++++++++++++++++
 drivers/iommu/iova.c            |  2 +-
 drivers/vfio/vfio.c             |  5 ++--
 drivers/vfio/vfio_iommu_type1.c | 63 ++++++++++++++++++++++++++++++++++++++++-
 include/linux/dma-iommu.h       |  9 ++++++
 include/linux/iommu.h           | 23 +++++++++++++++
 include/uapi/linux/vfio.h       | 16 ++++++++++-
 10 files changed, 275 insertions(+), 18 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1514872 — [RFC 2/8] iommu/iova: fix __alloc_and_insert_iova_range

Fix the size check within start_pfn and limit_pfn.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

the issue was observed when playing with 1 page iova domain with
higher iova reserved.
---
 drivers/iommu/iova.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index e23001b..ee29dbf 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -147,7 +147,7 @@ static int __alloc_and_insert_iova_range(struct iova_domain *iovad,
 	if (!curr) {
 		if (size_aligned)
 			pad_size = iova_get_pad_size(size, limit_pfn);
-		if ((iovad->start_pfn + size + pad_size) > limit_pfn) {
+		if ((iovad->start_pfn + size + pad_size - 1) > limit_pfn) {
 			spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
 			return -ENOMEM;
 		}
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1514873 — [RFC 4/8] iommu: Add a list of iommu_reserved_region in iommu_domain

Introduce a new iommu_reserved_region struct. This embodies
an IOVA reserved region that cannot be used along with the IOMMU
API. The list is protected by a dedicated mutex.

An iommu domain now owns a list of those.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---
---
 drivers/iommu/iommu.c |  2 ++
 include/linux/iommu.h | 17 +++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 9a2f196..0af07492 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1061,6 +1061,8 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
 
 	domain->ops  = bus->iommu_ops;
 	domain->type = type;
+	INIT_LIST_HEAD(&domain->reserved_regions);
+	mutex_init(&domain->resv_mutex);
 	/* Assume all sizes by default; the driver may override this later */
 	domain->pgsize_bitmap  = bus->iommu_ops->pgsize_bitmap;
 
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 436dc21..0f2eb64 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -84,6 +84,8 @@ struct iommu_domain {
 	void *handler_token;
 	struct iommu_domain_geometry geometry;
 	void *iova_cookie;
+	struct list_head reserved_regions;
+	struct mutex resv_mutex; /* protects the reserved region list */
 };
 
 enum iommu_cap {
@@ -131,6 +133,21 @@ struct iommu_dm_region {
 	int			prot;
 };
 
+/**
+ * struct iommu_reserved_region - descriptor for a reserved iova region
+ * @list: Linked list pointers
+ * @start: IOVA base address of the region
+ * @length: Length of the region in bytes
+ */
+struct iommu_reserved_region {
+	struct list_head	list;
+	dma_addr_t		start;
+	size_t			length;
+};
+
+#define iommu_reserved_region_for_each(resv, d) \
+	list_for_each_entry(resv, &(d)->reserved_regions, list)
+
 #ifdef CONFIG_IOMMU_API
 
 /**
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1514874 — [RFC 5/8] vfio/type1: Introduce RESV_IOVA_RANGE capability

This patch allows the user-space to retrieve the reserved IOVA range(s),
if any. The implementation is based on capability chains, now also added
to VFIO_IOMMU_GET_INFO.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---
---
 drivers/vfio/vfio_iommu_type1.c | 63 ++++++++++++++++++++++++++++++++++++++++-
 include/uapi/linux/vfio.h       | 16 ++++++++++-
 2 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 2ba1942..0d53ac1 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -965,6 +965,42 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu)
 	return ret;
 }
 
+static int add_resv_iova_range(struct vfio_info_cap *caps, u64 start, u64 end)
+{
+	struct vfio_iommu_type1_info_cap_resv_iova_range *cap;
+	struct vfio_info_cap_header *header;
+
+	header = vfio_info_cap_add(caps, sizeof(*cap),
+			VFIO_IOMMU_TYPE1_INFO_CAP_RESV_IOVA_RANGE, 1);
+
+	if (IS_ERR(header))
+		return PTR_ERR(header);
+
+	cap = container_of(header,
+			   struct vfio_iommu_type1_info_cap_resv_iova_range,
+			   header);
+
+	cap->start = start;
+	cap->end = end;
+	return 0;
+}
+
+static int build_resv_iova_range_caps(struct vfio_iommu *iommu,
+				      struct vfio_info_cap *caps)
+{
+	struct vfio_domain *domain;
+	struct iommu_reserved_region *resv;
+
+	domain = list_first_entry(&iommu->domain_list,
+				  struct vfio_domain, next);
+
+	iommu_reserved_region_for_each(resv, domain->domain)
+		add_resv_iova_range(caps, resv->start,
+				    resv->start + resv->length - 1);
+
+	return 0;
+}
+
 static long vfio_iommu_type1_ioctl(void *iommu_data,
 				   unsigned int cmd, unsigned long arg)
 {
@@ -986,8 +1022,10 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
 		}
 	} else if (cmd == VFIO_IOMMU_GET_INFO) {
 		struct vfio_iommu_type1_info info;
+		struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
+		int ret;
 
-		minsz = offsetofend(struct vfio_iommu_type1_info, iova_pgsizes);
+		minsz = offsetofend(struct vfio_iommu_type1_info, cap_offset);
 
 		if (copy_from_user(&info, (void __user *)arg, minsz))
 			return -EFAULT;
@@ -999,6 +1037,29 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
 
 		info.iova_pgsizes = vfio_pgsize_bitmap(iommu);
 
+		ret = build_resv_iova_range_caps(iommu, &caps);
+		if (ret)
+			return ret;
+
+		if (caps.size) {
+			info.flags |= VFIO_IOMMU_INFO_CAPS;
+			if (info.argsz < sizeof(info) + caps.size) {
+				info.argsz = sizeof(info) + caps.size;
+				info.cap_offset = 0;
+			} else {
+				vfio_info_cap_shift(&caps, sizeof(info));
+				if (copy_to_user((void __user *)arg +
+						sizeof(info), caps.buf,
+						caps.size)) {
+					kfree(caps.buf);
+					return -EFAULT;
+				}
+				info.cap_offset = sizeof(info);
+			}
+
+			kfree(caps.buf);
+		}
+
 		return copy_to_user((void __user *)arg, &info, minsz) ?
 			-EFAULT : 0;
 
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 255a211..ae96f2c 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -488,7 +488,21 @@ struct vfio_iommu_type1_info {
 	__u32	argsz;
 	__u32	flags;
 #define VFIO_IOMMU_INFO_PGSIZES (1 << 0)	/* supported page sizes info */
-	__u64	iova_pgsizes;		/* Bitmap of supported page sizes */
+#define VFIO_IOMMU_INFO_CAPS	(1 << 1)	/* Info supports caps */
+	__u64	iova_pgsizes;	/* Bitmap of supported page sizes */
+	__u32   cap_offset;	/* Offset within info struct of first cap */
+	__u32	__resv;
+};
+
+/*
+ * The RESV_IOVA_RANGE capability allows to report the reserved IOVA range(s),
+ * if any.
+ */
+#define VFIO_IOMMU_TYPE1_INFO_CAP_RESV_IOVA_RANGE  1
+struct vfio_iommu_type1_info_cap_resv_iova_range {
+	struct vfio_info_cap_header header;
+	__u64 start;
+	__u64 end;
 };
 
 #define VFIO_IOMMU_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1514875 — [RFC 3/8] iommu/dma: Allow MSI-only cookies

From: Robin Murphy <robin.murphy@arm.com>

IOMMU domain users such as VFIO face a similar problem to DMA API ops
with regard to mapping MSI messages in systems where the MSI write is
subject to IOMMU translation. With the relevant infrastructure now in
place for managed DMA domains, it's actually really simple for other
users to piggyback off that and reap the benefits without giving up
their own IOVA management, and without having to reinvent their own
wheel in the MSI layer.

Allow such users to opt into automatic MSI remapping by dedicating a
region of their IOVA space to a managed cookie.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v13 -> v14:
- restore reserve_iova for iova >= base + size

v1 -> v13 incorpration:
- compared to Robin's version
- add NULL last param to iommu_dma_init_domain
- set the msi_geometry aperture
- I removed
  if (base < U64_MAX - size)
     reserve_iova(iovad, iova_pfn(iovad, base + size), ULONG_MAX);
  don't get why we would reserve something out of the scope of the iova domain?
  what do I miss?
---
 drivers/iommu/dma-iommu.c | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/dma-iommu.h |  9 +++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index c5ab866..d45f9a0 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -716,3 +716,42 @@ void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
 		msg->address_lo += lower_32_bits(msi_page->iova);
 	}
 }
+
+/**
+ * iommu_get_dma_msi_region_cookie - Configure a domain for MSI remapping only
+ * @domain: IOMMU domain to prepare
+ * @base: Base address of IOVA region to use as the MSI remapping aperture
+ * @size: Size of the desired MSI aperture
+ *
+ * Users who manage their own IOVA allocation and do not want DMA API support,
+ * but would still like to take advantage of automatic MSI remapping, can use
+ * this to initialise their own domain appropriately.
+ */
+int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
+			       dma_addr_t base, u64 size)
+{
+	struct iommu_dma_cookie *cookie;
+	struct iova_domain *iovad;
+	int ret;
+
+	if (domain->type == IOMMU_DOMAIN_DMA)
+		return -EINVAL;
+
+	ret = iommu_get_dma_cookie(domain);
+	if (ret)
+		return ret;
+
+	ret = iommu_dma_init_domain(domain, base, size, NULL);
+	if (ret) {
+		iommu_put_dma_cookie(domain);
+		return ret;
+	}
+
+	cookie = domain->iova_cookie;
+	iovad = &cookie->iovad;
+	if (base < U64_MAX - size)
+		reserve_iova(iovad, iova_pfn(iovad, base + size), ULONG_MAX);
+
+	return 0;
+}
+EXPORT_SYMBOL(iommu_get_dma_msi_region_cookie);
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 32c5890..05ab5b4 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -67,6 +67,9 @@ void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg, int nents,
 /* The DMA API isn't _quite_ the whole story, though... */
 void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
 
+int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
+				    dma_addr_t base, u64 size);
+
 #else
 
 struct iommu_domain;
@@ -90,6 +93,12 @@ static inline void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
 {
 }
 
+static inline int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
+						  dma_addr_t base, u64 size)
+{
+	return -ENODEV;
+}
+
 #endif	/* CONFIG_IOMMU_DMA */
 #endif	/* __KERNEL__ */
 #endif	/* __DMA_IOMMU_H */
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1514876 — [RFC 7/8] iommu/vt-d: Implement add_reserved_regions callback

Implement the add_reserved_regions callback by registering
the [FEE0_0000h - FEF0_000h] 1MB range as a reserved region
(MSI address space).

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 drivers/iommu/intel-iommu.c | 48 +++++++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index a4407ea..fb64418 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5173,6 +5173,27 @@ static void intel_iommu_remove_device(struct device *dev)
 	iommu_device_unlink(iommu->iommu_dev, dev);
 }
 
+static int intel_add_reserved_regions(struct iommu_domain *domain,
+				      struct device *device)
+{
+	struct iommu_reserved_region *region;
+	const dma_addr_t msi_base = 0xfee00000;
+	const size_t msi_length	= 0x00100000;
+
+	iommu_reserved_region_for_each(region, domain) {
+		if (region->start == msi_base && region->length == msi_length)
+			continue;
+	}
+	region = kzalloc(sizeof(*region), GFP_KERNEL);
+	if (!region)
+		return -ENOMEM;
+
+	region->start  = msi_base;
+	region->length = msi_length;
+	list_add_tail(&region->list, &domain->reserved_regions);
+	return 0;
+}
+
 #ifdef CONFIG_INTEL_IOMMU_SVM
 int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sdev)
 {
@@ -5282,19 +5303,20 @@ struct intel_iommu *intel_svm_device_to_iommu(struct device *dev)
 #endif /* CONFIG_INTEL_IOMMU_SVM */
 
 static const struct iommu_ops intel_iommu_ops = {
-	.capable	= intel_iommu_capable,
-	.domain_alloc	= intel_iommu_domain_alloc,
-	.domain_free	= intel_iommu_domain_free,
-	.attach_dev	= intel_iommu_attach_device,
-	.detach_dev	= intel_iommu_detach_device,
-	.map		= intel_iommu_map,
-	.unmap		= intel_iommu_unmap,
-	.map_sg		= default_iommu_map_sg,
-	.iova_to_phys	= intel_iommu_iova_to_phys,
-	.add_device	= intel_iommu_add_device,
-	.remove_device	= intel_iommu_remove_device,
-	.device_group   = pci_device_group,
-	.pgsize_bitmap	= INTEL_IOMMU_PGSIZES,
+	.capable		= intel_iommu_capable,
+	.domain_alloc		= intel_iommu_domain_alloc,
+	.domain_free		= intel_iommu_domain_free,
+	.attach_dev		= intel_iommu_attach_device,
+	.detach_dev		= intel_iommu_detach_device,
+	.map			= intel_iommu_map,
+	.unmap			= intel_iommu_unmap,
+	.map_sg			= default_iommu_map_sg,
+	.iova_to_phys		= intel_iommu_iova_to_phys,
+	.add_device		= intel_iommu_add_device,
+	.remove_device		= intel_iommu_remove_device,
+	.add_reserved_regions	= intel_iommu_add_reserved_regions,
+	.device_group		= pci_device_group,
+	.pgsize_bitmap		= INTEL_IOMMU_PGSIZES,
 };
 
 static void quirk_iommu_g4x_gfx(struct pci_dev *dev)
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1514877 — [RFC 1/8] vfio: fix vfio_info_cap_add/shift

Capability header next field is an offset relative to the start of
the INFO buffer. tmp->next is assigned the proper value but iterations
implemented in vfio_info_cap_add and vfio_info_cap_shift use next
as an offset between the headers. When coping with multiple capabilities
this leads to an Oops.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 drivers/vfio/vfio.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index d1d70e0..1e838d1 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1763,7 +1763,7 @@ struct vfio_info_cap_header *vfio_info_cap_add(struct vfio_info_cap *caps,
 	header->version = version;
 
 	/* Add to the end of the capability chain */
-	for (tmp = caps->buf; tmp->next; tmp = (void *)tmp + tmp->next)
+	for (tmp = buf; tmp->next; tmp = buf + tmp->next)
 		; /* nothing */
 
 	tmp->next = caps->size;
@@ -1776,8 +1776,9 @@ struct vfio_info_cap_header *vfio_info_cap_add(struct vfio_info_cap *caps,
 void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset)
 {
 	struct vfio_info_cap_header *tmp;
+	void *buf = (void *)caps->buf;
 
-	for (tmp = caps->buf; tmp->next; tmp = (void *)tmp + tmp->next - offset)
+	for (tmp = buf; tmp->next; tmp = buf + tmp->next - offset)
 		tmp->next += offset;
 }
 EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1514878 — [RFC 6/8] iommu: Handle the list of reserved regions

A new callback is introduced, add_reserved_regions. This function
aims at populating the iommu domain reserved region list with
regions associated with the device. The function is called on
device attachment. The list is freed on iommu_domain_free().

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 drivers/iommu/iommu.c | 23 +++++++++++++++++++++++
 include/linux/iommu.h |  6 ++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0af07492..300af44 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1077,6 +1077,10 @@ struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
 
 void iommu_domain_free(struct iommu_domain *domain)
 {
+	struct iommu_reserved_region *entry, *next;
+
+	list_for_each_entry_safe(entry, next, &domain->reserved_regions, list)
+		kfree(entry);
 	domain->ops->domain_free(domain);
 }
 EXPORT_SYMBOL_GPL(iommu_domain_free);
@@ -1089,6 +1093,15 @@ static int __iommu_attach_device(struct iommu_domain *domain,
 		return -ENODEV;
 
 	ret = domain->ops->attach_dev(domain, dev);
+	if (ret)
+		return ret;
+
+	if (domain->type == IOMMU_DOMAIN_UNMANAGED) {
+		mutex_lock(&domain->resv_mutex);
+		ret = iommu_add_reserved_regions(domain, dev);
+		mutex_unlock(&domain->resv_mutex);
+	}
+
 	if (!ret)
 		trace_attach_device_to_domain(dev);
 	return ret;
@@ -1548,6 +1561,16 @@ int iommu_domain_set_attr(struct iommu_domain *domain,
 }
 EXPORT_SYMBOL_GPL(iommu_domain_set_attr);
 
+int iommu_add_reserved_regions(struct iommu_domain *domain, struct device *dev)
+{
+	const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+	if (ops && ops->add_reserved_regions)
+		return ops->add_reserved_regions(domain, dev);
+	else
+		return 0;
+}
+
 void iommu_get_dm_regions(struct device *dev, struct list_head *list)
 {
 	const struct iommu_ops *ops = dev->bus->iommu_ops;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 0f2eb64..7748a1b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -170,6 +170,7 @@ struct iommu_reserved_region {
  * @get_dm_regions: Request list of direct mapping requirements for a device
  * @put_dm_regions: Free list of direct mapping requirements for a device
  * @apply_dm_region: Temporary helper call-back for iova reserved ranges
+ * @add_reserved_regions: Populate reserved regions for a device
  * @domain_window_enable: Configure and enable a particular window for a domain
  * @domain_window_disable: Disable a particular window for a domain
  * @domain_set_windows: Set the number of windows for a domain
@@ -207,6 +208,10 @@ struct iommu_ops {
 	void (*apply_dm_region)(struct device *dev, struct iommu_domain *domain,
 				struct iommu_dm_region *region);
 
+	/* Add reserved regions for a device */
+	int (*add_reserved_regions)(struct iommu_domain *domain,
+				     struct device *device);
+
 	/* Window handling functions */
 	int (*domain_window_enable)(struct iommu_domain *domain, u32 wnd_nr,
 				    phys_addr_t paddr, u64 size, int prot);
@@ -289,6 +294,7 @@ struct device *iommu_device_create(struct device *parent, void *drvdata,
 void iommu_device_destroy(struct device *dev);
 int iommu_device_link(struct device *dev, struct device *link);
 void iommu_device_unlink(struct device *dev, struct device *link);
+int iommu_add_reserved_regions(struct iommu_domain *domain, struct device *dev);
 
 /* Window handling function prototypes */
 extern int iommu_domain_window_enable(struct iommu_domain *domain, u32 wnd_nr,
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1514987

On Thu,  3 Nov 2016 21:39:30 +0000
Eric Auger <eric.auger@redhat.com> wrote:

> Following Will & Robin's suggestions, this series attempts to propose
> an alternative to [1] where the host would arbitrarily decide the
> location of the IOVA MSI window and would be able to report to the
> userspace the list of reserved IOVA regions that cannot be used
> along with VFIO_IOMMU_MAP_DMA. This would allow the userspace to react
> in case of conflict.
> 
> Userspace can retrieve all the reserved regions through the VFIO_IOMMU_GET_INFO
> IOCTL by querying the new RESV_IOVA_RANGE chained capability. Each reserved
> IOVA range is put in a separate capability.

Doesn't it make more sense to describe the non-holes (ie. what I
can use for DMA) rather the holes (what I can't use for DMA)?  For
example on VT-d, the IOMMU not only has the block of MSI addresses
handled through interrupt remapping, but it also has a maximum address
width.  Rather than describing the reserved space we could describe the
usable DMA ranges above and below that reserved block.

Anyway, there's also a pretty harsh problem that I came up with in
talking to Will.  If the platform describes a fixed IOVA range as
reserved, that's great for the use case when a VM is instantiated with
a device attached, but it seems like it nearly excludes the case of
hotplugging a device.  We can't dynamically decide that a set of RAM
pages in the VM cannot be used as a DMA target.  Does the user need to
create the VM with a predefined hole that lines up with the reserved
regions for this platform?  How do they know the reserved regions for
this platform?  How would we handle migration where an assigned device
hot-add might not occur until after we've migrated to a slightly
different platform from the one we started on, that might have
different reserved memory requirements?

We can always have QEMU reject hot-adding the device if the reserved
region overlaps existing guest RAM, but I don't even really see how we
advise users to give them a reasonable chance of avoiding that
possibility.  Apparently there are also ARM platforms where MSI pages
cannot be remapped to support the previous programmable user/VM
address, is it even worthwhile to support those platforms?  Does that
decision influence whether user programmable MSI reserved regions are
really a second class citizen to fixed reserved regions?  I expect
we'll be talking about this tomorrow morning, but I certainly haven't
come up with any viable solutions to this.  Thanks,

Alex

> At IOMMU level, the reserved regions are stored in an iommu_domain list
> which is populated on each device attachment. An IOMMU add_reserved_regions
> callback specializes the registration of the reserved regions.
> 
> On x86, the [FEE0_0000h - FEF0_000h] MSI window is registered (NOT tested).
> 
> On ARM, the PCI host bridge windows (ACS check to be added?) + the MSI IOVA
> reserved regions are populated by the arm-smmu driver. Currently the MSI
> IOVA region is arbitrarily located at 0x8000000 and 1MB sized.  An IOVA domain
> is created in add_reserved_regions callback. Then MSIs are transparently
> mapped using this IOVA domain.
> 
> This series currently does not address some features addressed in [1]:
> - MSI IOVA size requirement computation
> - IRQ safety assessment
> 
> This RFC was just tested on ARM Overdrive with QEMU and is sent to help
> potential discussions at LPC. Additionnal development + testing is needed.
> 
> 2 tentative fixes may be submitted separately:
> - vfio: fix vfio_info_cap_add/shift
> - iommu/iova: fix __alloc_and_insert_iova_range
> 
> Best Regards
> 
> Eric
> 
> [1] [PATCH v14 00/16] KVM PCIe/MSI passthrough on ARM/ARM64
> https://lkml.org/lkml/2016/10/12/347
> 
> Git: complete series available at
> https://github.com/eauger/linux/tree/v4.9-rc3-reserved-rfc
> 
> 
> Eric Auger (7):
>   vfio: fix vfio_info_cap_add/shift
>   iommu/iova: fix __alloc_and_insert_iova_range
>   iommu: Add a list of iommu_reserved_region in iommu_domain
>   vfio/type1: Introduce RESV_IOVA_RANGE capability
>   iommu: Handle the list of reserved regions
>   iommu/vt-d: Implement add_reserved_regions callback
>   iommu/arm-smmu: implement add_reserved_regions callback
> 
> Robin Murphy (1):
>   iommu/dma: Allow MSI-only cookies
> 
>  drivers/iommu/arm-smmu.c        | 63 +++++++++++++++++++++++++++++++++++++++++
>  drivers/iommu/dma-iommu.c       | 39 +++++++++++++++++++++++++
>  drivers/iommu/intel-iommu.c     | 48 ++++++++++++++++++++++---------
>  drivers/iommu/iommu.c           | 25 ++++++++++++++++
>  drivers/iommu/iova.c            |  2 +-
>  drivers/vfio/vfio.c             |  5 ++--
>  drivers/vfio/vfio_iommu_type1.c | 63 ++++++++++++++++++++++++++++++++++++++++-
>  include/linux/dma-iommu.h       |  9 ++++++
>  include/linux/iommu.h           | 23 +++++++++++++++
>  include/uapi/linux/vfio.h       | 16 ++++++++++-
>  10 files changed, 275 insertions(+), 18 deletions(-)
> 

[toc] | [prev] | [next] | [standalone]

#1516747 — Summary of LPC guest MSI discussion in Santa Fe (was: Re: [RFC 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 (Alt II))

Hi all,

I figured this was a reasonable post to piggy-back on for the LPC minutes
relating to guest MSIs on arm64.

On Thu, Nov 03, 2016 at 10:02:05PM -0600, Alex Williamson wrote:
> We can always have QEMU reject hot-adding the device if the reserved
> region overlaps existing guest RAM, but I don't even really see how we
> advise users to give them a reasonable chance of avoiding that
> possibility.  Apparently there are also ARM platforms where MSI pages
> cannot be remapped to support the previous programmable user/VM
> address, is it even worthwhile to support those platforms?  Does that
> decision influence whether user programmable MSI reserved regions are
> really a second class citizen to fixed reserved regions?  I expect
> we'll be talking about this tomorrow morning, but I certainly haven't
> come up with any viable solutions to this.  Thanks,

At LPC last week, we discussed guest MSIs on arm64 as part of the PCI
microconference. I presented some slides to illustrate some of the issues
we're trying to solve:

  http://www.willdeacon.ukfsn.org/bitbucket/lpc-16/msi-in-guest-arm64.pdf

Punit took some notes (thanks!) on the etherpad here:

  https://etherpad.openstack.org/p/LPC2016_PCI

although the discussion was pretty lively and jumped about, so I've had
to go from memory where the notes didn't capture everything that was
said.

To summarise, arm64 platforms differ in their handling of MSIs when compared
to x86:

  1. The physical memory map is not standardised (Jon pointed out that
     this is something that was realised late on)
  2. MSIs are usually treated the same as DMA writes, in that they must be
     mapped by the SMMU page tables so that they target a physical MSI
     doorbell
  3. On some platforms, MSIs bypass the SMMU entirely (e.g. due to an MSI
     doorbell built into the PCI RC)
  4. Platforms typically have some set of addresses that abort before
     reaching the SMMU (e.g. because the PCI identifies them as P2P).

All of this means that userspace (QEMU) needs to identify the memory
regions corresponding to points (3) and (4) and ensure that they are
not allocated in the guest physical (IPA) space. For platforms that can
remap the MSI doorbell as in (2), then some space also needs to be
allocated for that.

Rather than treat these as separate problems, a better interface is to
tell userspace about a set of reserved regions, and have this include
the MSI doorbell, irrespective of whether or not it can be remapped.
Don suggested that we statically pick an address for the doorbell in a
similar way to x86, and have the kernel map it there. We could even pick
0xfee00000. If it conflicts with a reserved region on the platform (due
to (4)), then we'd obviously have to (deterministically?) allocate it
somewhere else, but probably within the bottom 4G.

The next question is how to tell userspace about all of the reserved
regions. Initially, the idea was to extend VFIO, however Alex pointed
out a horrible scenario:

  1. QEMU spawns a VM on system 0
  2. VM is migrated to system 1
  3. QEMU attempts to passthrough a device using PCI hotplug

In this scenario, the guest memory map is chosen at step (1), yet there
is no VFIO fd available to determine the reserved regions. Furthermore,
the reserved regions may vary between system 0 and system 1. This pretty
much rules out using VFIO to determine the reserved regions. Alex suggested
that the SMMU driver can advertise the regions via /sys/class/iommu/. This
would solve part of the problem, but migration between systems with
different memory maps can still cause problems if the reserved regions
of the new system conflict with the guest memory map chosen by QEMU.
Jon pointed out that most people are pretty conservative about hardware
choices when migrating between them -- that is, they may only migrate
between different revisions of the same SoC, or they know ahead of time
all of the memory maps they want to support and this could be communicated
by way of configuration to libvirt. It would be up to QEMU to fail the
hotplug if it detected a conflict. Alex asked if there was a security
issue with DMA bypassing the SMMU, but there aren't currently any systems
where that is known to happen. Such a system would surely not be safe for
passthrough.

Ben mused that a way to handle conflicts dynamically might be to hotplug
on the entire host bridge in the guest, passing firmware tables describing
the new reserved regions as a property of the host bridge. Whilst this
may well solve the issue, it was largely considered future work due to
its invasive nature and dependency on firmware tables (and guest support)
that do not currently exist.

Will

[toc] | [prev] | [next] | [standalone]

#1517240 — Re: Summary of LPC guest MSI discussion in Santa Fe

Hi Will,

On 08/11/2016 03:45, Will Deacon wrote:
> Hi all,
> 
> I figured this was a reasonable post to piggy-back on for the LPC minutes
> relating to guest MSIs on arm64.
> 
> On Thu, Nov 03, 2016 at 10:02:05PM -0600, Alex Williamson wrote:
>> We can always have QEMU reject hot-adding the device if the reserved
>> region overlaps existing guest RAM, but I don't even really see how we
>> advise users to give them a reasonable chance of avoiding that
>> possibility.  Apparently there are also ARM platforms where MSI pages
>> cannot be remapped to support the previous programmable user/VM
>> address, is it even worthwhile to support those platforms?  Does that
>> decision influence whether user programmable MSI reserved regions are
>> really a second class citizen to fixed reserved regions?  I expect
>> we'll be talking about this tomorrow morning, but I certainly haven't
>> come up with any viable solutions to this.  Thanks,
> 
> At LPC last week, we discussed guest MSIs on arm64 as part of the PCI
> microconference. I presented some slides to illustrate some of the issues
> we're trying to solve:
> 
>   http://www.willdeacon.ukfsn.org/bitbucket/lpc-16/msi-in-guest-arm64.pdf
> 
> Punit took some notes (thanks!) on the etherpad here:
> 
>   https://etherpad.openstack.org/p/LPC2016_PCI

Thanks to both of you for the minutes and slides. Unfortunately I could
not travel but my ears were burning ;-)
> 
> although the discussion was pretty lively and jumped about, so I've had
> to go from memory where the notes didn't capture everything that was
> said.
> 
> To summarise, arm64 platforms differ in their handling of MSIs when compared
> to x86:
> 
>   1. The physical memory map is not standardised (Jon pointed out that
>      this is something that was realised late on)
>   2. MSIs are usually treated the same as DMA writes, in that they must be
>      mapped by the SMMU page tables so that they target a physical MSI
>      doorbell
>   3. On some platforms, MSIs bypass the SMMU entirely (e.g. due to an MSI
>      doorbell built into the PCI RC)
>   4. Platforms typically have some set of addresses that abort before
>      reaching the SMMU (e.g. because the PCI identifies them as P2P).
> 
> All of this means that userspace (QEMU) needs to identify the memory
> regions corresponding to points (3) and (4) and ensure that they are
> not allocated in the guest physical (IPA) space. For platforms that can
> remap the MSI doorbell as in (2), then some space also needs to be
> allocated for that.
> 
> Rather than treat these as separate problems, a better interface is to
> tell userspace about a set of reserved regions, and have this include
> the MSI doorbell, irrespective of whether or not it can be remapped.
> Don suggested that we statically pick an address for the doorbell in a
> similar way to x86, and have the kernel map it there. We could even pick
> 0xfee00000. If it conflicts with a reserved region on the platform (due
> to (4)), then we'd obviously have to (deterministically?) allocate it
> somewhere else, but probably within the bottom 4G.

This is tentatively achieved now with
[1] [RFC v2 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 - Alt II
(http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1264506.html)
> 
> The next question is how to tell userspace about all of the reserved
> regions. Initially, the idea was to extend VFIO, however Alex pointed
> out a horrible scenario:
> 
>   1. QEMU spawns a VM on system 0
>   2. VM is migrated to system 1
>   3. QEMU attempts to passthrough a device using PCI hotplug
> 
> In this scenario, the guest memory map is chosen at step (1), yet there
> is no VFIO fd available to determine the reserved regions. Furthermore,
> the reserved regions may vary between system 0 and system 1. This pretty
> much rules out using VFIO to determine the reserved regions.Alex suggested
> that the SMMU driver can advertise the regions via /sys/class/iommu/. This
> would solve part of the problem, but migration between systems with
> different memory maps can still cause problems if the reserved regions
> of the new system conflict with the guest memory map chosen by QEMU.


OK so I understand we do not want anymore the VFIO chain capability API
(patch 5 of above series) but we prefer a sysfs approach instead.

I understand the sysfs approach which allows the userspace to get the
info earlier and independently on VFIO. Keeping in mind current QEMU
virt - which is not the only userspace - will not do much from this info
until we bring upheavals in virt address space management. So if I am
not wrong, at the moment the main action to be undertaken is the
rejection of the PCI hotplug in case we detect a collision.

I can respin [1]
- studying and taking into account Robin's comments about dm_regions
similarities
- removing the VFIO capability chain and replacing this by a sysfs API

Would that be OK?

What about Alex comments who wanted to report the usable memory ranges
instead of unusable memory ranges?

Also did you have a chance to discuss the following items:
1) the VFIO irq safety assessment
2) the MSI reserved size computation (is an arbitrary size OK?)

Thanks

Eric

> Jon pointed out that most people are pretty conservative about hardware
> choices when migrating between them -- that is, they may only migrate
> between different revisions of the same SoC, or they know ahead of time
> all of the memory maps they want to support and this could be communicated
> by way of configuration to libvirt. It would be up to QEMU to fail the
> hotplug if it detected a conflict. Alex asked if there was a security
> issue with DMA bypassing the SMMU, but there aren't currently any systems
> where that is known to happen. Such a system would surely not be safe for
> passthrough.
> 
> Ben mused that a way to handle conflicts dynamically might be to hotplug
> on the entire host bridge in the guest, passing firmware tables describing
> the new reserved regions as a property of the host bridge. Whilst this
> may well solve the issue, it was largely considered future work due to
> its invasive nature and dependency on firmware tables (and guest support)
> that do not currently exist.
> 
> Will
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[toc] | [prev] | [next] | [standalone]

#1517434 — Re: Summary of LPC guest MSI discussion in Santa Fe

On Tue, Nov 08, 2016 at 03:27:23PM +0100, Auger Eric wrote:
> On 08/11/2016 03:45, Will Deacon wrote:
> > Rather than treat these as separate problems, a better interface is to
> > tell userspace about a set of reserved regions, and have this include
> > the MSI doorbell, irrespective of whether or not it can be remapped.
> > Don suggested that we statically pick an address for the doorbell in a
> > similar way to x86, and have the kernel map it there. We could even pick
> > 0xfee00000. If it conflicts with a reserved region on the platform (due
> > to (4)), then we'd obviously have to (deterministically?) allocate it
> > somewhere else, but probably within the bottom 4G.
> 
> This is tentatively achieved now with
> [1] [RFC v2 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 - Alt II
> (http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1264506.html)

Yup, I saw that fly by. Hopefully some of the internals can be reused
with the current thinking on user ABI.

> > The next question is how to tell userspace about all of the reserved
> > regions. Initially, the idea was to extend VFIO, however Alex pointed
> > out a horrible scenario:
> > 
> >   1. QEMU spawns a VM on system 0
> >   2. VM is migrated to system 1
> >   3. QEMU attempts to passthrough a device using PCI hotplug
> > 
> > In this scenario, the guest memory map is chosen at step (1), yet there
> > is no VFIO fd available to determine the reserved regions. Furthermore,
> > the reserved regions may vary between system 0 and system 1. This pretty
> > much rules out using VFIO to determine the reserved regions.Alex suggested
> > that the SMMU driver can advertise the regions via /sys/class/iommu/. This
> > would solve part of the problem, but migration between systems with
> > different memory maps can still cause problems if the reserved regions
> > of the new system conflict with the guest memory map chosen by QEMU.
> 
> 
> OK so I understand we do not want anymore the VFIO chain capability API
> (patch 5 of above series) but we prefer a sysfs approach instead.

Right.

> I understand the sysfs approach which allows the userspace to get the
> info earlier and independently on VFIO. Keeping in mind current QEMU
> virt - which is not the only userspace - will not do much from this info
> until we bring upheavals in virt address space management. So if I am
> not wrong, at the moment the main action to be undertaken is the
> rejection of the PCI hotplug in case we detect a collision.

I don't think so; it should be up to userspace to reject the hotplug.
If userspace doesn't have support for the regions, then that's fine --
you just end up in a situation where the CPU page table maps memory
somewhere that the device can't see. In other words, you'll end up with
spurious DMA failures, but that's exactly what happens with current systems
if you passthrough an overlapping region (Robin demonstrated this on Juno).

Additionally, you can imagine some future support where you can tell the
guest not to use certain regions of its memory for DMA. In this case, you
wouldn't want to refuse the hotplug in the case of overlapping regions.

Really, I think the kernel side just needs to enumerate the fixed reserved
regions, place the doorbell at a fixed address and then advertise these
via sysfs.

> I can respin [1]
> - studying and taking into account Robin's comments about dm_regions
> similarities
> - removing the VFIO capability chain and replacing this by a sysfs API

Ideally, this would be reusable between different SMMU drivers so the sysfs
entries have the same format etc.

> Would that be OK?

Sounds good to me. Are you in a position to prototype something on the qemu
side once we've got kernel-side agreement?

> What about Alex comments who wanted to report the usable memory ranges
> instead of unusable memory ranges?
> 
> Also did you have a chance to discuss the following items:
> 1) the VFIO irq safety assessment

The discussion really focussed on system topology, as opposed to properties
of the doorbell. Regardless of how the device talks to the doorbell, if
the doorbell can't protect against things like MSI spoofing, then it's
unsafe. My opinion is that we shouldn't allow passthrough by default on
systems with unsafe doorbells (we could piggyback on allow_unsafe_interrupts
cmdline option to VFIO).

A first step would be making all this opt-in, and only supporting GICv3
ITS for now.

> 2) the MSI reserved size computation (is an arbitrary size OK?)

If we fix the base address, we could fix a size too. However, we'd still
need to enumerate the doorbells to check that they fit in the region we
have. If not, then we can warn during boot and treat it the same way as
a resource conflict (that is, reallocate the region in some deterministic
way).

Will

[toc] | [prev] | [next] | [standalone]

#1517491 — Re: Summary of LPC guest MSI discussion in Santa Fe

On 11/08/2016 12:54 PM, Will Deacon wrote:
> On Tue, Nov 08, 2016 at 03:27:23PM +0100, Auger Eric wrote:
>> On 08/11/2016 03:45, Will Deacon wrote:
>>> Rather than treat these as separate problems, a better interface is to
>>> tell userspace about a set of reserved regions, and have this include
>>> the MSI doorbell, irrespective of whether or not it can be remapped.
>>> Don suggested that we statically pick an address for the doorbell in a
>>> similar way to x86, and have the kernel map it there. We could even pick
>>> 0xfee00000. If it conflicts with a reserved region on the platform (due
>>> to (4)), then we'd obviously have to (deterministically?) allocate it
>>> somewhere else, but probably within the bottom 4G.
>> This is tentatively achieved now with
>> [1] [RFC v2 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 - Alt II
>> (http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1264506.html)
> Yup, I saw that fly by. Hopefully some of the internals can be reused
> with the current thinking on user ABI.
>
>>> The next question is how to tell userspace about all of the reserved
>>> regions. Initially, the idea was to extend VFIO, however Alex pointed
>>> out a horrible scenario:
>>>
>>>    1. QEMU spawns a VM on system 0
>>>    2. VM is migrated to system 1
>>>    3. QEMU attempts to passthrough a device using PCI hotplug
>>>
>>> In this scenario, the guest memory map is chosen at step (1), yet there
>>> is no VFIO fd available to determine the reserved regions. Furthermore,
>>> the reserved regions may vary between system 0 and system 1. This pretty
>>> much rules out using VFIO to determine the reserved regions.Alex suggested
>>> that the SMMU driver can advertise the regions via /sys/class/iommu/. This
>>> would solve part of the problem, but migration between systems with
>>> different memory maps can still cause problems if the reserved regions
>>> of the new system conflict with the guest memory map chosen by QEMU.
>>
>> OK so I understand we do not want anymore the VFIO chain capability API
>> (patch 5 of above series) but we prefer a sysfs approach instead.
> Right.
>
>> I understand the sysfs approach which allows the userspace to get the
>> info earlier and independently on VFIO. Keeping in mind current QEMU
>> virt - which is not the only userspace - will not do much from this info
>> until we bring upheavals in virt address space management. So if I am
>> not wrong, at the moment the main action to be undertaken is the
>> rejection of the PCI hotplug in case we detect a collision.
> I don't think so; it should be up to userspace to reject the hotplug.
> If userspace doesn't have support for the regions, then that's fine --
> you just end up in a situation where the CPU page table maps memory
> somewhere that the device can't see. In other words, you'll end up with
> spurious DMA failures, but that's exactly what happens with current systems
> if you passthrough an overlapping region (Robin demonstrated this on Juno).
>
> Additionally, you can imagine some future support where you can tell the
> guest not to use certain regions of its memory for DMA. In this case, you
> wouldn't want to refuse the hotplug in the case of overlapping regions.
>
> Really, I think the kernel side just needs to enumerate the fixed reserved
> regions, place the doorbell at a fixed address and then advertise these
> via sysfs.
>
>> I can respin [1]
>> - studying and taking into account Robin's comments about dm_regions
>> similarities
>> - removing the VFIO capability chain and replacing this by a sysfs API
> Ideally, this would be reusable between different SMMU drivers so the sysfs
> entries have the same format etc.
>
>> Would that be OK?
> Sounds good to me. Are you in a position to prototype something on the qemu
> side once we've got kernel-side agreement?
>
>> What about Alex comments who wanted to report the usable memory ranges
>> instead of unusable memory ranges?
>>
>> Also did you have a chance to discuss the following items:
>> 1) the VFIO irq safety assessment
> The discussion really focussed on system topology, as opposed to properties
> of the doorbell. Regardless of how the device talks to the doorbell, if
> the doorbell can't protect against things like MSI spoofing, then it's
> unsafe. My opinion is that we shouldn't allow passthrough by default on
> systems with unsafe doorbells (we could piggyback on allow_unsafe_interrupts
> cmdline option to VFIO).
>
> A first step would be making all this opt-in, and only supporting GICv3
> ITS for now.
You're trying to support a config that is < GICv3 and no ITS ? ...
That would be the equiv. of x86 pre-intr-remap, and that's why allow_unsafe_interrupts
hook was created ... to enable devel/kick-the-tires.
>> 2) the MSI reserved size computation (is an arbitrary size OK?)
> If we fix the base address, we could fix a size too. However, we'd still
> need to enumerate the doorbells to check that they fit in the region we
> have. If not, then we can warn during boot and treat it the same way as
> a resource conflict (that is, reallocate the region in some deterministic
> way).
>
> Will

[toc] | [prev] | [next] | [standalone]

#1517505 — Re: Summary of LPC guest MSI discussion in Santa Fe

On Tue, Nov 08, 2016 at 02:02:39PM -0500, Don Dutile wrote:
> On 11/08/2016 12:54 PM, Will Deacon wrote:
> >A first step would be making all this opt-in, and only supporting GICv3
> >ITS for now.
> You're trying to support a config that is < GICv3 and no ITS ? ...
> That would be the equiv. of x86 pre-intr-remap, and that's why allow_unsafe_interrupts
> hook was created ... to enable devel/kick-the-tires.

Yup, that's exactly what I was envisaging. For systems that can't do
passthrough safely, we'll always have to throw a devel switch.

Will

[toc] | [prev] | [next] | [standalone]

#1517805 — Re: Summary of LPC guest MSI discussion in Santa Fe

Hi Will,
On 08/11/2016 20:02, Don Dutile wrote:
> On 11/08/2016 12:54 PM, Will Deacon wrote:
>> On Tue, Nov 08, 2016 at 03:27:23PM +0100, Auger Eric wrote:
>>> On 08/11/2016 03:45, Will Deacon wrote:
>>>> Rather than treat these as separate problems, a better interface is to
>>>> tell userspace about a set of reserved regions, and have this include
>>>> the MSI doorbell, irrespective of whether or not it can be remapped.
>>>> Don suggested that we statically pick an address for the doorbell in a
>>>> similar way to x86, and have the kernel map it there. We could even
>>>> pick
>>>> 0xfee00000. If it conflicts with a reserved region on the platform (due
>>>> to (4)), then we'd obviously have to (deterministically?) allocate it
>>>> somewhere else, but probably within the bottom 4G.
>>> This is tentatively achieved now with
>>> [1] [RFC v2 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 - Alt II
>>> (http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1264506.html)
>>>
>> Yup, I saw that fly by. Hopefully some of the internals can be reused
>> with the current thinking on user ABI.
>>
>>>> The next question is how to tell userspace about all of the reserved
>>>> regions. Initially, the idea was to extend VFIO, however Alex pointed
>>>> out a horrible scenario:
>>>>
>>>>    1. QEMU spawns a VM on system 0
>>>>    2. VM is migrated to system 1
>>>>    3. QEMU attempts to passthrough a device using PCI hotplug
>>>>
>>>> In this scenario, the guest memory map is chosen at step (1), yet there
>>>> is no VFIO fd available to determine the reserved regions. Furthermore,
>>>> the reserved regions may vary between system 0 and system 1. This
>>>> pretty
>>>> much rules out using VFIO to determine the reserved regions.Alex
>>>> suggested
>>>> that the SMMU driver can advertise the regions via
>>>> /sys/class/iommu/. This
>>>> would solve part of the problem, but migration between systems with
>>>> different memory maps can still cause problems if the reserved regions
>>>> of the new system conflict with the guest memory map chosen by QEMU.
>>>
>>> OK so I understand we do not want anymore the VFIO chain capability API
>>> (patch 5 of above series) but we prefer a sysfs approach instead.
>> Right.
>>
>>> I understand the sysfs approach which allows the userspace to get the
>>> info earlier and independently on VFIO. Keeping in mind current QEMU
>>> virt - which is not the only userspace - will not do much from this info
>>> until we bring upheavals in virt address space management. So if I am
>>> not wrong, at the moment the main action to be undertaken is the
>>> rejection of the PCI hotplug in case we detect a collision.
>> I don't think so; it should be up to userspace to reject the hotplug.
>> If userspace doesn't have support for the regions, then that's fine --
>> you just end up in a situation where the CPU page table maps memory
>> somewhere that the device can't see. In other words, you'll end up with
>> spurious DMA failures, but that's exactly what happens with current
>> systems
>> if you passthrough an overlapping region (Robin demonstrated this on
>> Juno).
>>
>> Additionally, you can imagine some future support where you can tell the
>> guest not to use certain regions of its memory for DMA. In this case, you
>> wouldn't want to refuse the hotplug in the case of overlapping regions.
>>
>> Really, I think the kernel side just needs to enumerate the fixed
>> reserved
>> regions, place the doorbell at a fixed address and then advertise these
>> via sysfs.
>>
>>> I can respin [1]
>>> - studying and taking into account Robin's comments about dm_regions
>>> similarities
>>> - removing the VFIO capability chain and replacing this by a sysfs API
>> Ideally, this would be reusable between different SMMU drivers so the
>> sysfs
>> entries have the same format etc.
>>
>>> Would that be OK?
>> Sounds good to me. Are you in a position to prototype something on the
>> qemu
>> side once we've got kernel-side agreement?
yes sure.
>>
>>> What about Alex comments who wanted to report the usable memory ranges
>>> instead of unusable memory ranges?
>>>
>>> Also did you have a chance to discuss the following items:
>>> 1) the VFIO irq safety assessment
>> The discussion really focussed on system topology, as opposed to
>> properties
>> of the doorbell. Regardless of how the device talks to the doorbell, if
>> the doorbell can't protect against things like MSI spoofing, then it's
>> unsafe. My opinion is that we shouldn't allow passthrough by default on
>> systems with unsafe doorbells (we could piggyback on
>> allow_unsafe_interrupts
>> cmdline option to VFIO).
OK.
>>
>> A first step would be making all this opt-in, and only supporting GICv3
>> ITS for now.
> You're trying to support a config that is < GICv3 and no ITS ? ...
> That would be the equiv. of x86 pre-intr-remap, and that's why
> allow_unsafe_interrupts
> hook was created ... to enable devel/kick-the-tires.
>>> 2) the MSI reserved size computation (is an arbitrary size OK?)
>> If we fix the base address, we could fix a size too. However, we'd still
>> need to enumerate the doorbells to check that they fit in the region we
>> have. If not, then we can warn during boot and treat it the same way as
>> a resource conflict (that is, reallocate the region in some deterministic
>> way).
OK

Thanks

Eric
>>
>> Will
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[toc] | [prev] | [next] | [standalone]

#1517323 — Re: Summary of LPC guest MSI discussion in Santa Fe

On 11/07/2016 09:45 PM, Will Deacon wrote:
> Hi all,
>
> I figured this was a reasonable post to piggy-back on for the LPC minutes
> relating to guest MSIs on arm64.
>
> On Thu, Nov 03, 2016 at 10:02:05PM -0600, Alex Williamson wrote:
>> We can always have QEMU reject hot-adding the device if the reserved
>> region overlaps existing guest RAM, but I don't even really see how we
>> advise users to give them a reasonable chance of avoiding that
>> possibility.  Apparently there are also ARM platforms where MSI pages
>> cannot be remapped to support the previous programmable user/VM
>> address, is it even worthwhile to support those platforms?  Does that
>> decision influence whether user programmable MSI reserved regions are
>> really a second class citizen to fixed reserved regions?  I expect
>> we'll be talking about this tomorrow morning, but I certainly haven't
>> come up with any viable solutions to this.  Thanks,
>
> At LPC last week, we discussed guest MSIs on arm64 as part of the PCI
> microconference. I presented some slides to illustrate some of the issues
> we're trying to solve:
>
>    http://www.willdeacon.ukfsn.org/bitbucket/lpc-16/msi-in-guest-arm64.pdf
>
> Punit took some notes (thanks!) on the etherpad here:
>
>    https://etherpad.openstack.org/p/LPC2016_PCI
>
> although the discussion was pretty lively and jumped about, so I've had
> to go from memory where the notes didn't capture everything that was
> said.
>
> To summarise, arm64 platforms differ in their handling of MSIs when compared
> to x86:
>
>    1. The physical memory map is not standardised (Jon pointed out that
>       this is something that was realised late on)
>    2. MSIs are usually treated the same as DMA writes, in that they must be
>       mapped by the SMMU page tables so that they target a physical MSI
>       doorbell
>    3. On some platforms, MSIs bypass the SMMU entirely (e.g. due to an MSI
>       doorbell built into the PCI RC)
Chaulk this case to 'the learning curve'.
Q35 chipset (the one being use for x86-PCIe qemu model) had no intr-remap hw,
only DMA addrs destined for real memory. assigned-device intrs had to be caught
by kvm & injected into guests, and yes, a DoS was possible... and thus,
the intr-remap support being done after initial iommu support.


>    4. Platforms typically have some set of addresses that abort before
>       reaching the SMMU (e.g. because the PCI identifies them as P2P).
ARM platforms that don't implement the equivalent of ACS (in PCI bridges within
a PCIe switch) are either not device-assignment capable, or the IOMMU domain
expands across the entire peer-to-peer (sub-)tree.
ACS(-like) functionality is a fundamental component to the security model,
as is the IOMMU itself.  Without it, it's equivalent to not having an IOMMU.

Dare I ask?: Can these systems, or parts of these systems, just be deemed
"incomplete" or "not 100% secure" wrt device assignment, and other systems
can or will be ???
Not much different then the first x86 systems that tried to get it right
the first few times... :-/
I'm hearing (parts of) systems that are just not properly designed
for device-assignment use-case, probably b/c this (system) architecture
hasn't been pulled together from the variouis component architectures
(CPU, SMMU, IOT, etc.).

>
> All of this means that userspace (QEMU) needs to identify the memory
> regions corresponding to points (3) and (4) and ensure that they are
> not allocated in the guest physical (IPA) space. For platforms that can
> remap the MSI doorbell as in (2), then some space also needs to be
> allocated for that.
>
Again, proper ACS features/control eliminates this need.
A (multi-function) device should never be able to perform
IO to itself via its PCIe interface.  Bridge-ACS pushes everything
up to SMMU for desitination resolution.
Without ACS, I don't see how a guest is migratible from one system to
another, unless the system-migration-group consists of system that
are exactly the same (wrt IO) [less/more system memory &/or cpu does
not affect VM system map.
  
Again, the initial Linux implementation did not have ACS,
but was 'resolved' by the default/common system mapping putting the PCI devices
into an area that was blocked from memory use (generally 3G->4G space).
ARM may not have that single, simple implementation, but a method to
indicated reserved regions, and then a check for matching/non-matching reserved
regions for guest migration, is the only way I see to resolve this issue
until ACS is sufficiently supported int the hw subsystems to be used
for device-assignment.

> Rather than treat these as separate problems, a better interface is to
> tell userspace about a set of reserved regions, and have this include
> the MSI doorbell, irrespective of whether or not it can be remapped.
> Don suggested that we statically pick an address for the doorbell in a
> similar way to x86, and have the kernel map it there. We could even pick
> 0xfee00000.
I suggest picking a 'relative-fixed' address: the last n-pages of system memory
address space, i.e.,
    0xfff[....]fee00000.   ... sign-extended 0xfee00000.
That way, no matter how large memory is, there is no hole, it's just the
last 2M of memory ... every system has an end of memory. :-p

> If it conflicts with a reserved region on the platform (due
>to (4)), then we'd obviously have to (deterministically?) allocate it
> somewhere else, but probably within the bottom 4G.
>
why? It's more likely for a hw platform to use this <4G address range
for device mmio, then the upper-most address space for device space.  Even if
platforms use upper-most address space now, it's not rocket science to subtract
upper 2M from existing use, then allocate device space from there (downward).
... and if you pull a 'there are systems that have hard-wired addresses
in upper-most 2M', then we can look at if we can quirk these systems, or
just not support them for this use-case.

> The next question is how to tell userspace about all of the reserved
> regions. Initially, the idea was to extend VFIO, however Alex pointed
Won't need to, if upper memory space is passed; take upper 2M and done. ;-)
For now, you'll need a whole new qemu paradigm, for a (hopefully) short-term
problem. I suggest coming up with a short-term, default 'safe' place for
devices & memory to avoid the qemu disruption.

> out a horrible scenario:
>
>    1. QEMU spawns a VM on system 0
>    2. VM is migrated to system 1
>    3. QEMU attempts to passthrough a device using PCI hotplug
>
> In this scenario, the guest memory map is chosen at step (1), yet there
> is no VFIO fd available to determine the reserved regions. Furthermore,
> the reserved regions may vary between system 0 and system 1. This pretty
> much rules out using VFIO to determine the reserved regions. Alex suggested
> that the SMMU driver can advertise the regions via /sys/class/iommu/. This
> would solve part of the problem, but migration between systems with
> different memory maps can still cause problems if the reserved regions
> of the new system conflict with the guest memory map chosen by QEMU.
> Jon pointed out that most people are pretty conservative about hardware
> choices when migrating between them -- that is, they may only migrate
> between different revisions of the same SoC, or they know ahead of time
> all of the memory maps they want to support and this could be communicated
> by way of configuration to libvirt. It would be up to QEMU to fail the
> hotplug if it detected a conflict. Alex asked if there was a security
> issue with DMA bypassing the SMMU, but there aren't currently any systems
> where that is known to happen. Such a system would surely not be safe for
> passthrough.
>
> Ben mused that a way to handle conflicts dynamically might be to hotplug
> on the entire host bridge in the guest, passing firmware tables describing
> the new reserved regions as a property of the host bridge. Whilst this
> may well solve the issue, it was largely considered future work due to
> its invasive nature and dependency on firmware tables (and guest support)
> that do not currently exist.
>
> Will
>

[toc] | [prev] | [next] | [standalone]

#1517559 — Re: Summary of LPC guest MSI discussion in Santa Fe (was: Re: [RFC 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 (Alt II))

Hi Will,

On Tue, Nov 08, 2016 at 02:45:59AM +0000, Will Deacon wrote:
> Hi all,
> 
> I figured this was a reasonable post to piggy-back on for the LPC minutes
> relating to guest MSIs on arm64.
> 
> On Thu, Nov 03, 2016 at 10:02:05PM -0600, Alex Williamson wrote:
> > We can always have QEMU reject hot-adding the device if the reserved
> > region overlaps existing guest RAM, but I don't even really see how we
> > advise users to give them a reasonable chance of avoiding that
> > possibility.  Apparently there are also ARM platforms where MSI pages
> > cannot be remapped to support the previous programmable user/VM
> > address, is it even worthwhile to support those platforms?  Does that
> > decision influence whether user programmable MSI reserved regions are
> > really a second class citizen to fixed reserved regions?  I expect
> > we'll be talking about this tomorrow morning, but I certainly haven't
> > come up with any viable solutions to this.  Thanks,
> 
> At LPC last week, we discussed guest MSIs on arm64 as part of the PCI
> microconference. I presented some slides to illustrate some of the issues
> we're trying to solve:
> 
>   http://www.willdeacon.ukfsn.org/bitbucket/lpc-16/msi-in-guest-arm64.pdf
> 
> Punit took some notes (thanks!) on the etherpad here:
> 
>   https://etherpad.openstack.org/p/LPC2016_PCI
> 
> although the discussion was pretty lively and jumped about, so I've had
> to go from memory where the notes didn't capture everything that was
> said.
> 
> To summarise, arm64 platforms differ in their handling of MSIs when compared
> to x86:
> 
>   1. The physical memory map is not standardised (Jon pointed out that
>      this is something that was realised late on)
>   2. MSIs are usually treated the same as DMA writes, in that they must be
>      mapped by the SMMU page tables so that they target a physical MSI
>      doorbell
>   3. On some platforms, MSIs bypass the SMMU entirely (e.g. due to an MSI
>      doorbell built into the PCI RC)
>   4. Platforms typically have some set of addresses that abort before
>      reaching the SMMU (e.g. because the PCI identifies them as P2P).
> 
> All of this means that userspace (QEMU) needs to identify the memory
> regions corresponding to points (3) and (4) and ensure that they are
> not allocated in the guest physical (IPA) space. For platforms that can
> remap the MSI doorbell as in (2), then some space also needs to be
> allocated for that.
> 
> Rather than treat these as separate problems, a better interface is to
> tell userspace about a set of reserved regions, and have this include
> the MSI doorbell, irrespective of whether or not it can be remapped.

Is my understanding correct, that you need to tell userspace about the
location of the doorbell (in the IOVA space) in case (2), because even
though the configuration of the device is handled by the (host) kernel
through trapping of the BARs, we have to avoid the VFIO user programming
the device to create other DMA transactions to this particular address,
since that will obviously conflict and either not produce the desired
DMA transactions or result in unintended weird interrupts?

Thanks,
Christoffer

[toc] | [prev] | [next] | [standalone]

#1517650 — Re: Summary of LPC guest MSI discussion in Santa Fe (was: Re: [RFC 0/8] KVM PCIe/MSI passthrough on ARM/ARM64 (Alt II))

On Tue, 8 Nov 2016 21:29:22 +0100
Christoffer Dall <christoffer.dall@linaro.org> wrote:

> Hi Will,
> 
> On Tue, Nov 08, 2016 at 02:45:59AM +0000, Will Deacon wrote:
> > Hi all,
> > 
> > I figured this was a reasonable post to piggy-back on for the LPC minutes
> > relating to guest MSIs on arm64.
> > 
> > On Thu, Nov 03, 2016 at 10:02:05PM -0600, Alex Williamson wrote:  
> > > We can always have QEMU reject hot-adding the device if the reserved
> > > region overlaps existing guest RAM, but I don't even really see how we
> > > advise users to give them a reasonable chance of avoiding that
> > > possibility.  Apparently there are also ARM platforms where MSI pages
> > > cannot be remapped to support the previous programmable user/VM
> > > address, is it even worthwhile to support those platforms?  Does that
> > > decision influence whether user programmable MSI reserved regions are
> > > really a second class citizen to fixed reserved regions?  I expect
> > > we'll be talking about this tomorrow morning, but I certainly haven't
> > > come up with any viable solutions to this.  Thanks,  
> > 
> > At LPC last week, we discussed guest MSIs on arm64 as part of the PCI
> > microconference. I presented some slides to illustrate some of the issues
> > we're trying to solve:
> > 
> >   http://www.willdeacon.ukfsn.org/bitbucket/lpc-16/msi-in-guest-arm64.pdf
> > 
> > Punit took some notes (thanks!) on the etherpad here:
> > 
> >   https://etherpad.openstack.org/p/LPC2016_PCI
> > 
> > although the discussion was pretty lively and jumped about, so I've had
> > to go from memory where the notes didn't capture everything that was
> > said.
> > 
> > To summarise, arm64 platforms differ in their handling of MSIs when compared
> > to x86:
> > 
> >   1. The physical memory map is not standardised (Jon pointed out that
> >      this is something that was realised late on)
> >   2. MSIs are usually treated the same as DMA writes, in that they must be
> >      mapped by the SMMU page tables so that they target a physical MSI
> >      doorbell
> >   3. On some platforms, MSIs bypass the SMMU entirely (e.g. due to an MSI
> >      doorbell built into the PCI RC)
> >   4. Platforms typically have some set of addresses that abort before
> >      reaching the SMMU (e.g. because the PCI identifies them as P2P).
> > 
> > All of this means that userspace (QEMU) needs to identify the memory
> > regions corresponding to points (3) and (4) and ensure that they are
> > not allocated in the guest physical (IPA) space. For platforms that can
> > remap the MSI doorbell as in (2), then some space also needs to be
> > allocated for that.
> > 
> > Rather than treat these as separate problems, a better interface is to
> > tell userspace about a set of reserved regions, and have this include
> > the MSI doorbell, irrespective of whether or not it can be remapped.  
> 
> Is my understanding correct, that you need to tell userspace about the
> location of the doorbell (in the IOVA space) in case (2), because even
> though the configuration of the device is handled by the (host) kernel
> through trapping of the BARs, we have to avoid the VFIO user programming
> the device to create other DMA transactions to this particular address,
> since that will obviously conflict and either not produce the desired
> DMA transactions or result in unintended weird interrupts?

Correct, if the MSI doorbell IOVA range overlaps RAM in the VM, then
it's potentially a DMA target and we'll get bogus data on DMA read from
the device, and lose data and potentially trigger spurious interrupts on
DMA write from the device.  Thanks,

Alex

[toc] | [prev] | [next] | [standalone]

#1517743 — Re: Summary of LPC guest MSI discussion in Santa Fe

On 11/08/2016 06:35 PM, Alex Williamson wrote:
> On Tue, 8 Nov 2016 21:29:22 +0100
> Christoffer Dall <christoffer.dall@linaro.org> wrote:
>
>> Hi Will,
>>
>> On Tue, Nov 08, 2016 at 02:45:59AM +0000, Will Deacon wrote:
>>> Hi all,
>>>
>>> I figured this was a reasonable post to piggy-back on for the LPC minutes
>>> relating to guest MSIs on arm64.
>>>
>>> On Thu, Nov 03, 2016 at 10:02:05PM -0600, Alex Williamson wrote:
>>>> We can always have QEMU reject hot-adding the device if the reserved
>>>> region overlaps existing guest RAM, but I don't even really see how we
>>>> advise users to give them a reasonable chance of avoiding that
>>>> possibility.  Apparently there are also ARM platforms where MSI pages
>>>> cannot be remapped to support the previous programmable user/VM
>>>> address, is it even worthwhile to support those platforms?  Does that
>>>> decision influence whether user programmable MSI reserved regions are
>>>> really a second class citizen to fixed reserved regions?  I expect
>>>> we'll be talking about this tomorrow morning, but I certainly haven't
>>>> come up with any viable solutions to this.  Thanks,
>>>
>>> At LPC last week, we discussed guest MSIs on arm64 as part of the PCI
>>> microconference. I presented some slides to illustrate some of the issues
>>> we're trying to solve:
>>>
>>>    http://www.willdeacon.ukfsn.org/bitbucket/lpc-16/msi-in-guest-arm64.pdf
>>>
>>> Punit took some notes (thanks!) on the etherpad here:
>>>
>>>    https://etherpad.openstack.org/p/LPC2016_PCI
>>>
>>> although the discussion was pretty lively and jumped about, so I've had
>>> to go from memory where the notes didn't capture everything that was
>>> said.
>>>
>>> To summarise, arm64 platforms differ in their handling of MSIs when compared
>>> to x86:
>>>
>>>    1. The physical memory map is not standardised (Jon pointed out that
>>>       this is something that was realised late on)
>>>    2. MSIs are usually treated the same as DMA writes, in that they must be
>>>       mapped by the SMMU page tables so that they target a physical MSI
>>>       doorbell
>>>    3. On some platforms, MSIs bypass the SMMU entirely (e.g. due to an MSI
>>>       doorbell built into the PCI RC)
>>>    4. Platforms typically have some set of addresses that abort before
>>>       reaching the SMMU (e.g. because the PCI identifies them as P2P).
>>>
>>> All of this means that userspace (QEMU) needs to identify the memory
>>> regions corresponding to points (3) and (4) and ensure that they are
>>> not allocated in the guest physical (IPA) space. For platforms that can
>>> remap the MSI doorbell as in (2), then some space also needs to be
>>> allocated for that.
>>>
>>> Rather than treat these as separate problems, a better interface is to
>>> tell userspace about a set of reserved regions, and have this include
>>> the MSI doorbell, irrespective of whether or not it can be remapped.
>>
>> Is my understanding correct, that you need to tell userspace about the
>> location of the doorbell (in the IOVA space) in case (2), because even
>> though the configuration of the device is handled by the (host) kernel
>> through trapping of the BARs, we have to avoid the VFIO user programming
>> the device to create other DMA transactions to this particular address,
>> since that will obviously conflict and either not produce the desired
>> DMA transactions or result in unintended weird interrupts?
>
> Correct, if the MSI doorbell IOVA range overlaps RAM in the VM, then
> it's potentially a DMA target and we'll get bogus data on DMA read from
> the device, and lose data and potentially trigger spurious interrupts on
> DMA write from the device.  Thanks,
>
> Alex
>
That's b/c the MSI doorbells are not positioned *above* the SMMU, i.e.,
they address match before the SMMU checks are done.  if
all DMA addrs had to go through SMMU first, then the DMA access could
be ignored/rejected.
For bare-metal, memory can't be put in the same place as MSI addrs, or
DMA could never reach it.  So, only a virt issue, unless the VMs mem address
range mimic the host layout.

- Don

[toc] | [prev] | [next] | [standalone]

#1518317 — Re: Summary of LPC guest MSI discussion in Santa Fe

On Tue, Nov 08, 2016 at 09:52:33PM -0500, Don Dutile wrote:
> On 11/08/2016 06:35 PM, Alex Williamson wrote:
> >On Tue, 8 Nov 2016 21:29:22 +0100
> >Christoffer Dall <christoffer.dall@linaro.org> wrote:
> >>Is my understanding correct, that you need to tell userspace about the
> >>location of the doorbell (in the IOVA space) in case (2), because even
> >>though the configuration of the device is handled by the (host) kernel
> >>through trapping of the BARs, we have to avoid the VFIO user programming
> >>the device to create other DMA transactions to this particular address,
> >>since that will obviously conflict and either not produce the desired
> >>DMA transactions or result in unintended weird interrupts?

Yes, that's the crux of the issue.

> >Correct, if the MSI doorbell IOVA range overlaps RAM in the VM, then
> >it's potentially a DMA target and we'll get bogus data on DMA read from
> >the device, and lose data and potentially trigger spurious interrupts on
> >DMA write from the device.  Thanks,
> >
> That's b/c the MSI doorbells are not positioned *above* the SMMU, i.e.,
> they address match before the SMMU checks are done.  if
> all DMA addrs had to go through SMMU first, then the DMA access could
> be ignored/rejected.

That's actually not true :( The SMMU can't generally distinguish between MSI
writes and DMA writes, so it would just see a write transaction to the
doorbell address, regardless of how it was generated by the endpoint.

Will

[toc] | [prev] | [next] | [standalone]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web