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

[Xen-devel] [RFC PATCH 2/2] x86/mm: Add mem access rights to NPT



From: Isaila Alexandru <aisaila@xxxxxxxxxxxxxxx>

This patch adds access control for NPT mode.

There aren’t enough extra bits to store the access rights in the NPT p2m
table, so we add a radix tree to store extra information.

For efficiency:
 - Only allocate this radix tree when we first store "non-default"
   extra information

 - Remove entires which match the default extra information rather
   than continuing to store them

 - For superpages, only store an entry for the first gfn in the
   superpage.  Use the order of the p2m entry being read to determine
   the proper place to look in the radix table.

Modify p2m_type_to_flags() to accept and interpret an access value,
parallel to the ept code.

Add a set_default_access() method to the p2m-pt and p2m-ept versions
of the p2m rather than setting it directly, to deal with different
default permitted access values.

Signed-off-by: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>
Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
---
NB, this is compile-tested only.

cc'ing Paul because this is functionality he may want at some point in
the future.

I'm not sure why we only allow 'int' to be stored in the radix tree,
but that throws away 30-some bits we could otherwise use.  We might
consider revising this if we run out of bits here.

CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
CC: Tim Deegan <tim@xxxxxxx>
CC: Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>
CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
CC: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
---
 xen/arch/x86/mm/mem_access.c     |   5 +-
 xen/arch/x86/mm/p2m-ept.c        |   9 ++
 xen/arch/x86/mm/p2m-pt.c         | 173 ++++++++++++++++++++++++++++---
 xen/arch/x86/mm/p2m.c            |   6 ++
 xen/arch/x86/monitor.c           |   1 +
 xen/include/asm-x86/mem_access.h |   2 +-
 xen/include/asm-x86/p2m.h        |  18 ++++
 7 files changed, 192 insertions(+), 22 deletions(-)

diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 699a315076..0f96c43f88 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -389,10 +389,7 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, 
uint32_t nr,
 
     /* If request to set default access. */
     if ( gfn_eq(gfn, INVALID_GFN) )
-    {
-        p2m->default_access = a;
-        return 0;
-    }
+        return p2m->set_default_access(p2m, a);
 
     p2m_lock(p2m);
     if ( ap2m )
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 1ff4f14ae4..37bdbcf186 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -667,6 +667,14 @@ bool_t ept_handle_misconfig(uint64_t gpa)
     return spurious ? (rc >= 0) : (rc > 0);
 }
 
