Re: [PATCH 1/2] x86 emulator: Add unary mul, imul, div, and idiv instructions

[Avi Kivity <avi@redhat.com>]

  On 08/08/2010 05:27 AM, Mohammed Gamal wrote:
> This adds unary mul, imul, div, and idiv instructions (group 3 r/m 4-7).
>
> Signed-off-by: Mohammed Gamal<m.gamal005@gmail.com>
> ---
>   arch/x86/kvm/emulate.c |   41 ++++++++++++++++++++++++++++++++++++++++-
>   1 files changed, 40 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 7d78832..6d1ec53 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -315,6 +315,31 @@ struct group_dual {
>   		}							\
>   	} while (0)
>
> +#define __emulate_1op_src(_op, _src, _ax, _dx, _eflags, _suffix)		\

Not just 1op - add rax_rdx to the name to indicate these are implicit 
operands.

> +	do {								\
> +		unsigned long _tmp;					\
> +									\
> +		__asm__ __volatile__ (					\
> +			_PRE_EFLAGS("0", "4", "1")			\
> +			_op _suffix " %5; "				\
> +			_POST_EFLAGS("0", "4", "1")			\
> +			: "=m" (_eflags), "=&r" (_tmp),			\
> +			  "=a" (_ax), "=d" (_dx)			\
> +			: "i" (EFLAGS_MASK), "m" ((_src).val),		\
> +			  "a" (_ax), "d" (_dx));			\
> +	} while (0)							

The byte form of the instruction doesn't update dx, and the word form 
doesn't update edx[16:31].  So the "=a" and "=d" operands need to be 
"+a" and "+d" so the compiler loads them before the operation is started.

Please add a test to the effect, for example start with eax=0x12345678, 
and multiply (byte size) 0x80 by 0x40, and observe that the upper 16 
bits of eax are preserved (and that rdx is not modified).

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.