Re: [Xen-devel] [PATCH] EPT: refine epte_present test

[Tim Deegan <tim@xxxxxxx>]

Hi, 

I agree with the idea of this patch but: 

> +static inline bool_t is_epte_valid(ept_entry_t *e)
> +{
> +    ept_entry_t rights = { .epte = 0 };
> +
> +    /* Not valid if empty */
> +    if (e->epte == 0) return 0;
> +
> +    /* Not valid if mfn not valid */
> +    if ( !mfn_valid(e->mfn) ) return 0;
> +
> +    /* Not valid if rights different from those of 
> +     * its p2m type and access combination */
> +    ept_p2m_type_to_flags(&rights, e->sa_p2mt, e->access);
> +    if ( epte_permissions(&rights) != epte_permissions(e) )
> +        return 0;

This is a rather odd set of tests.  Since we construct EPT pagetables
ourselves, I think that we only need to test for uninitialised entries
and explicitly invalid entries.

Does this patch work for you?

diff -r f5b366c6c4c6 xen/arch/x86/mm/p2m-ept.c
--- a/xen/arch/x86/mm/p2m-ept.c Thu Jan 19 10:42:42 2012 +0000
+++ b/xen/arch/x86/mm/p2m-ept.c Thu Jan 19 11:01:34 2012 +0000
@@ -41,6 +41,10 @@
 
 #define is_epte_present(ept_entry)      ((ept_entry)->epte & 0x7)
 #define is_epte_superpage(ept_entry)    ((ept_entry)->sp)
+static inline bool_t is_epte_valid(ept_entry_t *e)
+{
+    return (e->epte != 0 && e->sa_p2mt != p2m_invalid);
+}
 
 /* Non-ept "lock-and-check" wrapper */
 static int ept_pod_check_and_populate(struct p2m_domain *p2m, unsigned long 
gfn,
@@ -777,7 +781,7 @@ static void ept_change_entry_type_page(m
 
     for ( int i = 0; i < EPT_PAGETABLE_ENTRIES; i++ )
     {
-        if ( !is_epte_present(epte + i) )
+        if ( !is_epte_valid(epte + i) )
             continue;
 
         if ( (ept_page_level > 0) && !is_epte_superpage(epte + i) )

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel