* [PATCH 1/3] intel-iommu: add some traces for PSIs
2018-04-18 4:41 [PATCH 0/3] intel-iommu: fix mapping PSI missing for iommu_map() Peter Xu
@ 2018-04-18 4:41 ` Peter Xu
2018-04-18 4:41 ` [PATCH 2/3] intel-iommu: generalize __mapping_notify_one() Peter Xu
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Peter Xu @ 2018-04-18 4:41 UTC (permalink / raw)
To: linux-kernel; +Cc: Alex Williamson, Joerg Roedel, David Woodhouse, peterx
It is helpful to debug and triage PSI notification missings.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
drivers/iommu/dmar.c | 3 +++
drivers/iommu/intel-iommu.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 9a7ffd13c7f0..62ae26c3f7b7 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1325,6 +1325,9 @@ void qi_flush_iotlb(struct intel_iommu *iommu, u16 did, u64 addr,
struct qi_desc desc;
int ih = 0;
+ pr_debug("%s: iommu %d did %u addr 0x%llx order %u type %llx\n",
+ __func__, iommu->seq_id, did, addr, size_order, type);
+
if (cap_write_drain(iommu->cap))
dw = 1;
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 582fd01cb7d1..a64da83e867c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1396,6 +1396,9 @@ static void __iommu_flush_iotlb(struct intel_iommu *iommu, u16 did,
u64 val = 0, val_iva = 0;
unsigned long flag;
+ pr_debug("%s: iommu %d did %u addr 0x%llx order %u type %llx\n",
+ __func__, iommu->seq_id, did, addr, size_order, type);
+
switch (type) {
case DMA_TLB_GLOBAL_FLUSH:
/* global flush doesn't need set IVA_REG */
--
2.14.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] intel-iommu: generalize __mapping_notify_one()
2018-04-18 4:41 [PATCH 0/3] intel-iommu: fix mapping PSI missing for iommu_map() Peter Xu
2018-04-18 4:41 ` [PATCH 1/3] intel-iommu: add some traces for PSIs Peter Xu
@ 2018-04-18 4:41 ` Peter Xu
2018-04-18 4:41 ` [PATCH 3/3] intel-iommu: fix iotlb psi missing for mappings Peter Xu
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Peter Xu @ 2018-04-18 4:41 UTC (permalink / raw)
To: linux-kernel; +Cc: Alex Williamson, Joerg Roedel, David Woodhouse, peterx
Generalize this new helper to notify one newly created mapping on one
single IOMMU. We can further leverage this helper in the next patch.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
drivers/iommu/intel-iommu.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index a64da83e867c..bf111e60857c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1607,6 +1607,18 @@ static void iommu_flush_iotlb_psi(struct intel_iommu *iommu,
iommu_flush_dev_iotlb(domain, addr, mask);
}
+/* Notification for newly created mappings */
+static inline void __mapping_notify_one(struct intel_iommu *iommu,
+ struct dmar_domain *domain,
+ unsigned long pfn, unsigned int pages)
+{
+ /* It's a non-present to present mapping. Only flush if caching mode */
+ if (cap_caching_mode(iommu->cap))
+ iommu_flush_iotlb_psi(iommu, domain, pfn, pages, 0, 1);
+ else
+ iommu_flush_write_buffer(iommu);
+}
+
static void iommu_flush_iova(struct iova_domain *iovad)
{
struct dmar_domain *domain;
@@ -3626,13 +3638,7 @@ static dma_addr_t __intel_map_single(struct device *dev, phys_addr_t paddr,
if (ret)
goto error;
- /* it's a non-present to present mapping. Only flush if caching mode */
- if (cap_caching_mode(iommu->cap))
- iommu_flush_iotlb_psi(iommu, domain,
- mm_to_dma_pfn(iova_pfn),
- size, 0, 1);
- else
- iommu_flush_write_buffer(iommu);
+ __mapping_notify_one(iommu, domain, mm_to_dma_pfn(iova_pfn), size);
start_paddr = (phys_addr_t)iova_pfn << PAGE_SHIFT;
start_paddr += paddr & ~PAGE_MASK;
@@ -3851,11 +3857,7 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
return 0;
}
- /* it's a non-present to present mapping. Only flush if caching mode */
- if (cap_caching_mode(iommu->cap))
- iommu_flush_iotlb_psi(iommu, domain, start_vpfn, size, 0, 1);
- else
- iommu_flush_write_buffer(iommu);
+ __mapping_notify_one(iommu, domain, start_vpfn, size);
return nelems;
}
--
2.14.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] intel-iommu: fix iotlb psi missing for mappings
2018-04-18 4:41 [PATCH 0/3] intel-iommu: fix mapping PSI missing for iommu_map() Peter Xu
2018-04-18 4:41 ` [PATCH 1/3] intel-iommu: add some traces for PSIs Peter Xu
2018-04-18 4:41 ` [PATCH 2/3] intel-iommu: generalize __mapping_notify_one() Peter Xu
@ 2018-04-18 4:41 ` Peter Xu
2018-04-18 4:46 ` [PATCH 0/3] intel-iommu: fix mapping PSI missing for iommu_map() Peter Xu
2018-04-18 8:36 ` Peter Xu
4 siblings, 0 replies; 6+ messages in thread
From: Peter Xu @ 2018-04-18 4:41 UTC (permalink / raw)
To: linux-kernel; +Cc: Alex Williamson, Joerg Roedel, David Woodhouse, peterx
When caching mode is enabled for IOMMU, we should send explicit IOTLB
PSIs even for newly created mappings. However these events are missing
for all intel_iommu_map() callers, e.g., iommu_map(). One direct user
is the vfio-pci driver.
To make sure we'll send the PSIs always when necessary, this patch
firstly introduced domain_mapping() helper for page mappings, then fixed
the problem by generalizing the explicit map IOTLB PSI logic into that
new helper. With that, we let iommu_domain_identity_map() to use the
simplified version to avoid sending the notifications, while for all the
rest of cases we send the notifications always.
For VM case, we send the PSIs to all the backend IOMMUs for the domain.
This patch allows the nested device assignment to work with QEMU (assign
device firstly to L1 guest, then assign it again to L2 guest).
Signed-off-by: Peter Xu <peterx@redhat.com>
---
drivers/iommu/intel-iommu.c | 43 ++++++++++++++++++++++++++++++++++---------
1 file changed, 34 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index bf111e60857c..eb0f0911342f 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2353,18 +2353,47 @@ static int __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
return 0;
}
+static int domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
+ struct scatterlist *sg, unsigned long phys_pfn,
+ unsigned long nr_pages, int prot)
+{
+ int ret;
+ struct intel_iommu *iommu;
+
+ /* Do the real mapping first */
+ ret = __domain_mapping(domain, iov_pfn, sg, phys_pfn, nr_pages, prot);
+ if (ret)
+ return ret;
+
+ /* Notify about the new mapping */
+ if (domain_type_is_vm(domain)) {
+ /* VM typed domains can have more than one IOMMUs */
+ int iommu_id;
+ for_each_domain_iommu(iommu_id, domain) {
+ iommu = g_iommus[iommu_id];
+ __mapping_notify_one(iommu, domain, iov_pfn, nr_pages);
+ }
+ } else {
+ /* General domains only have one IOMMU */
+ iommu = domain_get_iommu(domain);
+ __mapping_notify_one(iommu, domain, iov_pfn, nr_pages);
+ }
+
+ return 0;
+}
+
static inline int domain_sg_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
struct scatterlist *sg, unsigned long nr_pages,
int prot)
{
- return __domain_mapping(domain, iov_pfn, sg, 0, nr_pages, prot);
+ return domain_mapping(domain, iov_pfn, sg, 0, nr_pages, prot);
}
static inline int domain_pfn_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
unsigned long phys_pfn, unsigned long nr_pages,
int prot)
{
- return __domain_mapping(domain, iov_pfn, NULL, phys_pfn, nr_pages, prot);
+ return domain_mapping(domain, iov_pfn, NULL, phys_pfn, nr_pages, prot);
}
static void domain_context_clear_one(struct intel_iommu *iommu, u8 bus, u8 devfn)
@@ -2669,9 +2698,9 @@ static int iommu_domain_identity_map(struct dmar_domain *domain,
*/
dma_pte_clear_range(domain, first_vpfn, last_vpfn);
- return domain_pfn_mapping(domain, first_vpfn, first_vpfn,
- last_vpfn - first_vpfn + 1,
- DMA_PTE_READ|DMA_PTE_WRITE);
+ return __domain_mapping(domain, first_vpfn, NULL,
+ first_vpfn, last_vpfn - first_vpfn + 1,
+ DMA_PTE_READ|DMA_PTE_WRITE);
}
static int domain_prepare_identity_map(struct device *dev,
@@ -3638,8 +3667,6 @@ static dma_addr_t __intel_map_single(struct device *dev, phys_addr_t paddr,
if (ret)
goto error;
- __mapping_notify_one(iommu, domain, mm_to_dma_pfn(iova_pfn), size);
-
start_paddr = (phys_addr_t)iova_pfn << PAGE_SHIFT;
start_paddr += paddr & ~PAGE_MASK;
return start_paddr;
@@ -3857,8 +3884,6 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
return 0;
}
- __mapping_notify_one(iommu, domain, start_vpfn, size);
-
return nelems;
}
--
2.14.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] intel-iommu: fix mapping PSI missing for iommu_map()
2018-04-18 4:41 [PATCH 0/3] intel-iommu: fix mapping PSI missing for iommu_map() Peter Xu
` (2 preceding siblings ...)
2018-04-18 4:41 ` [PATCH 3/3] intel-iommu: fix iotlb psi missing for mappings Peter Xu
@ 2018-04-18 4:46 ` Peter Xu
2018-04-18 8:36 ` Peter Xu
4 siblings, 0 replies; 6+ messages in thread
From: Peter Xu @ 2018-04-18 4:46 UTC (permalink / raw)
To: linux-kernel
Cc: Alex Williamson, Joerg Roedel, David Woodhouse, Alexander Witte,
Jintack Lim
On Wed, Apr 18, 2018 at 12:41:27PM +0800, Peter Xu wrote:
> (PSI stands for: Page Selective Invalidations)
>
> Intel IOMMU has the caching mode to ease emulation of the device.
> When that bit is set, we need to send PSIs even for newly mapped
> pages. However current driver is not fully obey the rule. E.g.,
> iommu_map() API will only do the mapping but it never sent the PSIs
> before. That can be problematic to emulated IOMMU devices since
> they'll never be able to build up the shadow page tables if without
> such information. This patchset tries to fix the problem.
>
> Patch 1 is a tracing enhancement that helped me to triage the problem.
> It might even be useful in the future.
>
> Patch 2 generalized a helper to notify the MAP PSIs.
>
> Patch 3 fixes the real problem by making sure every domain mapping
> will trigger the MAP PSI notifications.
>
> Without the patchset, nested device assignment (assign one device
> firstly to L1 guest, then to L2 guest) won't work for QEMU. After
> applying the patchset, it works.
>
> Please review. Thanks.
>
> Peter Xu (3):
> intel-iommu: add some traces for PSIs
> intel-iommu: generalize __mapping_notify_one()
> intel-iommu: fix iotlb psi missing for mappings
>
> drivers/iommu/dmar.c | 3 ++
> drivers/iommu/intel-iommu.c | 68 ++++++++++++++++++++++++++++++++-------------
> 2 files changed, 52 insertions(+), 19 deletions(-)
Adding CC to Alexander Witte and Jintack.
--
Peter Xu
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] intel-iommu: fix mapping PSI missing for iommu_map()
2018-04-18 4:41 [PATCH 0/3] intel-iommu: fix mapping PSI missing for iommu_map() Peter Xu
` (3 preceding siblings ...)
2018-04-18 4:46 ` [PATCH 0/3] intel-iommu: fix mapping PSI missing for iommu_map() Peter Xu
@ 2018-04-18 8:36 ` Peter Xu
4 siblings, 0 replies; 6+ messages in thread
From: Peter Xu @ 2018-04-18 8:36 UTC (permalink / raw)
To: linux-kernel; +Cc: Alex Williamson, Joerg Roedel, David Woodhouse
On Wed, Apr 18, 2018 at 12:41:27PM +0800, Peter Xu wrote:
> (PSI stands for: Page Selective Invalidations)
>
> Intel IOMMU has the caching mode to ease emulation of the device.
> When that bit is set, we need to send PSIs even for newly mapped
> pages. However current driver is not fully obey the rule. E.g.,
> iommu_map() API will only do the mapping but it never sent the PSIs
> before. That can be problematic to emulated IOMMU devices since
> they'll never be able to build up the shadow page tables if without
> such information. This patchset tries to fix the problem.
>
> Patch 1 is a tracing enhancement that helped me to triage the problem.
> It might even be useful in the future.
>
> Patch 2 generalized a helper to notify the MAP PSIs.
>
> Patch 3 fixes the real problem by making sure every domain mapping
> will trigger the MAP PSI notifications.
>
> Without the patchset, nested device assignment (assign one device
> firstly to L1 guest, then to L2 guest) won't work for QEMU. After
> applying the patchset, it works.
>
> Please review. Thanks.
I even forgot to CC iommu list. I'll just repost. Sorry for the
troublesome.
--
Peter Xu
^ permalink raw reply [flat|nested] 6+ messages in thread