[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2] x86/mm: switch to new APIs in arch_init_memory



Hi Hongyan,

On 27/02/2020 10:27, Hongyan Xia wrote:
From: Wei Liu <wei.liu2@xxxxxxxxxx>

The function will map and unmap pages on demand.

Since we now map and unmap Xen PTE pages, we would like to track the
lifetime of mappings so that 1) we do not dereference memory through a
variable after it is unmapped, 2) we do not unmap more than once.
Therefore, we introduce the UNMAP_DOMAIN_PAGE macro to nullify the
variable after unmapping, and ignore NULL.

Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Signed-off-by: Hongyan Xia <hongyxia@xxxxxxxxxx>

---
Changed in v2:
- let UNMAP_DOMAIN_PAGE itself check whether the input is NULL to avoid
   adding the check in unmap_domain_page.
- reword the commit message.
---
  xen/arch/x86/mm.c             | 14 ++++++++------
  xen/include/xen/domain_page.h |  7 +++++++
  2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 70b87c4830..9fcdcde5b7 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -356,19 +356,21 @@ void __init arch_init_memory(void)
              ASSERT(root_pgt_pv_xen_slots < ROOT_PAGETABLE_PV_XEN_SLOTS);
              if ( l4_table_offset(split_va) == l4_table_offset(split_va - 1) )
              {
-                l3_pgentry_t *l3tab = alloc_xen_pagetable();
+                mfn_t l3mfn = alloc_xen_pagetable_new();
- if ( l3tab )
+                if ( !mfn_eq(l3mfn, INVALID_MFN) )
                  {
-                    const l3_pgentry_t *l3idle =
-                        l4e_to_l3e(idle_pg_table[l4_table_offset(split_va)]);
+                    l3_pgentry_t *l3idle = map_l3t_from_l4e(
+                            idle_pg_table[l4_table_offset(split_va)]);
+                    l3_pgentry_t *l3tab = map_domain_page(l3mfn);
for ( i = 0; i < l3_table_offset(split_va); ++i )
                          l3tab[i] = l3idle[i];
                      for ( ; i < L3_PAGETABLE_ENTRIES; ++i )
                          l3tab[i] = l3e_empty();
-                    split_l4e = l4e_from_mfn(virt_to_mfn(l3tab),
-                                             __PAGE_HYPERVISOR_RW);
+                    split_l4e = l4e_from_mfn(l3mfn, __PAGE_HYPERVISOR_RW);
+                    UNMAP_DOMAIN_PAGE(l3idle);
+                    UNMAP_DOMAIN_PAGE(l3tab);
                  }
                  else
                      ++root_pgt_pv_xen_slots;
diff --git a/xen/include/xen/domain_page.h b/xen/include/xen/domain_page.h
index 32669a3339..bfc3bf6aeb 100644
--- a/xen/include/xen/domain_page.h
+++ b/xen/include/xen/domain_page.h
@@ -72,4 +72,11 @@ static inline void unmap_domain_page_global(const void *va) 
{};
#endif /* !CONFIG_DOMAIN_PAGE */ +#define UNMAP_DOMAIN_PAGE(p) do { \
+    if ( p ) {                      \
+        unmap_domain_page(p);       \
+        (p) = NULL;                 \
+    }                               \
+} while ( false )

Do we need to keep the do {} while ()?

Cheers,


--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.