[PATCH 0/6] IOMMU, vtd and iotlb flush rework

[PATCH 0/6] IOMMU, vtd and iotlb flush rework

[Jean Guyader]

In one of my previous email I detailed a bug I was seeing when passing
through a Intel GPU on a guest that has more that 4G or RAM.

Allen suggested that I go for the Plan B but after a discussion with Tim
we agreed that Plan B was way to disruptive in term of code change.

This patch series implements Plan A.

http://xen.1045712.n5.nabble.com/VTD-Intel-iommu-IOTLB-flush-really-slow-td4952866.html

Jean Guyader (6):
      vtd: Refactor iotlb flush code
      iommu: Introduce iommu_flush and iommu_flush_all.
      add_to_physmap: Move the code for XENMEM_add_to_physmap.
      mm: Add new map space for add_to_physmap, XENMAPSPACE_gmfn_range.
      hvmloader: Change memory relocation loop when overlap with PCI hole.
      Introduce domain flag (dont_flush_iotlb) to avoid unnecessary iotlb flush.

 tools/firmware/hvmloader/pci.c      |   20 +++-
 xen/arch/x86/mm.c                   |  197 ++++++++++++++++++++---------------
 xen/drivers/passthrough/iommu.c     |   26 +++++
 xen/drivers/passthrough/vtd/iommu.c |  100 ++++++++++--------
 xen/include/public/memory.h         |    4 +
 xen/include/xen/iommu.h             |    5 +
 xen/include/xen/sched.h             |    1 +
 7 files changed, 222 insertions(+), 131 deletions(-)

[PATCH 1/6] vtd: Refactor iotlb flush code

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 279 bytes --]


Factorize the iotlb flush code from map_page and unmap_page into
it's own function.

Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
---
 xen/drivers/passthrough/vtd/iommu.c |   86 +++++++++++++++++-----------------
 1 files changed, 43 insertions(+), 43 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vtd-Refactor-iotlb-flush-code.patch --]
[-- Type: text/x-patch; name="0001-vtd-Refactor-iotlb-flush-code.patch", Size: 4122 bytes --]

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 7717ab4..5a5b6be 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -578,16 +578,53 @@ static void iommu_flush_all(void)
     }
 }
 
+static void __intel_iommu_iotlb_flush(struct domain *d, unsigned long gfn,
+        int dma_old_pte_present, unsigned int page_count)
+{
+    struct hvm_iommu *hd = domain_hvm_iommu(d);
+    struct acpi_drhd_unit *drhd;
+    struct iommu *iommu;
+    int flush_dev_iotlb;
+    int iommu_domid;
+
+    /*
+     * No need pcideves_lock here because we have flush
+     * when assign/deassign device
+     */
+    for_each_drhd_unit ( drhd )
+    {
+        iommu = drhd->iommu;
+
+        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
+            continue;
+
+        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+        iommu_domid= domain_iommu_domid(d, iommu);
+        if ( iommu_domid == -1 )
+            continue;
+
+        if ( page_count > 1 || gfn == -1 )
+        {
+            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
+                        0, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+        else
+        {
+            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
+                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
+                        !dma_old_pte_present, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+    }
+}
+
 /* clear one page's page table */
 static void dma_pte_clear_one(struct domain *domain, u64 addr)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(domain);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL;
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
     struct mapped_rmrr *mrmrr;
 
     spin_lock(&hd->mapping_lock);
@@ -613,21 +650,7 @@ static void dma_pte_clear_one(struct domain *domain, u64 addr)
     spin_unlock(&hd->mapping_lock);
     iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
 
-    /* No need pcidevs_lock here since do that on assign/deassign device*/
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-        if ( test_bit(iommu->index, &hd->iommu_bitmap) )
-        {
-            flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-            iommu_domid= domain_iommu_domid(domain, iommu);
-            if ( iommu_domid == -1 )
-                continue;
-            if ( iommu_flush_iotlb_psi(iommu, iommu_domid, addr,
-                                       0, 0, flush_dev_iotlb) )
-                iommu_flush_write_buffer(iommu);
-        }
-    }
+    __intel_iommu_iotlb_flush(domain, addr >> PAGE_SHIFT_4K , 0, 1);
 
     unmap_vtd_domain_page(page);
 
@@ -1677,12 +1700,8 @@ static int intel_iommu_map_page(
     unsigned int flags)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(d);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
 
     /* Do nothing if VT-d shares EPT page table */
     if ( iommu_use_hap_pt(d) )
@@ -1724,26 +1743,7 @@ static int intel_iommu_map_page(
     spin_unlock(&hd->mapping_lock);
     unmap_vtd_domain_page(page);
 
-    /*
-     * No need pcideves_lock here because we have flush
-     * when assign/deassign device
-     */
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-
-        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
-            continue;
-
-        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-        iommu_domid= domain_iommu_domid(d, iommu);
-        if ( iommu_domid == -1 )
-            continue;
-        if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
-                                   (paddr_t)gfn << PAGE_SHIFT_4K, 0,
-                                   !dma_pte_present(old), flush_dev_iotlb) )
-            iommu_flush_write_buffer(iommu);
-    }
+    __intel_iommu_iotlb_flush(d, gfn, dma_pte_present(old), 1);
 
     return 0;
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 2/6] iommu: Introduce iommu_flush and iommu_flush_all.

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 286 bytes --]


Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
---
 xen/drivers/passthrough/iommu.c     |   20 ++++++++++++++++++++
 xen/drivers/passthrough/vtd/iommu.c |   12 ++++++++++++
 xen/include/xen/iommu.h             |    5 +++++
 3 files changed, 37 insertions(+), 0 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-iommu-Introduce-iommu_flush-and-iommu_flush_all.patch --]
[-- Type: text/x-patch; name="0002-iommu-Introduce-iommu_flush-and-iommu_flush_all.patch", Size: 2742 bytes --]

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index cd6174d..9c62861 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -301,6 +301,26 @@ int iommu_unmap_page(struct domain *d, unsigned long gfn)
     return hd->platform_ops->unmap_page(d, gfn);
 }
 
+void iommu_iotlb_flush(struct domain *d, unsigned long gfn, unsigned int page_count)
+{
+    struct hvm_iommu *hd = domain_hvm_iommu(d);
+
+    if ( !iommu_enabled || !hd->platform_ops || !hd->platform_ops->iotlb_flush )
+        return;
+
+    hd->platform_ops->iotlb_flush(d, gfn, page_count);
+}
+
+void iommu_iotlb_flush_all(struct domain *d)
+{
+    struct hvm_iommu *hd = domain_hvm_iommu(d);
+
+    if ( !iommu_enabled || !hd->platform_ops )
+        return;
+
+    hd->platform_ops->iotlb_flush_all(d);
+}
+
 /* caller should hold the pcidevs_lock */
 int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
 {
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 5a5b6be..7ec9541 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -619,6 +619,16 @@ static void __intel_iommu_iotlb_flush(struct domain *d, unsigned long gfn,
     }
 }
 
+static void intel_iommu_iotlb_flush(struct domain *d, unsigned long gfn, unsigned int page_count)
+{
+    __intel_iommu_iotlb_flush(d, gfn, 1, page_count);
+}
+
+static void intel_iommu_iotlb_flush_all(struct domain *d)
+{
+    __intel_iommu_iotlb_flush(d, 0, 0, 0);
+}
+
 /* clear one page's page table */
 static void dma_pte_clear_one(struct domain *domain, u64 addr)
 {
@@ -2329,6 +2339,8 @@ const struct iommu_ops intel_iommu_ops = {
     .resume = vtd_resume,
     .share_p2m = iommu_set_pgd,
     .crash_shutdown = vtd_crash_shutdown,
+    .iotlb_flush = intel_iommu_iotlb_flush,
+    .iotlb_flush_all = intel_iommu_iotlb_flush_all,
 };
 
 /*
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 837e60d..a1034df 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -139,6 +139,8 @@ struct iommu_ops {
     void (*resume)(void);
     void (*share_p2m)(struct domain *d);
     void (*crash_shutdown)(void);
+    void (*iotlb_flush)(struct domain *d, unsigned long gfn, unsigned int page_count);
+    void (*iotlb_flush_all)(struct domain *d);
 };
 
 void iommu_update_ire_from_apic(unsigned int apic, unsigned int reg, unsigned int value);
@@ -155,4 +157,7 @@ void iommu_share_p2m_table(struct domain *d);
 
 int iommu_do_domctl(struct xen_domctl *, XEN_GUEST_HANDLE(xen_domctl_t));
 
+void iommu_iotlb_flush(struct domain *d, unsigned long gfn, unsigned int page_count);
+void iommu_iotlb_flush_all(struct domain *d);
+
 #endif /* _IOMMU_H_ */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 3/6] add_to_physmap: Move the code for XENMEM_add_to_physmap.

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 293 bytes --]


Move the code for the XENMEM_add_to_physmap case into it's own
function (xenmem_add_to_physmap).

Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
---
 xen/arch/x86/mm.c |  188 ++++++++++++++++++++++++++++-------------------------
 1 files changed, 99 insertions(+), 89 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-add_to_physmap-Move-the-code-for-XENMEM_add_to_physm.patch --]
[-- Type: text/x-patch; name="0003-add_to_physmap-Move-the-code-for-XENMEM_add_to_physm.patch", Size: 7065 bytes --]

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index acc1f34..f75011e 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4592,119 +4592,129 @@ static int handle_iomem_range(unsigned long s, unsigned long e, void *p)
     return 0;
 }
 
