[PATCH]: Prevent in-sync L1s to become writable (was: Re: [Xen-devel] Xen crash with latest 3.3 bits)

[Gianluca Guida <gianluca.guida@xxxxxxxxxxxxx>]

Hi,

Gianluca Guida wrote:
> John Levon wrote:
> > On Tue, Mar 10, 2009 at 04:23:25PM +0000, Gianluca Guida wrote:
> >
> > > > Some time after starting an 64-bit SMP Solaris 10 domain (HVM with PV 
> > > > drivers),
> > > > I get the below crash. Any ideas?
> > > This should be fixed by xen-unstable.hg's changeset 18806, named 
> > > "shadow: fix race between resync and page promotion".
> > > It should be in xen-3.3.hg, can't check right now because xenbits web 
> > > page seems to be dead/slow.
> > I have the patch you're referring to, but still see the problem. It's
> > pretty rare and only seems to have happened with S10 SMP so far...
> Ah, interesting. I'll look into it.
Meanwhile, Christian Limpach was hit by the same bug and fixed it.

Versions for Xen-3.3 and unstable attached.

Gianluca

diff -r b249f3e979a5 xen/arch/x86/mm/shadow/multi.c
--- a/xen/arch/x86/mm/shadow/multi.c    Mon Mar 09 10:32:24 2009 +0000
+++ b/xen/arch/x86/mm/shadow/multi.c    Tue Mar 17 12:45:56 2009 +0000
@@ -3112,6 +3112,19 @@ static int sh_page_fault(struct vcpu *v,
     shadow_lock(d);
 
     TRACE_CLEAR_PATH_FLAGS;
+
+    /* Make sure there is enough free shadow memory to build a chain of
+     * shadow tables. (We never allocate a top-level shadow on this path,
+     * only a 32b l1, pae l1, or 64b l3+2+1. Note that while
+     * SH_type_l1_shadow isn't correct in the latter case, all page
+     * tables are the same size there.)
+     *
+     * Preallocate shadow pages *before* removing writable accesses
+     * otherwhise an OOS L1 might be demoted and promoted again with
+     * writable mappings. */
+    shadow_prealloc(d,
+                    SH_type_l1_shadow,
+                    GUEST_PAGING_LEVELS < 4 ? 1 : GUEST_PAGING_LEVELS - 1);
     
     rc = gw_remove_write_accesses(v, va, &gw);
 
@@ -3144,15 +3157,6 @@ static int sh_page_fault(struct vcpu *v,
 
     shadow_audit_tables(v);
     sh_audit_gw(v, &gw);
-
-    /* Make sure there is enough free shadow memory to build a chain of
-     * shadow tables. (We never allocate a top-level shadow on this path,
-     * only a 32b l1, pae l1, or 64b l3+2+1. Note that while
-     * SH_type_l1_shadow isn't correct in the latter case, all page
-     * tables are the same size there.) */
-    shadow_prealloc(d,
-                    SH_type_l1_shadow,
-                    GUEST_PAGING_LEVELS < 4 ? 1 : GUEST_PAGING_LEVELS - 1);
 
     /* Acquire the shadow.  This must happen before we figure out the rights 
      * for the shadow entry, since we might promote a page here. */

diff -r 587e81dd3540 xen/arch/x86/mm/shadow/multi.c
--- a/xen/arch/x86/mm/shadow/multi.c    Mon Mar 02 14:19:35 2009 +0000
+++ b/xen/arch/x86/mm/shadow/multi.c    Tue Mar 17 12:31:10 2009 +0000
@@ -3257,6 +3257,19 @@ static int sh_page_fault(struct vcpu *v,
 
     shadow_lock(d);
 
+    /* Make sure there is enough free shadow memory to build a chain of
+     * shadow tables. (We never allocate a top-level shadow on this path,
+     * only a 32b l1, pae l1, or 64b l3+2+1. Note that while
+     * SH_type_l1_shadow isn't correct in the latter case, all page
+     * tables are the same size there.)
+     *
+     * Preallocate shadow pages *before* removing writable accesses
+     * otherwhise an OOS L1 might be demoted and promoted again with
+     * writable mappings. */
+    shadow_prealloc(d,
+                    SH_type_l1_shadow,
+                    GUEST_PAGING_LEVELS < 4 ? 1 : GUEST_PAGING_LEVELS - 1);
+    
     rc = gw_remove_write_accesses(v, va, &gw);
 
     /* First bit set: Removed write access to a page. */
@@ -3288,15 +3301,6 @@ static int sh_page_fault(struct vcpu *v,
 
     shadow_audit_tables(v);
     sh_audit_gw(v, &gw);
-
-    /* Make sure there is enough free shadow memory to build a chain of
-     * shadow tables. (We never allocate a top-level shadow on this path,
-     * only a 32b l1, pae l1, or 64b l3+2+1. Note that while
-     * SH_type_l1_shadow isn't correct in the latter case, all page
-     * tables are the same size there.) */
-    shadow_prealloc(d,
-                    SH_type_l1_shadow,
-                    GUEST_PAGING_LEVELS < 4 ? 1 : GUEST_PAGING_LEVELS - 1);
 
     /* Acquire the shadow.  This must happen before we figure out the rights 
      * for the shadow entry, since we might promote a page here. */

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