All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH][RFC]Provide fast write emulation path to release shadow lock
@ 2008-01-22  2:20 Tian, Kevin
  2008-01-22  9:26 ` Tim Deegan
  0 siblings, 1 reply; 17+ messages in thread
From: Tian, Kevin @ 2008-01-22  2:20 UTC (permalink / raw)
  To: Tim Deegan; +Cc: xen-devel

[-- Attachment #1: Type: text/plain, Size: 1700 bytes --]

Provide fast write emulation path to release shadow lock.

Basically we can consider shadow fault logic into two parts,
with 1st part to cover logistic work like validating guest
page table or fix shadow table, and the 2nd part for write
emulation.

However there's one scenario we can optimize to skip the
1st part. For previous successfully emulated virtual frame,
it's very likely approaching at write emulation logic again
if next adjacent shadow fault is hitting same virtual frame.
It's wasteful to re-walk 1st part which is already covered
by last shadow fault. In this case, actually we can jump to
emulation code early, without any lock acquisition until
final shadow validation for write emulation. By perfc counts
on 64bit SMP HVM guest, 89% of total shadow write emulation
are observed falling into this fast path when doing kernel
build in guest.

We also did series of tests on 32/32pae/32e: (host is 32e)
                   32       32pae       32e
----Linux----
kernel build   +1%    +0.86%    +1.9%
Specjbb        +0.9% +1.61%    +0.32%

----XP----
Sysbench     N/A     -0.05%     -0.32%(*)

* Sysbench score is not very stable on 32e guest, with up
to 6% variation observed in 5 rounds running. 32pae is
stable. 32 XP image was unfortunately corrupted at test 
cycle, so not test yet. Don't want to hold here from getting
early comments. :-)

I thought the performance gain should be straightforward
with this patch, and thus would like to know comment
like:
	- Is it a right direction?
	- Is there anything wrong or missed in patch?
	- Any more benchmarks should we test?

Signed-off-by Kevin Tian <kevin.tian@intel.com>

Thanks,
Kevin

[-- Attachment #2: fast_emul.patch --]
[-- Type: application/octet-stream, Size: 11602 bytes --]

Provide fast write emulation path to release shadow lock.

Basically we can consider shadow fault logic into two parts,
with 1st part to cover logistic work like validating guest
page table or fix shadow table, and the 2nd part for write
emulation.

However there's one scenario we can optimize to skip the
1st part. For previous successfully emulated virtual frame,
it's very likely approaching at write emulation logic again
if next adjacent shadow fault is hitting same virtual frame.
It's wasteful to re-walk 1st part which is already covered
by last shadow fault. In this case, actually we can jump to
emulation code early, without any lock acquisition until
final shadow validation for write emulation. By perfc counts
on 64bit SMP HVM guest, 89% of total shadow write emulation
are observed falling into this fast path when doing kernel
build in guest.

Signed-off-by Kevin Tian <kevin.tian@intel.com>

diff -r 64088e89251d xen/arch/x86/mm/shadow/common.c
--- a/xen/arch/x86/mm/shadow/common.c	Tue Jan 15 04:49:23 2008 +0800
+++ b/xen/arch/x86/mm/shadow/common.c	Wed Jan 16 05:29:43 2008 +0800
@@ -1017,13 +1017,18 @@ void shadow_free(struct domain *d, mfn_t
 
     for ( i = 0; i < 1<<order; i++ ) 
     {
-#if SHADOW_OPTIMIZATIONS & SHOPT_WRITABLE_HEURISTIC
+#if SHADOW_OPTIMIZATIONS & (SHOPT_WRITABLE_HEURISTIC | SHOPT_FAST_EMULATION)
         struct vcpu *v;
         for_each_vcpu(d, v) 
         {
+#if SHADOW_OPTIMIZATIONS & SHOPT_WRITABLE_HEURISTIC
             /* No longer safe to look for a writeable mapping in this shadow */
             if ( v->arch.paging.shadow.last_writeable_pte_smfn == mfn_x(smfn) + i ) 
                 v->arch.paging.shadow.last_writeable_pte_smfn = 0;
+#endif
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+            v->arch.paging.last_write_emul_ok = 0;
+#endif
         }
 #endif
         /* Strip out the type: this is now a free shadow page */
diff -r 64088e89251d xen/arch/x86/mm/shadow/multi.c
--- a/xen/arch/x86/mm/shadow/multi.c	Tue Jan 15 04:49:23 2008 +0800
+++ b/xen/arch/x86/mm/shadow/multi.c	Tue Jan 22 09:41:11 2008 +0800
@@ -2594,8 +2594,8 @@ static inline void check_for_early_unsha
 static inline void check_for_early_unshadow(struct vcpu *v, mfn_t gmfn)
 {
 #if SHADOW_OPTIMIZATIONS & SHOPT_EARLY_UNSHADOW
-    if ( v->arch.paging.shadow.last_emulated_mfn == mfn_x(gmfn) &&
-         sh_mfn_is_a_page_table(gmfn) )
+    if ( v->arch.paging.shadow.last_emulated_mfn_for_unshadow == mfn_x(gmfn)
+         && sh_mfn_is_a_page_table(gmfn) )
     {
         u32 flags = mfn_to_page(gmfn)->shadow_flags;
         if ( !(flags & (SHF_L2_32|SHF_L2_PAE|SHF_L2H_PAE|SHF_L4_64)) )
@@ -2604,7 +2604,7 @@ static inline void check_for_early_unsha
             sh_remove_shadows(v, gmfn, 1, 0 /* Fast, can fail to unshadow */ );
         } 
     }
-    v->arch.paging.shadow.last_emulated_mfn = mfn_x(gmfn);
+    v->arch.paging.shadow.last_emulated_mfn_for_unshadow = mfn_x(gmfn);
 #endif
 }
 
@@ -2612,7 +2612,7 @@ static inline void reset_early_unshadow(
 static inline void reset_early_unshadow(struct vcpu *v)
 {
 #if SHADOW_OPTIMIZATIONS & SHOPT_EARLY_UNSHADOW
-    v->arch.paging.shadow.last_emulated_mfn = INVALID_MFN;
+    v->arch.paging.shadow.last_emulated_mfn_for_unshadow = INVALID_MFN;
 #endif
 }
 
@@ -2720,12 +2720,40 @@ static int sh_page_fault(struct vcpu *v,
     int r;
     fetch_type_t ft = 0;
     p2m_type_t p2mt;
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+    int fast_emul = 0;
+#endif
 
     SHADOW_PRINTK("d:v=%u:%u va=%#lx err=%u, rip=%lx\n",
                   v->domain->domain_id, v->vcpu_id, va, regs->error_code,
                   regs->rip);
 
     perfc_incr(shadow_fault);
+
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+    /* If faulting virtual frame is successfully emulated in closely last
+     * shadow fault, it's highly likely to reach same emulation action
+     * for same virtual frame. Try to emulate early which can release
+     * shadow lock further.
+     */
+    if ( v->arch.paging.last_write_emul_ok
+         && v->arch.paging.shadow.last_emulated_vfn == (va >> PAGE_SHIFT) ) 
+    {
+        /* check whether error code is 3, or else fall back to normal path
+         * in case of some validation is required
+         */
+        if ( regs->error_code == (PFEC_write_access | PFEC_page_present) )
+        {
+            fast_emul = 1;
+            gmfn = v->arch.paging.shadow.last_emulated_mfn;
+            perfc_incr(shadow_fault_fast_emulate);
+            goto early_emulation;
+        }
+        else
+            v->arch.paging.last_write_emul_ok = 0;
+    }
+#endif
+
     //
     // XXX: Need to think about eventually mapping superpages directly in the
     //      shadow (when possible), as opposed to splintering them into a
@@ -2936,6 +2964,17 @@ static int sh_page_fault(struct vcpu *v,
         goto done;
     }
 
+    /*
+     * We don't need to hold the lock for the whole emulation; we will
+     * take it again when we write to the pagetables.
+     */
+    sh_audit_gw(v, &gw);
+    shadow_audit_tables(v);
+    shadow_unlock(d);
+
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+ early_emulation:
+#endif
     if ( is_hvm_domain(d) )
     {
         /*
@@ -2947,6 +2986,13 @@ static int sh_page_fault(struct vcpu *v,
          */
         if ( unlikely(hvm_event_pending(v)) )
         {
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+            if ( fast_emul )
+            {
+                perfc_incr(shadow_fault_fast_emulate_fail);
+                v->arch.paging.last_write_emul_ok = 0;
+            }
+#endif
             gdprintk(XENLOG_DEBUG, "write to pagetable during event "
                      "injection: cr2=%#lx, mfn=%#lx\n", 
                      va, mfn_x(gmfn));
@@ -2957,14 +3003,6 @@ static int sh_page_fault(struct vcpu *v,
 
     SHADOW_PRINTK("emulate: eip=%#lx esp=%#lx\n", 
                   (unsigned long)regs->eip, (unsigned long)regs->esp);
-
-    /*
-     * We don't need to hold the lock for the whole emulation; we will
-     * take it again when we write to the pagetables.
-     */
-    sh_audit_gw(v, &gw);
-    shadow_audit_tables(v);
-    shadow_unlock(d);
 
     emul_ops = shadow_init_emulation(&emul_ctxt, regs);
 
@@ -2979,14 +3017,43 @@ static int sh_page_fault(struct vcpu *v,
      */
     if ( r == X86EMUL_UNHANDLEABLE )
     {
+        perfc_incr(shadow_fault_emulate_failed);
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+        if ( fast_emul )
+        {
+            perfc_incr(shadow_fault_fast_emulate_fail);
+            v->arch.paging.last_write_emul_ok = 0;
+        }
+#endif
         SHADOW_PRINTK("emulator failure, unshadowing mfn %#lx\n", 
                        mfn_x(gmfn));
-        perfc_incr(shadow_fault_emulate_failed);
         /* If this is actually a page table, then we have a bug, and need 
          * to support more operations in the emulator.  More likely, 
          * though, this is a hint that this page should not be shadowed. */
         shadow_remove_all_shadows(v, gmfn);
     }
+
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+    /* Record successfully emulated information as heuristics to next
+     * fault on same virtual frame for acceleration. But be careful
+     * to verify its attribute still as page table, or else unshadow
+     * triggered in write emulation normally requires a re-sync with
+     * guest page table to recover r/w permission. Incorrect record
+     * for such case will cause unexpected more shadow faults due to
+     * propagation is skipped.
+     */
+    if ( (r == X86EMUL_OKAY) && sh_mfn_is_a_page_table(gmfn) )
+    {
+        if ( !fast_emul )
+        {
+            v->arch.paging.shadow.last_emulated_vfn = va >> PAGE_SHIFT;
+            v->arch.paging.shadow.last_emulated_mfn = gmfn;
+            v->arch.paging.last_write_emul_ok = 1;
+        }
+    }
+    else if ( fast_emul )
+        v->arch.paging.last_write_emul_ok = 0;
+#endif
 
 #if GUEST_PAGING_LEVELS == 3 /* PAE guest */
     if ( r == X86EMUL_OKAY ) {
@@ -3057,6 +3124,10 @@ sh_invlpg(struct vcpu *v, unsigned long 
 #if (SHADOW_OPTIMIZATIONS & SHOPT_VIRTUAL_TLB)
     /* No longer safe to use cached gva->gfn translations */
     vtlb_flush(v);
+#endif
+
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+    v->arch.paging.last_write_emul_ok = 0;
 #endif
 
     /* First check that we can safely read the shadow l2e.  SMP/PAE linux can
@@ -3786,6 +3857,10 @@ sh_update_cr3(struct vcpu *v, int do_loc
     vtlb_flush(v);
 #endif
 
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+    v->arch.paging.last_write_emul_ok = 0;
+#endif
+
     /* Release the lock, if we took it (otherwise it's the caller's problem) */
     if ( do_locking ) shadow_unlock(v->domain);
 }
diff -r 64088e89251d xen/arch/x86/mm/shadow/private.h
--- a/xen/arch/x86/mm/shadow/private.h	Tue Jan 15 04:49:23 2008 +0800
+++ b/xen/arch/x86/mm/shadow/private.h	Wed Jan 16 05:38:01 2008 +0800
@@ -62,8 +62,9 @@ extern int shadow_audit_enable;
 #define SHOPT_LINUX_L3_TOPLEVEL   0x10  /* Pin l3es on early 64bit linux */
 #define SHOPT_SKIP_VERIFY         0x20  /* Skip PTE v'fy when safe to do so */
 #define SHOPT_VIRTUAL_TLB         0x40  /* Cache guest v->p translations */
-
-#define SHADOW_OPTIMIZATIONS      0x7f
+#define SHOPT_FAST_EMULATION      0x80  /* Fast write emulation */
+
+#define SHADOW_OPTIMIZATIONS      0xff
 
 
 /******************************************************************************
diff -r 64088e89251d xen/include/asm-x86/domain.h
--- a/xen/include/asm-x86/domain.h	Tue Jan 15 04:49:23 2008 +0800
+++ b/xen/include/asm-x86/domain.h	Wed Jan 16 05:02:08 2008 +0800
@@ -108,10 +108,14 @@ struct shadow_vcpu {
 #endif
     /* Non-PAE guests: pointer to guest top-level pagetable */
     void *guest_vtable;
-    /* Last MFN that we emulated a write to. */
-    unsigned long last_emulated_mfn;
+    /* Last MFN that we emulated a write to as unshadow heuristics. */
+    unsigned long last_emulated_mfn_for_unshadow;
     /* MFN of the last shadow that we shot a writeable mapping in */
     unsigned long last_writeable_pte_smfn;
+    /* Last virtual frame number (VFN) that we emulated a write to. */
+    unsigned long last_emulated_vfn;
+    /* Last MFN that we emulated a write successfully */
+    unsigned long last_emulated_mfn;
 };
 
 /************************************************/
@@ -189,6 +193,8 @@ struct paging_vcpu {
     struct paging_mode *mode;
     /* HVM guest: last emulate was to a pagetable */
     unsigned int last_write_was_pt:1;
+    /* HVM guest: last write emulation succeeds */
+    unsigned int last_write_emul_ok:1;
     /* Translated guest: virtual TLB */
     struct shadow_vtlb *vtlb;
     spinlock_t          vtlb_lock;
diff -r 64088e89251d xen/include/asm-x86/perfc_defn.h
--- a/xen/include/asm-x86/perfc_defn.h	Tue Jan 15 04:49:23 2008 +0800
+++ b/xen/include/asm-x86/perfc_defn.h	Tue Jan 22 09:38:57 2008 +0800
@@ -57,6 +57,9 @@ PERFCOUNTER(shadow_fault_emulate_failed,
 PERFCOUNTER(shadow_fault_emulate_failed, "shadow_fault emulator fails")
 PERFCOUNTER(shadow_fault_emulate_stack, "shadow_fault emulate stack write")
 PERFCOUNTER(shadow_fault_emulate_wp, "shadow_fault emulate for CR0.WP=0")
+PERFCOUNTER(shadow_fault_fast_emulate, "shadow_fault fast emulate")
+PERFCOUNTER(shadow_fault_fast_emulate_fail,
+                                   "shadow_fault fast emulate failed")
 PERFCOUNTER(shadow_fault_mmio,     "shadow_fault handled as mmio")
 PERFCOUNTER(shadow_fault_fixed,    "shadow_fault fixed fault")
 PERFCOUNTER(shadow_fault_disable_write, "shadow_fault disable write perm")

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH][RFC]Provide fast write emulation path to release shadow lock
  2008-01-22  2:20 [PATCH][RFC]Provide fast write emulation path to release shadow lock Tian, Kevin
@ 2008-01-22  9:26 ` Tim Deegan
  2008-01-22 10:01   ` Tian, Kevin
  2008-01-25  1:41   ` Re: [PATCH][RFC]Provide fast write emulation path torelease " Xu, Dongxiao
  0 siblings, 2 replies; 17+ messages in thread