-long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+static int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap xatp)
 {
     struct page_info *page = NULL;
+    unsigned long prev_mfn, mfn = 0, gpfn;
     int rc;
 
-    switch ( op )
-    {
-    case XENMEM_add_to_physmap:
+    switch ( xatp.space )
     {
-        struct xen_add_to_physmap xatp;
-        unsigned long prev_mfn, mfn = 0, gpfn;
-        struct domain *d;
-
-        if ( copy_from_guest(&xatp, arg, 1) )
-            return -EFAULT;
+    case XENMAPSPACE_shared_info:
+        if ( xatp.idx == 0 )
+            mfn = virt_to_mfn(d->shared_info);
+        break;
+    case XENMAPSPACE_grant_table:
+        spin_lock(&d->grant_table->lock);
 
-        rc = rcu_lock_target_domain_by_id(xatp.domid, &d);
-        if ( rc != 0 )
-            return rc;
+        if ( d->grant_table->gt_version == 0 )
+            d->grant_table->gt_version = 1;
 
-        if ( xsm_add_to_physmap(current->domain, d) )
+        if ( d->grant_table->gt_version == 2 &&
+             (xatp.idx & XENMAPIDX_grant_table_status) )
         {
-            rcu_unlock_domain(d);
-            return -EPERM;
+            xatp.idx &= ~XENMAPIDX_grant_table_status;
+            if ( xatp.idx < nr_status_frames(d->grant_table) )
+                mfn = virt_to_mfn(d->grant_table->status[xatp.idx]);
+        }
+        else
+        {
+            if ( (xatp.idx >= nr_grant_frames(d->grant_table)) &&
+                 (xatp.idx < max_nr_grant_frames) )
+                gnttab_grow_table(d, xatp.idx + 1);
+
+            if ( xatp.idx < nr_grant_frames(d->grant_table) )
+                mfn = virt_to_mfn(d->grant_table->shared_raw[xatp.idx]);
         }
 
-        switch ( xatp.space )
+        spin_unlock(&d->grant_table->lock);
+        break;
+    case XENMAPSPACE_gmfn:
+    {
+        p2m_type_t p2mt;
+
+        xatp.idx = mfn_x(gfn_to_mfn_unshare(d, xatp.idx, &p2mt));
+        /* If the page is still shared, exit early */
+        if ( p2m_is_shared(p2mt) )
         {
-        case XENMAPSPACE_shared_info:
-            if ( xatp.idx == 0 )
-                mfn = virt_to_mfn(d->shared_info);
+            rcu_unlock_domain(d);
+            return -ENOMEM;
+        }
+        if ( !get_page_from_pagenr(xatp.idx, d) )
             break;
-        case XENMAPSPACE_grant_table:
-            spin_lock(&d->grant_table->lock);
+        mfn = xatp.idx;
+        page = mfn_to_page(mfn);
+        break;
+    }
+    default:
+        break;
+    }
 
-            if ( d->grant_table->gt_version == 0 )
-                d->grant_table->gt_version = 1;
+    if ( !paging_mode_translate(d) || (mfn == 0) )
+    {
+        if ( page )
+            put_page(page);
+        rcu_unlock_domain(d);
+        return -EINVAL;
+    }
 
-            if ( d->grant_table->gt_version == 2 &&
-                 (xatp.idx & XENMAPIDX_grant_table_status) )
-            {
-                xatp.idx &= ~XENMAPIDX_grant_table_status;
-                if ( xatp.idx < nr_status_frames(d->grant_table) )
-                    mfn = virt_to_mfn(d->grant_table->status[xatp.idx]);
-            }
-            else
-            {
-                if ( (xatp.idx >= nr_grant_frames(d->grant_table)) &&
-                     (xatp.idx < max_nr_grant_frames) )
-                    gnttab_grow_table(d, xatp.idx + 1);
+    domain_lock(d);
 
-                if ( xatp.idx < nr_grant_frames(d->grant_table) )
-                    mfn = virt_to_mfn(d->grant_table->shared_raw[xatp.idx]);
-            }
+    if ( page )
+        put_page(page);
 
-            spin_unlock(&d->grant_table->lock);
-            break;
-        case XENMAPSPACE_gmfn:
-        {
-            p2m_type_t p2mt;
+    /* Remove previously mapped page if it was present. */
+    prev_mfn = gmfn_to_mfn(d, xatp.gpfn);
+    if ( mfn_valid(prev_mfn) )
+    {
+        if ( is_xen_heap_mfn(prev_mfn) )
+            /* Xen heap frames are simply unhooked from this phys slot. */
+            guest_physmap_remove_page(d, xatp.gpfn, prev_mfn, 0);
+        else
+            /* Normal domain memory is freed, to avoid leaking memory. */
+            guest_remove_page(d, xatp.gpfn);
+    }
 
-            xatp.idx = mfn_x(gfn_to_mfn_unshare(d, xatp.idx, &p2mt));
-            /* If the page is still shared, exit early */
-            if ( p2m_is_shared(p2mt) )
-            {
-                rcu_unlock_domain(d);
-                return -ENOMEM;
-            }
-            if ( !get_page_from_pagenr(xatp.idx, d) )
-                break;
-            mfn = xatp.idx;
-            page = mfn_to_page(mfn);
-            break;
-        }
-        default:
-            break;
-        }
+    /* Unmap from old location, if any. */
+    gpfn = get_gpfn_from_mfn(mfn);
+    ASSERT( gpfn != SHARED_M2P_ENTRY );
+    if ( gpfn != INVALID_M2P_ENTRY )
+        guest_physmap_remove_page(d, gpfn, mfn, 0);
 
-        if ( !paging_mode_translate(d) || (mfn == 0) )
-        {
-            if ( page )
-                put_page(page);
-            rcu_unlock_domain(d);
-            return -EINVAL;
-        }
+    /* Map at new location. */
+    rc = guest_physmap_add_page(d, xatp.gpfn, mfn, 0);
 
-        domain_lock(d);
+    domain_unlock(d);
 
-        if ( page )
-            put_page(page);
+    return rc;
+}
 
-        /* Remove previously mapped page if it was present. */
-        prev_mfn = gmfn_to_mfn(d, xatp.gpfn);
-        if ( mfn_valid(prev_mfn) )
-        {
-            if ( is_xen_heap_mfn(prev_mfn) )
-                /* Xen heap frames are simply unhooked from this phys slot. */
-                guest_physmap_remove_page(d, xatp.gpfn, prev_mfn, 0);
-            else
-                /* Normal domain memory is freed, to avoid leaking memory. */
-                guest_remove_page(d, xatp.gpfn);
-        }
 
-        /* Unmap from old location, if any. */
-        gpfn = get_gpfn_from_mfn(mfn);
-        ASSERT( gpfn != SHARED_M2P_ENTRY );
-        if ( gpfn != INVALID_M2P_ENTRY )
-            guest_physmap_remove_page(d, gpfn, mfn, 0);
+long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+{
+    int rc;
+
+    switch ( op )
+    {
+    case XENMEM_add_to_physmap:
+    {
+        struct xen_add_to_physmap xatp;
+        struct domain *d;
 
-        /* Map at new location. */
-        rc = guest_physmap_add_page(d, xatp.gpfn, mfn, 0);
+        if ( copy_from_guest(&xatp, arg, 1) )
+            return -EFAULT;
+
+        rc = rcu_lock_target_domain_by_id(xatp.domid, &d);
+        if ( rc != 0 )
+            return rc;
+
+        if ( xsm_add_to_physmap(current->domain, d) )
+        {
+            rcu_unlock_domain(d);
+            return -EPERM;
+        }
 
-        domain_unlock(d);
+        xenmem_add_to_physmap(d, xatp);
 
         rcu_unlock_domain(d);
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 4/6] mm: Add new map space for add_to_physmap, XENMAPSPACE_gmfn_range.

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]


XENMAPSPACE_gmfn_range is like XENMAPSPACE_gmfn but with a size which
is the number of pages on which xen will iterate.

Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
---
 xen/arch/x86/mm.c           |   15 ++++++++++++++-
 xen/include/public/memory.h |    4 ++++
 2 files changed, 18 insertions(+), 1 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0004-mm-Add-new-map-space-for-add_to_physmap-XENMAPSPACE_.patch --]
[-- Type: text/x-patch; name="0004-mm-Add-new-map-space-for-add_to_physmap-XENMAPSPACE_.patch", Size: 1956 bytes --]

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index f75011e..77ae77f 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4629,6 +4629,7 @@ static int xenmem_add_to_physmap(struct domain *d, struct xen_add_to_physmap xat
 
         spin_unlock(&d->grant_table->lock);
         break;
+    case XENMAPSPACE_gmfn_range:
     case XENMAPSPACE_gmfn:
     {
         p2m_type_t p2mt;
@@ -4700,6 +4701,7 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
     {
         struct xen_add_to_physmap xatp;
         struct domain *d;
+        unsigned int size, i;
 
         if ( copy_from_guest(&xatp, arg, 1) )
             return -EFAULT;
@@ -4714,7 +4716,18 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
             return -EPERM;
         }
 
-        xenmem_add_to_physmap(d, xatp);
+        size = 1;
+        if ( xatp.space == XENMAPSPACE_gmfn_range )
+        {
+            size = xatp.size;
+        }
+
+        for ( i = 0; i < size; i++ )
+        {
+            xenmem_add_to_physmap(d, xatp);
+            xatp.idx++;
+            xatp.gpfn++;
+        }
 
         rcu_unlock_domain(d);
 
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 08355e3..860b8c0 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -212,6 +212,7 @@ struct xen_add_to_physmap {
 #define XENMAPSPACE_shared_info 0 /* shared info page */
 #define XENMAPSPACE_grant_table 1 /* grant table page */
 #define XENMAPSPACE_gmfn        2 /* GMFN */
+#define XENMAPSPACE_gmfn_range  3 /* GMFN range */
     unsigned int space;
 
 #define XENMAPIDX_grant_table_status 0x80000000
@@ -221,6 +222,9 @@ struct xen_add_to_physmap {
 
     /* GPFN where the source mapping page should appear. */
     xen_pfn_t     gpfn;
+
+    /* Number of page, used for GMFN range */
+    unsigned int size;
 };
 typedef struct xen_add_to_physmap xen_add_to_physmap_t;
 DEFINE_XEN_GUEST_HANDLE(xen_add_to_physmap_t);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 5/6] hvmloader: Change memory relocation loop when overlap with PCI hole.

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 715 bytes --]


Change the way we relocate the memory page if they overlap with pci hole.
Use new map space (XENMAPSPACE_gmfn_range) to move the loop into xen.

This code usually get triggered when a device is pass through to a guest
and the PCI hole has to be extended to have enough room to map the device BARs.
The PCI hole will starts lower and it might overlap with some RAM that has been
alocated for the guest. That usually happen if the guest has more than 4G of RAM.
We have to relocate those pages in high mem otherwise they won't be accessible.

Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
---
 tools/firmware/hvmloader/pci.c |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0005-hvmloader-Change-memory-relocation-loop-when-overlap.patch --]
