Re: [RFC PATCH 02/25] target/i386/emulate: allow instruction decoding from stream

[Paolo Bonzini <pbonzini@redhat.com>]

On 5/20/25 13:29, Magnus Kulke wrote:
> Introduce a new helper function to decode x86 instructions from a
> raw instruction byte stream. MSHV delivers an instruction stream in a
> buffer of the vm_exit message. It can be used to speed up MMIO
> emulation, since instructions do not have to be fetched and translated.
> 
> Signed-off-by: Magnus Kulke <magnuskulke@linux.microsoft.com>

Missing update of hvf_x86_emul_ops.

Paolo

> ---
>   target/i386/emulate/x86_decode.c | 32 +++++++++++++++++++++++++++-----
>   target/i386/emulate/x86_decode.h | 11 +++++++++++
>   target/i386/emulate/x86_emu.c    |  3 ++-
>   target/i386/emulate/x86_emu.h    |  1 +
>   4 files changed, 41 insertions(+), 6 deletions(-)
> 
> diff --git a/target/i386/emulate/x86_decode.c b/target/i386/emulate/x86_decode.c
> index 88be9479a8..7a862b976e 100644
> --- a/target/i386/emulate/x86_decode.c
> +++ b/target/i386/emulate/x86_decode.c
> @@ -60,6 +60,7 @@ static inline uint64_t decode_bytes(CPUX86State *env, struct x86_decode *decode,
>                                       int size)
>   {
>       uint64_t val = 0;
> +    target_ulong va;
>   
>       switch (size) {
>       case 1:
> @@ -71,10 +72,16 @@ static inline uint64_t decode_bytes(CPUX86State *env, struct x86_decode *decode,
>           VM_PANIC_EX("%s invalid size %d\n", __func__, size);
>           break;
>       }
> -    target_ulong va  = linear_rip(env_cpu(env), env->eip) + decode->len;
> -    emul_ops->read_mem(env_cpu(env), &val, va, size);
> +
> +	/* copy the bytes from the instruction stream, if available */
> +	if (decode->stream && decode->len + size <= decode->stream->len) {
> +		memcpy(&val, decode->stream->bytes + decode->len, size);
> +	} else {
> +		va = linear_rip(env_cpu(env), env->eip) + decode->len;
> +		emul_ops->fetch_instruction(env_cpu(env), &val, va, size);
> +	}
>       decode->len += size;
> -
> +
>       return val;
>   }
>   
> @@ -2076,9 +2083,8 @@ static void decode_opcodes(CPUX86State *env, struct x86_decode *decode)
>       }
>   }
>   
> -uint32_t decode_instruction(CPUX86State *env, struct x86_decode *decode)
> +static uint32_t decode_opcode(CPUX86State *env, struct x86_decode *decode)
>   {
> -    memset(decode, 0, sizeof(*decode));
>       decode_prefix(env, decode);
>       set_addressing_size(env, decode);
>       set_operand_size(env, decode);
> @@ -2088,6 +2094,22 @@ uint32_t decode_instruction(CPUX86State *env, struct x86_decode *decode)
>       return decode->len;
>   }
>   
> +uint32_t decode_instruction(CPUX86State *env, struct x86_decode *decode)
> +{
> +	memset(decode, 0, sizeof(*decode));
> +	return decode_opcode(env, decode);
> +}
> +
> +uint32_t decode_instruction_stream(CPUX86State *env, struct x86_decode *decode,
> +		                           struct x86_insn_stream *stream)
> +{
> +	memset(decode, 0, sizeof(*decode));
> +	if (stream != NULL) {
> +		decode->stream = stream;
> +	}
> +	return decode_opcode(env, decode);
> +}
> +
>   void init_decoder(void)
>   {
>       int i;
> diff --git a/target/i386/emulate/x86_decode.h b/target/i386/emulate/x86_decode.h
> index 87cc728598..9bc7d6cc49 100644
> --- a/target/i386/emulate/x86_decode.h
> +++ b/target/i386/emulate/x86_decode.h
> @@ -269,6 +269,11 @@ typedef struct x86_decode_op {
>       target_ulong ptr;
>   } x86_decode_op;
>   
> +typedef struct x86_insn_stream {
> +	const uint8_t *bytes;
> +	size_t len;
> +} x86_insn_stream;
> +
>   typedef struct x86_decode {
>       int len;
>       uint8_t opcode[4];
> @@ -295,12 +300,18 @@ typedef struct x86_decode {
>       struct x86_modrm modrm;
>       struct x86_decode_op op[4];
>       bool is_fpu;
> +
> +	x86_insn_stream *stream;
>   } x86_decode;
>   
>   uint64_t sign(uint64_t val, int size);
>   
>   uint32_t decode_instruction(CPUX86State *env, struct x86_decode *decode);
>   
> +uint32_t decode_instruction_stream(CPUX86State *env,
> +								   struct x86_decode *decode,
> +		                           struct x86_insn_stream *stream);
> +
>   target_ulong get_reg_ref(CPUX86State *env, int reg, int rex_present,
>                            int is_extended, int size);
>   target_ulong get_reg_val(CPUX86State *env, int reg, int rex_present,
> diff --git a/target/i386/emulate/x86_emu.c b/target/i386/emulate/x86_emu.c
> index 7773b51b95..73c9eb41d1 100644
> --- a/target/i386/emulate/x86_emu.c
> +++ b/target/i386/emulate/x86_emu.c
> @@ -1241,7 +1241,8 @@ static void init_cmd_handler(void)
>   bool exec_instruction(CPUX86State *env, struct x86_decode *ins)
>   {
>       if (!_cmd_handler[ins->cmd].handler) {
> -        printf("Unimplemented handler (" TARGET_FMT_lx ") for %d (%x %x) \n", env->eip,
> +        printf("Unimplemented handler (" TARGET_FMT_lx ") for %d (%x %x) \n",
> +                env->eip,
>                   ins->cmd, ins->opcode[0],
>                   ins->opcode_len > 1 ? ins->opcode[1] : 0);
>           env->eip += ins->len;
> diff --git a/target/i386/emulate/x86_emu.h b/target/i386/emulate/x86_emu.h
> index 555b567e2c..761e83fd6b 100644
> --- a/target/i386/emulate/x86_emu.h
> +++ b/target/i386/emulate/x86_emu.h
> @@ -24,6 +24,7 @@
>   #include "cpu.h"
>   
>   struct x86_emul_ops {
> +    void (*fetch_instruction)(CPUState *cpu, void *data, target_ulong addr, int bytes);
>       void (*read_mem)(CPUState *cpu, void *data, target_ulong addr, int bytes);
>       void (*write_mem)(CPUState *cpu, void *data, target_ulong addr, int bytes);
>       void (*read_segment_descriptor)(CPUState *cpu, struct x86_segment_descriptor *desc,