+int ept_set_default_access(struct p2m_domain *p2m, p2m_access_t p2ma)
+{
+    /* All access is permitted */
+    p2m->default_access = p2ma;
+    
+    return 0;
+}
+
 /*
  * ept_set_entry() computes 'need_modify_vtd_table' for itself,
  * by observing whether any gfn->mfn translations are modified.
@@ -1255,6 +1263,7 @@ int ept_p2m_init(struct p2m_domain *p2m)
     p2m->change_entry_type_global = ept_change_entry_type_global;
     p2m->change_entry_type_range = ept_change_entry_type_range;
     p2m->memory_type_changed = ept_memory_type_changed;
+    p2m->set_default_access = ept_set_default_access;
     p2m->audit_p2m = NULL;
     p2m->tlb_flush = ept_tlb_flush;
 
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index 40bfc76a6f..cd8b8c9187 100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -68,7 +68,8 @@
 static unsigned long p2m_type_to_flags(const struct p2m_domain *p2m,
                                        p2m_type_t t,
                                        mfn_t mfn,
-                                       unsigned int level)
+                                       unsigned int level,
+                                       p2m_access_t access)
 {
     unsigned long flags;
     /*
@@ -87,23 +88,28 @@ static unsigned long p2m_type_to_flags(const struct 
p2m_domain *p2m,
     case p2m_ram_paged:
     case p2m_ram_paging_in:
     default:
-        return flags | _PAGE_NX_BIT;
+        flags |= _PAGE_NX_BIT;
+        break;
     case p2m_grant_map_ro:
-        return flags | P2M_BASE_FLAGS | _PAGE_NX_BIT;
+        flags |= P2M_BASE_FLAGS | _PAGE_NX_BIT;
+        break;
     case p2m_ioreq_server:
         flags |= P2M_BASE_FLAGS | _PAGE_RW | _PAGE_NX_BIT;
         if ( p2m->ioreq.flags & XEN_DMOP_IOREQ_MEM_ACCESS_WRITE )
-            return flags & ~_PAGE_RW;
-        return flags;
+            flags &= ~_PAGE_RW;
+        break;
     case p2m_ram_ro:
     case p2m_ram_logdirty:
     case p2m_ram_shared:
-        return flags | P2M_BASE_FLAGS;
+        flags |= P2M_BASE_FLAGS;
+        break;
     case p2m_ram_rw:
-        return flags | P2M_BASE_FLAGS | _PAGE_RW;
+        flags |= P2M_BASE_FLAGS | _PAGE_RW;
+        break;
     case p2m_grant_map_rw:
     case p2m_map_foreign:
-        return flags | P2M_BASE_FLAGS | _PAGE_RW | _PAGE_NX_BIT;
+        flags |= P2M_BASE_FLAGS | _PAGE_RW | _PAGE_NX_BIT;
+        break;
     case p2m_mmio_direct:
         if ( !rangeset_contains_singleton(mmio_ro_ranges, mfn_x(mfn)) )
             flags |= _PAGE_RW;
@@ -112,8 +118,32 @@ static unsigned long p2m_type_to_flags(const struct 
p2m_domain *p2m,
             flags |= _PAGE_PWT;
             ASSERT(!level);
         }
-        return flags | P2M_BASE_FLAGS | _PAGE_PCD;
+        flags |= P2M_BASE_FLAGS | _PAGE_PCD;
+        break;
     }
+
+    switch ( access )
+    {
+    case p2m_access_r:
+        flags |= _PAGE_NX_BIT;
+        flags &= ~_PAGE_RW;
+        break;
+    case p2m_access_rw:
+        flags |= _PAGE_NX_BIT;
+        break;
+    case p2m_access_rx:
+    case p2m_access_rx2rw:
+        flags &= ~(_PAGE_NX_BIT | _PAGE_RW);
+        break;
+    case p2m_access_x:
+        flags &= ~_PAGE_RW;
+        break;
+    case p2m_access_rwx:
+    default:
+        break;
+    }
+
+    return flags;
 }
 
 
@@ -174,6 +204,48 @@ static void p2m_add_iommu_flags(l1_pgentry_t *p2m_entry,
         l1e_add_flags(*p2m_entry, iommu_nlevel_to_flags(nlevel, flags));
 }
 
+const p2m_extra_t pt_extra_default = { .access = p2m_access_rwx };
+
+static p2m_extra_t p2m_pt_get_extra(struct p2m_domain *p2m, unsigned long gfn,
+                                      unsigned level)
+{
+    void *ptr;
+    unsigned long mask = ~0UL << (level * PAGETABLE_ORDER);
+
+    if ( !p2m->extra )
+        return pt_extra_default;
+
+    ptr = radix_tree_lookup(p2m->extra, gfn & mask);
+    if ( !ptr )
+        return pt_extra_default;
+    else
+        return (p2m_extra_t)radix_tree_ptr_to_int(ptr);
+}
+
+static void p2m_pt_set_extra(struct p2m_domain *p2m, unsigned long gfn,
+                                      p2m_extra_t extra)
+{
+    int rc;
+
+    if ( extra.extra == pt_extra_default.extra )
+    {
+        if ( p2m->extra )
+            radix_tree_delete(p2m->extra, gfn);
+        return;
+    }
+
+    ASSERT(p2m->extra);
+
+    rc = radix_tree_insert(p2m->extra, gfn,
+                           radix_tree_int_to_ptr(extra.extra));
+    if ( rc == -EEXIST )
+        /* If a setting already exists, change it to the new one. */
+        radix_tree_replace_slot(
+            radix_tree_lookup_slot(
+                p2m->extra, gfn),
+            radix_tree_int_to_ptr(extra.extra));
+}
+
 /* Returns: 0 for success, -errno for failure */
 static int
 p2m_next_level(struct p2m_domain *p2m, void **table,
@@ -210,6 +282,7 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
         mfn_t mfn;
         l1_pgentry_t *l1_entry;
         unsigned int i;
+        p2m_extra_t extra;
 
         switch ( level )
         {
@@ -247,8 +320,14 @@ p2m_next_level(struct p2m_domain *p2m, void **table,
 
         for ( i = 0; i < (1u << PAGETABLE_ORDER); i++ )
         {
-            new_entry = l1e_from_pfn(pfn | (i << ((level - 1) * 
PAGETABLE_ORDER)),
-                                     flags);
+            unsigned long frame_offset = (i << ((level - 1) * 
PAGETABLE_ORDER));
+            new_entry = l1e_from_pfn(pfn | frame_offset, flags);
+
+            /* Shattering a p2m mapping requires duplicating extra info as 
well */
+            if ( i == 0 )
+                extra = p2m_pt_get_extra(p2m, gfn, level);
+            else
+                p2m_pt_set_extra(p2m, gfn + frame_offset, extra);
             p2m->write_p2m_entry(p2m, gfn, l1_entry + i, new_entry, level);
         }
 
@@ -420,8 +499,10 @@ static int do_recalc(struct p2m_domain *p2m, unsigned long 
gfn)
         if ( nt != ot )
         {
             unsigned long mfn = l1e_get_pfn(e);
+            p2m_extra_t extra = p2m_pt_get_extra(p2m, gfn, level);
             unsigned long flags = p2m_type_to_flags(p2m, nt,
-                                                    _mfn(mfn), level);
+                                                    _mfn(mfn), level,
+                                                    extra.access);
 
             if ( level )
             {
@@ -471,6 +552,32 @@ int p2m_pt_handle_deferred_changes(uint64_t gpa)
     return rc;
 }
 
+/* Returns: 0 for success, -errno for failure */
+static int p2m_pt_check_access(p2m_access_t p2ma)
+{
+    switch ( p2ma )
+    {
+    case p2m_access_n:
+    case p2m_access_w:
+    case p2m_access_wx:
+    case p2m_access_n2rwx:
+        return -EINVAL;
+    default:
+        break;
+    }
+    return 0;
+}
+
+static int p2m_pt_set_default_access(struct p2m_domain *p2m,
+                                     p2m_access_t p2ma) {
+    int rc = p2m_pt_check_access(p2ma);
+
+    if ( !rc )
+        p2m->default_access = p2ma;
+
+    return rc;
+}
+
 /* Returns: 0 for success, -errno for failure */
 static int
 p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t mfn,
@@ -500,9 +607,13 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t 
mfn,
      */
     unsigned int flags, iommu_old_flags = 0;
     unsigned long old_mfn = mfn_x(INVALID_MFN);
+    p2m_extra_t extra = pt_extra_default;
 
     ASSERT(sve != 0);
 
+    if ( (rc = p2m_pt_check_access(p2ma)) )
+        return rc;
+
     if ( tb_init_done )
     {
         struct {
@@ -532,6 +643,24 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t 
mfn,
     if ( rc < 0 )
         return rc;
 
+    if ( p2ma != extra.access )
+        extra.access = p2ma;
+
+    /* 
+     * If we need 'extra' bits and we haven't allocated space for it
+     * yet, do so
+     */
+    if ( (extra.extra != pt_extra_default.extra) && !p2m->extra )
+    {
+        p2m->extra = xmalloc(struct radix_tree_root);
+        
+        if( !p2m->extra )
+        {
+            return -ENOMEM;
+        }
+        radix_tree_init(p2m->extra);
+    }
+
     table = map_domain_page(pagetable_get_mfn(p2m_get_pagetable(p2m)));
     rc = p2m_next_level(p2m, &table, &gfn_remainder, gfn,
                         L4_PAGETABLE_SHIFT - PAGE_SHIFT,
@@ -569,7 +698,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t 
mfn,
         ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
         l3e_content = mfn_valid(mfn) || p2m_allows_invalid_mfn(p2mt)
             ? p2m_l3e_from_pfn(mfn_x(mfn),
-                               p2m_type_to_flags(p2m, p2mt, mfn, 2))
+                               p2m_type_to_flags(p2m, p2mt, mfn, 2, p2ma))
             : l3e_empty();
         entry_content.l1 = l3e_content.l3;
 
@@ -608,7 +737,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t 
mfn,
 
         if ( mfn_valid(mfn) || p2m_allows_invalid_mfn(p2mt) )
             entry_content = p2m_l1e_from_pfn(mfn_x(mfn),
-                                         p2m_type_to_flags(p2m, p2mt, mfn, 0));
+                                         p2m_type_to_flags(p2m, p2mt, mfn, 0, 
p2ma));
         else
             entry_content = l1e_empty();
 
@@ -661,7 +790,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t 
mfn,
         ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
         l2e_content = mfn_valid(mfn) || p2m_allows_invalid_mfn(p2mt)
             ? p2m_l2e_from_pfn(mfn_x(mfn),
-                               p2m_type_to_flags(p2m, p2mt, mfn, 1))
+                               p2m_type_to_flags(p2m, p2mt, mfn, 1, p2ma))
             : l2e_empty();
         entry_content.l1 = l2e_content.l2;
 
@@ -672,6 +801,13 @@ p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t 
mfn,
         /* NB: paging_write_p2m_entry() handles tlb flushes properly */
     }
 
+    /* 
+     * For storing of 'extra' information, only set the first gfn in
+     * any range; use the p2m table itself to record the order covered
+     * by that entry.
+     */
+    p2m_pt_set_extra(p2m, gfn, extra);
+
     /* Track the highest gfn for which we have ever had a valid mapping */
     if ( p2mt != p2m_invalid
          && (gfn + (1UL << page_order) - 1 > p2m->max_mapped_pfn) )
@@ -749,8 +885,7 @@ p2m_pt_get_entry(struct p2m_domain *p2m, gfn_t gfn_,
      * XXX Once we start explicitly registering MMIO regions in the p2m 
      * XXX we will return p2m_invalid for unmapped gfns */
     *t = p2m_mmio_dm;
-    /* Not implemented except with EPT */
-    *a = p2m_access_rwx; 
+    *a = p2m_access_n;
 
     if ( gfn > p2m->max_mapped_pfn )
     {
@@ -813,6 +948,7 @@ pod_retry_l3:
                        l1_table_offset(addr));
             *t = p2m_recalc_type(recalc || _needs_recalc(flags),
                                  p2m_flags_to_type(flags), p2m, gfn);
+            *a = p2m_pt_get_extra(p2m, gfn, 2).access;
             unmap_domain_page(l3e);
 
             ASSERT(mfn_valid(mfn) || !p2m_is_ram(*t));
@@ -852,6 +988,7 @@ pod_retry_l2:
         mfn = _mfn(l2e_get_pfn(*l2e) + l1_table_offset(addr));
         *t = p2m_recalc_type(recalc || _needs_recalc(flags),
                              p2m_flags_to_type(flags), p2m, gfn);
+        *a = p2m_pt_get_extra(p2m, gfn, 1).access;
         unmap_domain_page(l2e);
         
         ASSERT(mfn_valid(mfn) || !p2m_is_ram(*t));
@@ -888,6 +1025,7 @@ pod_retry_l1:
     }
     mfn = l1e_get_mfn(*l1e);
     *t = p2m_recalc_type(recalc || _needs_recalc(flags), l1t, p2m, gfn);
+    *a = p2m_pt_get_extra(p2m, gfn, 0).access;
     unmap_domain_page(l1e);
 
     ASSERT(mfn_valid(mfn) || !p2m_is_ram(*t) || p2m_is_paging(*t));
@@ -1129,6 +1267,7 @@ void p2m_pt_init(struct p2m_domain *p2m)
     p2m->change_entry_type_global = p2m_pt_change_entry_type_global;
     p2m->change_entry_type_range = p2m_pt_change_entry_type_range;
     p2m->write_p2m_entry = paging_write_p2m_entry;
+    p2m->set_default_access = p2m_pt_set_default_access;
 #if P2M_AUDIT
     p2m->audit_p2m = p2m_pt_audit_p2m;
 #else
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index ed2e8daf58..706a8ec8bb 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -682,6 +682,12 @@ void p2m_teardown(struct p2m_domain *p2m)
 
     d = p2m->domain;
 
+    if ( p2m->extra )
+    {
+        radix_tree_destroy(p2m->extra, NULL);
+        xfree(p2m->extra);
+    }
+
     p2m_lock(p2m);
     ASSERT(atomic_read(&d->shr_pages) == 0);
     p2m->phys_table = pagetable_null();
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 3c42e21906..2e6b1e75e4 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -20,6 +20,7 @@
  */
 
 #include <asm/monitor.h>
+#include <asm/p2m.h>
 #include <public/vm_event.h>
 
 int arch_monitor_init_domain(struct domain *d)
diff --git a/xen/include/asm-x86/mem_access.h b/xen/include/asm-x86/mem_access.h
index 4043c9fb4d..34f2c07088 100644
--- a/xen/include/asm-x86/mem_access.h
+++ b/xen/include/asm-x86/mem_access.h
@@ -46,7 +46,7 @@ bool p2m_mem_access_emulate_check(struct vcpu *v,
 /* Sanity check for mem_access hardware support */
 static inline bool p2m_mem_access_sanity_check(struct domain *d)
 {
-    return is_hvm_domain(d) && cpu_has_vmx && hap_enabled(d);
+    return is_hvm_domain(d) && hap_enabled(d);
 }
 
 #endif /*__ASM_X86_MEM_ACCESS_H__ */
diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index be3b6fcaf0..a5e1193dcc 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -185,6 +185,16 @@ typedef enum {
     p2m_alternate,
 } p2m_class_t;
 
+typedef union {
+    struct {
+        u32  access      :   4,  /* p2m_access_t */
+            _available   :  25,
+            _reserved    :   3;  /* Upper two bits reserved */
+    };
+    int extra;
+} p2m_extra_t;
+
+
 /* Per-p2m-table state */
 struct p2m_domain {
     /* Lock that protects updates to the p2m */
@@ -270,6 +280,8 @@ struct p2m_domain {
                                           unsigned long gfn, l1_pgentry_t *p,
                                           l1_pgentry_t new, unsigned int 
level);
     long               (*audit_p2m)(struct p2m_domain *p2m);
+    int                (*set_default_access)(struct p2m_domain *p2m,
+                                             p2m_access_t p2ma);
 
     /*
      * P2M updates may require TLBs to be flushed (invalidated).
@@ -292,6 +304,12 @@ struct p2m_domain {
      * retyped get this access type.  See definition of p2m_access_t. */
     p2m_access_t default_access;
 
+    /*
+     * Radix tree to store extra information which doesn't fit in the
+     * p2m's PTE
+     */
+    struct radix_tree_root *extra;
+
     /* If true, and an access fault comes in and there is no vm_event 
listener, 
      * pause domain.  Otherwise, remove access restrictions. */
     bool_t       access_required;
-- 
2.19.0


_______________________________________________
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®.