From: Tim Deegan @ 2008-01-22  9:26 UTC (permalink / raw)
  To: Tian, Kevin; +Cc: xen-devel

At 10:20 +0800 on 22 Jan (1200997253), Tian, Kevin wrote:
> We also did series of tests on 32/32pae/32e: (host is 32e)
>                    32       32pae       32e
> ----Linux----
> kernel build   +1%    +0.86%    +1.9%
> Specjbb        +0.9% +1.61%    +0.32%
> 
> ----XP----
> Sysbench     N/A     -0.05%     -0.32%(*)
> 
> * Sysbench score is not very stable on 32e guest, with up
> to 6% variation observed in 5 rounds running. 32pae is
> stable. 32 XP image was unfortunately corrupted at test 
> cycle, so not test yet. Don't want to hold here from getting
> early comments. :-)
> 
> I thought the performance gain should be straightforward
> with this patch, and thus would like to know comment
> like:
> 	- Is it a right direction?

Looks good to me! 

> 	- Is there anything wrong or missed in patch?

Nothing fundamental that I can see by reading through it.  One thing I'd
change is to avoid introducing "vfn": a virtual address >> PAGE_SIZE is
just a "page number".

> 	- Any more benchmarks should we test?

Anything and everything. :)  Specially multi-vcpu mixed operations
(e.g. kernel compile + ltp + network traffic) while doing live migrate.
Even when they look as clean as this one, changes in the shadow fault
handler tend to chase out implicit/forgotten assumptions.

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH][RFC]Provide fast write emulation path to release shadow lock
  2008-01-22  9:26 ` Tim Deegan
@ 2008-01-22 10:01   ` Tian, Kevin
  2008-01-22 10:06     ` Tim Deegan
  2008-01-25  1:41   ` Re: [PATCH][RFC]Provide fast write emulation path torelease " Xu, Dongxiao
  1 sibling, 1 reply; 17+ messages in thread
