Skip to contentSkip to conversation

amd iommu: Clean up iommu page table deallocation

PATCH In review Wei Wang 2 participants 6 messages Aug 13, 2012 – Aug 14, 2012

lore
Share & tools
lore

Wei Wang

author
#0 lore raw

PATCH 1/1 amd iommu: Clean up iommu page table deallocation

Hi Jan, this patch cleans up deallocate_next_page_table() function. Please take a look.

Thanks
Wei

author
#1 lore raw

PATCH 1/1 amd iommu: Add 2 helper functions: iommu_is_pte_present and iommu_next_level

# HG changeset patch
# User Wei Wang <[email protected]>
# Date 1344872310 -7200
# Node ID b6ca536658ac712c5f03b5ffedce3bb61d55adaf
# Parent 47080c96593702acd4145c5a1175b7d7f8f0679d
amd iommu: Add 2 helper functions: iommu_is_pte_present and iommu_next_level.

xen/drivers/passthrough/amd/iommu_map.c +2 -16
diff -r 47080c965937 -r b6ca536658ac xen/drivers/passthrough/amd/iommu_map.c
@@ -306,20 +306,6 @@ u64 amd_iommu_get_next_table_from_pte(u3
     return ptr;
 }
 
-static unsigned int iommu_next_level(u32 *entry)
-{
-    return get_field_from_reg_u32(entry[0],
-                                  IOMMU_PDE_NEXT_LEVEL_MASK,
-                                  IOMMU_PDE_NEXT_LEVEL_SHIFT);
-}
-
-static int amd_iommu_is_pte_present(u32 *entry)
-{
-    return get_field_from_reg_u32(entry[0],
-                                  IOMMU_PDE_PRESENT_MASK,
-                                  IOMMU_PDE_PRESENT_SHIFT);
-}
-
 /* For each pde, We use ignored bits (bit 1 - bit 8 and bit 63)
  * to save pde count, pde count = 511 is a candidate of page coalescing.
  */
@@ -489,7 +475,7 @@ static int iommu_pde_from_gfn(struct dom
                          >> PAGE_SHIFT;
 
         /* Split super page frame into smaller pieces.*/
-        if ( amd_iommu_is_pte_present((u32*)pde) &&
+        if ( iommu_is_pte_present((u32*)pde) &&
              (iommu_next_level((u32*)pde) == 0) &&
              next_table_mfn != 0 )
         {
@@ -526,7 +512,7 @@ static int iommu_pde_from_gfn(struct dom
         }
 
         /* Install lower level page table for non-present entries */
-        else if ( !amd_iommu_is_pte_present((u32*)pde) )
+        else if ( !iommu_is_pte_present((u32*)pde) )
         {
             if ( next_table_mfn == 0 )
             {
xen/drivers/passthrough/amd/pci_amd_iommu.c +3 -11
diff -r 47080c965937 -r b6ca536658ac xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -392,8 +392,7 @@ static void deallocate_next_page_table(s
 {
     void *table_vaddr, *pde;
     u64 next_table_maddr;
-    int index, next_level, present;
-    u32 *entry;
+    int index, next_level;
 
     table_vaddr = __map_domain_page(pg);
 
@@ -403,18 +402,11 @@ static void deallocate_next_page_table(s
         {
             pde = table_vaddr + (index * IOMMU_PAGE_TABLE_ENTRY_SIZE);
             next_table_maddr = amd_iommu_get_next_table_from_pte(pde);
-            entry = (u32*)pde;
 
-            next_level = get_field_from_reg_u32(entry[0],
-                                                IOMMU_PDE_NEXT_LEVEL_MASK,
-                                                IOMMU_PDE_NEXT_LEVEL_SHIFT);
-
-            present = get_field_from_reg_u32(entry[0],
-                                             IOMMU_PDE_PRESENT_MASK,
-                                             IOMMU_PDE_PRESENT_SHIFT);
+            next_level = iommu_next_level((u32*)pde);
 
             if ( (next_table_maddr != 0) && (next_level != 0)
-                && present )
+                && iommu_is_pte_present((u32*)pde) )
             {
                 deallocate_next_page_table(
                     maddr_to_page(next_table_maddr), level - 1);
xen/include/asm-x86/hvm/svm/amd-iommu-proto.h +14 -0
diff -r 47080c965937 -r b6ca536658ac xen/include/asm-x86/hvm/svm/amd-iommu-proto.h
@@ -257,4 +257,18 @@ static inline void iommu_set_addr_hi_to_
                          IOMMU_REG_BASE_ADDR_HIGH_SHIFT, reg);
 }
 
+static inline int iommu_is_pte_present(u32 *entry)
+{
+    return get_field_from_reg_u32(entry[0],
+                                  IOMMU_PDE_PRESENT_MASK,
+                                  IOMMU_PDE_PRESENT_SHIFT);
+}
+
+static inline unsigned int iommu_next_level(u32 *entry)
+{
+    return get_field_from_reg_u32(entry[0],
+                                  IOMMU_PDE_NEXT_LEVEL_MASK,
+                                  IOMMU_PDE_NEXT_LEVEL_SHIFT);
+}
+
 #endif /* _ASM_X86_64_AMD_IOMMU_PROTO_H */
author
#2 lore raw

PATCH 1/1 amd iommu: Use next_level instead of recalculating it

In reply to Wei Wang

# HG changeset patch
# User Wei Wang <[email protected]>
# Date 1344872313 -7200
# Node ID 273471c6dedd1e66caab7e4eede72130e4e0c00f # Parent b6ca536658ac712c5f03b5ffedce3bb61d55adaf amd iommu: Use next_level instead of recalculating it.

xen/drivers/passthrough/amd/pci_amd_iommu.c +3 -1
diff -r b6ca536658ac -r 273471c6dedd xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -408,8 +408,10 @@ static void deallocate_next_page_table(s
             if ( (next_table_maddr != 0) && (next_level != 0)
                 && iommu_is_pte_present((u32*)pde) )
             {
+                /* We do not support skip level yet */
+                ASSERT( next_level == level - 1 );
                 deallocate_next_page_table(
-                    maddr_to_page(next_table_maddr), level - 1);
+                    maddr_to_page(next_table_maddr), next_level);
             }
         }
     }
author
#3 lore raw

PATCH 1/1 amd iommu: Remove unnecessary map/unmap for l1 page tables

In reply to Wei Wang

# HG changeset patch
# User Wei Wang <[email protected]>
# Date 1344872316 -7200
# Node ID 076df9db4c273e9786ea373bda092716015d9403 # Parent 273471c6dedd1e66caab7e4eede72130e4e0c00f amd iommu: Remove unnecessary map/unmap for l1 page tables

xen/drivers/passthrough/amd/pci_amd_iommu.c +17 -15
diff -r 273471c6dedd -r 076df9db4c27 xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -394,25 +394,27 @@ static void deallocate_next_page_table(s
     u64 next_table_maddr;
     int index, next_level;
 
+    if ( level <= 1 )
+    {
+        free_amd_iommu_pgtable(pg);
+        return;
+    }
+
     table_vaddr = __map_domain_page(pg);
 
-    if ( level > 1 )
+    for ( index = 0; index < PTE_PER_TABLE_SIZE; index++ )
     {
-        for ( index = 0; index < PTE_PER_TABLE_SIZE; index++ )
+        pde = table_vaddr + (index * IOMMU_PAGE_TABLE_ENTRY_SIZE);
+        next_table_maddr = amd_iommu_get_next_table_from_pte(pde);
+        next_level = iommu_next_level((u32*)pde);
+
+        if ( (next_table_maddr != 0) && (next_level != 0)
+             && iommu_is_pte_present((u32*)pde) )
         {
-            pde = table_vaddr + (index * IOMMU_PAGE_TABLE_ENTRY_SIZE);
-            next_table_maddr = amd_iommu_get_next_table_from_pte(pde);
-
-            next_level = iommu_next_level((u32*)pde);
-
-            if ( (next_table_maddr != 0) && (next_level != 0)
-                && iommu_is_pte_present((u32*)pde) )
-            {
-                /* We do not support skip level yet */
-                ASSERT( next_level == level - 1 );
-                deallocate_next_page_table(
-                    maddr_to_page(next_table_maddr), next_level);
-            }
+            /* We do not support skip levels yet */
+            ASSERT( next_level == level - 1 );
+            deallocate_next_page_table(maddr_to_page(next_table_maddr), 
+                                       next_level);
         }
     }
#4 lore raw
In reply to Wei Wang
Quoted 3 lines
Quoted 1 line
Quoted 1 line · Wei Wang

Hi Jan, this patch cleans up deallocate_next_page_table() function. Please take a look.

Hi Wei,

looks okay, but unless I'm mistaken this is only cleanup, so I'd like to postpone this until after 4.2 unless you have a rather strong opinion towards getting it in now.

Jan

author
#5 lore raw
Quoted 4 lines · Jan Beulich
Quoted 3 lines
Quoted 1 line
Quoted 1 line · Wei Wang

Hi Jan, this patch cleans up deallocate_next_page_table() function. Please take a look.

Quoted 9 lines

Hi Wei,

looks okay, but unless I'm mistaken this is only cleanup, so I'd like to postpone this until after 4.2 unless you have a rather strong opinion towards getting it in now.

Jan

Yes, just clean up. After 4.2 is ok to me.
Thanks
Wei