Re: [PATCH] VT-d: IOTLB flush fixups

Re: [PATCH] VT-d: IOTLB flush fixups

[Yang, Xiaowei]

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

> On map: only flush when old PTE was valid or invalid PTE may be cached.
> On unmap: always flush old entry, but skip flush for unaffected IOMMUs.
> 
> Signed-off-by: Espen Skoglund <espen.skoglund@netronome.com>
> 
> --
>  iommu.c |   17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> --

Seems my last mail sent to the xen-devel is lost and I have no local
copy so I have to write again...

Espen,
Thanks for the patch! I also noticed context/iotlb flush need a cleanup.
As flush of present/non-present entry are different, your change to
iommu_intel_map_page are not that correct. So I made up anther patch.
iommu_flush is also removed, as VTd table is not shared with p2m any
more.

Signed-off-by: Xiaowei Yang <xiaowei.yang@intel.com>

Thanks,
Xiaowei

[-- Attachment #2: flush.diff --]
[-- Type: text/x-patch, Size: 7951 bytes --]

diff -r c93a913c221f xen/arch/x86/mm/hap/p2m-ept.c
--- a/xen/arch/x86/mm/hap/p2m-ept.c	Tue May 27 13:03:05 2008 +0100
+++ b/xen/arch/x86/mm/hap/p2m-ept.c	Wed May 28 06:24:27 2008 +0800
@@ -266,12 +266,6 @@ out:
                 iommu_unmap_page(d, gfn);
         }
     }
-
-#ifdef P2M_SHARE_WITH_VTD_PAGE_TABLE
-    /* If p2m table is shared with vtd page-table. */
-    if ( iommu_enabled && is_hvm_domain(d) && (p2mt == p2m_mmio_direct) )
-        iommu_flush(d, gfn, (u64*)ept_entry);
-#endif
 
     return rv;
 }
diff -r c93a913c221f xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Tue May 27 13:03:05 2008 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Wed May 28 06:59:25 2008 +0800
@@ -485,9 +485,12 @@ static int flush_iotlb_reg(void *_iommu,
     /* check IOTLB invalidation granularity */
     if ( DMA_TLB_IAIG(val) == 0 )
         printk(KERN_ERR VTDPREFIX "IOMMU: flush IOTLB failed\n");
+
+#ifdef VTD_DEBUG
     if ( DMA_TLB_IAIG(val) != DMA_TLB_IIRG(type) )
         printk(KERN_ERR VTDPREFIX "IOMMU: tlb flush request %x, actual %x\n",
                (u32)DMA_TLB_IIRG(type), (u32)DMA_TLB_IAIG(val));
+#endif
     /* flush context entry will implictly flush write buffer */
     return 0;
 }
@@ -581,30 +584,29 @@ static void dma_pte_clear_one(struct dom
     drhd = list_entry(acpi_drhd_units.next, typeof(*drhd), list);
 
     /* get last level pte */
-    pg_maddr = dma_addr_level_page_maddr(domain, addr, 1);
+    pg_maddr = dma_addr_level_page_maddr(domain, addr, 2);
     if ( pg_maddr == 0 )
         return;
     page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
     pte = page + address_level_offset(addr, 1);
-    if ( pte )
-    {
-        dma_clear_pte(*pte);
-        iommu_flush_cache_entry(drhd->iommu, pte);
-
-        for_each_drhd_unit ( drhd )
-        {
-            iommu = drhd->iommu;
-
-            if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
-                continue;
-
-            if ( cap_caching_mode(iommu->cap) )
-                iommu_flush_iotlb_psi(iommu, domain_iommu_domid(domain),
-                                      addr, 1, 0);
-            else if (cap_rwbf(iommu->cap))
-                iommu_flush_write_buffer(iommu);
-        }
-    }
+
+    if ( !dma_pte_present(*pte) )
+    {
+        unmap_vtd_domain_page(page);
+        return;
+    }
+
+    dma_clear_pte(*pte); 
+    iommu_flush_cache_entry(drhd->iommu, pte);
+
+    for_each_drhd_unit ( drhd )
+    {
+        iommu = drhd->iommu;
+
+        if ( test_bit(iommu->index, &hd->iommu_bitmap) )
+            iommu_flush_iotlb_psi(iommu, domain_iommu_domid(domain), addr, 1, 0);
+    }
+
     unmap_vtd_domain_page(page);
 }
 