From: Tian, Kevin @ 2008-01-22 10:01 UTC (permalink / raw)
  To: Tim Deegan; +Cc: xen-devel

>From: Tim Deegan [mailto:Tim.Deegan@citrix.com] 
>Sent: 2008年1月22日 17:26
>
>At 10:20 +0800 on 22 Jan (1200997253), Tian, Kevin wrote:
>> We also did series of tests on 32/32pae/32e: (host is 32e)
>>                    32       32pae       32e
>> ----Linux----
>> kernel build   +1%    +0.86%    +1.9%
>> Specjbb        +0.9% +1.61%    +0.32%
>> 
>> ----XP----
>> Sysbench     N/A     -0.05%     -0.32%(*)
>> 
>> * Sysbench score is not very stable on 32e guest, with up
>> to 6% variation observed in 5 rounds running. 32pae is
>> stable. 32 XP image was unfortunately corrupted at test 
>> cycle, so not test yet. Don't want to hold here from getting
>> early comments. :-)
>> 
>> I thought the performance gain should be straightforward
>> with this patch, and thus would like to know comment
>> like:
>> 	- Is it a right direction?
>
>Looks good to me! 
>
>> 	- Is there anything wrong or missed in patch?
>
>Nothing fundamental that I can see by reading through it.  One 
>thing I'd
>change is to avoid introducing "vfn": a virtual address >> PAGE_SIZE is
>just a "page number".

OK.

>
>> 	- Any more benchmarks should we test?
>
>Anything and everything. :)  Specially multi-vcpu mixed operations
>(e.g. kernel compile + ltp + network traffic) while doing live migrate.
>Even when they look as clean as this one, changes in the shadow fault
>handler tend to chase out implicit/forgotten assumptions.
>

Agree. I asked the question because the combinations are really too many
and usually we try those tests. So for the case you mentioned when doing
live migrate, can I consider the stability/correctness is the major test goal
since individual score may vary a lot in such complex environment?

Thanks,
Kevin

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH][RFC]Provide fast write emulation path to release shadow lock
  2008-01-22 10:01   ` Tian, Kevin
@ 2008-01-22 10:06     ` Tim Deegan
  2008-01-22 10:08       ` Tian, Kevin
  0 siblings, 1 reply; 17+ messages in thread
From: Tim Deegan @ 2008-01-22 10:06 UTC (permalink / raw)
  To: Tian, Kevin; +Cc: xen-devel

At 18:01 +0800 on 22 Jan (1201024893), Tian, Kevin wrote:
> Agree. I asked the question because the combinations are really too many
> and usually we try those tests. So for the case you mentioned when doing
> live migrate, can I consider the stability/correctness is the major test goal
> since individual score may vary a lot in such complex environment?

Absolutely. 

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH][RFC]Provide fast write emulation path to release shadow lock
  2008-01-22 10:06     ` Tim Deegan
