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

Magnus Kulke posted 25 patches 5 months, 4 weeks ago
There is a newer version of this series
[RFC PATCH 02/25] target/i386/emulate: allow instruction decoding from stream
Posted by Magnus Kulke 5 months, 4 weeks ago
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>
---
 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,
-- 
2.34.1
Re: [RFC PATCH 02/25] target/i386/emulate: allow instruction decoding from stream
Posted by Wei Liu 5 months, 4 weeks ago
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 <magnuskulke@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
>
Re: [RFC PATCH 02/25] target/i386/emulate: allow instruction decoding from stream
Posted by Paolo Bonzini 5 months, 4 weeks ago
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,