Re: [PATCH RFC v1 11/20] KVM: x86: Refactor opcode table lookup in instruction emulation

[Paolo Bonzini <pbonzini@redhat.com>]

On 11/10/25 19:01, Chang S. Bae wrote:
> Refactor opcode lookup to clearly separate handling of different byte
> sequences and prefix types, in preparation for REX2 support.
> 
> The decoder begins with a one-byte opcode table by default and falls
> through to other tables on escape bytes, but the logic is intertwined and
> hard to extend.
> 
> REX2 introduces a dedicated bit in its payload byte to indicate which
> opcode table to use. To accommodate this mapping bit, the existing lookup
> path needs to be restructured.
> 
> No functional changes intended.
> 
> Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
> ---
>   arch/x86/kvm/emulate.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 763fbd139242..9c98843094a1 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -4773,7 +4773,6 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len, int
>   	ctxt->_eip = ctxt->eip;
>   	ctxt->fetch.ptr = ctxt->fetch.data;
>   	ctxt->fetch.end = ctxt->fetch.data + insn_len;
> -	ctxt->opcode_len = 1;
>   	ctxt->intercept = x86_intercept_none;
>   	if (insn_len > 0)
>   		memcpy(ctxt->fetch.data, insn, insn_len);
> @@ -4877,20 +4876,24 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len, int
>   	if (ctxt->rex.bits.w)
>   		ctxt->op_bytes = 8;
>   
> -	/* Opcode byte(s). */
> -	opcode = opcode_table[ctxt->b];
> -	/* Two-byte opcode? */
> +	/* Determine opcode byte(s): */
>   	if (ctxt->b == 0x0f) {
> -		ctxt->opcode_len = 2;
> +		/* Escape byte: start two-byte opcode sequence */
>   		ctxt->b = insn_fetch(u8, ctxt);
> -		opcode = twobyte_table[ctxt->b];
> -
> -		/* 0F_38 opcode map */
>   		if (ctxt->b == 0x38) {
> +			/* Three-byte opcode */
>   			ctxt->opcode_len = 3;
>   			ctxt->b = insn_fetch(u8, ctxt);
>   			opcode = opcode_map_0f_38[ctxt->b];
> +		} else {
> +			/* Two-byte opcode */
> +			ctxt->opcode_len = 2;
> +			opcode = twobyte_table[ctxt->b];
>   		}
> +	} else {
> +		/* Single-byte opcode */
> +		ctxt->opcode_len = 1;
> +		opcode = opcode_table[ctxt->b];
>   	}
>   	ctxt->d = opcode.flags;
>   

This will also conflict with the VEX patches, overall I think all 10-12 
patches can be merged in one.

Paolo