[PATCH 0/3] Trap and propagate divide errors when emulating DIV

[PATCH 0/3] Trap and propagate divide errors when emulating DIV

[Avi Kivity]

KVM recently started emulating DIV and IDIV.  However, those instructions
trap when given the right operands.  Since figuring out when to trap or not
is difficult, we just execute the instruction and see if the processor trapped
or not.

tip: please queue the first patch on fast-forward-only branch kvm.git can
     merge, or we can carry the patch in kvm.git with your ack.

Avi Kivity (3):
  x86: allow kernel exception fixup for divide errors (#DE)
  KVM: x86 emulator: add macros for executing instructions that may
    trap
  KVM: x86 emulator: trap and propagate #DE from DIV and IDIV

 arch/x86/kernel/traps.c |   10 +++++++-
 arch/x86/kvm/emulate.c  |   60 ++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 66 insertions(+), 4 deletions(-)

[PATCH 1/3] x86: allow kernel exception fixup for divide errors (#DE)

[Avi Kivity]

KVM wants to emulate the DIV and IDIV instructions by executing them natively;
this can cause a #DE to be raised.

Allow the exception handling mechanism to process #DE exceptions so KVM can
catch and process them.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kernel/traps.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 725ef4d..dd313cf 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -205,7 +205,8 @@ dotraplinkage void do_##name(struct pt_regs *regs, long error_code)	\
 	do_trap(trapnr, signr, str, regs, error_code, &info);		\
 }
 
-DO_ERROR_INFO(0, SIGFPE, "divide error", divide_error, FPE_INTDIV, regs->ip)
+static DO_ERROR_INFO(0, SIGFPE, "divide error", divide_error_user,
+		     FPE_INTDIV, regs->ip)
 DO_ERROR(4, SIGSEGV, "overflow", overflow)
 DO_ERROR(5, SIGSEGV, "bounds", bounds)
 DO_ERROR_INFO(6, SIGILL, "invalid opcode", invalid_op, ILL_ILLOPN, regs->ip)
@@ -217,6 +218,13 @@ DO_ERROR(12, SIGBUS, "stack segment", stack_segment)
 #endif
 DO_ERROR_INFO(17, SIGBUS, "alignment check", alignment_check, BUS_ADRALN, 0)
 
+dotraplinkage void do_divide_error(struct pt_regs *regs, long error_code)
+{
+	if (!user_mode_vm(regs) && fixup_exception(regs))
+		return;
+	do_divide_error_user(regs, error_code);
+}
+
 #ifdef CONFIG_X86_64
 /* Runs on IST stack */
 dotraplinkage void do_stack_segment(struct pt_regs *regs, long error_code)
-- 
1.7.1

Re: [PATCH 1/3] x86: allow kernel exception fixup for divide errors (#DE)

[Brian Gerst]

On Tue, Aug 24, 2010 at 7:10 AM, Avi Kivity <avi@redhat.com> wrote:
> KVM wants to emulate the DIV and IDIV instructions by executing them natively;
> this can cause a #DE to be raised.
>
> Allow the exception handling mechanism to process #DE exceptions so KVM can
> catch and process them.
>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/kernel/traps.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index 725ef4d..dd313cf 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -205,7 +205,8 @@ dotraplinkage void do_##name(struct pt_regs *regs, long error_code) \
>        do_trap(trapnr, signr, str, regs, error_code, &info);           \
>  }
>
> -DO_ERROR_INFO(0, SIGFPE, "divide error", divide_error, FPE_INTDIV, regs->ip)
> +static DO_ERROR_INFO(0, SIGFPE, "divide error", divide_error_user,
> +                    FPE_INTDIV, regs->ip)
>  DO_ERROR(4, SIGSEGV, "overflow", overflow)
>  DO_ERROR(5, SIGSEGV, "bounds", bounds)
>  DO_ERROR_INFO(6, SIGILL, "invalid opcode", invalid_op, ILL_ILLOPN, regs->ip)
> @@ -217,6 +218,13 @@ DO_ERROR(12, SIGBUS, "stack segment", stack_segment)
>  #endif
>  DO_ERROR_INFO(17, SIGBUS, "alignment check", alignment_check, BUS_ADRALN, 0)
>
> +dotraplinkage void do_divide_error(struct pt_regs *regs, long error_code)
> +{
> +       if (!user_mode_vm(regs) && fixup_exception(regs))
> +               return;
> +       do_divide_error_user(regs, error_code);
> +}
> +
>  #ifdef CONFIG_X86_64
>  /* Runs on IST stack */
>  dotraplinkage void do_stack_segment(struct pt_regs *regs, long error_code)

Kernel mode exceptions should already be handled by do_trap().  This
is unnecessary.

--
Brian Gerst

Re: [PATCH 1/3] x86: allow kernel exception fixup for divide errors (#DE)

[Avi Kivity]

  On 08/24/2010 02:22 PM, Brian Gerst wrote:
>
>> +dotraplinkage void do_divide_error(struct pt_regs *regs, long error_code)
>> +{
>> +       if (!user_mode_vm(regs)&&  fixup_exception(regs))
>> +               return;
>> +       do_divide_error_user(regs, error_code);
>> +}
>> +
>>   #ifdef CONFIG_X86_64
>>   /* Runs on IST stack */
>>   dotraplinkage void do_stack_segment(struct pt_regs *regs, long error_code)
> Kernel mode exceptions should already be handled by do_trap().  This
> is unnecessary.

Correct.  Please ignore this patch.  I've verified that the other two 
work without it.

-- 
error compiling committee.c: too many arguments to function

[PATCH 2/3] KVM: x86 emulator: add macros for executing instructions that may trap

[Avi Kivity]

Like DIV and IDIV.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 808934c..f82e43a 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -331,6 +331,27 @@ struct group_dual {
 			  "a" (_rax), "d" (_rdx));			\
 	} while (0)
 
+#define __emulate_1op_rax_rdx_ex(_op, _src, _rax, _rdx, _eflags, _suffix, _ex) \
+	do {								\
+		unsigned long _tmp;					\
+									\
+		__asm__ __volatile__ (					\
+			_PRE_EFLAGS("0", "5", "1")			\
+			"1: \n\t"					\
+			_op _suffix " %6; "				\
+			"2: \n\t"					\
+			_POST_EFLAGS("0", "5", "1")			\
+			".pushsection .fixup,\"ax\" \n\t"		\
+			"3: movb $1, %4 \n\t"				\
+			"jmp 2b \n\t"					\
+			".popsection \n\t"				\
+			_ASM_EXTABLE(1b, 3b)				\
+			: "=m" (_eflags), "=&r" (_tmp),			\
+			  "+a" (_rax), "+d" (_rdx), "+qm"(_ex)		\
+			: "i" (EFLAGS_MASK), "m" ((_src).val),		\
+			  "a" (_rax), "d" (_rdx));			\
+	} while (0)
+
 /* instruction has only one source operand, destination is implicit (e.g. mul, div, imul, idiv) */
 #define emulate_1op_rax_rdx(_op, _src, _rax, _rdx, _eflags)			\
 	do {									\
@@ -342,6 +363,28 @@ struct group_dual {
 		}							\
 	} while (0)
 
+#define emulate_1op_rax_rdx_ex(_op, _src, _rax, _rdx, _eflags, _ex)	\
+	do {								\
+		switch((_src).bytes) {					\
+		case 1:							\
+			__emulate_1op_rax_rdx_ex(_op, _src, _rax, _rdx,	\
+						 _eflags, "b", _ex);	\
+			break;						\
+		case 2:							\
+			__emulate_1op_rax_rdx_ex(_op, _src, _rax, _rdx, \
+						 _eflags, "w", _ex);	\
+			break;						\
+		case 4:							\
+			__emulate_1op_rax_rdx_ex(_op, _src, _rax, _rdx, \
+						 _eflags, "l", _ex);	\
+			break;						\
+		case 8: ON64(						\
+			__emulate_1op_rax_rdx_ex(_op, _src, _rax, _rdx, \
+						 _eflags, "q", _ex));	\
+			break;						\
+		}							\
+	} while (0)
+
 /* Fetch next part of the instruction being emulated. */
 #define insn_fetch(_type, _size, _eip)                                  \
 ({	unsigned long _x;						\
-- 
1.7.1

[PATCH 3/3] KVM: x86 emulator: trap and propagate #DE from DIV and IDIV

[Avi Kivity]

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/emulate.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index f82e43a..a7e26d0 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -505,6 +505,12 @@ static void emulate_ts(struct x86_emulate_ctxt *ctxt, int err)
 	emulate_exception(ctxt, TS_VECTOR, err, true);
 }
 
+static int emulate_de(struct x86_emulate_ctxt *ctxt)
+{
+	emulate_exception(ctxt, DE_VECTOR, 0, false);
+	return X86EMUL_PROPAGATE_FAULT;
+}
+
 static int do_fetch_insn_byte(struct x86_emulate_ctxt *ctxt,
 			      struct x86_emulate_ops *ops,
 			      unsigned long eip, u8 *dest)
@@ -1459,6 +1465,7 @@ static inline int emulate_grp3(struct x86_emulate_ctxt *ctxt,
 	struct decode_cache *c = &ctxt->decode;
 	unsigned long *rax = &c->regs[VCPU_REGS_RAX];
 	unsigned long *rdx = &c->regs[VCPU_REGS_RDX];
+	u8 de = 0;
 
 	switch (c->modrm_reg) {
 	case 0 ... 1:	/* test */
@@ -1477,14 +1484,18 @@ static inline int emulate_grp3(struct x86_emulate_ctxt *ctxt,
 		emulate_1op_rax_rdx("imul", c->src, *rax, *rdx, ctxt->eflags);
 		break;
 	case 6: /* div */
-		emulate_1op_rax_rdx("div", c->src, *rax, *rdx, ctxt->eflags);
+		emulate_1op_rax_rdx_ex("div", c->src, *rax, *rdx,
+				       ctxt->eflags, de);
 		break;
 	case 7: /* idiv */
-		emulate_1op_rax_rdx("idiv", c->src, *rax, *rdx, ctxt->eflags);
+		emulate_1op_rax_rdx_ex("idiv", c->src, *rax, *rdx,
+				       ctxt->eflags, de);
 		break;
 	default:
 		return X86EMUL_UNHANDLEABLE;
 	}
+	if (de)
+		return emulate_de(ctxt);
 	return X86EMUL_CONTINUE;
 }
 
@@ -3363,7 +3374,7 @@ special_insn:
 		break;
 	case 0xf6 ... 0xf7:	/* Grp3 */
 		if (emulate_grp3(ctxt, ops) != X86EMUL_CONTINUE)
-			goto cannot_emulate;
+			goto done;
 		break;
 	case 0xf8: /* clc */
 		ctxt->eflags &= ~EFLG_CF;
-- 
1.7.1

Re: [PATCH 3/3] KVM: x86 emulator: trap and propagate #DE from DIV and IDIV

[Marcelo Tosatti]

On Tue, Aug 24, 2010 at 02:10:29PM +0300, Avi Kivity wrote:
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/kvm/emulate.c |   17 ++++++++++++++---
>  1 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index f82e43a..a7e26d0 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -505,6 +505,12 @@ static void emulate_ts(struct x86_emulate_ctxt *ctxt, int err)
>  	emulate_exception(ctxt, TS_VECTOR, err, true);
>  }
>  
> +static int emulate_de(struct x86_emulate_ctxt *ctxt)
> +{
> +	emulate_exception(ctxt, DE_VECTOR, 0, false);
> +	return X86EMUL_PROPAGATE_FAULT;
> +}
> +
>  static int do_fetch_insn_byte(struct x86_emulate_ctxt *ctxt,
>  			      struct x86_emulate_ops *ops,
>  			      unsigned long eip, u8 *dest)
> @@ -1459,6 +1465,7 @@ static inline int emulate_grp3(struct x86_emulate_ctxt *ctxt,
>  	struct decode_cache *c = &ctxt->decode;
>  	unsigned long *rax = &c->regs[VCPU_REGS_RAX];
>  	unsigned long *rdx = &c->regs[VCPU_REGS_RDX];
> +	u8 de = 0;
>  
>  	switch (c->modrm_reg) {
>  	case 0 ... 1:	/* test */
> @@ -1477,14 +1484,18 @@ static inline int emulate_grp3(struct x86_emulate_ctxt *ctxt,
>  		emulate_1op_rax_rdx("imul", c->src, *rax, *rdx, ctxt->eflags);
>  		break;
>  	case 6: /* div */
> -		emulate_1op_rax_rdx("div", c->src, *rax, *rdx, ctxt->eflags);
> +		emulate_1op_rax_rdx_ex("div", c->src, *rax, *rdx,
> +				       ctxt->eflags, de);
>  		break;
>  	case 7: /* idiv */
> -		emulate_1op_rax_rdx("idiv", c->src, *rax, *rdx, ctxt->eflags);
> +		emulate_1op_rax_rdx_ex("idiv", c->src, *rax, *rdx,
> +				       ctxt->eflags, de);
>  		break;
>  	default:
>  		return X86EMUL_UNHANDLEABLE;
>  	}
> +	if (de)
> +		return emulate_de(ctxt);
>  	return X86EMUL_CONTINUE;
>  }
>  
> @@ -3363,7 +3374,7 @@ special_insn:
>  		break;
>  	case 0xf6 ... 0xf7:	/* Grp3 */
>  		if (emulate_grp3(ctxt, ops) != X86EMUL_CONTINUE)
> -			goto cannot_emulate;
> +			goto done;
>  		break;

Must assign rc to emulate_grp3() retval.

Re: [PATCH 3/3] KVM: x86 emulator: trap and propagate #DE from DIV and IDIV

[Avi Kivity]

  On 08/26/2010 12:05 AM, Marcelo Tosatti wrote:
>
>> @@ -3363,7 +3374,7 @@ special_insn:
>>   		break;
>>   	case 0xf6 ... 0xf7:	/* Grp3 */
>>   		if (emulate_grp3(ctxt, ops) != X86EMUL_CONTINUE)
>> -			goto cannot_emulate;
>> +			goto done;
>>   		break;
> Must assign rc to emulate_grp3() retval.

While there's not much difference between X86EMUL_PROPAGATE_FAULT and 
X86EMUL_CONTINUE, it's bad form to break things internally.  I'll fix 
and repost.

-- 
error compiling committee.c: too many arguments to function