On Tue, May 20, 2025 at 01:29:55PM +0200, 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 <[email protected]>
> ---
> 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);
> + }
You're using tabs here.
> decode->len += size;
> -
> +
Unrelated whitespace change.
> 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));
Why lift this out to its callers when in both cases they need to call
memset anyway?
> 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;
Tab here.
> } 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,
Tabs here again.
I suppose you will need to configure your editor correctly and change
from tabs to spaces.
Thanks,
Wei.
> + 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,
> --
> 2.34.1
>