[-- Type: text/x-patch; name="0005-hvmloader-Change-memory-relocation-loop-when-overlap.patch", Size: 1576 bytes --]

diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
index 29ec011..fb451ef 100644
--- a/tools/firmware/hvmloader/pci.c
+++ b/tools/firmware/hvmloader/pci.c
@@ -185,17 +185,25 @@ void pci_setup(void)
             ((pci_mem_start << 1) != 0) )
         pci_mem_start <<= 1;
 
-    while ( (pci_mem_start >> PAGE_SHIFT) < hvm_info->low_mem_pgend )
+    if ((pci_mem_start >> PAGE_SHIFT) < hvm_info->low_mem_pgend)
     {
         struct xen_add_to_physmap xatp;
-        if ( hvm_info->high_mem_pgend == 0 )
-            hvm_info->high_mem_pgend = 1ull << (32 - PAGE_SHIFT);
+        unsigned int size = hvm_info->low_mem_pgend - (pci_mem_start >> PAGE_SHIFT);
         xatp.domid = DOMID_SELF;
-        xatp.space = XENMAPSPACE_gmfn;
-        xatp.idx   = --hvm_info->low_mem_pgend;
-        xatp.gpfn  = hvm_info->high_mem_pgend++;
+        xatp.space = XENMAPSPACE_gmfn_range;
+        xatp.size = size;
+        xatp.idx = (pci_mem_start >> PAGE_SHIFT);
+        xatp.gpfn = hvm_info->high_mem_pgend;
+
+        printf("Relocate page for pci hole:\n");
         if ( hypercall_memory_op(XENMEM_add_to_physmap, &xatp) != 0 )
             BUG();
+        printf("  - low_mem_pgend: %x -> %lx\n"
+                "  - high_mem_pgend: %x -> %x\n",
+                hvm_info->low_mem_pgend, (pci_mem_start >> PAGE_SHIFT),
+                hvm_info->high_mem_pgend, hvm_info->high_mem_pgend + size);
+        hvm_info->low_mem_pgend = pci_mem_start >> PAGE_SHIFT;
+        hvm_info->high_mem_pgend += size;
     }
 
     mem_resource.base = pci_mem_start;

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 6/6] Introduce domain flag (dont_flush_iotlb) to avoid unnecessary iotlb flush.

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 459 bytes --]


Add a domain level flag that will be check by the iommu low level code
to skip iotlb flush. iommu_iotlb_flush has to be called explicitly.

Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
---
 xen/arch/x86/mm.c                   |   12 +++++++++++-
 xen/drivers/passthrough/iommu.c     |    6 ++++++
 xen/drivers/passthrough/vtd/iommu.c |    6 ++++--
 xen/include/xen/sched.h             |    1 +
 4 files changed, 22 insertions(+), 3 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0006-Introduce-domain-flag-dont_flush_iotlb-to-avoid-unne.patch --]
[-- Type: text/x-patch; name="0006-Introduce-domain-flag-dont_flush_iotlb-to-avoid-unne.patch", Size: 3793 bytes --]

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 77ae77f..4e23875 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4699,7 +4699,7 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
     {
     case XENMEM_add_to_physmap:
     {
-        struct xen_add_to_physmap xatp;
+        struct xen_add_to_physmap xatp, start_xatp;
         struct domain *d;
         unsigned int size, i;
 
@@ -4720,8 +4720,11 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
         if ( xatp.space == XENMAPSPACE_gmfn_range )
         {
             size = xatp.size;
+            if ( need_iommu(d) )
+                d->dont_flush_iotlb = 1;
         }
 
+        start_xatp = xatp;
         for ( i = 0; i < size; i++ )
         {
             xenmem_add_to_physmap(d, xatp);
@@ -4729,6 +4732,13 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
             xatp.gpfn++;
         }
 
+        if ( xatp.space == XENMAPSPACE_gmfn_range && need_iommu(d) )
+        {
+            d->dont_flush_iotlb = 0;
+            iommu_iotlb_flush(d, start_xatp.idx, size);
+            iommu_iotlb_flush(d, start_xatp.gpfn, size);
+        }
+
         rcu_unlock_domain(d);
 
         return rc;
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 9c62861..7f87aed 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -113,10 +113,13 @@ void __init iommu_dom0_init(struct domain *d)
         return;
 
     d->need_iommu = !!iommu_dom0_strict;
+    d->dont_flush_iotlb = 0;
     if ( need_iommu(d) )
     {
         struct page_info *page;
         unsigned int i = 0;
+
+        d->dont_flush_iotlb = 1;
         page_list_for_each ( page, &d->page_list )
         {
             unsigned long mfn = page_to_mfn(page);
@@ -129,6 +132,8 @@ void __init iommu_dom0_init(struct domain *d)
             if ( !(i++ & 0xfffff) )
                 process_pending_softirqs();
         }
+        d->dont_flush_iotlb = 0;
+        iommu_iotlb_flush_all(d);
     }
 
     return hd->platform_ops->dom0_init(d);
@@ -210,6 +215,7 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
     if ( has_arch_pdevs(d) && !need_iommu(d) )
     {
         d->need_iommu = 1;
+        d->dont_flush_iotlb = 0;
         if ( !iommu_use_hap_pt(d) )
             rc = iommu_populate_page_table(d);
         goto done;
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 7ec9541..336232e 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -660,7 +660,8 @@ static void dma_pte_clear_one(struct domain *domain, u64 addr)
     spin_unlock(&hd->mapping_lock);
     iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
 
-    __intel_iommu_iotlb_flush(domain, addr >> PAGE_SHIFT_4K , 0, 1);
+    if ( !domain->dont_flush_iotlb )
+        __intel_iommu_iotlb_flush(domain, addr >> PAGE_SHIFT_4K , 0, 1);
 
     unmap_vtd_domain_page(page);
 
@@ -1753,7 +1754,8 @@ static int intel_iommu_map_page(
     spin_unlock(&hd->mapping_lock);
     unmap_vtd_domain_page(page);
 
-    __intel_iommu_iotlb_flush(d, gfn, dma_pte_present(old), 1);
+    if ( !d->dont_flush_iotlb )
+        __intel_iommu_iotlb_flush(d, gfn, dma_pte_present(old), 1);
 
     return 0;
 }
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 3ba5495..922f068 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -244,6 +244,7 @@ struct domain
     bool_t           is_hvm;
     /* Does this guest need iommu mappings? */
     bool_t           need_iommu;
+    bool_t           dont_flush_iotlb;
     /* Is this guest fully privileged (aka dom0)? */
     bool_t           is_privileged;
     /* Which guest this guest has privileges on */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 6/6] Introduce domain flag (dont_flush_iotlb) to avoid unnecessary iotlb flush.

[Jan Beulich]

>>> On 04.11.11 at 11:38, Jean Guyader <jean.guyader@eu.citrix.com> wrote:

> Add a domain level flag that will be check by the iommu low level code
> to skip iotlb flush. iommu_iotlb_flush has to be called explicitly.
> 
> Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
> ---
>  xen/arch/x86/mm.c                   |   12 +++++++++++-
>  xen/drivers/passthrough/iommu.c     |    6 ++++++
>  xen/drivers/passthrough/vtd/iommu.c |    6 ++++--
>  xen/include/xen/sched.h             |    1 +
>  4 files changed, 22 insertions(+), 3 deletions(-)

Neither the patch nor its description make clear what lock is used
to protect this flag from having an effect on other but the current
vCPU of the guest in question - I doubt that all other relevant code
paths indeed use domain_lock().

Jan

Re: [PATCH 5/6] hvmloader: Change memory relocation loop when overlap with PCI hole.

[Keir Fraser]

On 04/11/2011 10:38, "Jean Guyader" <jean.guyader@eu.citrix.com> wrote:

> 
> Change the way we relocate the memory page if they overlap with pci hole.
> Use new map space (XENMAPSPACE_gmfn_range) to move the loop into xen.
> 
> This code usually get triggered when a device is pass through to a guest
> and the PCI hole has to be extended to have enough room to map the device
> BARs.
> The PCI hole will starts lower and it might overlap with some RAM that has
> been
> alocated for the guest. That usually happen if the guest has more than 4G of
> RAM.
> We have to relocate those pages in high mem otherwise they won't be
> accessible.

If the size field in the add_to_physmap structure is reduced to 16 bits, the
new if() stmt will probably have to become a while() again.

The new printf's are just excess verbiage. We dump the memory map as we exit
hvmloader anyway.

 -- Keir

> Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
> ---
>  tools/firmware/hvmloader/pci.c |   20 ++++++++++++++------
>  1 files changed, 14 insertions(+), 6 deletions(-)
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 4/6] mm: Add new map space for add_to_physmap, XENMAPSPACE_gmfn_range.

[Jan Beulich]

>>> On 04.11.11 at 11:38, Jean Guyader <jean.guyader@eu.citrix.com> wrote:
> XENMAPSPACE_gmfn_range is like XENMAPSPACE_gmfn but with a size which
> is the number of pages on which xen will iterate.
> 
> Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
> ---
>  xen/arch/x86/mm.c           |   15 ++++++++++++++-
>  xen/include/public/memory.h |    4 ++++
>  2 files changed, 18 insertions(+), 1 deletions(-)

You can't add a member to a structure that's part of the public
interface. You'll need to create a new structure.

Jan

Re: [PATCH 4/6] mm: Add new map space for add_to_physmap, XENMAPSPACE_gmfn_range.

[Keir Fraser]

On 04/11/2011 10:38, "Jean Guyader" <jean.guyader@eu.citrix.com> wrote:

> 
> XENMAPSPACE_gmfn_range is like XENMAPSPACE_gmfn but with a size which
> is the number of pages on which xen will iterate.

You can't really extend the size of an existing ABI structure, and always
copy that extended size. Older guests won't know to guarantee that the
extended space is accessible. I suggest you make your new field a uint16_t,
placed directly after the domid field. Then you are making use of existing
pad space. 64k pages = 256MB at a time should be plenty of amortisation.

And, even with the reduced field width, I could imagine 64k remappings
taking a good while. The remapping loop should regularly (even every
iteration) check hypercall_preempt_check(), then
hypercall_create_continuation() and exit if the hypercall is being requested
to voluntarily yield.

 -- Keir

> Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
> ---
>  xen/arch/x86/mm.c           |   15 ++++++++++++++-
>  xen/include/public/memory.h |    4 ++++
>  2 files changed, 18 insertions(+), 1 deletions(-)
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

IOMMU, vtd and iotlb flush rework (v2)

[Jean Guyader]

In one of my previous email I detailed a bug I was seeing when passing
through a Intel GPU on a guest that has more that 4G or RAM.

Allen suggested that I go for the Plan B but after a discussion with Tim
we agreed that Plan B was way to disruptive in term of code change.

This patch series implements Plan A.

http://xen.1045712.n5.nabble.com/VTD-Intel-iommu-IOTLB-flush-really-slow-td4952866.html

Changes between v1 and v2:
        - Move size in struct xen_add_to_physmap in padding between .domid and .space.
        - Store iommu_dont_flush per cpu
        - Change the code in hvmloader to relocate by batch of 64K, .size is now 16 bits.

Jean Guyader (6):
      vtd: Refactor iotlb flush code
      iommu: Introduce iommu_flush and iommu_flush_all.
      add_to_physmap: Move the code for XENMEM_add_to_physmap.
      mm: New XENMEM, XENMEM_add_to_physmap_gmfn_range
      hvmloader: Change memory relocation loop when overlap with PCI hole.
      Introduce per cpu flag (iommu_dont_flush_iotlb) to avoid unnecessary iotlb flush
 
 tools/firmware/hvmloader/pci.c      |   20 +++-
 xen/arch/x86/mm.c                   |  203 +++++++++++++++++++++--------------
 xen/drivers/passthrough/iommu.c     |   25 +++++
 xen/drivers/passthrough/vtd/iommu.c |  100 ++++++++++--------
 xen/include/public/memory.h         |    5 +-
 xen/include/xen/iommu.h             |    7 ++
 6 files changed, 230 insertions(+), 130 deletions(-)

Jean

[PATCH 1/6] vtd: Refactor iotlb flush code

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 279 bytes --]


