Re: [qemu-upstream-unstable test] 21375: regressions - FAIL

[Anthony PERARD <anthony.perard@citrix.com>]

On Tue, Nov 19, 2013 at 01:05:37PM +0000, Jan Beulich wrote:
> >>> On 18.11.13 at 18:18, Anthony PERARD <anthony.perard@citrix.com> wrote:
> > An other update,
> > 
> > we had the idee of trying this on earlier versin of Xen, and it turns
> > out that Xen 4.3 works fine. One bisect later, and a commit turns out.
> > 
> > commit 86781624f8df1d50eb4185cfc2ddce926798f7aa
> > x86_emulate: PUSH <mem> must read source operand just once
> > ... for the case of accessing MMIO.
> > 
> > So after this commit, syslinux stop working correctly with the last
> > version of QEMU. This happen if QEMU is calling track_dirty_vram.
> > 
> > I also have use xentrace/xenalyze to try to grab more information about
> > the issue, it did not really help, but it's tell me that the guest is
> > stock on a specific instruction (it result in vmexit EPT_VIOLATION over
> > and over on xentrace). And that were the guest is stock:
> > 
> >    0xa126:  mov    %eax,%cr0
> >    0xa129:  ljmp   $0xf2e,$0xa12e
> >    0xa130:  mov    $0x26,%dl
> >    0xa132:  or     %bh,(%eax)
> >    0xa134:  movzww %sp,%sp
> >    0xa138:  mov    %edx,%ds
> >    0xa13a:  mov    %edx,%es
> >    0xa13c:  mov    %edx,%fs
> >    0xa13e:  mov    %edx,%gs
> >    0xa140:  jmp    *%ebx
> >    0xa142:  pushf  
> > => 0xa143:  lcall  *%cs:(%si)
> >    0xa147:  mov    $0x0,%ch
> > 
> > Before trying on earlier version of Xen, I try to understand what when
> > wrong on the Xen side, it turn out that, in the track_dirty_vram
> > hypercall, a call to hap_enable_log_dirty() is all that needed to break
> > the guest.
> > 
> > Jan, any idee of what the issue is?
> 
> Oh, indeed, I screwed this up without noticing. Please try the
> attached patch.

Looks like this patch fix the issue. The guest is now running fine.

Thanks.

> x86: fix emulation of indirect far calls and jumps
> 
> Commit 86781624 ("x86_emulate: PUSH <mem> must read source operand
> just once") corrected the operands of those of the operations of opcode
> extension group 5 that only read memory from SrcMem to DstMem, but
> failed to also switch the use of "dst" here to "src".
> 
> Reported-by: Anthony Perard <anthony.perard@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -3571,7 +3571,6 @@ x86_emulate(
>              _regs.eip = src.val;
>              src.val = dst.val;
>              goto push;
> -            break;
>          case 4: /* jmp (near) */
>              _regs.eip = src.val;
>              dst.type = OP_NONE;
> @@ -3580,9 +3579,9 @@ x86_emulate(
>          case 5: /* jmp (far, absolute indirect) */ {
>              unsigned long sel;
>  
> -            generate_exception_if(dst.type != OP_MEM, EXC_UD, -1);
> +            generate_exception_if(src.type != OP_MEM, EXC_UD, -1);
>  
> -            if ( (rc = read_ulong(dst.mem.seg, dst.mem.off+dst.bytes,
> +            if ( (rc = read_ulong(src.mem.seg, src.mem.off + op_bytes,
>                                    &sel, 2, ctxt, ops)) )
>                  goto done;
>  
> @@ -3600,7 +3599,7 @@ x86_emulate(
>  
>              if ( (rc = load_seg(x86_seg_cs, sel, ctxt, ops)) != 0 )
>                  goto done;
> -            _regs.eip = dst.val;
> +            _regs.eip = src.val;
>  
>              dst.type = OP_NONE;
>              break;


-- 
Anthony PERARD