Re: [PATCH] x86 emulator: Add Src2 decode set

[Avi Kivity <avi@redhat.com>]

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.