Re: [Xen-devel] [PATCH v2 09/12] x86/altp2m: add remaining support routines.

["Lengyel, Tamas" <tlengyel@xxxxxxxxxxx>]

On Mon, Jun 22, 2015 at 2:56 PM, Ed White <edmund.h.white@xxxxxxxxx> wrote:

Add the remaining routines required to support enabling the alternate
p2m functionality.

Signed-off-by: Ed White <edmund.h.white@xxxxxxxxx>
---
 xen/arch/x86/hvm/hvm.c              |  60 +++++-
 xen/arch/x86/mm/hap/Makefile        |   1 +
 xen/arch/x86/mm/hap/altp2m_hap.c    | 103 +++++++++
 xen/arch/x86/mm/p2m-ept.c           |   3 +
 xen/arch/x86/mm/p2m.c               | 405 ++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/hvm/altp2mhvm.h |   4 +
 xen/include/asm-x86/p2m.h           |  33 +++
 7 files changed, 601 insertions(+), 8 deletions(-)
 create mode 100644 xen/arch/x86/mm/hap/altp2m_hap.c

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index d75c12d..b758ee1 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2786,10 +2786,11 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
     p2m_access_t p2ma;
     mfn_t mfn;
     struct vcpu *v = current;
-    struct p2m_domain *p2m;
+    struct p2m_domain *p2m, *hostp2m;
     int rc, fall_through = 0, paged = 0;
     int sharing_enomem = 0;
     vm_event_request_t *req_ptr = NULL;
+    int altp2m_active = 0;

     /* On Nested Virtualization, walk the guest page table.
      * If this succeeds, all is fine.
@@ -2845,15 +2846,33 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
     {
         if ( !handle_mmio_with_translation(gla, gpa >> PAGE_SHIFT, npfec) )
             hvm_inject_hw_exception(TRAP_gp_fault, 0);
-        rc = 1;
-        goto out;
+        return 1;
     }

-    p2m = p2m_get_hostp2m(v->domain);
-    mfn = get_gfn_type_access(p2m, gfn, &p2mt, &p2ma,
+    altp2m_active = altp2mhvm_active(v->domain);
+
+    /* Take a lock on the host p2m speculatively, to avoid potential
+     * locking order problems later and to handle unshare etc.
+     */
+    hostp2m = p2m_get_hostp2m(v->domain);
+    mfn = get_gfn_type_access(hostp2m, gfn, &p2mt, &p2ma,
                               P2M_ALLOC | (npfec.write_access ? P2M_UNSHARE : 0),
                               NULL);

+    if ( altp2m_active )
+    {
+        if ( altp2mhvm_hap_nested_page_fault(v, gpa, gla, npfec, &p2m) == 1 )
+        {
+            /* entry was lazily copied from host -- retry */

So I'm not fully following this logic here. I can see that the altp2m entry got copied from the host. Why is there a need for the retry, why not just continue?

 

+            __put_gfn(hostp2m, gfn);
+            return 1;
+        }
+
+        mfn = get_gfn_type_access(p2m, gfn, &p2mt, &p2ma, 0, NULL);
+    }
+    else
+        p2m = hostp2m;
+
     /* Check access permissions first, then handle faults */
     if ( mfn_x(mfn) != INVALID_MFN )
     {

--

Tamas K Lengyel

Senior Security Researcher

7921 Jones Branch Drive

McLean VA 22102

Email  tlengyel@novetta.com

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