@@ -1191,12 +1193,13 @@ static int domain_context_mapping_one(
 
     unmap_vtd_domain_page(context_entries);
 
+    /* it's a non-present to present mapping */
     if ( iommu_flush_context_device(iommu, domain_iommu_domid(domain),
                                     (((u16)bus) << 8) | devfn,
                                     DMA_CCMD_MASK_NOBIT, 1) )
         iommu_flush_write_buffer(iommu);
     else
-        iommu_flush_iotlb_dsi(iommu, domain_iommu_domid(domain), 0);
+        iommu_flush_iotlb_dsi(iommu, 0, 0);
 
     set_bit(iommu->index, &hd->iommu_bitmap);
     spin_unlock_irqrestore(&iommu->lock, flags);
@@ -1526,6 +1529,7 @@ int intel_iommu_map_page(
     struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL;
     u64 pg_maddr;
+    int pte_present;
 
     drhd = list_entry(acpi_drhd_units.next, typeof(*drhd), list);
     iommu = drhd->iommu;
@@ -1541,6 +1545,7 @@ int intel_iommu_map_page(
         return -ENOMEM;
     page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
     pte = page + (gfn & LEVEL_MASK);
+    pte_present = dma_pte_present(*pte);
     dma_set_pte_addr(*pte, (paddr_t)mfn << PAGE_SHIFT_4K);
     dma_set_pte_prot(*pte, DMA_PTE_READ | DMA_PTE_WRITE);
     iommu_flush_cache_entry(iommu, pte);
@@ -1553,10 +1558,11 @@ int intel_iommu_map_page(
         if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
             continue;
 
-        if ( cap_caching_mode(iommu->cap) )
+        if ( pte_present )
             iommu_flush_iotlb_psi(iommu, domain_iommu_domid(d),
                                   (paddr_t)gfn << PAGE_SHIFT_4K, 1, 0);
-        else if ( cap_rwbf(iommu->cap) )
+        else if ( iommu_flush_iotlb_psi(iommu, domain_iommu_domid(d),
+                                   (paddr_t)gfn << PAGE_SHIFT_4K, 1, 1) )
             iommu_flush_write_buffer(iommu);
     }
 
@@ -1567,8 +1573,6 @@ int intel_iommu_unmap_page(struct domain
 {
     struct acpi_drhd_unit *drhd;
     struct iommu *iommu;
-    struct dma_pte *page = NULL, *pte = NULL;
-    u64 pg_maddr;
 
     drhd = list_entry(acpi_drhd_units.next, typeof(*drhd), list);
     iommu = drhd->iommu;
@@ -1579,24 +1583,7 @@ int intel_iommu_unmap_page(struct domain
         return 0;
 #endif
 
-    pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn << PAGE_SHIFT_4K);
-    if ( pg_maddr == 0 )
-        return -ENOMEM;
-    page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
-    pte = page + (gfn & LEVEL_MASK);
-    dma_clear_pte(*pte);
-    iommu_flush_cache_entry(drhd->iommu, pte);
-    unmap_vtd_domain_page(page);
-
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-        if ( cap_caching_mode(iommu->cap) )
-            iommu_flush_iotlb_psi(iommu, domain_iommu_domid(d),
-                                  (paddr_t)gfn << PAGE_SHIFT_4K, 1, 0);
-        else if ( cap_rwbf(iommu->cap) )
-            iommu_flush_write_buffer(iommu);
-    }
+    dma_pte_clear_one(d, (paddr_t)gfn << PAGE_SHIFT_4K);
 
     return 0;
 }
@@ -1642,10 +1629,8 @@ int iommu_page_mapping(struct domain *do
         if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
             continue;
 
-        if ( cap_caching_mode(iommu->cap) )
-            iommu_flush_iotlb_psi(iommu, domain_iommu_domid(domain),
-                                  iova, index, 0);
-        else if ( cap_rwbf(iommu->cap) )
+        if ( iommu_flush_iotlb_psi(iommu, domain_iommu_domid(domain),
+                                   iova, index, 1) )
             iommu_flush_write_buffer(iommu);
     }
 
@@ -1657,30 +1642,6 @@ int iommu_page_unmapping(struct domain *
     dma_pte_clear_range(domain, addr, addr + size);
 
     return 0;
-}
-
-void iommu_flush(struct domain *d, unsigned long gfn, u64 *p2m_entry)
-{
-    struct hvm_iommu *hd = domain_hvm_iommu(d);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu = NULL;
-    struct dma_pte *pte = (struct dma_pte *) p2m_entry;
-
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-
-        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
-            continue;
-
-        if ( cap_caching_mode(iommu->cap) )
-            iommu_flush_iotlb_psi(iommu, domain_iommu_domid(d),
-                                  (paddr_t)gfn << PAGE_SHIFT_4K, 1, 0);
-        else if ( cap_rwbf(iommu->cap) )
-            iommu_flush_write_buffer(iommu);
-    }
-
-    iommu_flush_cache_entry(iommu, pte);
 }
 
 static int iommu_prepare_rmrr_dev(
diff -r c93a913c221f xen/include/xen/iommu.h
--- a/xen/include/xen/iommu.h	Tue May 27 13:03:05 2008 +0100
+++ b/xen/include/xen/iommu.h	Wed May 28 06:24:44 2008 +0800
@@ -65,7 +65,6 @@ void reassign_device_ownership(struct do
                                u8 bus, u8 devfn);
 int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn);
 int iommu_unmap_page(struct domain *d, unsigned long gfn);
-void iommu_flush(struct domain *d, unsigned long gfn, u64 *p2m_entry);
 void iommu_set_pgd(struct domain *d);
 void iommu_free_pgd(struct domain *d);
 void iommu_domain_teardown(struct 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

Re: [PATCH] VT-d: IOTLB flush fixups

[Espen Skoglund]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 1451 bytes --]

[Xiaowei Yang]
>> On map: only flush when old PTE was valid or invalid PTE may be cached.
>> On unmap: always flush old entry, but skip flush for unaffected IOMMUs.
>> 
>> Signed-off-by: Espen Skoglund <espen.skoglund@netronome.com>
>> 
>> --
>>  iommu.c |   17 +++++++++++------
>>  1 file changed, 11 insertions(+), 6 deletions(-)
>> --
>
> Seems my last mail sent to the xen-devel is lost and I have no local
> copy so I have to write again...
>
> Espen,
> Thanks for the patch! I also noticed context/iotlb flush need a
> cleanup.  As flush of present/non-present entry are different, your
> change to iommu_intel_map_page are not that correct. So I made up
> anther patch.  iommu_flush is also removed, as VTd table is not
> shared with p2m any more.
>
> Signed-off-by: Xiaowei Yang <xiaowei.yang@intel.com>

Oh, right.  When flushing a non-present cached entry domid 0 must be
used.  Here's a modification of your patch:

 - Made the non-present flush testing a bit simpler.
 - Removed dma_addr_level_page_maddr().  Use a modified
   addr_to_dma_page_maddr() instead.
 - Upon mapping new context entry: flush old entry using domid 0 and
   always flush iotlb.

Signed-off-by: Espen Skoglund <espen.skoglund@netronome.com>

--
 arch/x86/mm/hap/p2m-ept.c       |    6 -
 drivers/passthrough/vtd/iommu.c |  150 ++++++++++------------------------------
 include/xen/iommu.h             |    1 
 3 files changed, 38 insertions(+), 119 deletions(-)



[-- Attachment #2: xen-vtd-flush.patch --]
[-- Type: application/octet-stream, Size: 10230 bytes --]

diff -r c93a913c221f xen/arch/x86/mm/hap/p2m-ept.c
--- a/xen/arch/x86/mm/hap/p2m-ept.c	Tue May 27 13:03:05 2008 +0100
+++ b/xen/arch/x86/mm/hap/p2m-ept.c	Wed May 28 15:48:06 2008 +0100
@@ -267,12 +267,6 @@
         }
     }
 
-#ifdef P2M_SHARE_WITH_VTD_PAGE_TABLE
-    /* If p2m table is shared with vtd page-table. */
-    if ( iommu_enabled && is_hvm_domain(d) && (p2mt == p2m_mmio_direct) )
-        iommu_flush(d, gfn, (u64*)ept_entry);
-#endif
-
     return rv;
 }
 
