Xen devel - Dec 2011 - [PATCH 0 of 2] Rebased mem access improvements and new type

We improve the handling of hap faults when both type and access
restrictions are present.

We also add a new p2m access type, n2rwx. It allows for implement a "log
access" mode in the hypervisor, aking to log dirty but for all types of
accesses. Faults caused by this access mode automatically promote the
access rights of the ofending p2m entry, place the event in the ring, and
let the vcpu keep on executing.

Rebased to apply cleanly on top of 537ceb11d51e.

Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
Signed-off-by: Adin Scannell <adin@scannell.ca>

 xen/arch/x86/hvm/hvm.c          |  20 +++++++++++++++-----
 xen/arch/x86/mm/p2m.c           |  10 ++++++----
 xen/include/asm-x86/p2m.h       |   9 +++++----
 xen/arch/x86/hvm/hvm.c          |   1 +
 xen/arch/x86/mm/p2m-ept.c       |   1 +
 xen/arch/x86/mm/p2m.c           |  30 +++++++++++++++++++++---------
 xen/include/asm-x86/p2m.h       |   3 +++
 xen/include/public/hvm/hvm_op.h |   3 +++
 8 files changed, 55 insertions(+), 22 deletions(-)

xen/arch/x86/hvm/hvm.c    |  20 +++++++++++++++-----
 xen/arch/x86/mm/p2m.c     |  10 ++++++----
 xen/include/asm-x86/p2m.h |   9 +++++----
 3 files changed, 26 insertions(+), 13 deletions(-)


The mem event fault handler in the p2m can automatically promote the access
rights of a p2m entry. In those scenarios, vcpu''s are not paused and
they will
immediately retry the faulting instructions. This will generate a second fault
if the underlying entry type requires so (paging, unsharing, pod, etc).
Collapse the two faults into a single one.

Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>

diff -r 3c240efdd6ad -r e315ce73f082 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1205,7 +1205,7 @@ int hvm_hap_nested_page_fault(unsigned l
     mfn_t mfn;
     struct vcpu *v = current;
     struct p2m_domain *p2m;
-    int rc;
+    int rc, fall_through = 0;
 
     /* On Nested Virtualization, walk the guest page table.
      * If this succeeds, all is fine.
@@ -1278,9 +1278,15 @@ int hvm_hap_nested_page_fault(unsigned l
 
         if ( violation )
         {
-            p2m_mem_access_check(gpa, gla_valid, gla, access_r, access_w,
access_x);
-            rc = 1;
-            goto out_put_gfn;
+            if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r, 
+                                        access_w, access_x) )
+            {
+                fall_through = 1;
+            } else {
+                /* Rights not promoted, vcpu paused, work here is done */
+                rc = 1;
+                goto out_put_gfn;
+            }
         }
     }
 
@@ -1339,7 +1345,11 @@ int hvm_hap_nested_page_fault(unsigned l
         goto out_put_gfn;
     }
 
-    rc = 0;
+    /* If we fell through, the vcpu will retry now that access restrictions
have
+     * been removed. It may fault again if the p2m entry type still requires
so.
+     * Otherwise, this is an error condition. */
+    rc = fall_through;
+
 out_put_gfn:
     put_gfn(p2m->domain, gfn);
     return rc;
diff -r 3c240efdd6ad -r e315ce73f082 xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1084,7 +1084,7 @@ void p2m_mem_paging_resume(struct domain
     mem_event_unpause_vcpus(d);
 }
 
-void p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, unsigned long
gla,
+bool_t p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, unsigned long
gla,
                           bool_t access_r, bool_t access_w, bool_t access_x)
 {
     struct vcpu *v = current;
@@ -1105,7 +1105,7 @@ void p2m_mem_access_check(unsigned long 
     {
         p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2mt, p2m_access_rw);
         p2m_unlock(p2m);
-        return;
+        return 1;
     }
     p2m_unlock(p2m);
 
@@ -1128,12 +1128,13 @@ void p2m_mem_access_check(unsigned long 
             p2m_lock(p2m);
             p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2mt,
p2m_access_rwx);
             p2m_unlock(p2m);
+            return 1;
         }
 