Factorize the iotlb flush code from map_page and unmap_page into
it's own function.

Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
---
 xen/drivers/passthrough/vtd/iommu.c |   86 +++++++++++++++++-----------------
 1 files changed, 43 insertions(+), 43 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vtd-Refactor-iotlb-flush-code.patch --]
[-- Type: text/x-patch; name="0001-vtd-Refactor-iotlb-flush-code.patch", Size: 4122 bytes --]

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 7717ab4..5a5b6be 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -578,16 +578,53 @@ static void iommu_flush_all(void)
     }
 }
 
+static void __intel_iommu_iotlb_flush(struct domain *d, unsigned long gfn,
+        int dma_old_pte_present, unsigned int page_count)
+{
+    struct hvm_iommu *hd = domain_hvm_iommu(d);
+    struct acpi_drhd_unit *drhd;
+    struct iommu *iommu;
+    int flush_dev_iotlb;
+    int iommu_domid;
+
+    /*
+     * No need pcideves_lock here because we have flush
+     * when assign/deassign device
+     */
+    for_each_drhd_unit ( drhd )
+    {
+        iommu = drhd->iommu;
+
+        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
+            continue;
+
+        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+        iommu_domid= domain_iommu_domid(d, iommu);
+        if ( iommu_domid == -1 )
+            continue;
+
+        if ( page_count > 1 || gfn == -1 )
+        {
+            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
+                        0, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+        else
+        {
+            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
+                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
+                        !dma_old_pte_present, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+    }
+}
+
 /* clear one page's page table */
 static void dma_pte_clear_one(struct domain *domain, u64 addr)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(domain);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL;
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
     struct mapped_rmrr *mrmrr;
 
     spin_lock(&hd->mapping_lock);
@@ -613,21 +650,7 @@ static void dma_pte_clear_one(struct domain *domain, u64 addr)
     spin_unlock(&hd->mapping_lock);
     iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
 
-    /* No need pcidevs_lock here since do that on assign/deassign device*/
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-        if ( test_bit(iommu->index, &hd->iommu_bitmap) )
-        {
-            flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-            iommu_domid= domain_iommu_domid(domain, iommu);
-            if ( iommu_domid == -1 )
-                continue;
-            if ( iommu_flush_iotlb_psi(iommu, iommu_domid, addr,
-                                       0, 0, flush_dev_iotlb) )
-                iommu_flush_write_buffer(iommu);
-        }
-    }
+    __intel_iommu_iotlb_flush(domain, addr >> PAGE_SHIFT_4K , 0, 1);
 
     unmap_vtd_domain_page(page);
 
@@ -1677,12 +1700,8 @@ static int intel_iommu_map_page(
     unsigned int flags)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(d);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
 
     /* Do nothing if VT-d shares EPT page table */
     if ( iommu_use_hap_pt(d) )
@@ -1724,26 +1743,7 @@ static int intel_iommu_map_page(
     spin_unlock(&hd->mapping_lock);
     unmap_vtd_domain_page(page);
 
-    /*
-     * No need pcideves_lock here because we have flush
-     * when assign/deassign device
-     */
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-
-        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
-            continue;
-
-        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-        iommu_domid= domain_iommu_domid(d, iommu);
-        if ( iommu_domid == -1 )
-            continue;
-        if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
-                                   (paddr_t)gfn << PAGE_SHIFT_4K, 0,
-                                   !dma_pte_present(old), flush_dev_iotlb) )
-            iommu_flush_write_buffer(iommu);
-    }
+    __intel_iommu_iotlb_flush(d, gfn, dma_pte_present(old), 1);
 
     return 0;
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

IOMMU, vtd and iotlb flush rework (v3)

[Jean Guyader]

In one of my previous email I detailed a bug I was seeing when passing
through a Intel GPU on a guest that has more that 4G or RAM.

Allen suggested that I go for the Plan B but after a discussion with Tim
we agreed that Plan B was way to disruptive in term of code change.

This patch series implements Plan A.

http://xen.1045712.n5.nabble.com/VTD-Intel-iommu-IOTLB-flush-really-slow-td4952866.html

Changes between v2 and v3:
        - Check for the presence iotlb_flush_all callback before calling it.

Changes between v1 and v2:
       - Move size in struct xen_add_to_physmap in padding between .domid and .space.
       - Store iommu_dont_flush per cpu
       - Change the code in hvmloader to relocate by batch of 64K, .size is now 16 bits.

Jean Guyader (6):
      vtd: Refactor iotlb flush code
      iommu: Introduce iommu_flush and iommu_flush_all.
      add_to_physmap: Move the code for XENMEM_add_to_physmap.
      mm: New XENMEM, XENMEM_add_to_physmap_gmfn_range
      hvmloader: Change memory relocation loop when overlap with PCI hole.
      Introduce per cpu flag (iommu_dont_flush_iotlb) to avoid unnecessary iotlb flush

 tools/firmware/hvmloader/pci.c      |   20 +++-
 xen/arch/x86/mm.c                   |  203 +++++++++++++++++++++--------------
 xen/drivers/passthrough/iommu.c     |   25 +++++
 xen/drivers/passthrough/vtd/iommu.c |  100 ++++++++++--------
 xen/include/public/memory.h         |    4 +
 xen/include/xen/iommu.h             |    7 ++
 6 files changed, 230 insertions(+), 129 deletions(-)

[PATCH 1/6] vtd: Refactor iotlb flush code

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 279 bytes --]


Factorize the iotlb flush code from map_page and unmap_page into
it's own function.

Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
---
 xen/drivers/passthrough/vtd/iommu.c |   86 +++++++++++++++++-----------------
 1 files changed, 43 insertions(+), 43 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vtd-Refactor-iotlb-flush-code.patch --]
[-- Type: text/x-patch; name="0001-vtd-Refactor-iotlb-flush-code.patch", Size: 4122 bytes --]

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 7717ab4..5a5b6be 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -578,16 +578,53 @@ static void iommu_flush_all(void)
     }
 }
 
