From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH] x86 emulator: Add Src2 decode set
Date: Thu, 09 Oct 2008 10:11:57 +0200
Message-ID: <48EDBCCD.4010705@redhat.com>
References: <20081008145245.78c7084d@frecb000711> <48ECC028.5070807@redhat.com> <20081009095253.66737bf1@frecb000711>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Cc: kvm <kvm@vger.kernel.org>
To: Guillaume Thouvenin <guillaume.thouvenin@ext.bull.net>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:45641 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1756122AbYJIIMe (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 9 Oct 2008 04:12:34 -0400
In-Reply-To: <20081009095253.66737bf1@frecb000711>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Guillaume Thouvenin wrote:
> Instruction like shld has three operands, so we need to add a Src2
> decode set. We start with Src2None, Src2CL, and Src2Imm8 to support
> shld and we will expand it later.
>
>   

Please add Src2One (implied '1') as well, so we can switch the existing
shift operators to Src2 later.

> Signed-off-by: Guillaume Thouvenin <guillaume.thouvenin@ext.bull.net>
> ---
>  arch/x86/kvm/x86_emulate.c        |   47 ++++++++++++++++++++++++++++----------
>  include/asm-x86/kvm_x86_emulate.h |    1 
>  2 files changed, 36 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
> index a391e21..c9ef2da 100644
> --- a/arch/x86/kvm/x86_emulate.c
> +++ b/arch/x86/kvm/x86_emulate.c
> @@ -59,16 +59,21 @@
>  #define SrcImm      (5<<4)	/* Immediate operand. */
>  #define SrcImmByte  (6<<4)	/* 8-bit sign-extended immediate operand. */
>  #define SrcMask     (7<<4)
> +/* Source 2 operand type */
> +#define Src2None    (0<<7)
> +#define Src2CL      (1<<7)
> +#define Src2Imm8    (2<<7)
> +#define Src2Mask    (7<<7)
>   

Please allocate bits for this at the end to avoid renumbering.

>  
> +	/*
> +	 * Decode and fetch the second source operand: register, memory
> +	 * or immediate.
> +	 */
> +	switch (c->d & Src2Mask) {
> +	case Src2None:
> +		break;
> +	case Src2CL:
> +		c->src2.val = c->regs[VCPU_REGS_RCX];
>   

Mask to a single byte; also set the operand length.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.