[Xen-devel] [PATCH] cleanup ept_set_entry

["Li, Xin" <xin.li@xxxxxxxxx>]

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