Re: [Xen-devel] [PATCH v2 2/4] x86emul: consolidate segment register handling

[Andrew Cooper]

On 30/09/16 14:16, Jan Beulich wrote:
> Use a single set of variables throughout the huge switch() statement,
> allowing to funnel SLDT/STR into the mov-from-sreg code path.
>
> Signed-off-by: Jan Beulich 
> Reviewed-by: Andrew Cooper 

Also looking better.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 2/4] x86emul: consolidate segment register handling

[Jan Beulich]

Use a single set of variables throughout the huge switch() statement,
allowing to funnel SLDT/STR into the mov-from-sreg code path.

Signed-off-by: Jan Beulich 
Reviewed-by: Andrew Cooper 
---
v2: s/store_seg/store_selector/g. Re-base.

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2505,7 +2505,8 @@ x86_emulate(
 
 switch ( ctxt->opcode )
 {
-struct segment_register cs;
+enum x86_segment seg;
+struct segment_register cs, sreg;
 
 case 0x00 ... 0x05: add: /* add */
 emulate_2op_SrcV("add", src, dst, _regs.eflags);
@@ -2541,22 +2542,20 @@ x86_emulate(
 dst.type = OP_NONE;
 break;
 
-case 0x06: /* push %%es */ {
-struct segment_register reg;
+case 0x06: /* push %%es */
 src.val = x86_seg_es;
 push_seg:
 generate_exception_if(mode_64bit() && !ext, EXC_UD, -1);
 fail_if(ops->read_segment == NULL);
-if ( (rc = ops->read_segment(src.val, , ctxt)) != 0 )
+if ( (rc = ops->read_segment(src.val, , ctxt)) != 0 )
 goto done;
 /* 64-bit mode: PUSH defaults to a 64-bit operand. */
 if ( mode_64bit() && (op_bytes == 4) )
 op_bytes = 8;
 if ( (rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes),
-  , op_bytes, ctxt)) != 0 )
+  , op_bytes, ctxt)) != 0 )
 goto done;
 break;
-}
 
 case 0x07: /* pop %%es */
 src.val = x86_seg_es;
@@ -2872,21 +2871,20 @@ x86_emulate(
 dst.val = src.val;
 break;
 
-case 0x8c: /* mov Sreg,r/m */ {
-struct segment_register reg;
-enum x86_segment seg = decode_segment(modrm_reg);
+case 0x8c: /* mov Sreg,r/m */
+seg = decode_segment(modrm_reg);
 generate_exception_if(seg == decode_segment_failed, EXC_UD, -1);
+store_selector:
 fail_if(ops->read_segment == NULL);
-if ( (rc = ops->read_segment(seg, , ctxt)) != 0 )
+if ( (rc = ops->read_segment(seg, , ctxt)) != 0 )
 goto done;
-dst.val = reg.sel;
+dst.val = sreg.sel;
 if ( dst.type == OP_MEM )
 dst.bytes = 2;
 break;
-}
 
-case 0x8e: /* mov r/m,Sreg */ {
-enum x86_segment seg = decode_segment(modrm_reg);
+case 0x8e: /* mov r/m,Sreg */
+seg = decode_segment(modrm_reg);
 generate_exception_if(seg == decode_segment_failed, EXC_UD, -1);
 generate_exception_if(seg == x86_seg_cs, EXC_UD, -1);
 if ( (rc = load_seg(seg, src.val, 0, NULL, ctxt, ops)) != 0 )
@@ -2895,7 +2893,6 @@ x86_emulate(
 ctxt->retire.flags.mov_ss = 1;
 dst.type = OP_NONE;
 break;
-}
 
 case 0x8d: /* lea */
 generate_exception_if(ea.type != OP_MEM, EXC_UD, -1);
@@ -2952,17 +2949,15 @@ x86_emulate(
 }
 break;
 
-case 0x9a: /* call (far, absolute) */ {
-struct segment_register reg;
-
+case 0x9a: /* call (far, absolute) */
 ASSERT(!mode_64bit());
 fail_if(ops->read_segment == NULL);
 
-if ( (rc = ops->read_segment(x86_seg_cs, , ctxt)) ||
+if ( (rc = ops->read_segment(x86_seg_cs, , ctxt)) ||
  (rc = load_seg(x86_seg_cs, imm2, 0, , ctxt, ops)) ||
  (validate_far_branch(, imm1),
   rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes),
-  , op_bytes, ctxt)) ||
+  , op_bytes, ctxt)) ||
  (rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes),
   &_regs.eip, op_bytes, ctxt)) ||
  (rc = ops->write_segment(x86_seg_cs, , ctxt)) )
@@ -2970,7 +2965,6 @@ x86_emulate(
 
 _regs.eip = imm1;
 break;
-}
 
 case 0x9b:  /* wait/fwait */
 emulate_fpu_insn("fwait");
@@ -4179,13 +4173,12 @@ x86_emulate(
 
 if ( (modrm_reg & 7) == 3 ) /* call */
 {
-struct segment_register reg;
 fail_if(ops->read_segment == NULL);
-if ( (rc = ops->read_segment(x86_seg_cs, , ctxt)) ||
+if ( (rc = ops->read_segment(x86_seg_cs, , ctxt)) ||
  (rc = load_seg(x86_seg_cs, sel, 0, , ctxt, ops)) ||
  (validate_far_branch(, src.val),
   rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes),
-  , op_bytes, ctxt)) ||
+  , op_bytes, ctxt)) ||
  (rc = ops->write(x86_seg_ss, sp_pre_dec(op_bytes),
   &_regs.eip, op_bytes, ctxt)) ||
  (rc = ops->write_segment(x86_seg_cs, , ctxt)) )
@@ -4207,23 +4200,13 @@ x86_emulate(
 break;
 
 case X86EMUL_OPC(0x0f, 0x00): /* Grp6 */
-{
-enum x86_segment seg = (modrm_reg & 1) ? x86_seg_tr :