@ 2008-01-22 10:08       ` Tian, Kevin
  0 siblings, 0 replies; 17+ messages in thread
From: Tian, Kevin @ 2008-01-22 10:08 UTC (permalink / raw)
  To: Tim Deegan; +Cc: xen-devel

>From: Tim Deegan [mailto:Tim.Deegan@citrix.com] 
>Sent: 2008年1月22日 18:07
>
>At 18:01 +0800 on 22 Jan (1201024893), Tian, Kevin wrote:
>> Agree. I asked the question because the combinations are 
>really too many
>> and usually we try those tests. So for the case you 
>mentioned when doing
>> live migrate, can I consider the stability/correctness is 
>the major test goal
>> since individual score may vary a lot in such complex environment?
>
>Absolutely. 
>
>Tim.
>

OK, thanks for your comment and I'll do more stability test
before next update. :-)

Thanks,
Kevin

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: Re: [PATCH][RFC]Provide fast write emulation path torelease shadow lock
  2008-01-22  9:26 ` Tim Deegan
  2008-01-22 10:01   ` Tian, Kevin
@ 2008-01-25  1:41   ` Xu, Dongxiao
  2008-01-25  1:54     ` Tian, Kevin
  2008-01-25 10:00     ` Tim Deegan
  1 sibling, 2 replies; 17+ messages in thread
From: Xu, Dongxiao @ 2008-01-25  1:41 UTC (permalink / raw)
  To: Tim Deegan, Tian, Kevin; +Cc: xen-devel

Hi, Tim and Kevin,
    I did some stability test with and without Kevin's patch: 

First guest (Linux):  kernel build;
Second guest (Linux): network copy;
Third guest (Windows): Sysmark 2007;

In dom0, I did local migration to the three guest.

But I found that even without Kevin's patch, the guest would be blue screen in Windows or print some call trace in Linux guest. Have you seen this issue before? Thanks!

Best Regards,
Xu Dongxiao

-----Original Message-----
From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Tim Deegan
Sent: 2008年1月22日 17:26
To: Tian, Kevin
Cc: xen-devel@lists.xensource.com
Subject: [Xen-devel] Re: [PATCH][RFC]Provide fast write emulation path torelease shadow lock

At 10:20 +0800 on 22 Jan (1200997253), Tian, Kevin wrote:
> We also did series of tests on 32/32pae/32e: (host is 32e)
>                    32       32pae       32e
> ----Linux----
> kernel build   +1%    +0.86%    +1.9%
> Specjbb        +0.9% +1.61%    +0.32%
> 
> ----XP----
> Sysbench     N/A     -0.05%     -0.32%(*)
> 
> * Sysbench score is not very stable on 32e guest, with up
> to 6% variation observed in 5 rounds running. 32pae is
> stable. 32 XP image was unfortunately corrupted at test 
> cycle, so not test yet. Don't want to hold here from getting
> early comments. :-)
> 
> I thought the performance gain should be straightforward
> with this patch, and thus would like to know comment
> like:
> 	- Is it a right direction?

Looks good to me! 

> 	- Is there anything wrong or missed in patch?

Nothing fundamental that I can see by reading through it.  One thing I'd
change is to avoid introducing "vfn": a virtual address >> PAGE_SIZE is
just a "page number".

> 	- Any more benchmarks should we test?

Anything and everything. :)  Specially multi-vcpu mixed operations
(e.g. kernel compile + ltp + network traffic) while doing live migrate.
Even when they look as clean as this one, changes in the shadow fault
handler tend to chase out implicit/forgotten assumptions.

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: Re: [PATCH][RFC]Provide fast write emulation path torelease shadow lock
  2008-01-25  1:41   ` Re: [PATCH][RFC]Provide fast write emulation path torelease " Xu, Dongxiao
@ 2008-01-25  1:54     ` Tian, Kevin
  2008-01-25 10:00     ` Tim Deegan
  1 sibling, 0 replies; 17+ messages in thread
From: Tian, Kevin @ 2008-01-25  1:54 UTC (permalink / raw)
  To: Xu, Dongxiao, Tim Deegan; +Cc: xen-devel

So Tim, can you help see whether our test approach is 
wrong or it's the fact with existing today's code? Or you 
may suggest some scenarios you're sure working with 
latest logic and then we can reproduce as comparison...

Thanks,
Kevin

>From: Xu, Dongxiao 
>Sent: 2008年1月25日 9:41
>
>Hi, Tim and Kevin,
>    I did some stability test with and without Kevin's patch: 
>
>First guest (Linux):  kernel build;
>Second guest (Linux): network copy;
>Third guest (Windows): Sysmark 2007;
>
>In dom0, I did local migration to the three guest.
>
>But I found that even without Kevin's patch, the guest would 
>be blue screen in Windows or print some call trace in Linux 
>guest. Have you seen this issue before? Thanks!
>
>Best Regards,
>Xu Dongxiao
>
>-----Original Message-----
>From: xen-devel-bounces@lists.xensource.com 
>[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Tim Deegan
>Sent: 2008年1月22日 17:26
>To: Tian, Kevin
>Cc: xen-devel@lists.xensource.com
>Subject: [Xen-devel] Re: [PATCH][RFC]Provide fast write 
>emulation path torelease shadow lock
>
>At 10:20 +0800 on 22 Jan (1200997253), Tian, Kevin wrote:
>> We also did series of tests on 32/32pae/32e: (host is 32e)
>>                    32       32pae       32e
>> ----Linux----
>> kernel build   +1%    +0.86%    +1.9%
>> Specjbb        +0.9% +1.61%    +0.32%
>> 
>> ----XP----
>> Sysbench     N/A     -0.05%     -0.32%(*)
>> 
>> * Sysbench score is not very stable on 32e guest, with up
>> to 6% variation observed in 5 rounds running. 32pae is
>> stable. 32 XP image was unfortunately corrupted at test 
>> cycle, so not test yet. Don't want to hold here from getting
>> early comments. :-)
>> 
>> I thought the performance gain should be straightforward
>> with this patch, and thus would like to know comment
>> like:
>> 	- Is it a right direction?
>
>Looks good to me! 
>
>> 	- Is there anything wrong or missed in patch?
>
>Nothing fundamental that I can see by reading through it.  One 
>thing I'd
>change is to avoid introducing "vfn": a virtual address >> PAGE_SIZE is
>just a "page number".
>
>> 	- Any more benchmarks should we test?
>
>Anything and everything. :)  Specially multi-vcpu mixed operations
>(e.g. kernel compile + ltp + network traffic) while doing live migrate.
>Even when they look as clean as this one, changes in the shadow fault
>handler tend to chase out implicit/forgotten assumptions.
>
>Cheers,
>
>Tim.
>
>-- 
>Tim Deegan <Tim.Deegan@citrix.com>
>Principal Software Engineer, Citrix Systems (R&D) Ltd.
>[Company #02300071, SL9 0DZ, UK.]
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Re: [PATCH][RFC]Provide fast write emulation path torelease shadow lock
  2008-01-25  1:41   ` Re: [PATCH][RFC]Provide fast write emulation path torelease " Xu, Dongxiao
  2008-01-25  1:54     ` Tian, Kevin
@ 2008-01-25 10:00     ` Tim Deegan
  2008-01-30  1:32       ` Re: [PATCH][RFC]Provide fast write emulation pathtorelease " Xu, Dongxiao
  1 sibling, 1 reply; 17+ messages in thread
From: Tim Deegan @ 2008-01-25 10:00 UTC (permalink / raw)
  To: Xu, Dongxiao; +Cc: Tian, Kevin, xen-devel

Hi,

At 09:41 +0800 on 25 Jan (1201254064), Xu, Dongxiao wrote:
> But I found that even without Kevin's patch, the guest would be blue
> screen in Windows or print some call trace in Linux guest. Have you
> seen this issue before? Thanks!

Can you try building Xen with "vmxassist=y" on the make command-line?

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: Re: [PATCH][RFC]Provide fast write emulation pathtorelease shadow lock
  2008-01-25 10:00     ` Tim Deegan
@ 2008-01-30  1:32       ` Xu, Dongxiao
  2008-01-30  9:46         ` Tim Deegan
  0 siblings, 1 reply; 17+ messages in thread
From: Xu, Dongxiao @ 2008-01-30  1:32 UTC (permalink / raw)
  To: Tim Deegan; +Cc: Tian, Kevin, xen-devel

Hi, Tim and Kevin,
    I used the changeset 16704 to take the stability test, and at that changeset, vmxassist=y is the default. Later I used the Xen3.2 test tree to have a test, it met the same issue, that is, windows guest would be blue screen, and linux guest would pop up some call trace. Also Dom0 will pop up some call trace too. My test environment is 

First guest (Linux):  kernel build;
Second guest (Linux): network copy;
Third guest (Windows): Sysmark 2007;

In dom0, I did local migration to the three guest.

Tim, could you tell me what's the test case and test environment (changeset) when you take the stability test? Then I may use your environment to test Kevin's patch. Thanks very much!

Best Regards, 
Xu Dongxiao 

-----Original Message-----
From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Tim Deegan
Sent: 2008年1月25日 18:01
To: Xu, Dongxiao
Cc: Tian, Kevin; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] Re: [PATCH][RFC]Provide fast write emulation pathtorelease shadow lock

Hi,

At 09:41 +0800 on 25 Jan (1201254064), Xu, Dongxiao wrote:
> But I found that even without Kevin's patch, the guest would be blue
> screen in Windows or print some call trace in Linux guest. Have you
> seen this issue before? Thanks!

Can you try building Xen with "vmxassist=y" on the make command-line?

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Re: [PATCH][RFC]Provide fast write emulation pathtorelease shadow lock
  2008-01-30  1:32       ` Re: [PATCH][RFC]Provide fast write emulation pathtorelease " Xu, Dongxiao
@ 2008-01-30  9:46         ` Tim Deegan
  2008-01-30 13:07           ` Re: [PATCH][RFC]Provide fast write emulationpathtorelease " Xu, Dongxiao
  0 siblings, 1 reply; 17+ messages in thread
From: Tim Deegan @ 2008-01-30  9:46 UTC (permalink / raw)
  To: Xu, Dongxiao; +Cc: Tian, Kevin, xen-devel

At 09:32 +0800 on 30 Jan (1201685565), Xu, Dongxiao wrote:
> Tim, could you tell me what's the test case and test environment
> (changeset) when you take the stability test? Then I may use your
> environment to test Kevin's patch. Thanks very much!

The 3.2 release (tagged version) should have passed some migration
stress testing; apparently the -unstable branch is not getting automatic
testing for this at the moment.  I'll look at it manually.  Sorry about
that.

Can you tell me what the STOP codes were on the bluescreens that you
saw?

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: Re: [PATCH][RFC]Provide fast write emulationpathtorelease shadow lock
  2008-01-30  9:46         ` Tim Deegan
@ 2008-01-30 13:07           ` Xu, Dongxiao
  2008-02-12  8:25             ` Tim Deegan
  0 siblings, 1 reply; 17+ messages in thread
From: Xu, Dongxiao @ 2008-01-30 13:07 UTC (permalink / raw)
  To: Tim Deegan; +Cc: Tian, Kevin, xen-devel

Hi, Tim,

The stop code is: 
STOP: 0x0000000A (0xF94EAFFC, 0x000000FF, 0x00000001, 0x805434A2)

Best Regards,
Xu Dongxiao

-----Original Message-----
From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Tim Deegan
Sent: 2008年1月30日 17:46
To: Xu, Dongxiao
Cc: Tian, Kevin; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] Re: [PATCH][RFC]Provide fast write emulationpathtorelease shadow lock

At 09:32 +0800 on 30 Jan (1201685565), Xu, Dongxiao wrote:
> Tim, could you tell me what's the test case and test environment
> (changeset) when you take the stability test? Then I may use your
> environment to test Kevin's patch. Thanks very much!

The 3.2 release (tagged version) should have passed some migration
stress testing; apparently the -unstable branch is not getting automatic
testing for this at the moment.  I'll look at it manually.  Sorry about
that.

Can you tell me what the STOP codes were on the bluescreens that you
saw?

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Re: [PATCH][RFC]Provide fast write emulationpathtorelease shadow lock
  2008-01-30 13:07           ` Re: [PATCH][RFC]Provide fast write emulationpathtorelease " Xu, Dongxiao
@ 2008-02-12  8:25             ` Tim Deegan
  2008-02-14  1:54               ` Tian, Kevin
  0 siblings, 1 reply; 17+ messages in thread
From: Tim Deegan @ 2008-02-12  8:25 UTC (permalink / raw)
  To: Xu, Dongxiao; +Cc: Tian, Kevin, xen-devel

At 21:07 +0800 on 30 Jan (1201727271), Xu, Dongxiao wrote:
> Hi, Tim,
> 
> The stop code is: 
> STOP: 0x0000000A (0xF94EAFFC, 0x000000FF, 0x00000001, 0x805434A2)

I haven't been able to reproduce this crash, but it certainly looks like
the sort of thing a shadow bug would do.  Were you seeing this on a
particular version of Windows?

In the meantime, if you're happy with the shadow lock patch, I think we
can take it into unstable. 

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: Re: [PATCH][RFC]Provide fast write emulationpathtorelease shadow lock
  2008-02-12  8:25             ` Tim Deegan
@ 2008-02-14  1:54               ` Tian, Kevin
  2008-02-15  6:06                 ` [PATCH][UPDATE]Provide fast write emulation path Tian, Kevin
  0 siblings, 1 reply; 17+ messages in thread
From: Tian, Kevin @ 2008-02-14  1:54 UTC (permalink / raw)
  To: Tim Deegan, Xu, Dongxiao; +Cc: xen-devel

>From: Tim Deegan [mailto:Tim.Deegan@citrix.com] 
>Sent: 2008年2月12日 16:26
>
>At 21:07 +0800 on 30 Jan (1201727271), Xu, Dongxiao wrote:
>> Hi, Tim,
>> 
>> The stop code is: 
>> STOP: 0x0000000A (0xF94EAFFC, 0x000000FF, 0x00000001, 0x805434A2)
>
>I haven't been able to reproduce this crash, but it certainly 
>looks like
>the sort of thing a shadow bug would do.  Were you seeing this on a
>particular version of Windows?

I think Dongxiao only tried one version of Windows. I'll ask his help to try
other versions after back from his vacation.

>
>In the meantime, if you're happy with the shadow lock patch, I think we
>can take it into unstable. 

That'd be great.

