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 <magnusku...@linux.microsoft.com>
> ---
>  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
> 

Reply via email to