+static void __intel_iommu_iotlb_flush(struct domain *d, unsigned long gfn,
+        int dma_old_pte_present, unsigned int page_count)
+{
+    struct hvm_iommu *hd = domain_hvm_iommu(d);
+    struct acpi_drhd_unit *drhd;
+    struct iommu *iommu;
+    int flush_dev_iotlb;
+    int iommu_domid;
+
+    /*
+     * No need pcideves_lock here because we have flush
+     * when assign/deassign device
+     */
+    for_each_drhd_unit ( drhd )
+    {
+        iommu = drhd->iommu;
+
+        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
+            continue;
+
+        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+        iommu_domid= domain_iommu_domid(d, iommu);
+        if ( iommu_domid == -1 )
+            continue;
+
+        if ( page_count > 1 || gfn == -1 )
+        {
+            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
+                        0, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+        else
+        {
+            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
+                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
+                        !dma_old_pte_present, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+    }
+}
+
 /* clear one page's page table */
 static void dma_pte_clear_one(struct domain *domain, u64 addr)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(domain);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL;
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
     struct mapped_rmrr *mrmrr;
 
     spin_lock(&hd->mapping_lock);
@@ -613,21 +650,7 @@ static void dma_pte_clear_one(struct domain *domain, u64 addr)
     spin_unlock(&hd->mapping_lock);
     iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
 
-    /* No need pcidevs_lock here since do that on assign/deassign device*/
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-        if ( test_bit(iommu->index, &hd->iommu_bitmap) )
-        {
-            flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-            iommu_domid= domain_iommu_domid(domain, iommu);
-            if ( iommu_domid == -1 )
-                continue;
-            if ( iommu_flush_iotlb_psi(iommu, iommu_domid, addr,
-                                       0, 0, flush_dev_iotlb) )
-                iommu_flush_write_buffer(iommu);
-        }
-    }
+    __intel_iommu_iotlb_flush(domain, addr >> PAGE_SHIFT_4K , 0, 1);
 
     unmap_vtd_domain_page(page);
 
@@ -1677,12 +1700,8 @@ static int intel_iommu_map_page(
     unsigned int flags)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(d);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
 
     /* Do nothing if VT-d shares EPT page table */
     if ( iommu_use_hap_pt(d) )
@@ -1724,26 +1743,7 @@ static int intel_iommu_map_page(
     spin_unlock(&hd->mapping_lock);
     unmap_vtd_domain_page(page);
 
-    /*
-     * No need pcideves_lock here because we have flush
-     * when assign/deassign device
-     */
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-
-        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
-            continue;
-
-        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-        iommu_domid= domain_iommu_domid(d, iommu);
-        if ( iommu_domid == -1 )
-            continue;
-        if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
-                                   (paddr_t)gfn << PAGE_SHIFT_4K, 0,
-                                   !dma_pte_present(old), flush_dev_iotlb) )
-            iommu_flush_write_buffer(iommu);
-    }
+    __intel_iommu_iotlb_flush(d, gfn, dma_pte_present(old), 1);
 
     return 0;
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: [PATCH 1/6] vtd: Refactor iotlb flush code

[Kay, Allen M]

[-- Attachment #1: Type: text/plain, Size: 1215 bytes --]

Jean,

The original code does not call iommu_flush_iotlb_dsi().  What is the reason the refractored code need to use domain selective invalidation?

Allen
-----

+        if ( page_count > 1 || gfn == -1 )
+        {
+            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
+                        0, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+        else
+        {
+            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
+                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
+                        !dma_old_pte_present, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);

-----Original Message-----
From: Jean Guyader [mailto:jean.guyader@eu.citrix.com] 
Sent: Monday, November 07, 2011 10:25 AM
To: xen-devel@lists.xensource.com
Cc: tim@xen.org; Kay, Allen M; Jean Guyader
Subject: [PATCH 1/6] vtd: Refactor iotlb flush code


Factorize the iotlb flush code from map_page and unmap_page into it's own function.

Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
---
 xen/drivers/passthrough/vtd/iommu.c |   86 +++++++++++++++++-----------------
 1 files changed, 43 insertions(+), 43 deletions(-)


[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/6] vtd: Refactor iotlb flush code

[Jean Guyader]

Allen,

You are probably talking about __intel_iommu_iotlb_flush.
This function takes a range of address to flush. I haven't
found a function in the vtd code to invalidate a range on
address without doing a loop of flush_iotlb_psi, so I thought
that the most efficient and quick way to flush a range would
be to use a domain selective invalidation.

Jean

On 08/11 03:10, Kay, Allen M wrote:
> Jean,
> 
> The original code does not call iommu_flush_iotlb_dsi().  What is the reason the refractored code need to use domain selective invalidation?
> 
> Allen
> -----
> 
> +        if ( page_count > 1 || gfn == -1 )
> +        {
> +            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
> +                        0, flush_dev_iotlb) )
> +                iommu_flush_write_buffer(iommu);
> +        }
> +        else
> +        {
> +            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
> +                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
> +                        !dma_old_pte_present, flush_dev_iotlb) )
> +                iommu_flush_write_buffer(iommu);
> 
> -----Original Message-----
> From: Jean Guyader [mailto:jean.guyader@eu.citrix.com] 
> Sent: Monday, November 07, 2011 10:25 AM
> To: xen-devel@lists.xensource.com
> Cc: tim@xen.org; Kay, Allen M; Jean Guyader
> Subject: [PATCH 1/6] vtd: Refactor iotlb flush code
> 
> 
> Factorize the iotlb flush code from map_page and unmap_page into it's own function.
> 
> Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
> ---
>  xen/drivers/passthrough/vtd/iommu.c |   86 +++++++++++++++++-----------------
>  1 files changed, 43 insertions(+), 43 deletions(-)
> 

RE: [PATCH 1/6] vtd: Refactor iotlb flush code

[Kay, Allen M]

Jean,

Page 122 of VT-d spec indicates you can invalidate multiple pages by setting the Address Mask (AM) field.  Will this achieve what you wanted to do?

http://download.intel.com/technology/computing/vptech/Intel(r)_VT_for_Direct_IO.pdf

Allen

-----Original Message-----
From: Jean Guyader [mailto:jean.guyader@eu.citrix.com] 
Sent: Monday, November 07, 2011 11:42 PM
To: Kay, Allen M
Cc: Jean Guyader; xen-devel@lists.xensource.com; Tim (Xen.org)
Subject: Re: [PATCH 1/6] vtd: Refactor iotlb flush code


Allen,

You are probably talking about __intel_iommu_iotlb_flush.
This function takes a range of address to flush. I haven't found a function in the vtd code to invalidate a range on address without doing a loop of flush_iotlb_psi, so I thought that the most efficient and quick way to flush a range would be to use a domain selective invalidation.

Jean

On 08/11 03:10, Kay, Allen M wrote:
> Jean,
> 
> The original code does not call iommu_flush_iotlb_dsi().  What is the reason the refractored code need to use domain selective invalidation?
> 
> Allen
> -----
> 
> +        if ( page_count > 1 || gfn == -1 )
> +        {
> +            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
> +                        0, flush_dev_iotlb) )
> +                iommu_flush_write_buffer(iommu);
> +        }
> +        else
> +        {
> +            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
> +                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
> +                        !dma_old_pte_present, flush_dev_iotlb) )
> +                iommu_flush_write_buffer(iommu);
> 
> -----Original Message-----
> From: Jean Guyader [mailto:jean.guyader@eu.citrix.com]
> Sent: Monday, November 07, 2011 10:25 AM
> To: xen-devel@lists.xensource.com
> Cc: tim@xen.org; Kay, Allen M; Jean Guyader
> Subject: [PATCH 1/6] vtd: Refactor iotlb flush code
> 
> 
> Factorize the iotlb flush code from map_page and unmap_page into it's own function.
> 
> Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
> ---
>  xen/drivers/passthrough/vtd/iommu.c |   86 +++++++++++++++++-----------------
>  1 files changed, 43 insertions(+), 43 deletions(-)
> 

Re: [PATCH 1/6] vtd: Refactor iotlb flush code

[Jean Guyader]

Allen,

The addresses in input are not necessarily aligned, to be able to use this feature
I will have to break the range into aligned chunks of different sizes and flush
them separatly.

I personally don't think this optimization is worth the effort.

Jean

On 09/11 02:41, Kay, Allen M wrote:
> Jean,
> 
> Page 122 of VT-d spec indicates you can invalidate multiple pages by setting the Address Mask (AM) field.  Will this achieve what you wanted to do?
> 
> http://download.intel.com/technology/computing/vptech/Intel(r)_VT_for_Direct_IO.pdf
> 
> Allen
> 
> -----Original Message-----
> From: Jean Guyader [mailto:jean.guyader@eu.citrix.com] 
> Sent: Monday, November 07, 2011 11:42 PM
> To: Kay, Allen M
> Cc: Jean Guyader; xen-devel@lists.xensource.com; Tim (Xen.org)
> Subject: Re: [PATCH 1/6] vtd: Refactor iotlb flush code
> 
> 
> Allen,
> 
> You are probably talking about __intel_iommu_iotlb_flush.
> This function takes a range of address to flush. I haven't found a function in the vtd code to invalidate a range on address without doing a loop of flush_iotlb_psi, so I thought that the most efficient and quick way to flush a range would be to use a domain selective invalidation.
> 
> Jean
> 
> On 08/11 03:10, Kay, Allen M wrote:
> > Jean,
> > 
> > The original code does not call iommu_flush_iotlb_dsi().  What is the reason the refractored code need to use domain selective invalidation?
> > 
> > Allen
> > -----
> > 
> > +        if ( page_count > 1 || gfn == -1 )
> > +        {
> > +            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
> > +                        0, flush_dev_iotlb) )
> > +                iommu_flush_write_buffer(iommu);
> > +        }
> > +        else
> > +        {
> > +            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
> > +                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
> > +                        !dma_old_pte_present, flush_dev_iotlb) )
> > +                iommu_flush_write_buffer(iommu);
> > 
> > -----Original Message-----
> > From: Jean Guyader [mailto:jean.guyader@eu.citrix.com]
> > Sent: Monday, November 07, 2011 10:25 AM
> > To: xen-devel@lists.xensource.com
> > Cc: tim@xen.org; Kay, Allen M; Jean Guyader
> > Subject: [PATCH 1/6] vtd: Refactor iotlb flush code
> > 
> > 
> > Factorize the iotlb flush code from map_page and unmap_page into it's own function.
> > 
> > Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
> > ---
> >  xen/drivers/passthrough/vtd/iommu.c |   86 +++++++++++++++++-----------------
> >  1 files changed, 43 insertions(+), 43 deletions(-)
> > 

RE: [PATCH 1/6] vtd: Refactor iotlb flush code

[Kay, Allen M]

Jean,

I think this is fine.  Ack for VT-d related changes.

Allen

-----Original Message-----
From: Jean Guyader [mailto:jean.guyader@eu.citrix.com] 
Sent: Wednesday, November 09, 2011 2:16 PM
To: Kay, Allen M
Cc: Jean Guyader; xen-devel@lists.xensource.com; Tim (Xen.org)
Subject: Re: [PATCH 1/6] vtd: Refactor iotlb flush code

Allen,

The addresses in input are not necessarily aligned, to be able to use this feature I will have to break the range into aligned chunks of different sizes and flush them separatly.

I personally don't think this optimization is worth the effort.

Jean

On 09/11 02:41, Kay, Allen M wrote:
> Jean,
> 
> Page 122 of VT-d spec indicates you can invalidate multiple pages by setting the Address Mask (AM) field.  Will this achieve what you wanted to do?
> 
> http://download.intel.com/technology/computing/vptech/Intel(r)_VT_for_
> Direct_IO.pdf
> 
> Allen
> 
> -----Original Message-----
> From: Jean Guyader [mailto:jean.guyader@eu.citrix.com]
> Sent: Monday, November 07, 2011 11:42 PM
> To: Kay, Allen M
> Cc: Jean Guyader; xen-devel@lists.xensource.com; Tim (Xen.org)
> Subject: Re: [PATCH 1/6] vtd: Refactor iotlb flush code
> 
> 
> Allen,
> 
> You are probably talking about __intel_iommu_iotlb_flush.
> This function takes a range of address to flush. I haven't found a function in the vtd code to invalidate a range on address without doing a loop of flush_iotlb_psi, so I thought that the most efficient and quick way to flush a range would be to use a domain selective invalidation.
> 
> Jean
> 
> On 08/11 03:10, Kay, Allen M wrote:
> > Jean,
> > 
> > The original code does not call iommu_flush_iotlb_dsi().  What is the reason the refractored code need to use domain selective invalidation?
> > 
> > Allen
> > -----
> > 
> > +        if ( page_count > 1 || gfn == -1 )
> > +        {
> > +            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
> > +                        0, flush_dev_iotlb) )
> > +                iommu_flush_write_buffer(iommu);
> > +        }
> > +        else
> > +        {
> > +            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
> > +                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
> > +                        !dma_old_pte_present, flush_dev_iotlb) )
> > +                iommu_flush_write_buffer(iommu);
> > 
> > -----Original Message-----
> > From: Jean Guyader [mailto:jean.guyader@eu.citrix.com]
> > Sent: Monday, November 07, 2011 10:25 AM
> > To: xen-devel@lists.xensource.com
> > Cc: tim@xen.org; Kay, Allen M; Jean Guyader
> > Subject: [PATCH 1/6] vtd: Refactor iotlb flush code
> > 
> > 
> > Factorize the iotlb flush code from map_page and unmap_page into it's own function.
> > 
> > Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
> > ---
> >  xen/drivers/passthrough/vtd/iommu.c |   86 +++++++++++++++++-----------------
> >  1 files changed, 43 insertions(+), 43 deletions(-)
> > 

RE: [PATCH 1/6] vtd: Refactor iotlb flush code

[Kay, Allen M]

The fourth parameter, currently 0,  is supposed to be the AM field.  It is named order now.

+            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
+                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
+                        !dma_old_pte_present, flush_dev_iotlb) )

-----Original Message-----
From: Kay, Allen M 
Sent: Tuesday, November 08, 2011 6:42 PM
To: 'Jean Guyader'
Cc: Jean Guyader; xen-devel@lists.xensource.com; Tim (Xen.org)
Subject: RE: [PATCH 1/6] vtd: Refactor iotlb flush code

Jean,

Page 122 of VT-d spec indicates you can invalidate multiple pages by setting the Address Mask (AM) field.  Will this achieve what you wanted to do?

http://download.intel.com/technology/computing/vptech/Intel(r)_VT_for_Direct_IO.pdf

Allen

-----Original Message-----
From: Jean Guyader [mailto:jean.guyader@eu.citrix.com] 
Sent: Monday, November 07, 2011 11:42 PM
To: Kay, Allen M
Cc: Jean Guyader; xen-devel@lists.xensource.com; Tim (Xen.org)
Subject: Re: [PATCH 1/6] vtd: Refactor iotlb flush code


Allen,

You are probably talking about __intel_iommu_iotlb_flush.
This function takes a range of address to flush. I haven't found a function in the vtd code to invalidate a range on address without doing a loop of flush_iotlb_psi, so I thought that the most efficient and quick way to flush a range would be to use a domain selective invalidation.

Jean

On 08/11 03:10, Kay, Allen M wrote:
> Jean,
> 
> The original code does not call iommu_flush_iotlb_dsi().  What is the reason the refractored code need to use domain selective invalidation?
> 
> Allen
> -----
> 
> +        if ( page_count > 1 || gfn == -1 )
> +        {
> +            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
> +                        0, flush_dev_iotlb) )
> +                iommu_flush_write_buffer(iommu);
> +        }
> +        else
> +        {
> +            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
> +                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
> +                        !dma_old_pte_present, flush_dev_iotlb) )
> +                iommu_flush_write_buffer(iommu);
> 
> -----Original Message-----
> From: Jean Guyader [mailto:jean.guyader@eu.citrix.com]
> Sent: Monday, November 07, 2011 10:25 AM
> To: xen-devel@lists.xensource.com
> Cc: tim@xen.org; Kay, Allen M; Jean Guyader
> Subject: [PATCH 1/6] vtd: Refactor iotlb flush code
> 
> 
> Factorize the iotlb flush code from map_page and unmap_page into it's own function.
> 
> Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
> ---
>  xen/drivers/passthrough/vtd/iommu.c |   86 +++++++++++++++++-----------------
>  1 files changed, 43 insertions(+), 43 deletions(-)
> 

[PATCH 0/6] IOMMU, vtd and iotlb flush rework (v4)

[Jean Guyader]

In one of my previous email I detailed a bug I was seeing when passing
through a Intel GPU on a guest that has more that 4G or RAM.

Allen suggested that I go for the Plan B but after a discussion with Tim
we agreed that Plan B was way to disruptive in term of code change.

This patch series implements Plan A.

http://xen.1045712.n5.nabble.com/VTD-Intel-iommu-IOTLB-flush-really-slow-td4952866.html

Changes between v3 and v4:
        - Move the loop for gmfn_range from arch_memory_op to xenmem_add_to_physmap.
        - Add a comment to comment to explain the purpose of iommu_dont_flush_iotlb.

Changes between v2 and v3:
       - Check for the presence iotlb_flush_all callback before calling it.

Changes between v1 and v2:
      - Move size in struct xen_add_to_physmap in padding between .domid and .space.
      - Store iommu_dont_flush per cpu
      - Change the code in hvmloader to relocate by batch of 64K, .size is now 16 bits.


 tools/firmware/hvmloader/pci.c      |   20 +++++--
 xen/arch/x86/mm.c                   |   82 ++++++++++++++++++++++------
 xen/arch/x86/x86_64/compat/mm.c     |    4 ++
 xen/drivers/passthrough/iommu.c     |   25 +++++++++
 xen/drivers/passthrough/vtd/iommu.c |  100 ++++++++++++++++++++---------------
 xen/include/public/memory.h         |    4 ++
 xen/include/xen/iommu.h             |   17 ++++++
 7 files changed, 186 insertions(+), 66 deletions(-)

Jean Guyader (6):
      vtd: Refactor iotlb flush code
      iommu: Introduce iommu_flush and iommu_flush_all.
      add_to_physmap: Move the code for XENMEM_add_to_physmap.
      mm: New XENMEM space, XENMAPSPACE_gmfn_range
      hvmloader: Change memory relocation loop when overlap with PCI hole.
      Introduce per cpu flag (iommu_dont_flush_iotlb) to avoid unnecessary iotlb flush

[PATCH 1/6] vtd: Refactor iotlb flush code

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 279 bytes --]


Factorize the iotlb flush code from map_page and unmap_page into
it's own function.

Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
---
 xen/drivers/passthrough/vtd/iommu.c |   86 +++++++++++++++++-----------------
 1 files changed, 43 insertions(+), 43 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vtd-Refactor-iotlb-flush-code.patch --]
[-- Type: text/x-patch; name="0001-vtd-Refactor-iotlb-flush-code.patch", Size: 4122 bytes --]

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 7717ab4..5a5b6be 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -578,16 +578,53 @@ static void iommu_flush_all(void)
     }
 }
 
+static void __intel_iommu_iotlb_flush(struct domain *d, unsigned long gfn,
+        int dma_old_pte_present, unsigned int page_count)
+{
+    struct hvm_iommu *hd = domain_hvm_iommu(d);
+    struct acpi_drhd_unit *drhd;
+    struct iommu *iommu;
+    int flush_dev_iotlb;
+    int iommu_domid;
+
+    /*
+     * No need pcideves_lock here because we have flush
+     * when assign/deassign device
+     */
+    for_each_drhd_unit ( drhd )
+    {
+        iommu = drhd->iommu;
+
+        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
+            continue;
+
+        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+        iommu_domid= domain_iommu_domid(d, iommu);
+        if ( iommu_domid == -1 )
+            continue;
+
+        if ( page_count > 1 || gfn == -1 )
+        {
+            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
+                        0, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+        else
+        {
+            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
+                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
+                        !dma_old_pte_present, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+    }
+}
+
 /* clear one page's page table */
 static void dma_pte_clear_one(struct domain *domain, u64 addr)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(domain);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL;
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
     struct mapped_rmrr *mrmrr;
 
     spin_lock(&hd->mapping_lock);
@@ -613,21 +650,7 @@ static void dma_pte_clear_one(struct domain *domain, u64 addr)
     spin_unlock(&hd->mapping_lock);
     iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
 
-    /* No need pcidevs_lock here since do that on assign/deassign device*/
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-        if ( test_bit(iommu->index, &hd->iommu_bitmap) )
-        {
-            flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-            iommu_domid= domain_iommu_domid(domain, iommu);
-            if ( iommu_domid == -1 )
-                continue;
-            if ( iommu_flush_iotlb_psi(iommu, iommu_domid, addr,
-                                       0, 0, flush_dev_iotlb) )
-                iommu_flush_write_buffer(iommu);
-        }
-    }
+    __intel_iommu_iotlb_flush(domain, addr >> PAGE_SHIFT_4K , 0, 1);
 
     unmap_vtd_domain_page(page);
 
@@ -1677,12 +1700,8 @@ static int intel_iommu_map_page(
     unsigned int flags)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(d);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
 
     /* Do nothing if VT-d shares EPT page table */
     if ( iommu_use_hap_pt(d) )
@@ -1724,26 +1743,7 @@ static int intel_iommu_map_page(
     spin_unlock(&hd->mapping_lock);
     unmap_vtd_domain_page(page);
 
-    /*
-     * No need pcideves_lock here because we have flush
-     * when assign/deassign device
-     */
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-
-        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
-            continue;
-
-        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-        iommu_domid= domain_iommu_domid(d, iommu);
-        if ( iommu_domid == -1 )
-            continue;
-        if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
-                                   (paddr_t)gfn << PAGE_SHIFT_4K, 0,
-                                   !dma_pte_present(old), flush_dev_iotlb) )
-            iommu_flush_write_buffer(iommu);
-    }
+    __intel_iommu_iotlb_flush(d, gfn, dma_pte_present(old), 1);
 
     return 0;
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 0/6] IOMMU, vtd and iotlb flush rework (v5)

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]

