[PATCH] xen/grant-table: Add a mechanism for cleaning frame GFN

[PATCH] xen/grant-table: Add a mechanism for cleaning frame GFN

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Introduce new gnttab_clean_frame_gfn() which purpose is to
locate a GFN corresponding to the passed MFN and remove it
from the gnttab database. This manual updating is only needed
on arch without M2P support.
So, call it from p2m_put_l3_page() on Arm after making sure
that we release a grant table page.

This patch is intended to fix a possible bug on Arm which might
happen when remapping grant-table frame. From the discussion
on the ML:
"The function gnttab_map_frame() is used to map the grant table
frame. If there is an old mapping, it will first remove it.
The function is using the helper gnttab_get_frame_gfn() to find
the corresponding GFN or return INVALID_GFN if not mapped.
On Arm, gnttab_map_frame() is implementing using an array index
by the grant table frame number. The trouble is we don't update
the array when the page is unmapped. So if the GFN is re-used
before the grant-table is remapped, then we will end up to remove
whatever was mapped there (this could be a foreign page...)."

Suggested-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
You can find the discussion at:
https://lore.kernel.org/xen-devel/93d0df14-2c8a-c2e3-8c51-54412190171c@xen.org/

I am sure that gnttab_clean_frame_gfn() is not needed on x86 which
has M2P support. But, I was thinking where to keep it, but couldn't
find any suitable place other than common grant_table.c.
I thought, new function could be neither placed in arch header as
static inline nor in "new" arch C file without reworking common
grant_table.c by moving all involved stuff (struct declarations,
helpers, etc) to the common header to make them visible from
the outside.
---
 xen/arch/arm/p2m.c            | 20 ++++++++++++++++----
 xen/common/grant_table.c      | 42 ++++++++++++++++++++++++++++++++++++++++++
 xen/include/xen/grant_table.h |  4 ++++
 3 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index eff9a10..e921be2 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -2,6 +2,7 @@
 #include <xen/domain_page.h>
 #include <xen/iocap.h>
 #include <xen/ioreq.h>
+#include <xen/grant_table.h>
 #include <xen/lib.h>
 #include <xen/sched.h>
 #include <xen/softirq.h>
@@ -718,8 +719,10 @@ static int p2m_mem_access_radix_set(struct p2m_domain *p2m, gfn_t gfn,
  * TODO: Handle superpages, for now we only take special references for leaf
  * pages (specifically foreign ones, which can't be super mapped today).
  */
-static void p2m_put_l3_page(const lpae_t pte)
+static void p2m_put_l3_page(struct p2m_domain *p2m, const lpae_t pte)
 {
+    mfn_t mfn = lpae_get_mfn(pte);
+
     ASSERT(p2m_is_valid(pte));
 
     /*
@@ -731,11 +734,20 @@ static void p2m_put_l3_page(const lpae_t pte)
      */
     if ( p2m_is_foreign(pte.p2m.type) )
     {
-        mfn_t mfn = lpae_get_mfn(pte);
-
         ASSERT(mfn_valid(mfn));
         put_page(mfn_to_page(mfn));
     }
+
+#ifdef CONFIG_GRANT_TABLE
+    /*
+     * Check whether we deal with grant-table page which GFN is stored
+     * in the gnttab database, so also needs to be marked as invalid.
+     * As the grant-table page is xen_heap page and its entry has known
+     * p2m type, detect it and let the gnttab code do the job.
+     */
+    if ( p2m_is_ram(pte.p2m.type) && is_xen_heap_mfn(mfn) )
+        gnttab_clean_frame_gfn(p2m->domain, mfn);
+#endif
 }
 
 /* Free lpae sub-tree behind an entry */
@@ -768,7 +780,7 @@ static void p2m_free_entry(struct p2m_domain *p2m,
         p2m->stats.mappings[level]--;
         /* Nothing to do if the entry is a super-page. */
         if ( level == 3 )
-            p2m_put_l3_page(entry);
+            p2m_put_l3_page(p2m, entry);
         return;
     }
 
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index fab77ab..4559524 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -4104,6 +4104,48 @@ int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn, mfn_t *mfn)
     return rc;
 }
 
+/*
+ * The only purpose of this function is to locate GFN corresponding to
+ * the passed MFN and remove it from the gnttab database.
+ */
+void gnttab_clean_frame_gfn(struct domain *d, mfn_t mfn)
+{
+    struct grant_table *gt = d->grant_table;
+    unsigned int i;
+    mfn_t tmp;
+
+    grant_write_lock(gt);
+
+    for ( i = 0; i < gt->max_grant_frames; i++ )
+    {
+        if ( gt->shared_raw[i] == NULL )
+            continue;
+
+        tmp = _mfn(virt_to_mfn(gt->shared_raw[i]));
+        if ( mfn_eq(tmp, mfn) )
+        {
+            gnttab_set_frame_gfn(gt, false, i, INVALID_GFN);
+            goto unlock;
+        }
+    }
+
+    for ( i = 0; i < grant_to_status_frames(gt->max_grant_frames); i++ )
+    {
+        if ( gt->status[i] == NULL )
+            continue;
+
+        tmp = _mfn(virt_to_mfn(gt->status[i]));
+        if ( mfn_eq(tmp, mfn) )
+        {
+            gnttab_set_frame_gfn(gt, true, i, INVALID_GFN);
+            goto unlock;
+        }
+    }
+
+unlock:
+    grant_write_unlock(gt);
+}
+
 static void gnttab_usage_print(struct domain *rd)
 {
     int first = 1;
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 9f8b7e6..62bae72 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -63,6 +63,8 @@ int gnttab_acquire_resource(
     struct domain *d, unsigned int id, unsigned int frame,
     unsigned int nr_frames, xen_pfn_t mfn_list[]);
 
+void gnttab_clean_frame_gfn(struct domain *d, mfn_t mfn);
+
 #else
 
 #define opt_max_grant_frames 0
@@ -108,6 +110,8 @@ static inline int gnttab_acquire_resource(
     return -EINVAL;
 }
 
+static inline void gnttab_clean_frame_gfn(struct domain *d, mfn_t mfn) {}
+
 #endif /* CONFIG_GRANT_TABLE */
 
 #endif /* __XEN_GRANT_TABLE_H__ */
-- 
2.7.4