diff -r c93a913c221f xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Tue May 27 13:03:05 2008 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Wed May 28 15:48:06 2008 +0100
@@ -191,7 +191,7 @@
     return ret;
 }
 
-static u64 addr_to_dma_page_maddr(struct domain *domain, u64 addr)
+static u64 addr_to_dma_page_maddr(struct domain *domain, u64 addr, int alloc)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(domain);
     struct acpi_drhd_unit *drhd;
@@ -201,7 +201,7 @@
     int level = agaw_to_level(hd->agaw);
     int offset;
     unsigned long flags;
-    u64 pte_maddr = 0;
+    u64 pte_maddr = 0, maddr;
     u64 *vaddr = NULL;
 
     drhd = list_entry(acpi_drhd_units.next, typeof(*drhd), list);
@@ -211,6 +211,8 @@
     spin_lock_irqsave(&hd->mapping_lock, flags);
     if ( hd->pgd_maddr == 0 )
     {
+        if ( !alloc )
+            return 0;
         hd->pgd_maddr = alloc_pgtable_maddr();
         if ( hd->pgd_maddr == 0 )
             return 0;
@@ -224,7 +226,9 @@
 
         if ( dma_pte_addr(*pte) == 0 )
         {
-            u64 maddr = alloc_pgtable_maddr();
+            if ( !alloc )
+                break;
+            maddr = alloc_pgtable_maddr();
             dma_set_pte_addr(*pte, maddr);
             vaddr = map_vtd_domain_page(maddr);
             if ( !vaddr )
@@ -261,41 +265,6 @@
     unmap_vtd_domain_page(parent);
     spin_unlock_irqrestore(&hd->mapping_lock, flags);
     return pte_maddr;
-}
-
-/* return address's page at specific level */
-static u64 dma_addr_level_page_maddr(
-    struct domain *domain, u64 addr, int level)
-{
-    struct hvm_iommu *hd = domain_hvm_iommu(domain);
-    struct dma_pte *parent, *pte = NULL;
-    int total = agaw_to_level(hd->agaw);
-    int offset;
-    u64 pg_maddr = hd->pgd_maddr;
-
-    if ( pg_maddr == 0 )
-        return 0;
-
-    parent = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
-    while ( level <= total )
-    {
-        offset = address_level_offset(addr, total);
-        pte = &parent[offset];
-        if ( dma_pte_addr(*pte) == 0 )
-            break;
-
-        pg_maddr = pte->val & PAGE_MASK_4K;
-        unmap_vtd_domain_page(parent);
-
-        if ( level == total )
-            return pg_maddr;
-
-        parent = map_vtd_domain_page(pte->val);
-        total--;
-    }
-
-    unmap_vtd_domain_page(parent);
-    return 0;
 }
 
 static void iommu_flush_write_buffer(struct iommu *iommu)
@@ -485,9 +454,12 @@
     /* check IOTLB invalidation granularity */
     if ( DMA_TLB_IAIG(val) == 0 )
         printk(KERN_ERR VTDPREFIX "IOMMU: flush IOTLB failed\n");
+
+#ifdef VTD_DEBUG
     if ( DMA_TLB_IAIG(val) != DMA_TLB_IIRG(type) )
         printk(KERN_ERR VTDPREFIX "IOMMU: tlb flush request %x, actual %x\n",
                (u32)DMA_TLB_IIRG(type), (u32)DMA_TLB_IAIG(val));
+#endif
     /* flush context entry will implictly flush write buffer */
     return 0;
 }
@@ -581,30 +553,29 @@
     drhd = list_entry(acpi_drhd_units.next, typeof(*drhd), list);
 
     /* get last level pte */
-    pg_maddr = dma_addr_level_page_maddr(domain, addr, 1);
+    pg_maddr = addr_to_dma_page_maddr(domain, addr, 0);
     if ( pg_maddr == 0 )
         return;
     page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
     pte = page + address_level_offset(addr, 1);
-    if ( pte )
+
+    if ( !dma_pte_present(*pte) )
     {
-        dma_clear_pte(*pte);
-        iommu_flush_cache_entry(drhd->iommu, pte);
+        unmap_vtd_domain_page(page);
+        return;
+    }
 
-        for_each_drhd_unit ( drhd )
-        {
-            iommu = drhd->iommu;
+    dma_clear_pte(*pte); 
+    iommu_flush_cache_entry(drhd->iommu, pte);
 
-            if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
-                continue;
+    for_each_drhd_unit ( drhd )
+    {
+        iommu = drhd->iommu;
 
-            if ( cap_caching_mode(iommu->cap) )
-                iommu_flush_iotlb_psi(iommu, domain_iommu_domid(domain),
-                                      addr, 1, 0);
-            else if (cap_rwbf(iommu->cap))
-                iommu_flush_write_buffer(iommu);
-        }
+        if ( test_bit(iommu->index, &hd->iommu_bitmap) )
+            iommu_flush_iotlb_psi(iommu, domain_iommu_domid(domain), addr, 1, 0);
     }
+
     unmap_vtd_domain_page(page);
 }
 
@@ -1191,12 +1162,11 @@
 
     unmap_vtd_domain_page(context_entries);
 
-    if ( iommu_flush_context_device(iommu, domain_iommu_domid(domain),
-                                    (((u16)bus) << 8) | devfn,
-                                    DMA_CCMD_MASK_NOBIT, 1) )
+    /* Context entry was previously non-present (with domid 0). */
+    iommu_flush_context_device(iommu, 0, (((u16)bus) << 8) | devfn,
+                               DMA_CCMD_MASK_NOBIT, 1);
+    if ( iommu_flush_iotlb_dsi(iommu, 0, 1) )
         iommu_flush_write_buffer(iommu);
-    else
-        iommu_flush_iotlb_dsi(iommu, domain_iommu_domid(domain), 0);
 
     set_bit(iommu->index, &hd->iommu_bitmap);
     spin_unlock_irqrestore(&iommu->lock, flags);
@@ -1526,6 +1496,7 @@
     struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL;
     u64 pg_maddr;
+    int pte_present;
 
     drhd = list_entry(acpi_drhd_units.next, typeof(*drhd), list);
     iommu = drhd->iommu;
@@ -1536,11 +1507,12 @@
         return 0;
 #endif
 
-    pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn << PAGE_SHIFT_4K);
+    pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn << PAGE_SHIFT_4K, 1);
     if ( pg_maddr == 0 )
         return -ENOMEM;
     page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
     pte = page + (gfn & LEVEL_MASK);
+    pte_present = dma_pte_present(*pte);
     dma_set_pte_addr(*pte, (paddr_t)mfn << PAGE_SHIFT_4K);
     dma_set_pte_prot(*pte, DMA_PTE_READ | DMA_PTE_WRITE);
     iommu_flush_cache_entry(iommu, pte);
@@ -1553,10 +1525,9 @@
         if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
             continue;
 
-        if ( cap_caching_mode(iommu->cap) )
-            iommu_flush_iotlb_psi(iommu, domain_iommu_domid(d),
-                                  (paddr_t)gfn << PAGE_SHIFT_4K, 1, 0);
-        else if ( cap_rwbf(iommu->cap) )
+        if ( iommu_flush_iotlb_psi(iommu, domain_iommu_domid(d),
+                                   (paddr_t)gfn << PAGE_SHIFT_4K, 1,
+                                   !pte_present) )
             iommu_flush_write_buffer(iommu);
     }
 
@@ -1567,8 +1538,6 @@
 {
     struct acpi_drhd_unit *drhd;
     struct iommu *iommu;
-    struct dma_pte *page = NULL, *pte = NULL;
-    u64 pg_maddr;
 
     drhd = list_entry(acpi_drhd_units.next, typeof(*drhd), list);
     iommu = drhd->iommu;
@@ -1579,24 +1548,7 @@
         return 0;
 #endif
 
-    pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn << PAGE_SHIFT_4K);
-    if ( pg_maddr == 0 )
-        return -ENOMEM;
-    page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
-    pte = page + (gfn & LEVEL_MASK);
-    dma_clear_pte(*pte);
-    iommu_flush_cache_entry(drhd->iommu, pte);
-    unmap_vtd_domain_page(page);
-
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-        if ( cap_caching_mode(iommu->cap) )
-            iommu_flush_iotlb_psi(iommu, domain_iommu_domid(d),
-                                  (paddr_t)gfn << PAGE_SHIFT_4K, 1, 0);
-        else if ( cap_rwbf(iommu->cap) )
-            iommu_flush_write_buffer(iommu);
-    }
+    dma_pte_clear_one(d, (paddr_t)gfn << PAGE_SHIFT_4K);
 
     return 0;
 }
@@ -1622,7 +1574,7 @@
     index = 0;
     while ( start_pfn < end_pfn )
     {
-        pg_maddr = addr_to_dma_page_maddr(domain, iova + PAGE_SIZE_4K * index);
+        pg_maddr = addr_to_dma_page_maddr(domain, iova + PAGE_SIZE_4K*index, 1);
         if ( pg_maddr == 0 )
             return -ENOMEM;
         page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
@@ -1642,10 +1594,8 @@
         if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
             continue;
 
-        if ( cap_caching_mode(iommu->cap) )
-            iommu_flush_iotlb_psi(iommu, domain_iommu_domid(domain),
-                                  iova, index, 0);
-        else if ( cap_rwbf(iommu->cap) )
+        if ( iommu_flush_iotlb_psi(iommu, domain_iommu_domid(domain),
+                                   iova, index, 1) )
             iommu_flush_write_buffer(iommu);
     }
 
@@ -1657,30 +1607,6 @@
     dma_pte_clear_range(domain, addr, addr + size);
 
     return 0;
-}
-
-void iommu_flush(struct domain *d, unsigned long gfn, u64 *p2m_entry)
-{
-    struct hvm_iommu *hd = domain_hvm_iommu(d);
-    struct acpi_drhd_unit *drhd;
-    struct iommu *iommu = NULL;
-    struct dma_pte *pte = (struct dma_pte *) p2m_entry;
-
-    for_each_drhd_unit ( drhd )
-    {
-        iommu = drhd->iommu;
-
-        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
-            continue;
-
-        if ( cap_caching_mode(iommu->cap) )
-            iommu_flush_iotlb_psi(iommu, domain_iommu_domid(d),
-                                  (paddr_t)gfn << PAGE_SHIFT_4K, 1, 0);
-        else if ( cap_rwbf(iommu->cap) )
-            iommu_flush_write_buffer(iommu);
-    }
-
-    iommu_flush_cache_entry(iommu, pte);
 }
 
 static int iommu_prepare_rmrr_dev(
diff -r c93a913c221f xen/include/xen/iommu.h
--- a/xen/include/xen/iommu.h	Tue May 27 13:03:05 2008 +0100
+++ b/xen/include/xen/iommu.h	Wed May 28 15:48:06 2008 +0100
@@ -65,7 +65,6 @@
                                u8 bus, u8 devfn);
 int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn);
 int iommu_unmap_page(struct domain *d, unsigned long gfn);
-void iommu_flush(struct domain *d, unsigned long gfn, u64 *p2m_entry);
 void iommu_set_pgd(struct domain *d);
 void iommu_free_pgd(struct domain *d);
 void iommu_domain_teardown(struct 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

Re: [PATCH] VT-d: IOTLB flush fixups

[Yang, Xiaowei]

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

On Wed, 2008-05-28 at 23:00 +0800, Espen Skoglund wrote:

> Oh, right.  When flushing a non-present cached entry domid 0 must be
> used.  Here's a modification of your patch:
> 
>  - Made the non-present flush testing a bit simpler.
>  - Removed dma_addr_level_page_maddr().  Use a modified
>    addr_to_dma_page_maddr() instead.
Yes, it's simpler. Thanks! However, you forgot spin_unlock before
return. so it leads to deadlock. Here's a small fix.

>  - Upon mapping new context entry: flush old entry using domid 0 and
>    always flush iotlb.
Actually, you may find before/after your modification, the code are the
same functionally. We can pass domid=0 or non_present_entry_flush=1 to
flush non present tlb.

Thanks,
Xiaowei
> 


[-- Attachment #2: spin.patch --]
[-- Type: text/x-patch, Size: 797 bytes --]

diff -r f681c4de91fc xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Wed May 28 16:14:10 2008 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Thu May 29 03:48:33 2008 +0800
@@ -212,10 +212,10 @@ static u64 addr_to_dma_page_maddr(struct
     if ( hd->pgd_maddr == 0 )
     {
         if ( !alloc )
-            return 0;
+            goto out;
         hd->pgd_maddr = alloc_pgtable_maddr();
         if ( hd->pgd_maddr == 0 )
-            return 0;
+            goto out;
     }
 
     parent = (struct dma_pte *)map_vtd_domain_page(hd->pgd_maddr);
@@ -263,6 +263,7 @@ static u64 addr_to_dma_page_maddr(struct
     }
 
     unmap_vtd_domain_page(parent);
+ out:
     spin_unlock_irqrestore(&hd->mapping_lock, flags);
     return pte_maddr;
 }

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

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

[PATCH] VT-d: IOTLB flush fixups

[Espen Skoglund]

On map: only flush when old PTE was valid or invalid PTE may be cached.
On unmap: always flush old entry, but skip flush for unaffected IOMMUs.

Signed-off-by: Espen Skoglund <espen.skoglund@netronome.com>

--
 iommu.c |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
--
diff -r 8187fd8113f9 xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Tue May 27 11:46:52 2008 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Tue May 27 17:16:51 2008 +0100
@@ -1525,6 +1525,7 @@
     struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL;
     u64 pg_maddr;
+    int pte_present;
 
     drhd = list_entry(acpi_drhd_units.next, typeof(*drhd), list);
     iommu = drhd->iommu;
@@ -1540,6 +1541,7 @@
         return -ENOMEM;
     page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
     pte = page + (gfn & LEVEL_MASK);
+    pte_present = dma_pte_present(*pte);
     dma_set_pte_addr(*pte, (paddr_t)mfn << PAGE_SHIFT_4K);
     dma_set_pte_prot(*pte, DMA_PTE_READ | DMA_PTE_WRITE);
     iommu_flush_cache_entry(iommu, pte);
@@ -1552,7 +1554,7 @@
         if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
             continue;
 
-        if ( cap_caching_mode(iommu->cap) )
+        if ( pte_present || cap_caching_mode(iommu->cap) )
             iommu_flush_iotlb_psi(iommu, domain_iommu_domid(d),
                                   (paddr_t)gfn << PAGE_SHIFT_4K, 1, 0);
         else if ( cap_rwbf(iommu->cap) )
@@ -1564,6 +1566,7 @@
 
 int intel_iommu_unmap_page(struct domain *d, unsigned long gfn)
 {
+    struct hvm_iommu *hd = domain_hvm_iommu(d);
     struct acpi_drhd_unit *drhd;
     struct iommu *iommu;
     struct dma_pte *page = NULL, *pte = NULL;
@@ -1590,11 +1593,13 @@
     for_each_drhd_unit ( drhd )
     {
         iommu = drhd->iommu;
-        if ( cap_caching_mode(iommu->cap) )
-            iommu_flush_iotlb_psi(iommu, domain_iommu_domid(d),
-                                  (paddr_t)gfn << PAGE_SHIFT_4K, 1, 0);
-        else if ( cap_rwbf(iommu->cap) )
-            iommu_flush_write_buffer(iommu);
+ 
+        if ( !test_bit(iommu->index, &hd->iommu_bitmap) )
+            continue;
+
+       if ( iommu_flush_iotlb_psi(iommu, domain_iommu_domid(d),
+                                  (paddr_t)gfn << PAGE_SHIFT_4K, 1, 0) )
+           iommu_flush_write_buffer(iommu);
     }
 
     return 0;