[PATCH 7/8] HVM save restore: shadow fix

[PATCH 7/8] HVM save restore: shadow fix

[Zhai, Edwin]

[PATCH 7/8] HVM save restore: shadow fix

Signed-off-by: Zhai Edwin <edwin.zhai@intel.com>

fix 2 shadow issues:
* restored guest's mem type_info are not correct
* remove annoying warnings when restore

enable smp:
turn on the vcpu when set_vcpu_context in restore as cpu hotplug is not feasible for HVM

diff -r b8fd7c5be4eb xen/arch/x86/mm/shadow/common.c
--- a/xen/arch/x86/mm/shadow/common.c	Tue Jan 09 16:44:52 2007 +0800
+++ b/xen/arch/x86/mm/shadow/common.c	Tue Jan 09 16:44:52 2007 +0800
@@ -2176,7 +2176,7 @@ int sh_remove_all_mappings(struct vcpu *
         /* Don't complain if we're in HVM and there's one extra mapping: 
          * The qemu helper process has an untyped mapping of this dom's RAM */
         if ( !(shadow_mode_external(v->domain)
-               && (page->count_info & PGC_count_mask) <= 2
+               && (page->count_info & PGC_count_mask) <= 3 /* vmx restore add one extra mapping*/
                && (page->u.inuse.type_info & PGT_count_mask) == 0) )
         {
             SHADOW_ERROR("can't find all mappings of mfn %lx: "
diff -r b8fd7c5be4eb xen/arch/x86/mm/shadow/multi.c
--- a/xen/arch/x86/mm/shadow/multi.c	Tue Jan 09 16:44:52 2007 +0800
+++ b/xen/arch/x86/mm/shadow/multi.c	Tue Jan 09 16:44:52 2007 +0800
@@ -1630,6 +1630,14 @@ sh_make_shadow(struct vcpu *v, mfn_t gmf
         }
     }
     
+    {
+        struct page_info *page = mfn_to_page(gmfn);
+        /* XXX: add it to emulate a touched page */
+        if ((page->u.inuse.type_info & PGT_type_mask) == PGT_none){
+            page->u.inuse.type_info |= (PGT_writable_page | PGT_validated);
+        }
+    }
+
     shadow_promote(v, gmfn, shadow_type);
     set_shadow_status(v, gmfn, shadow_type, smfn);
 
diff -r b8fd7c5be4eb xen/common/domain.c
--- a/xen/common/domain.c	Tue Jan 09 16:44:52 2007 +0800
+++ b/xen/common/domain.c	Tue Jan 09 16:44:52 2007 +0800
@@ -24,6 +24,7 @@
 #include <xen/percpu.h>
 #include <xen/multicall.h>
 #include <asm/debugger.h>
+#include <asm/hvm/support.h>
 #include <public/sched.h>
 #include <public/vcpu.h>
 #ifdef CONFIG_COMPAT
@@ -505,6 +506,14 @@ int set_info_guest(struct domain *d,
     if ( rc == 0 )
         rc = arch_set_info_guest(v, c);
 
+    /*XXX: hvm smp guest restore support */
+    if ( rc == 0 &&
+            v->vcpu_id != 0 &&
+            is_hvm_vcpu(v) &&
+            test_and_clear_bit(_VCPUF_down, &v->vcpu_flags) ) {
+        vcpu_wake(v);
+    }
+
     domain_unpause(d);
 
     xfree(c.nat);

Re: [PATCH 7/8] HVM save restore: shadow fix

[Tim Deegan]

At 22:12 +0800 on 11 Jan (1168553551), Zhai, Edwin wrote:
> [PATCH 7/8] HVM save restore: shadow fix
> 
> Signed-off-by: Zhai Edwin <edwin.zhai@intel.com>
> 
> fix 2 shadow issues:
> * restored guest's mem type_info are not correct

In what way?  And why are you fixing that in the shadow
pagetable-promotion path rather than the restore path?  
If the pagetable pages are wrong, are other pages wrong too?

> +    {
> +        struct page_info *page = mfn_to_page(gmfn);
> +        /* XXX: add it to emulate a touched page */
> +        if ((page->u.inuse.type_info & PGT_type_mask) == PGT_none){
> +            page->u.inuse.type_info |= (PGT_writable_page | PGT_validated);
> +        }
> +    }
> +

Should this be gated to only happen to HVM guests?

Cheers,

Tim.

Re: [PATCH 7/8] HVM save restore: shadow fix

[Zhai, Edwin]

On Thu, Jan 18, 2007 at 04:31:13PM +0000, Tim Deegan wrote:
> At 22:12 +0800 on 11 Jan (1168553551), Zhai, Edwin wrote:
> > [PATCH 7/8] HVM save restore: shadow fix
> > 
> > Signed-off-by: Zhai Edwin <edwin.zhai@intel.com>
> > 
> > fix 2 shadow issues:
> > * restored guest's mem type_info are not correct
> 
> In what way?  And why are you fixing that in the shadow
> pagetable-promotion path rather than the restore path?  
> If the pagetable pages are wrong, are other pages wrong too?
> 

my comments are not accurate. long time ago i have fixed this bug that happen 
when restore. pls. correct me if something wrong. 

sh_make_shadow=>shadow_promote=>sh_remove_write_access would promote a shadow 
page and crash domain if type_info has no PGT_writable_page set. 

comments as: /* If this isn't a "normal" writeable page, the domain is trying to 
* put pagetables in special memory of some kind.  We can't allow that. */

at that time, HVM restore always reach this bug point, while creation is okay.
it's reasonable since created guest would access the page before promotion, but 
restored guest always start with "untouched" page.

now, this bug seems disappeared, maybe fixed by new shadow changes:)
we can revisit it in future.

> > +    {
> > +        struct page_info *page = mfn_to_page(gmfn);
> > +        /* XXX: add it to emulate a touched page */
> > +        if ((page->u.inuse.type_info & PGT_type_mask) == PGT_none){
> > +            page->u.inuse.type_info |= (PGT_writable_page | PGT_validated);
> > +        }
> > +    }
> > +
> 
> Should this be gated to only happen to HVM guests?
> 
> Cheers,
> 
> Tim.
> 

-- 
best rgds,
edwin