In one of my previous email I detailed a bug I was seeing when passing
through a Intel GPU on a guest that has more that 4G or RAM.

Allen suggested that I go for the Plan B but after a discussion with Tim
we agreed that Plan B was way to disruptive in term of code change.

This patch series implements Plan A.

http://xen.1045712.n5.nabble.com/VTD-Intel-iommu-IOTLB-flush-really-slow-td4952866.html

changes between v4 and v5:
        - Fix hypercall continuation for add_to_physmap in compat mode.

Changes between v3 and v4:
       - Move the loop for gmfn_range from arch_memory_op to xenmem_add_to_physmap.
       - Add a comment to comment to explain the purpose of iommu_dont_flush_iotlb.

Changes between v2 and v3:
      - Check for the presence iotlb_flush_all callback before calling it.

Changes between v1 and v2:
     - Move size in struct xen_add_to_physmap in padding between .domid and .space.
     - Store iommu_dont_flush per cpu
     - Change the code in hvmloader to relocate by batch of 64K, .size is now 16 bits.

Jean Guyader (6):
  vtd: Refactor iotlb flush code
  iommu: Introduce iommu_flush and iommu_flush_all.
  add_to_physmap: Move the code for XENMEM_add_to_physmap.
  mm: New XENMEM space, XENMAPSPACE_gmfn_range
  hvmloader: Change memory relocation loop when overlap with PCI hole.
  Introduce per cpu flag (iommu_dont_flush_iotlb) to avoid unnecessary
    iotlb flush

 tools/firmware/hvmloader/pci.c      |   20 +++++--
 xen/arch/x86/mm.c                   |   82 ++++++++++++++++++++++------
 xen/arch/x86/x86_64/compat/mm.c     |   10 ++++
 xen/drivers/passthrough/iommu.c     |   25 +++++++++
 xen/drivers/passthrough/vtd/iommu.c |  100 ++++++++++++++++++++---------------
 xen/include/public/memory.h         |    4 ++
 xen/include/xen/iommu.h             |   17 ++++++
 7 files changed, 192 insertions(+), 66 deletions(-)


[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 1/6] vtd: Refactor iotlb flush code

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 325 bytes --]


Factorize the iotlb flush code from map_page and unmap_page into
it's own function.

Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
Acked-by: Allen M Kay <allen.m.kay@intel.com>
---
 xen/drivers/passthrough/vtd/iommu.c |   86 +++++++++++++++++-----------------
 1 files changed, 43 insertions(+), 43 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vtd-Refactor-iotlb-flush-code.patch --]
[-- Type: text/x-patch; name="0001-vtd-Refactor-iotlb-flush-code.patch", Size: 4122 bytes --]

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 7717ab4..5a5b6be 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -578,16 +578,53 @@ static void iommu_flush_all(void)
     }
 }
 
+static void __intel_iommu_iotlb_flush(struct domain *d, unsigned long gfn,
+        int dma_old_pte_present, unsigned int page_count)
+{
+    struct hvm_iommu *hd = domain_hvm_iommu(d);
+    struct acpi_drhd_unit *drhd;
+    struct iommu *iommu;
+    int flush_dev_iotlb;
+    int iommu_domid;
+
+    /*
+     * No need pcideves_lock here because we have flush
+     * when assign/deassign device
+     */
+    for_each_drhd_unit ( drhd )
+    {
+        iommu = drhd->iommu;
+
+        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
+            continue;
+
+        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+        iommu_domid= domain_iommu_domid(d, iommu);
+        if ( iommu_domid == -1 )
+            continue;
+
+        if ( page_count > 1 || gfn == -1 )
+        {
+            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
+                        0, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+        else
+        {
+            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
+                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
+                        !dma_old_pte_present, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+    }
+}
+
 /* clear one page's page table */
 static void dma_pte_clear_one(struct domain *domain, u64 addr)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(domain);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL;
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
     struct mapped_rmrr *mrmrr;
 
     spin_lock(&hd->mapping_lock);
@@ -613,21 +650,7 @@ static void dma_pte_clear_one(struct domain *domain, u64 addr)
     spin_unlock(&hd->mapping_lock);
     iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
 
-    /* No need pcidevs_lock here since do that on assign/deassign device*/
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-        if ( test_bit(iommu->index, &hd->iommu_bitmap) )
-        {
-            flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-            iommu_domid= domain_iommu_domid(domain, iommu);
-            if ( iommu_domid == -1 )
-                continue;
-            if ( iommu_flush_iotlb_psi(iommu, iommu_domid, addr,
-                                       0, 0, flush_dev_iotlb) )
-                iommu_flush_write_buffer(iommu);
-        }
-    }
+    __intel_iommu_iotlb_flush(domain, addr >> PAGE_SHIFT_4K , 0, 1);
 
     unmap_vtd_domain_page(page);
 
@@ -1677,12 +1700,8 @@ static int intel_iommu_map_page(
     unsigned int flags)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(d);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
 
     /* Do nothing if VT-d shares EPT page table */
     if ( iommu_use_hap_pt(d) )
@@ -1724,26 +1743,7 @@ static int intel_iommu_map_page(
     spin_unlock(&hd->mapping_lock);
     unmap_vtd_domain_page(page);
 
-    /*
-     * No need pcideves_lock here because we have flush
-     * when assign/deassign device
-     */
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-
-        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
-            continue;
-
-        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-        iommu_domid= domain_iommu_domid(d, iommu);
-        if ( iommu_domid == -1 )
-            continue;
-        if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
-                                   (paddr_t)gfn << PAGE_SHIFT_4K, 0,
-                                   !dma_pte_present(old), flush_dev_iotlb) )
-            iommu_flush_write_buffer(iommu);
-    }
+    __intel_iommu_iotlb_flush(d, gfn, dma_pte_present(old), 1);
 
     return 0;
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 0/6] IOMMU, vtd and iotlb flush rework (v7)

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 2480 bytes --]

In one of my previous email I detailed a bug I was seeing when passing
through a Intel GPU on a guest that has more that 4G or RAM.

Allen suggested that I go for the Plan B but after a discussion with Tim
we agreed that Plan B was way to disruptive in term of code change.

This patch series implements Plan A.

http://xen.1045712.n5.nabble.com/VTD-Intel-iommu-IOTLB-flush-really-slow-td4952866.html

Changes between v6 and v7:
        - xenmem_add_to_physmap_once can't take a pointer on xatp because
          it modifies .idx and .gpfn.
        - Cancel hypercall continuation in the compat layer if the copy_to_guest
          failed.

Changes between v5 and v6:
       - Rework the patch queue to make it more readable.
       - Modify xatp in place in xenmem_add_to_physmap
       - Only check for preemption if we are not at the last iteration
       - Copy xatp guest handler back to the guest only in case of continuation
       - Add continuation only when dealing with the new xenmem space
         (XENMAPSPACE_gmfn_range).

Changes between v4 and v5:
       - Fix hypercall continuation for add_to_physmap in compat mode.

Changes between v3 and v4:
       - Move the loop for gmfn_range from arch_memory_op to xenmem_add_to_physmap.
       - Add a comment to comment to explain the purpose of iommu_dont_flush_iotlb.

Changes between v2 and v3:
       - Check for the presence iotlb_flush_all callback before calling it.

Changes between v1 and v2:
       - Move size in struct xen_add_to_physmap in padding between .domid and .space.
       - Store iommu_dont_flush per cpu
       - Change the code in hvmloader to relocate by batch of 64K, .size is now 16 bits.

Jean Guyader (6):
  vtd: Refactor iotlb flush code
  iommu: Introduce iommu_flush and iommu_flush_all.
  add_to_physmap: Move the code for XENMEM_add_to_physmap
  mm: New XENMEM space, XENMAPSPACE_gmfn_range
  hvmloader: Change memory relocation loop when overlap with PCI hole
  Introduce per cpu flag (iommu_dont_flush_iotlb) to avoid unnecessary
    iotlb flush

 tools/firmware/hvmloader/pci.c      |   20 ++-
 xen/arch/x86/mm.c                   |  231 ++++++++++++++++++++++-------------
 xen/arch/x86/x86_64/compat/mm.c     |   14 ++
 xen/drivers/passthrough/iommu.c     |   25 ++++
 xen/drivers/passthrough/vtd/iommu.c |  100 +++++++++-------
 xen/include/public/memory.h         |    4 +
 xen/include/xen/iommu.h             |   17 +++
 7 files changed, 279 insertions(+), 132 deletions(-)


[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 1/6] vtd: Refactor iotlb flush code

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 326 bytes --]


Factorize the iotlb flush code from map_page and unmap_page into
it's own function.

Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
Acked-by: Allen M Kay <allen.m.kay@intel.com>

---
 xen/drivers/passthrough/vtd/iommu.c |   86 +++++++++++++++++-----------------
 1 files changed, 43 insertions(+), 43 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vtd-Refactor-iotlb-flush-code.patch --]
[-- Type: text/x-patch; name="0001-vtd-Refactor-iotlb-flush-code.patch", Size: 4122 bytes --]

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 7717ab4..5a5b6be 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -578,16 +578,53 @@ static void iommu_flush_all(void)
     }
 }
 
