From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH 04/13] KVM: X86: Add x86 callback for intercept check
Date: Sat, 26 Mar 2011 11:23:02 +0200
Message-ID: <4D8DB076.50408@redhat.com>
References: <1301045356-25257-1-git-send-email-joerg.roedel@amd.com> <1301045356-25257-5-git-send-email-joerg.roedel@amd.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Marcelo Tosatti <mtosatti@redhat.com>, kvm@vger.kernel.org
To: Joerg Roedel <joerg.roedel@amd.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:56735 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752583Ab1CZJXG (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sat, 26 Mar 2011 05:23:06 -0400
In-Reply-To: <1301045356-25257-5-git-send-email-joerg.roedel@amd.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 03/25/2011 11:29 AM, Joerg Roedel wrote:
> This patch adds a callback into kvm_x86_ops so that svm and
> vmx code can do intercept checks on emulated instructions.
>
>
>
> +/*
> + * This struct is used to carry enough information from the instruction
> + * decoder to main KVM so that a decision can be made whether the
> + * instruction needs to be intercepted or not.
> + */
> +struct x86_instruction_info {
> +	u8  intercept;		/* which intercept			*/
> +	u8  rep_prefix;		/* rep prefix?				*/
> +	u8  modrm;		/* index of register used		*/
> +	u64 src_val;		/* value of source operand		*/
> +	u8  src_bytes;		/* size of source operand		*/
> +	u8  dst_bytes;		/* size of destination operand		*/
> +	u8  ad_bytes;		/* size of src/dst address		*/
> +	u64 next_rip;		/* rip following the instruction	*/
> +};

Should be in kvm_emulate.h.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 90a41aa..bf72ec6 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4245,7 +4245,25 @@ static int emulator_intercept(struct x86_emulate_ctxt *ctxt,
>   			      enum x86_intercept intercept,
>   			      enum x86_intercept_stage stage)
>   {
> -	return X86EMUL_CONTINUE;
> +	struct x86_instruction_info info = {
> +		.intercept  = intercept,
> +		.rep_prefix = ctxt->decode.rep_prefix,
> +		.modrm      = ctxt->decode.modrm,
> +		.src_val    = ctxt->decode.src.val64,
> +		.src_bytes  = ctxt->decode.src.bytes,
> +		.dst_bytes  = ctxt->decode.dst.bytes,
> +		.ad_bytes   = ctxt->decode.ad_bytes,
> +		.next_rip   = ctxt->eip,
> +	};

And this should be in emulate.c, so kvm code doesn't have to peek into 
the emulator internals.


> +
> +	/*
> +	 * The callback only needs to be implemented if the architecture
> +	 * supports emulated guest-mode. This BUG_ON reminds the
> +	 * programmer that this callback needs to be implemented.
> +	 */
> +	BUG_ON(kvm_x86_ops->check_intercept == NULL);
> +

BUG_ON()s are nasty.  I prefer a null implementation for vmx.

> +	return kvm_x86_ops->check_intercept(ctxt->vcpu,&info, stage);
>   }
>
>   static struct x86_emulate_ops emulate_ops = {


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