[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v2] x86/hvm: Use for_each_set_bit() in hvm_emulate_writeback()



... which is more consise than the opencoded form, and more efficient when
compiled.

For production VMs, ~100% of emulations are simple MOVs, so it is likely that
there are no segments to write back.

Furthermore, now that find_{first,next}_bit() are no longer in use, the
seg_reg_{accessed,dirty} fields aren't forced to be unsigned long, although
they do need to remain unsigned int because of __set_bit() elsewhere.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>

v2:
 * Don't clear reg_dirty.  That mess can be sorted out later.

Bloat-o-meter reports:

  add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-43 (-43)
  Function                                     old     new   delta
  hvm_emulate_writeback                        131      88     -43

find_{first,next}_bit() did expand inline without function calls, but the loop
they create has two exit paths, and repeatedly shifts seg_reg by the last
answer before doing BSF on the result.

I still can't persuade GCC to do the early exit prior to establishing the
stack frame, and unlike do_livepatch_work(), it's not critical enough to
require noinline games.
---
 xen/arch/x86/hvm/emulate.c             | 18 ++++++++----------
 xen/arch/x86/include/asm/hvm/emulate.h |  4 ++--
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 1db14642b362..e8c704ab041c 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -3022,18 +3022,16 @@ void hvm_emulate_init_per_insn(
 void hvm_emulate_writeback(
     struct hvm_emulate_ctxt *hvmemul_ctxt)
 {
-    enum x86_segment seg;
+    struct vcpu *curr;
+    unsigned int dirty = hvmemul_ctxt->seg_reg_dirty;
 
-    seg = find_first_bit(&hvmemul_ctxt->seg_reg_dirty,
-                         ARRAY_SIZE(hvmemul_ctxt->seg_reg));
+    if ( likely(!dirty) )
+        return;
 
-    while ( seg < ARRAY_SIZE(hvmemul_ctxt->seg_reg) )
-    {
-        hvm_set_segment_register(current, seg, &hvmemul_ctxt->seg_reg[seg]);
-        seg = find_next_bit(&hvmemul_ctxt->seg_reg_dirty,
-                            ARRAY_SIZE(hvmemul_ctxt->seg_reg),
-                            seg+1);
-    }
+    curr = current;
+
+    for_each_set_bit ( seg, dirty )
+        hvm_set_segment_register(curr, seg, &hvmemul_ctxt->seg_reg[seg]);
 }
 
 /*
diff --git a/xen/arch/x86/include/asm/hvm/emulate.h 
b/xen/arch/x86/include/asm/hvm/emulate.h
index d61eacfaa779..c7a2d2a5be4e 100644
--- a/xen/arch/x86/include/asm/hvm/emulate.h
+++ b/xen/arch/x86/include/asm/hvm/emulate.h
@@ -37,8 +37,8 @@ struct hvm_emulate_ctxt {
     unsigned int insn_buf_bytes;
 
     struct segment_register seg_reg[10];
-    unsigned long seg_reg_accessed;
-    unsigned long seg_reg_dirty;
+    unsigned int seg_reg_accessed;
+    unsigned int seg_reg_dirty;
 
     /*
      * MFNs behind temporary mappings in the write callback.  The length is
-- 
2.39.5




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.