-        return;
+        return 0;
     }
     else if ( res > 0 )
-        return;  /* No space in buffer; VCPU paused */
+        return 0;  /* No space in buffer; VCPU paused */
 
     memset(&req, 0, sizeof(req));
     req.type = MEM_EVENT_TYPE_ACCESS;
@@ -1157,6 +1158,7 @@ void p2m_mem_access_check(unsigned long 
     mem_event_put_request(d, &d->mem_event->access, &req);
 
     /* VCPU paused, mem event request sent */
+    return 0;
 }
 
 void p2m_mem_access_resume(struct domain *d)
diff -r 3c240efdd6ad -r e315ce73f082 xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -491,8 +491,9 @@ static inline void p2m_mem_paging_popula
 
 #ifdef __x86_64__
 /* Send mem event based on the access (gla is -1ull if not available).  Handles
- * the rw2rx conversion */
-void p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, unsigned long
gla,
+ * the rw2rx conversion. Boolean return value indicates if access rights have 
+ * been promoted with no underlying vcpu pause. */
+bool_t p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, unsigned long
gla,
                           bool_t access_r, bool_t access_w, bool_t access_x);
 /* Resumes the running of the VCPU, restarting the last instruction */
 void p2m_mem_access_resume(struct domain *d);
@@ -508,10 +509,10 @@ int p2m_get_mem_access(struct domain *d,
                        hvmmem_access_t *access);
 
 #else
-static inline void p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, 
+static inline bool_t p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, 
                                         unsigned long gla, bool_t access_r, 
                                         bool_t access_w, bool_t access_x)
-{ }
+{ return 1; }
 static inline int p2m_set_mem_access(struct domain *d, 
                                      unsigned long start_pfn, 
                                      uint32_t nr, hvmmem_access_t access)

xen/arch/x86/hvm/hvm.c          |   1 +
 xen/arch/x86/mm/p2m-ept.c       |   1 +
 xen/arch/x86/mm/p2m.c           |  30 +++++++++++++++++++++---------
 xen/include/asm-x86/p2m.h       |   3 +++
 xen/include/public/hvm/hvm_op.h |   3 +++
 5 files changed, 29 insertions(+), 9 deletions(-)


This patch adds a new p2m access type, n2rwx. It allows for implement a
"log
access" mode in the hypervisor, aking to log dirty but for all types of
accesses. Faults caused by this access mode automatically promote the
access rights of the ofending p2m entry, place the event in the ring, and
let the vcpu keep on executing.

Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
Signed-off-by: Adin Scannell <adin@scannell.ca>

diff -r e315ce73f082 -r 66ca1a02082f xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1250,6 +1250,7 @@ int hvm_hap_nested_page_fault(unsigned l
         switch (p2ma) 
         {
         case p2m_access_n:
+        case p2m_access_n2rwx:
         default:
             violation = access_r || access_w || access_x;
             break;
diff -r e315ce73f082 -r 66ca1a02082f xen/arch/x86/mm/p2m-ept.c
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -111,6 +111,7 @@ static void ept_p2m_type_to_flags(ept_en
     switch (access) 
     {
         case p2m_access_n:
+        case p2m_access_n2rwx:
             entry->r = entry->w = entry->x = 0;
             break;
         case p2m_access_r:
diff -r e315ce73f082 -r 66ca1a02082f xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1107,6 +1107,11 @@ bool_t p2m_mem_access_check(unsigned lon
         p2m_unlock(p2m);
         return 1;
     }
+    else if ( p2ma == p2m_access_n2rwx )
+    {
+        ASSERT(access_w || access_r || access_x);
+        p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2mt, p2m_access_rwx);
+    }
     p2m_unlock(p2m);
 
     /* Otherwise, check if there is a memory event listener, and send the
message along */
@@ -1124,10 +1129,13 @@ bool_t p2m_mem_access_check(unsigned lon
         }
         else
         {
-            /* A listener is not required, so clear the access restrictions */
-            p2m_lock(p2m);
-            p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2mt,
p2m_access_rwx);
-            p2m_unlock(p2m);
+            if ( p2ma != p2m_access_n2rwx )
+            {
+                /* A listener is not required, so clear the access restrictions
*/
+                p2m_lock(p2m);
+                p2m->set_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2mt,
p2m_access_rwx);
+                p2m_unlock(p2m);
+            }
             return 1;
         }
 
@@ -1140,9 +1148,12 @@ bool_t p2m_mem_access_check(unsigned lon
     req.type = MEM_EVENT_TYPE_ACCESS;
     req.reason = MEM_EVENT_REASON_VIOLATION;
 
-    /* Pause the current VCPU unconditionally */
-    vcpu_pause_nosync(v);
-    req.flags |= MEM_EVENT_FLAG_VCPU_PAUSED;    
+    /* Pause the current VCPU */
+    if ( p2ma != p2m_access_n2rwx )
+    {
+        vcpu_pause_nosync(v);
+        req.flags |= MEM_EVENT_FLAG_VCPU_PAUSED;
+    } 
 
     /* Send request to mem event */
     req.gfn = gfn;
@@ -1157,8 +1168,8 @@ bool_t p2m_mem_access_check(unsigned lon
 
     mem_event_put_request(d, &d->mem_event->access, &req);
 
-    /* VCPU paused, mem event request sent */
-    return 0;
+    /* VCPU may be paused, return whether we promoted automatically */
+    return (p2ma == p2m_access_n2rwx);
 }
 
 void p2m_mem_access_resume(struct domain *d)
@@ -1204,6 +1215,7 @@ int p2m_set_mem_access(struct domain *d,
         p2m_access_wx,
         p2m_access_rwx,
         p2m_access_rx2rw,
+        p2m_access_n2rwx,
         p2m->default_access,
     };
 
diff -r e315ce73f082 -r 66ca1a02082f xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -108,6 +108,9 @@ typedef enum {
     p2m_access_wx    = 6, 
     p2m_access_rwx   = 7,
     p2m_access_rx2rw = 8, /* Special: page goes from RX to RW on write */
+    p2m_access_n2rwx = 9, /* Special: page goes from N to RWX on access, *
+                           * generates an event but does not pause the
+                           * vcpu */
 
     /* NOTE: Assumed to be only 4 bits right now */
 } p2m_access_t;
diff -r e315ce73f082 -r 66ca1a02082f xen/include/public/hvm/hvm_op.h
--- a/xen/include/public/hvm/hvm_op.h
+++ b/xen/include/public/hvm/hvm_op.h
@@ -174,6 +174,9 @@ typedef enum {
     HVMMEM_access_rwx,
     HVMMEM_access_rx2rw,       /* Page starts off as r-x, but automatically
                                 * change to r-w on a write */
+    HVMMEM_access_n2rwx,       /* Log access: starts off as n, automatically 
+                                * goes to rwx, generating an event without
+                                * pausing the vcpu */
     HVMMEM_access_default      /* Take the domain default */
 } hvmmem_access_t;
 /* Notify that a region of memory is to have specific access types */

At 16:04 -0500 on 06 Dec (1323187469), Andres Lagar-Cavilla
wrote:
> We improve the handling of hap faults when both type and access
> restrictions are present.
> 
> We also add a new p2m access type, n2rwx. It allows for implement a
"log
> access" mode in the hypervisor, aking to log dirty but for all types
of
> accesses. Faults caused by this access mode automatically promote the
> access rights of the ofending p2m entry, place the event in the ring, and
> let the vcpu keep on executing.
> 
> Rebased to apply cleanly on top of 537ceb11d51e.
Applied, thanks.

Tim.