Re: [PATCH] KVM: vmx: VMXOFF emulation in vm86 should cause #UD

[Paolo Bonzini <pbonzini@redhat.com>]

Il 29/08/2014 10:26, Nadav Amit ha scritto:
> Unlike VMCALL, the instructions VMXOFF, VMLAUNCH and VMRESUME should cause a UD
> exception in real-mode or vm86.  However, the emulator considers all these
> instructions the same for the matter of mode checks, and emulation upon exit
> due to #UD exception.
> 
> As a result, the hypervisor behaves incorrectly on vm86 mode. VMXOFF, VMLAUNCH
> or VMRESUME cause on vm86 exit due to #UD. The hypervisor then emulates these
> instruction and inject #GP to the guest instead of #UD.
> 
> This patch creates a new group for these instructions and mark only VMCALL as
> an instruction which can be emulated.
> 
> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>

Patch looks good, but where is the check that MOD == 3 in the "case
RMExt"?  Am I just not seeing it?

Paolo

> ---
>  arch/x86/kvm/emulate.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index e5bf130..a240fac 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -3139,12 +3139,8 @@ static int em_clts(struct x86_emulate_ctxt *ctxt)
>  
>  static int em_vmcall(struct x86_emulate_ctxt *ctxt)
>  {
> -	int rc;
> -
> -	if (ctxt->modrm_mod != 3 || ctxt->modrm_rm != 1)
> -		return X86EMUL_UNHANDLEABLE;
> +	int rc = ctxt->ops->fix_hypercall(ctxt);
>  
> -	rc = ctxt->ops->fix_hypercall(ctxt);
>  	if (rc != X86EMUL_CONTINUE)
>  		return rc;
>  
> @@ -3562,6 +3558,12 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
>  		F2bv(((_f) | DstReg | SrcMem | ModRM) & ~Lock, _e),	\
>  		F2bv(((_f) & ~Lock) | DstAcc | SrcImm, _e)
>  
> +static const struct opcode group7_rm0[] = {
> +	N,
> +	I(SrcNone | Priv | EmulateOnUD,	em_vmcall),
> +	N, N, N, N, N, N,
> +};
> +
>  static const struct opcode group7_rm1[] = {
>  	DI(SrcNone | Priv, monitor),
>  	DI(SrcNone | Priv, mwait),
> @@ -3655,7 +3657,7 @@ static const struct group_dual group7 = { {
>  	II(SrcMem16 | Mov | Priv,		em_lmsw, lmsw),
>  	II(SrcMem | ByteOp | Priv | NoAccess,	em_invlpg, invlpg),
>  }, {
> -	I(SrcNone | Priv | EmulateOnUD,	em_vmcall),
> +	EXT(0, group7_rm0),
>  	EXT(0, group7_rm1),
>  	N, EXT(0, group7_rm3),
>  	II(SrcNone | DstMem | Mov,		em_smsw, smsw), N,
>