Thanks,
Kevn

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH][UPDATE]Provide fast write emulation path
  2008-02-14  1:54               ` Tian, Kevin
@ 2008-02-15  6:06                 ` Tian, Kevin
  2008-02-15 10:01                   ` Keir Fraser
  0 siblings, 1 reply; 17+ messages in thread
From: Tian, Kevin @ 2008-02-15  6:06 UTC (permalink / raw)
  To: Tian, Kevin, Tim Deegan, Xu, Dongxiao; +Cc: xen-devel

[-- Attachment #1: Type: text/plain, Size: 843 bytes --]

So, attached updated patch.

Thanks,
Kevin

>From: Tian, Kevin
>Sent: 2008年2月14日 9:54
>
>>From: Tim Deegan [mailto:Tim.Deegan@citrix.com] 
>>Sent: 2008年2月12日 16:26
>>
>>At 21:07 +0800 on 30 Jan (1201727271), Xu, Dongxiao wrote:
>>> Hi, Tim,
>>> 
>>> The stop code is: 
>>> STOP: 0x0000000A (0xF94EAFFC, 0x000000FF, 0x00000001, 0x805434A2)
>>
>>I haven't been able to reproduce this crash, but it certainly 
>>looks like
>>the sort of thing a shadow bug would do.  Were you seeing this on a
>>particular version of Windows?
>
>I think Dongxiao only tried one version of Windows. I'll ask 
>his help to try
>other versions after back from his vacation.
>
>>
>>In the meantime, if you're happy with the shadow lock patch, 
>I think we
>>can take it into unstable. 
>
>That'd be great.
>
>Thanks,
>Kevn
>

[-- Attachment #2: fast_emul.patch --]
[-- Type: application/octet-stream, Size: 11339 bytes --]

Provide fast write emulation path

Basically we can consider shadow fault logic into two parts,
with 1st part to cover logistic work like validating guest
page table or fix shadow table, and the 2nd part for write
emulation.

However there's one scenario we can optimize to skip the
1st part. For previous successfully emulated frame, it's
very likely approaching at write emulation logic again if
next adjacent shadow fault is hitting same frame. It's
wasteful to re-walk 1st part which is already covered by
last shadow fault. In this case, actually we can jump to
emulation code early, without any lock acquisition until
final shadow validation for write emulation. By perfc counts
on 64bit SMP HVM guest, 89% of total shadow write emulation
are observed falling into this fast path when doing kernel
build in guest.

Signed-off-by Kevin Tian <kevin.tian@intel.com>

diff -r 888820beef84 xen/arch/x86/mm/shadow/common.c
--- a/xen/arch/x86/mm/shadow/common.c	Thu Feb 14 10:59:48 2008 +0800
+++ b/xen/arch/x86/mm/shadow/common.c	Fri Feb 15 13:10:48 2008 +0800
@@ -1039,13 +1039,18 @@ void shadow_free(struct domain *d, mfn_t
 
     for ( i = 0; i < 1<<order; i++ ) 
     {
-#if SHADOW_OPTIMIZATIONS & SHOPT_WRITABLE_HEURISTIC
+#if SHADOW_OPTIMIZATIONS & (SHOPT_WRITABLE_HEURISTIC | SHOPT_FAST_EMULATION)
         struct vcpu *v;
         for_each_vcpu(d, v) 
         {
+#if SHADOW_OPTIMIZATIONS & SHOPT_WRITABLE_HEURISTIC
             /* No longer safe to look for a writeable mapping in this shadow */
             if ( v->arch.paging.shadow.last_writeable_pte_smfn == mfn_x(smfn) + i ) 
                 v->arch.paging.shadow.last_writeable_pte_smfn = 0;
+#endif
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+            v->arch.paging.last_write_emul_ok = 0;
+#endif
         }
 #endif
         /* Strip out the type: this is now a free shadow page */
diff -r 888820beef84 xen/arch/x86/mm/shadow/multi.c
--- a/xen/arch/x86/mm/shadow/multi.c	Thu Feb 14 10:59:48 2008 +0800
+++ b/xen/arch/x86/mm/shadow/multi.c	Fri Feb 15 13:42:37 2008 +0800
@@ -2623,13 +2623,13 @@ static inline void check_for_early_unsha
 static inline void check_for_early_unshadow(struct vcpu *v, mfn_t gmfn)
 {
 #if SHADOW_OPTIMIZATIONS & SHOPT_EARLY_UNSHADOW
-    if ( v->arch.paging.shadow.last_emulated_mfn == mfn_x(gmfn) &&
-         sh_mfn_is_a_page_table(gmfn) )
+    if ( v->arch.paging.shadow.last_emulated_mfn_for_unshadow == mfn_x(gmfn)
+         && sh_mfn_is_a_page_table(gmfn) )
     {
         perfc_incr(shadow_early_unshadow);
         sh_remove_shadows(v, gmfn, 1, 0 /* Fast, can fail to unshadow */ );
     }
-    v->arch.paging.shadow.last_emulated_mfn = mfn_x(gmfn);
+    v->arch.paging.shadow.last_emulated_mfn_for_unshadow = mfn_x(gmfn);
 #endif
 }
 
@@ -2637,7 +2637,7 @@ static inline void reset_early_unshadow(
 static inline void reset_early_unshadow(struct vcpu *v)
 {
 #if SHADOW_OPTIMIZATIONS & SHOPT_EARLY_UNSHADOW
-    v->arch.paging.shadow.last_emulated_mfn = INVALID_MFN;
+    v->arch.paging.shadow.last_emulated_mfn_for_unshadow = INVALID_MFN;
 #endif
 }
 
@@ -2744,12 +2744,39 @@ static int sh_page_fault(struct vcpu *v,
     int r;
     fetch_type_t ft = 0;
     p2m_type_t p2mt;
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+    int fast_emul = 0;
+#endif
 
     SHADOW_PRINTK("d:v=%u:%u va=%#lx err=%u, rip=%lx\n",
                   v->domain->domain_id, v->vcpu_id, va, regs->error_code,
                   regs->rip);
 
     perfc_incr(shadow_fault);
+
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+    /* If faulting frame is successfully emulated in last shadow fault
+     * it's highly likely to reach same emulation action for this frame.
+     * Then try to emulate early to avoid lock aquisition.
+     */
+    if ( v->arch.paging.last_write_emul_ok
+         && v->arch.paging.shadow.last_emulated_frame == (va >> PAGE_SHIFT) ) 
+    {
+        /* check whether error code is 3, or else fall back to normal path
+         * in case of some validation is required
+         */
+        if ( regs->error_code == (PFEC_write_access | PFEC_page_present) )
+        {
+            fast_emul = 1;
+            gmfn = v->arch.paging.shadow.last_emulated_mfn;
+            perfc_incr(shadow_fault_fast_emulate);
+            goto early_emulation;
+        }
+        else
+            v->arch.paging.last_write_emul_ok = 0;
+    }
+#endif
+
     //
     // XXX: Need to think about eventually mapping superpages directly in the
     //      shadow (when possible), as opposed to splintering them into a
@@ -2960,6 +2987,17 @@ static int sh_page_fault(struct vcpu *v,
         goto done;
     }
 
+    /*
+     * We don't need to hold the lock for the whole emulation; we will
+     * take it again when we write to the pagetables.
+     */
+    sh_audit_gw(v, &gw);
+    shadow_audit_tables(v);
+    shadow_unlock(d);
+
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+ early_emulation:
+#endif
     if ( is_hvm_domain(d) )
     {
         /*
@@ -2971,6 +3009,13 @@ static int sh_page_fault(struct vcpu *v,
          */
         if ( unlikely(hvm_event_pending(v)) )
         {
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+            if ( fast_emul )
+            {
+                perfc_incr(shadow_fault_fast_emulate_fail);
+                v->arch.paging.last_write_emul_ok = 0;
+            }
+#endif
             gdprintk(XENLOG_DEBUG, "write to pagetable during event "
                      "injection: cr2=%#lx, mfn=%#lx\n", 
                      va, mfn_x(gmfn));
@@ -2981,14 +3026,6 @@ static int sh_page_fault(struct vcpu *v,
 
     SHADOW_PRINTK("emulate: eip=%#lx esp=%#lx\n", 
                   (unsigned long)regs->eip, (unsigned long)regs->esp);
-
-    /*
-     * We don't need to hold the lock for the whole emulation; we will
-     * take it again when we write to the pagetables.
-     */
-    sh_audit_gw(v, &gw);
-    shadow_audit_tables(v);
-    shadow_unlock(d);
 
     emul_ops = shadow_init_emulation(&emul_ctxt, regs);
 
@@ -3001,14 +3038,43 @@ static int sh_page_fault(struct vcpu *v,
      */
     if ( r == X86EMUL_UNHANDLEABLE )
     {
+        perfc_incr(shadow_fault_emulate_failed);
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+        if ( fast_emul )
+        {
+            perfc_incr(shadow_fault_fast_emulate_fail);
+            v->arch.paging.last_write_emul_ok = 0;
+        }
+#endif
         SHADOW_PRINTK("emulator failure, unshadowing mfn %#lx\n", 
                        mfn_x(gmfn));
-        perfc_incr(shadow_fault_emulate_failed);
         /* If this is actually a page table, then we have a bug, and need 
          * to support more operations in the emulator.  More likely, 
          * though, this is a hint that this page should not be shadowed. */
         shadow_remove_all_shadows(v, gmfn);
     }
+
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+    /* Record successfully emulated information as heuristics to next
+     * fault on same frame for acceleration. But be careful to verify
+     * its attribute still as page table, or else unshadow triggered
+     * in write emulation normally requires a re-sync with guest page
+     * table to recover r/w permission. Incorrect record for such case
+     * will cause unexpected more shadow faults due to propagation is
+     * skipped.
+     */
+    if ( (r == X86EMUL_OKAY) && sh_mfn_is_a_page_table(gmfn) )
+    {
+        if ( !fast_emul )
+        {
+            v->arch.paging.shadow.last_emulated_frame = va >> PAGE_SHIFT;
+            v->arch.paging.shadow.last_emulated_mfn = gmfn;
+            v->arch.paging.last_write_emul_ok = 1;
+        }
+    }
+    else if ( fast_emul )
+        v->arch.paging.last_write_emul_ok = 0;
+#endif
 
 #if GUEST_PAGING_LEVELS == 3 /* PAE guest */
     if ( r == X86EMUL_OKAY ) {
@@ -3077,6 +3143,10 @@ sh_invlpg(struct vcpu *v, unsigned long 
 #if (SHADOW_OPTIMIZATIONS & SHOPT_VIRTUAL_TLB)
     /* No longer safe to use cached gva->gfn translations */
     vtlb_flush(v);
+#endif
+
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+    v->arch.paging.last_write_emul_ok = 0;
 #endif
 
     /* First check that we can safely read the shadow l2e.  SMP/PAE linux can
@@ -3815,6 +3885,10 @@ sh_update_cr3(struct vcpu *v, int do_loc
     vtlb_flush(v);
 #endif
 
+#if SHADOW_OPTIMIZATIONS & SHOPT_FAST_EMULATION
+    v->arch.paging.last_write_emul_ok = 0;
+#endif
+
     /* Release the lock, if we took it (otherwise it's the caller's problem) */
     if ( do_locking ) shadow_unlock(v->domain);
 }
diff -r 888820beef84 xen/arch/x86/mm/shadow/private.h
--- a/xen/arch/x86/mm/shadow/private.h	Thu Feb 14 10:59:48 2008 +0800
+++ b/xen/arch/x86/mm/shadow/private.h	Fri Feb 15 13:10:48 2008 +0800
@@ -62,8 +62,9 @@ extern int shadow_audit_enable;
 #define SHOPT_LINUX_L3_TOPLEVEL   0x10  /* Pin l3es on early 64bit linux */
 #define SHOPT_SKIP_VERIFY         0x20  /* Skip PTE v'fy when safe to do so */
 #define SHOPT_VIRTUAL_TLB         0x40  /* Cache guest v->p translations */
-
-#define SHADOW_OPTIMIZATIONS      0x7f
+#define SHOPT_FAST_EMULATION      0x80  /* Fast write emulation */
+
+#define SHADOW_OPTIMIZATIONS      0xff
 
 
 /******************************************************************************
diff -r 888820beef84 xen/include/asm-x86/domain.h
--- a/xen/include/asm-x86/domain.h	Thu Feb 14 10:59:48 2008 +0800
+++ b/xen/include/asm-x86/domain.h	Fri Feb 15 13:35:52 2008 +0800
@@ -108,10 +108,14 @@ struct shadow_vcpu {
 #endif
     /* Non-PAE guests: pointer to guest top-level pagetable */
     void *guest_vtable;
-    /* Last MFN that we emulated a write to. */
-    unsigned long last_emulated_mfn;
+    /* Last MFN that we emulated a write to as unshadow heuristics. */
+    unsigned long last_emulated_mfn_for_unshadow;
     /* MFN of the last shadow that we shot a writeable mapping in */
     unsigned long last_writeable_pte_smfn;
+    /* Last frame number that we emulated a write to. */
+    unsigned long last_emulated_frame;
+    /* Last MFN that we emulated a write successfully */
+    unsigned long last_emulated_mfn;
 };
 
 /************************************************/
@@ -189,6 +193,8 @@ struct paging_vcpu {
     struct paging_mode *mode;
     /* HVM guest: last emulate was to a pagetable */
     unsigned int last_write_was_pt:1;
+    /* HVM guest: last write emulation succeeds */
+    unsigned int last_write_emul_ok:1;
     /* Translated guest: virtual TLB */
     struct shadow_vtlb *vtlb;
     spinlock_t          vtlb_lock;
diff -r 888820beef84 xen/include/asm-x86/perfc_defn.h
--- a/xen/include/asm-x86/perfc_defn.h	Thu Feb 14 10:59:48 2008 +0800
+++ b/xen/include/asm-x86/perfc_defn.h	Fri Feb 15 13:10:48 2008 +0800
@@ -57,6 +57,9 @@ PERFCOUNTER(shadow_fault_emulate_failed,
 PERFCOUNTER(shadow_fault_emulate_failed, "shadow_fault emulator fails")
 PERFCOUNTER(shadow_fault_emulate_stack, "shadow_fault emulate stack write")
 PERFCOUNTER(shadow_fault_emulate_wp, "shadow_fault emulate for CR0.WP=0")
+PERFCOUNTER(shadow_fault_fast_emulate, "shadow_fault fast emulate")
+PERFCOUNTER(shadow_fault_fast_emulate_fail,
+                                   "shadow_fault fast emulate failed")
 PERFCOUNTER(shadow_fault_mmio,     "shadow_fault handled as mmio")
 PERFCOUNTER(shadow_fault_fixed,    "shadow_fault fixed fault")
 PERFCOUNTER(shadow_ptwr_emulate,   "shadow causes ptwr to emulate")

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH][UPDATE]Provide fast write emulation path
  2008-02-15  6:06                 ` [PATCH][UPDATE]Provide fast write emulation path Tian, Kevin
@ 2008-02-15 10:01                   ` Keir Fraser
  2008-02-15 11:36                     ` Tim Deegan
  0 siblings, 1 reply; 17+ messages in thread
From: Keir Fraser @ 2008-02-15 10:01 UTC (permalink / raw)
  To: Tian, Kevin, Tim Deegan, Xu, Dongxiao; +Cc: xen-devel

I'd like a final ack from Tim on this one before applying.

 -- Keir

On 15/2/08 06:06, "Tian, Kevin" <kevin.tian@intel.com> wrote:

> So, attached updated patch.
> 
> Thanks,
> Kevin
> 
>> From: Tian, Kevin
>> Sent: 2008年2月14日 9:54
>> 
>>> From: Tim Deegan [mailto:Tim.Deegan@citrix.com]
>>> Sent: 2008年2月12日 16:26
>>> 
>>> At 21:07 +0800 on 30 Jan (1201727271), Xu, Dongxiao wrote:
>>>> Hi, Tim,
>>>> 
>>>> The stop code is:
>>>> STOP: 0x0000000A (0xF94EAFFC, 0x000000FF, 0x00000001, 0x805434A2)
>>> 
>>> I haven't been able to reproduce this crash, but it certainly
>>> looks like
>>> the sort of thing a shadow bug would do.  Were you seeing this on a
>>> particular version of Windows?
>> 
>> I think Dongxiao only tried one version of Windows. I'll ask
>> his help to try
>> other versions after back from his vacation.
>> 
>>> 
>>> In the meantime, if you're happy with the shadow lock patch,
>> I think we
>>> can take it into unstable.
>> 
>> That'd be great.
>> 
>> Thanks,
>> Kevn
>> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH][UPDATE]Provide fast write emulation path
  2008-02-15 10:01                   ` Keir Fraser
@ 2008-02-15 11:36                     ` Tim Deegan
  2008-02-15 12:21                       ` Tian, Kevin
  0 siblings, 1 reply; 17+ messages in thread
From: Tim Deegan @ 2008-02-15 11:36 UTC (permalink / raw)
  To: Keir Fraser; +Cc: Tian, Kevin, xen-devel, Xu, Dongxiao

At 10:01 +0000 on 15 Feb (1203069706), Keir Fraser wrote:
> I'd like a final ack from Tim on this one before applying.

Ack.  

I read through again and the only thing I can see is a risk that on SMP
guests, a foreign vcpu might change the guest pagetable under our feet,
leading to a risk of using inconsistent mappings for reads and writes.
I think that's OK, though, because
 - we already have similar exposure from vTLB &c, and will do from OOS;
 - a guest with good TLB flush discipline should be entirely safe.

Cheers,

Tim.

> On 15/2/08 06:06, "Tian, Kevin" <kevin.tian@intel.com> wrote:
> 
> > So, attached updated patch.
> > 
> > Thanks,
> > Kevin
> > 
> >> From: Tian, Kevin
> >> Sent: 2008?$BG/2?$B7n14?$BF| 9:54
> >> 
> >>> From: Tim Deegan [mailto:Tim.Deegan@citrix.com]
> >>> Sent: 2008?$BG/2?$B7n12?$BF| 16:26
> >>> 
> >>> At 21:07 +0800 on 30 Jan (1201727271), Xu, Dongxiao wrote:
> >>>> Hi, Tim,
> >>>> 
> >>>> The stop code is:
> >>>> STOP: 0x0000000A (0xF94EAFFC, 0x000000FF, 0x00000001, 0x805434A2)
> >>> 
> >>> I haven't been able to reproduce this crash, but it certainly
> >>> looks like
> >>> the sort of thing a shadow bug would do.  Were you seeing this on a
> >>> particular version of Windows?
> >> 
> >> I think Dongxiao only tried one version of Windows. I'll ask
> >> his help to try
> >> other versions after back from his vacation.
> >> 
> >>> 
> >>> In the meantime, if you're happy with the shadow lock patch,
> >> I think we
> >>> can take it into unstable.
> >> 
> >> That'd be great.
> >> 
> >> Thanks,
> >> Kevn
> >> 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> 
> 

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH][UPDATE]Provide fast write emulation path
  2008-02-15 11:36                     ` Tim Deegan
