WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] cleanup ept_set_entry

To: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] cleanup ept_set_entry
From: "Li, Xin" <xin.li@xxxxxxxxx>
Date: Thu, 1 Jul 2010 20:42:24 +0800
Accept-language: zh-CN, en-US
Acceptlanguage: zh-CN, en-US
Delivery-date: Thu, 01 Jul 2010 05:43:28 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcsZGtsT18+1SsIOTHOVKvGb+swPXw==
Thread-topic: [PATCH] cleanup ept_set_entry
VMX: cleanup ept_set_entry:
1) more strict input parameters checking;
2) better comments;
3) fewer variable;
4) change direct_mmio type to bool_t.

Signed-off-by: Xin Li <xin.li@xxxxxxxxx>

diff -r aecf092da748 xen/arch/x86/hvm/mtrr.c
--- a/xen/arch/x86/hvm/mtrr.c   Wed Jun 30 22:12:54 2010 +0100
+++ b/xen/arch/x86/hvm/mtrr.c   Fri Jul 02 04:24:31 2010 +0800
@@ -707,7 +707,7 @@
                           1, HVMSR_PER_VCPU);
 
 uint8_t epte_get_entry_emt(struct domain *d, unsigned long gfn, mfn_t mfn,
-                           uint8_t *ipat, int direct_mmio)
+                           uint8_t *ipat, bool_t direct_mmio)
 {
     uint8_t gmtrr_mtype, hmtrr_mtype;
     uint32_t type;
diff -r aecf092da748 xen/arch/x86/mm/hap/p2m-ept.c
--- a/xen/arch/x86/mm/hap/p2m-ept.c     Wed Jun 30 22:12:54 2010 +0100
+++ b/xen/arch/x86/mm/hap/p2m-ept.c     Fri Jul 02 04:24:31 2010 +0800
@@ -229,33 +229,41 @@
  * by observing whether any gfn->mfn translations are modified.
  */
 static int
-ept_set_entry(struct domain *d, unsigned long gfn, mfn_t mfn, 
+ept_set_entry(struct domain *d, unsigned long gfn, mfn_t mfn,
               unsigned int order, p2m_type_t p2mt)
 {
-    ept_entry_t *table = NULL;
+    ept_entry_t *table, *ept_entry;
     unsigned long gfn_remainder = gfn;
-    unsigned long offset = 0;
-    ept_entry_t *ept_entry = NULL;
+    unsigned long offset;
     u32 index;
-    int i;
+    int i, target = order / EPT_TABLE_ORDER;
     int rv = 0;
     int ret = 0;
-    int split_level = 0;
-    int walk_level = order / EPT_TABLE_ORDER;
-    int direct_mmio = (p2mt == p2m_mmio_direct);
+    bool_t direct_mmio = (p2mt == p2m_mmio_direct);
     uint8_t ipat = 0;
     int need_modify_vtd_table = 1;
     int needs_sync = 1;
 
-    if (  order != 0 )
-        if ( (gfn & ((1UL << order) - 1)) )
-            return 1;
+    /*
+     * the caller must make sure:
+     * 1. passing valid gfn and mfn at order boundary.
+     * 2. gfn not exceeding guest physical address width.
+     * 3. passing a valid order.
+     */
+    if ( ((gfn | mfn_x(mfn)) & ((1UL << order) - 1)) ||
+         (gfn >> ((ept_get_wl(d) + 1) * EPT_TABLE_ORDER)) ||
+         (order % EPT_TABLE_ORDER) )
+        return 0;
+
+    ASSERT((target == 2 && hvm_hap_has_1gb(d)) ||
+           (target == 1 && hvm_hap_has_2mb(d)) ||
+           (target == 0));
 
     table = map_domain_page(ept_get_asr(d));
 
     ASSERT(table != NULL);
 
-    for ( i = ept_get_wl(d); i > walk_level; i-- )
+    for ( i = ept_get_wl(d); i > target; i-- )
     {
         ret = ept_next_level(d, 0, &table, &gfn_remainder, i * 
EPT_TABLE_ORDER);
         if ( !ret )
@@ -264,21 +272,23 @@
             break;
     }
 
-    /* If order == 0, we should only get POD if we have a POD superpage.
-     * If i > walk_level, we need to split the page; otherwise,
-     * just behave as normal. */
-    ASSERT(ret != GUEST_TABLE_POD_PAGE || i != walk_level);
+    ASSERT(ret != GUEST_TABLE_POD_PAGE || i != target);
 
-    index = gfn_remainder >> ( i ?  (i * EPT_TABLE_ORDER): order);
-    offset = (gfn_remainder & ( ((1 << (i*EPT_TABLE_ORDER)) - 1)));
-
-    split_level = i;
+    index = gfn_remainder >> (i * EPT_TABLE_ORDER);
+    gfn_remainder &= (1UL << (i * EPT_TABLE_ORDER)) - 1;
 
     ept_entry = table + index;
 
-    if ( i == walk_level )
+    offset = gfn_remainder;
+
+    /*
+     * When we are here, we must be on a leaf ept entry
+     * with i == target or i > target.
+     */
+
+    if ( i == target )
     {
-        /* We reached the level we're looking for */
+        /* We reached the target level. */
 
         /* No need to flush if the old entry wasn't valid */
         if ( !is_epte_present(ept_entry) )
@@ -307,12 +317,14 @@
     }
     else
     {
-        int level;
+        /* We need to split the original page. */
         ept_entry_t *split_ept_entry;
 
-        for ( level = split_level; level > walk_level ; level-- )
+        ASSERT(is_epte_superpage(ept_entry));
+
+        for ( ; i > target; i-- )
         {
-            rv = ept_split_large_page(d, &table, &index, gfn, level);
+            rv = ept_split_large_page(d, &table, &index, gfn, i);
             if ( !rv )
                 goto out;
         }
@@ -559,7 +571,7 @@
     return ept_get_entry(current->domain, gfn, t, q);
 }
 
-/* 
+/*
  * To test if the new emt type is the same with old,
  * return 1 to not to reset ept entry.
  */
@@ -569,14 +581,14 @@
 {
     uint8_t ipat;
     uint8_t emt;
-    int direct_mmio = (p2mt == p2m_mmio_direct);
+    bool_t direct_mmio = (p2mt == p2m_mmio_direct);
 
     emt = epte_get_entry_emt(d, gfn, mfn, &ipat, direct_mmio);
 
     if ( (emt == o_emt) && (ipat == o_ipat) )
         return 0;
 
-    return 1; 
+    return 1;
 }
 
 void ept_change_entry_emt_with_range(struct domain *d, unsigned long start_gfn,
diff -r aecf092da748 xen/include/asm-x86/mtrr.h
--- a/xen/include/asm-x86/mtrr.h        Wed Jun 30 22:12:54 2010 +0100
+++ b/xen/include/asm-x86/mtrr.h        Fri Jul 02 04:24:31 2010 +0800
@@ -65,7 +65,7 @@
 extern u32 get_pat_flags(struct vcpu *v, u32 gl1e_flags, paddr_t gpaddr,
                   paddr_t spaddr, uint8_t gmtrr_mtype);
 extern uint8_t epte_get_entry_emt(struct domain *d, unsigned long gfn,
-                                  mfn_t mfn, uint8_t *ipat, int direct_mmio);
+                                  mfn_t mfn, uint8_t *ipat, bool_t 
direct_mmio);
 extern void ept_change_entry_emt_with_range(
     struct domain *d, unsigned long start_gfn, unsigned long end_gfn);
 extern unsigned char pat_type_2_pte_flags(unsigned char pat_type);

Attachment: ept_set_entry_cleanup.patch
Description: ept_set_entry_cleanup.patch

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH] cleanup ept_set_entry, Li, Xin <=