+static void __intel_iommu_iotlb_flush(struct domain *d, unsigned long gfn,
+        int dma_old_pte_present, unsigned int page_count)
+{
+    struct hvm_iommu *hd = domain_hvm_iommu(d);
+    struct acpi_drhd_unit *drhd;
+    struct iommu *iommu;
+    int flush_dev_iotlb;
+    int iommu_domid;
+
+    /*
+     * No need pcideves_lock here because we have flush
+     * when assign/deassign device
+     */
+    for_each_drhd_unit ( drhd )
+    {
+        iommu = drhd->iommu;
+
+        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
+            continue;
+
+        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+        iommu_domid= domain_iommu_domid(d, iommu);
+        if ( iommu_domid == -1 )
+            continue;
+
+        if ( page_count > 1 || gfn == -1 )
+        {
+            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
+                        0, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+        else
+        {
+            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
+                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
+                        !dma_old_pte_present, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+    }
+}
+
 /* clear one page's page table */
 static void dma_pte_clear_one(struct domain *domain, u64 addr)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(domain);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL;
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
     struct mapped_rmrr *mrmrr;
 
     spin_lock(&hd->mapping_lock);
@@ -613,21 +650,7 @@ static void dma_pte_clear_one(struct domain *domain, u64 addr)
     spin_unlock(&hd->mapping_lock);
     iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
 
-    /* No need pcidevs_lock here since do that on assign/deassign device*/
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-        if ( test_bit(iommu->index, &hd->iommu_bitmap) )
-        {
-            flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-            iommu_domid= domain_iommu_domid(domain, iommu);
-            if ( iommu_domid == -1 )
-                continue;
-            if ( iommu_flush_iotlb_psi(iommu, iommu_domid, addr,
-                                       0, 0, flush_dev_iotlb) )
-                iommu_flush_write_buffer(iommu);
-        }
-    }
+    __intel_iommu_iotlb_flush(domain, addr >> PAGE_SHIFT_4K , 0, 1);
 
     unmap_vtd_domain_page(page);
 
@@ -1677,12 +1700,8 @@ static int intel_iommu_map_page(
     unsigned int flags)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(d);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
 
     /* Do nothing if VT-d shares EPT page table */
     if ( iommu_use_hap_pt(d) )
@@ -1724,26 +1743,7 @@ static int intel_iommu_map_page(
     spin_unlock(&hd->mapping_lock);
     unmap_vtd_domain_page(page);
 
-    /*
-     * No need pcideves_lock here because we have flush
-     * when assign/deassign device
-     */
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-
-        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
-            continue;
-
-        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-        iommu_domid= domain_iommu_domid(d, iommu);
-        if ( iommu_domid == -1 )
-            continue;
-        if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
-                                   (paddr_t)gfn << PAGE_SHIFT_4K, 0,
-                                   !dma_pte_present(old), flush_dev_iotlb) )
-            iommu_flush_write_buffer(iommu);
-    }
+    __intel_iommu_iotlb_flush(d, gfn, dma_pte_present(old), 1);
 
     return 0;
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 0/6] IOMMU, vtd and iotlb flush rework (v8)

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 2807 bytes --]


In one of my previous email I detailed a bug I was seeing when passing
through a Intel GPU on a guest that has more that 4G or RAM.

Allen suggested that I go for the Plan B but after a discussion with Tim
we agreed that Plan B was way to disruptive in term of code change.

This patch series implements Plan A.

http://xen.1045712.n5.nabble.com/VTD-Intel-iommu-IOTLB-flush-really-slow-td4952866.html

Changes between v7 and v8:
        - Rebase on new add_to_physmap from Andres Lagar-Cavilla
        - Only do copy_to_guest on the new XENMAPSPACE (XENMAPSPACE_gmfn_range).
        - Convert the xatp argument to a pointer in xenmem_add_to_physmap.
        - Modify xenmem_add_to_physmap so it doesn't touchs the input (xatp is const now).

Changes between v6 and v7:
       - xenmem_add_to_physmap_once can't take a pointer on xatp because
         it modifies .idx and .gpfn.
       - Cancel hypercall continuation in the compat layer if the copy_to_guest
         failed.

Changes between v5 and v6:
      - Rework the patch queue to make it more readable.
      - Modify xatp in place in xenmem_add_to_physmap
      - Only check for preemption if we are not at the last iteration
      - Copy xatp guest handler back to the guest only in case of continuation
      - Add continuation only when dealing with the new xenmem space
        (XENMAPSPACE_gmfn_range).

Changes between v4 and v5:
      - Fix hypercall continuation for add_to_physmap in compat mode.

Changes between v3 and v4:
      - Move the loop for gmfn_range from arch_memory_op to xenmem_add_to_physmap.
      - Add a comment to comment to explain the purpose of iommu_dont_flush_iotlb.

Changes between v2 and v3:
      - Check for the presence iotlb_flush_all callback before calling it.

Changes between v1 and v2:
      - Move size in struct xen_add_to_physmap in padding between .domid and .space.
      - Store iommu_dont_flush per cpu
      - Change the code in hvmloader to relocate by batch of 64K, .size is now 16 bits.

Jean Guyader (6):
  vtd: Refactor iotlb flush code
  iommu: Introduce iommu_flush and iommu_flush_all.
  add_to_physmap: Move the code for XENMEM_add_to_physmap
  mm: New XENMEM space, XENMAPSPACE_gmfn_range
  hvmloader: Change memory relocation loop when overlap with PCI hole
  Introduce per cpu flag (iommu_dont_flush_iotlb) to avoid unnecessary
    iotlb flush

 tools/firmware/hvmloader/pci.c      |   20 +++-
 xen/arch/x86/mm.c                   |  212 +++++++++++++++++++++++------------
 xen/arch/x86/x86_64/compat/mm.c     |   18 +++
 xen/drivers/passthrough/iommu.c     |   25 ++++
 xen/drivers/passthrough/vtd/iommu.c |  100 ++++++++++-------
 xen/include/public/memory.h         |    4 +
 xen/include/xen/iommu.h             |   17 +++
 7 files changed, 277 insertions(+), 119 deletions(-)


[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

[PATCH 1/6] vtd: Refactor iotlb flush code

[Jean Guyader]

[-- Attachment #1: Type: text/plain, Size: 279 bytes --]


Factorize the iotlb flush code from map_page and unmap_page into
it's own function.

Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com>
---
 xen/drivers/passthrough/vtd/iommu.c |   86 +++++++++++++++++-----------------
 1 files changed, 43 insertions(+), 43 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vtd-Refactor-iotlb-flush-code.patch --]
[-- Type: text/x-patch; name="0001-vtd-Refactor-iotlb-flush-code.patch", Size: 4122 bytes --]

diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index d125b93..97c7ffa 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -579,16 +579,53 @@ static void iommu_flush_all(void)
     }
 }
 
+static void __intel_iommu_iotlb_flush(struct domain *d, unsigned long gfn,
+        int dma_old_pte_present, unsigned int page_count)
+{
+    struct hvm_iommu *hd = domain_hvm_iommu(d);
+    struct acpi_drhd_unit *drhd;
+    struct iommu *iommu;
+    int flush_dev_iotlb;
+    int iommu_domid;
+
+    /*
+     * No need pcideves_lock here because we have flush
+     * when assign/deassign device
+     */
+    for_each_drhd_unit ( drhd )
+    {
+        iommu = drhd->iommu;
+
+        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
+            continue;
+
+        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
+        iommu_domid= domain_iommu_domid(d, iommu);
+        if ( iommu_domid == -1 )
+            continue;
+
+        if ( page_count > 1 || gfn == -1 )
+        {
+            if ( iommu_flush_iotlb_dsi(iommu, iommu_domid,
+                        0, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+        else
+        {
+            if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
+                        (paddr_t)gfn << PAGE_SHIFT_4K, 0,
+                        !dma_old_pte_present, flush_dev_iotlb) )
+                iommu_flush_write_buffer(iommu);
+        }
+    }
+}
+
 /* clear one page's page table */
 static void dma_pte_clear_one(struct domain *domain, u64 addr)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(domain);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL;
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
     struct mapped_rmrr *mrmrr;
 
     spin_lock(&hd->mapping_lock);
@@ -614,21 +651,7 @@ static void dma_pte_clear_one(struct domain *domain, u64 addr)
     spin_unlock(&hd->mapping_lock);
     iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
 
-    /* No need pcidevs_lock here since do that on assign/deassign device*/
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-        if ( test_bit(iommu->index, &hd->iommu_bitmap) )
-        {
-            flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-            iommu_domid= domain_iommu_domid(domain, iommu);
-            if ( iommu_domid == -1 )
-                continue;
-            if ( iommu_flush_iotlb_psi(iommu, iommu_domid, addr,
-                                       0, 0, flush_dev_iotlb) )
-                iommu_flush_write_buffer(iommu);
-        }
-    }
+    __intel_iommu_iotlb_flush(domain, addr >> PAGE_SHIFT_4K , 0, 1);
 
     unmap_vtd_domain_page(page);
 
@@ -1678,12 +1701,8 @@ static int intel_iommu_map_page(
     unsigned int flags)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(d);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
     u64 pg_maddr;
-    int flush_dev_iotlb;
-    int iommu_domid;
 
     /* Do nothing if VT-d shares EPT page table */
     if ( iommu_use_hap_pt(d) )
@@ -1725,26 +1744,7 @@ static int intel_iommu_map_page(
     spin_unlock(&hd->mapping_lock);
     unmap_vtd_domain_page(page);
 
-    /*
-     * No need pcideves_lock here because we have flush
-     * when assign/deassign device
-     */
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-
-        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
-            continue;
-
-        flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
-        iommu_domid= domain_iommu_domid(d, iommu);
-        if ( iommu_domid == -1 )
-            continue;
-        if ( iommu_flush_iotlb_psi(iommu, iommu_domid,
-                                   (paddr_t)gfn << PAGE_SHIFT_4K, 0,
-                                   !dma_pte_present(old), flush_dev_iotlb) )
-            iommu_flush_write_buffer(iommu);
-    }
+    __intel_iommu_iotlb_flush(d, gfn, dma_pte_present(old), 1);
 
     return 0;
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel