[Xen-devel] [PATCH 4/4] xen/arm: p2m: Remove translation table when it's empty

[Julien Grall <julien.grall@xxxxxxxxxx>]

Currently, the translation table is left in place even if no entries is
inuse. Because of how the p2m code has been implemented, replacing a
translation table by a block (i.e superpage) is not supported. Therefore,
any mapping of a superpage size will be split in smaller chunks making
the translation less efficient.

Replacing a table by a block when a new mapping is added would be too
complicated because it requires to check if all the upper levels are not
inuse and free them if necessary.

Instead, we will remove the empty translation table when the mapping are
removed. To avoid going through all the table checking if no entry is
inuse, a counter representing the number of entry currently inuse is
kept per table translation and updated when an entry change state (i.e
valid <-> invalid).

As Xen allocates a page for each translation table, it's possible to
store the counter in the struct page_info.The field type_info has been
choosen for this purpose as the p2m owns the page and nobody should used
it.

Once Xen has finished to remove a mapping and all the reference to each
translation table has been updated, the level will be lookup backward to
check if we need first need to free an unused translation table at an
higher level and then the lower levels. This will allow to propagate the
number of reference and free multiple translation table at different level
in one go.

Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
---
 xen/arch/arm/p2m.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index ae0acf0..9e3aced 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -427,6 +427,8 @@ static int p2m_create_table(struct domain *d, lpae_t *entry,
 
              write_pte(&p[i], pte);
          }
+
+         page->u.inuse.type_info = LPAE_ENTRIES;
     }
     else
         clear_page(p);
@@ -936,6 +938,16 @@ static int apply_one_level(struct domain *d,
     BUG(); /* Should never get here */
 }
 
+static void update_reference_mapping(struct page_info *page,
+                                     lpae_t old_entry,
+                                     lpae_t new_entry)
+{
+    if ( p2m_valid(old_entry) && !p2m_valid(new_entry) )
+        page->u.inuse.type_info--;
+    else if ( !p2m_valid(old_entry) && !p2m_valid(new_entry) )
+        page->u.inuse.type_info++;
+}
+
 static int apply_p2m_changes(struct domain *d,
                      enum p2m_operation op,
                      paddr_t start_gpaddr,
@@ -961,6 +973,8 @@ static int apply_p2m_changes(struct domain *d,
     const bool_t preempt = !is_idle_vcpu(current);
     bool_t flush = false;
     bool_t flush_pt;
+    PAGE_LIST_HEAD(free_pages);
+    struct page_info *pg;
 
     /* Some IOMMU don't support coherent PT walk. When the p2m is
      * shared with the CPU, Xen has to make sure that the PT changes have
@@ -1070,6 +1084,7 @@ static int apply_p2m_changes(struct domain *d,
         {
             unsigned offset = offsets[level];
             lpae_t *entry = &mappings[level][offset];
+            lpae_t old_entry = *entry;
 
             ret = apply_one_level(d, entry,
                                   level, flush_pt, op,
@@ -1078,6 +1093,10 @@ static int apply_p2m_changes(struct domain *d,
                                   mattr, t, a);
             if ( ret < 0 ) { rc = ret ; goto out; }
             count += ret;
+
+            if ( ret != P2M_ONE_PROGRESS_NOP )
+                update_reference_mapping(pages[level], old_entry, *entry);
+
             /* L3 had better have done something! We cannot descend any 
further */
             BUG_ON(level == 3 && ret == P2M_ONE_DESCEND);
             if ( ret != P2M_ONE_DESCEND ) break;
@@ -1099,6 +1118,45 @@ static int apply_p2m_changes(struct domain *d,
             }
             /* else: next level already valid */
         }
+
+        BUG_ON(level > 3);
+
+        if ( op == REMOVE )
+        {
+           for ( ; level > P2M_ROOT_LEVEL; level-- )
+            {
+                lpae_t old_entry;
+                lpae_t *entry;
+                unsigned int offset;
+
+                pg = pages[level];
+
+                /*
+                 * No need to try the previous level if the current one
+                 * still contains some mappings
+                 */
+                if ( pg->u.inuse.type_info )
+                    break;
+
+                offset = offsets[level - 1];
+                entry = &mappings[level - 1][offset];
+                old_entry = *entry;
+
+                page_list_del(pg, &p2m->pages);
+
+                p2m_remove_pte(entry, flush_pt);
+
+                p2m->stats.mappings[level - 1]--;
+                update_reference_mapping(pages[level - 1], old_entry, *entry);
+
+                /*
+                 * We can't free the page now because it may be present
+                 * in the guest TLB. Queue it and free it after the TLB
+                 * has been flushed.
+                 */
+                page_list_add(pg, &free_pages);
+            }
+        }
     }
 
     if ( op == ALLOCATE || op == INSERT )
@@ -1116,6 +1174,9 @@ out:
         iommu_iotlb_flush(d, sgfn, egfn - sgfn);
     }
 
+    while ( (pg = page_list_remove_head(&free_pages)) )
+        free_domheap_page(pg);
+
     if ( rc < 0 && ( op == INSERT || op == ALLOCATE ) &&
          addr != start_gpaddr )
     {
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel