Re: [Xen-devel] [PATCH] x86emul: correct CF output of SHLD/SHRD

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 21/06/17 13:45, Jan Beulich wrote:
> CF reflects the last bit shifted out, i.e. can't possibly be derived
> from the result value.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

I agree with your reasoning, but the test code doesn't compile.

GCC 4.9 complains with:

test_x86_emulator.c: Assembler messages:
test_x86_emulator.c:795: Error: operand type mismatch for `shld'
test_x86_emulator.c:795: Error: operand type mismatch for `pop'
Makefile:110: recipe for target 'test_x86_emulator.o' failed

while Clang 3.5 is a little more helpful:

test_x86_emulator.c:795:15: error: unknown use of instruction mnemonic
without a size suffix
        asm ( "shld $1,%2,%0; pushf; pop %1"
              ^
<inline asm>:1:2: note: instantiated into assembly here
        shld $1,%rax,%ecx; pushf; pop 1272(%rsp)
        ^
1 error generated.
Makefile:110: recipe for target 'test_x86_emulator.o' failed

~Andrew

> ---
> This will only apply cleanly on top of
> https://lists.xenproject.org/archives/html/xen-devel/2017-06/msg02137.html 
>
> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -783,6 +783,29 @@ int main(int argc, char **argv)
>      printf("okay\n");
>  #endif
>  
> +    printf("%-40s", "Testing shld $1,%ecx,(%edx)...");
> +    res[0]      = 0x12345678;
> +    regs.edx    = (unsigned long)res;
> +    regs.ecx    = 0x9abcdef0;
> +    instr[0] = 0x0f; instr[1] = 0xa4; instr[2] = 0x0a; instr[3] = 0x01;
> +    for ( i = 0; i < 0x20; ++i )
> +    {
> +        uint32_t r = res[0], f, m = X86_EFLAGS_ARITH_MASK & ~X86_EFLAGS_AF;
> +
> +        asm ( "shld $1,%2,%0; pushf; pop %1"
> +              : "+rm" (r), "=rm" (f) : "r" (regs.ecx) );
> +        regs.eflags = f ^ m;
> +        regs.eip    = (unsigned long)&instr[0];
> +        rc = x86_emulate(&ctxt, &emulops);
> +        if ( (rc != X86EMUL_OKAY) ||
> +             (regs.eip != (unsigned long)&instr[4]) ||
> +             (res[0] != r) ||
> +             ((regs.eflags ^ f) & m) )
> +            goto fail;
> +        regs.ecx <<= 1;
> +    }
> +    printf("okay\n");
> +
>      printf("%-40s", "Testing movbe (%ecx),%eax...");
>      instr[0] = 0x0f; instr[1] = 0x38; instr[2] = 0xf0; instr[3] = 0x01;
>      regs.eflags = 0x200;
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -6424,7 +6424,7 @@ x86_emulate(
>                     (src.val >> (width - shift)));
>          _regs.eflags &= ~(X86_EFLAGS_OF | X86_EFLAGS_SF | X86_EFLAGS_ZF |
>                            X86_EFLAGS_PF | X86_EFLAGS_CF);
> -        if ( (dst.val >> ((b & 8) ? (shift - 1) : (width - shift))) & 1 )
> +        if ( (dst.orig_val >> ((b & 8) ? (shift - 1) : (width - shift))) & 1 
> )
>              _regs.eflags |= X86_EFLAGS_CF;
>          if ( ((dst.val ^ dst.orig_val) >> (width - 1)) & 1 )
>              _regs.eflags |= X86_EFLAGS_OF;
>
>
>


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