@ 2008-02-15 12:21                       ` Tian, Kevin
  0 siblings, 0 replies; 17+ messages in thread
From: Tian, Kevin @ 2008-02-15 12:21 UTC (permalink / raw)
  To: Tim Deegan, Keir Fraser; +Cc: Xu, Dongxiao, xen-devel

>From: Tim Deegan [mailto:Tim.Deegan@citrix.com] 
>Sent: 2008年2月15日 19:36
>
>At 10:01 +0000 on 15 Feb (1203069706), Keir Fraser wrote:
>> I'd like a final ack from Tim on this one before applying.
>
>Ack.  
>
>I read through again and the only thing I can see is a risk that on SMP
>guests, a foreign vcpu might change the guest pagetable under our feet,
>leading to a risk of using inconsistent mappings for reads and writes.
>I think that's OK, though, because
> - we already have similar exposure from vTLB &c, and will do from OOS;
> - a guest with good TLB flush discipline should be entirely safe.
>

Yep. This is actually same as physical cpu behavior, as long as
inconsistent mappings are not stamped into shadow page table.

Thanks for comments,
Kevin

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2008-02-15 12:21 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-22  2:20 [PATCH][RFC]Provide fast write emulation path to release shadow lock Tian, Kevin
2008-01-22  9:26 ` Tim Deegan
2008-01-22 10:01   ` Tian, Kevin
2008-01-22 10:06     ` Tim Deegan
2008-01-22 10:08       ` Tian, Kevin
2008-01-25  1:41   ` Re: [PATCH][RFC]Provide fast write emulation path torelease " Xu, Dongxiao
2008-01-25  1:54     ` Tian, Kevin
2008-01-25 10:00     ` Tim Deegan
2008-01-30  1:32       ` Re: [PATCH][RFC]Provide fast write emulation pathtorelease " Xu, Dongxiao
2008-01-30  9:46         ` Tim Deegan
2008-01-30 13:07           ` Re: [PATCH][RFC]Provide fast write emulationpathtorelease " Xu, Dongxiao
2008-02-12  8:25             ` Tim Deegan
2008-02-14  1:54               ` Tian, Kevin
2008-02-15  6:06                 ` [PATCH][UPDATE]Provide fast write emulation path Tian, Kevin
2008-02-15 10:01                   ` Keir Fraser
2008-02-15 11:36                     ` Tim Deegan
2008-02-15 12:21                       ` Tian, Kevin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.