diff mbox series

[06/11] i386/hvf: APIC access exit with fast-path for common mov cases

Message ID 20241209203629.74436-7-phil@philjordan.eu (mailing list archive)
State New
Headers show
Series hvf and APIC fixes, improvements, and optimisations | expand

Commit Message

Phil Dennis-Jordan Dec. 9, 2024, 8:36 p.m. UTC
From: Phil Dennis-Jordan <phil@philjordan.eu>

The implementation of the EXIT_REASON_APIC_ACCESS vm exit handler has so far
been essentially the same as a regular EPT fault handler, performing a full
simulation of the faulted instruction. The code path has also not been used at
all because the APIC base address setter in Hypervisor.framework was never
called. This change improves the former.

In particular, the APIC_ACCESS exit provides us some additional metadata which
in many cases allows us to avoid a full instruction emulation.

There is no need to walk the memory hierarchy, because exit_qual contains the
APIC MMIO offset. It also tells us whether it's an MMIO read or write. So
we can detect common mov instructions and directly call the relevant APIC
accessor functions.

For more complex instructions, we can fall back to the usual instruction
emulation.

Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
---
 hw/intc/apic.c               |  4 +-
 include/hw/i386/apic.h       |  2 +
 meson.build                  |  1 +
 target/i386/hvf/hvf.c        | 18 +++++++-
 target/i386/hvf/trace-events |  9 ++++
 target/i386/hvf/trace.h      |  1 +
 target/i386/hvf/x86_emu.c    | 84 ++++++++++++++++++++++++++++++++++++
 target/i386/hvf/x86_emu.h    |  2 +
 8 files changed, 117 insertions(+), 4 deletions(-)
 create mode 100644 target/i386/hvf/trace-events
 create mode 100644 target/i386/hvf/trace.h
diff mbox series

Patch

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 4186c57b34..add99f01e5 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -788,7 +788,7 @@  static void apic_timer(void *opaque)
     apic_timer_update(s, s->next_time);
 }
 
-static int apic_register_read(int index, uint64_t *value)
+int apic_register_read(int index, uint64_t *value)
 {
     DeviceState *dev;
     APICCommonState *s;
@@ -936,7 +936,7 @@  static void apic_send_msi(MSIMessage *msi)
     apic_deliver_irq(dest, dest_mode, delivery, vector, trigger_mode);
 }
 
-static int apic_register_write(int index, uint64_t val)
+int apic_register_write(int index, uint64_t val)
 {
     DeviceState *dev;
     APICCommonState *s;
diff --git a/include/hw/i386/apic.h b/include/hw/i386/apic.h
index eb606d6076..47946e5581 100644
--- a/include/hw/i386/apic.h
+++ b/include/hw/i386/apic.h
@@ -20,6 +20,8 @@  void apic_designate_bsp(DeviceState *d, bool bsp);
 int apic_get_highest_priority_irr(DeviceState *dev);
 int apic_msr_read(int index, uint64_t *val);
 int apic_msr_write(int index, uint64_t val);
+int apic_register_read(int index, uint64_t *value);
+int apic_register_write(int index, uint64_t val);
 bool is_x2apic_mode(DeviceState *d);
 
 /* pc.c */
diff --git a/meson.build b/meson.build
index 147097c652..0846c09bdb 100644
--- a/meson.build
+++ b/meson.build
@@ -3606,6 +3606,7 @@  if have_system or have_user
     'target/arm/hvf',
     'target/hppa',
     'target/i386',
+    'target/i386/hvf',
     'target/i386/kvm',
     'target/loongarch',
     'target/mips/tcg',
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 3f1ff0f013..2a13a9e49b 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -75,6 +75,7 @@ 
 #include "qemu/main-loop.h"
 #include "qemu/accel.h"
 #include "target/i386/cpu.h"
+#include "trace.h"
 
 static Error *invtsc_mig_blocker;
 
@@ -666,8 +667,21 @@  int hvf_vcpu_exec(CPUState *cpu)
             store_regs(cpu);
             break;
         }
-        case EXIT_REASON_APIC_ACCESS: { /* TODO */
-            exec_instruction(env, &decode);
+        case EXIT_REASON_APIC_ACCESS: {
+            bool is_load = (exit_qual & 0x1000) == 0;
+            uint32_t apic_register_idx = (exit_qual & 0xff0) >> 4;
+
+            if (simulate_fast_path_apic_mmio(is_load, apic_register_idx,
+                                             env, &decode)) {
+                env->eip += ins_len;
+            } else {
+                trace_hvf_x86_vcpu_exec_apic_access_slowpath(
+                    is_load ? "load from" : "store to", apic_register_idx,
+                    ins_len, decode.prefetch_buf[0], decode.prefetch_buf[1],
+                    decode.prefetch_buf[2], decode.prefetch_buf[3],
+                    decode.prefetch_buf[4], decode.prefetch_buf[5]);
+                exec_instruction(env, &decode);
+            }
             store_regs(cpu);
             break;
         }
diff --git a/target/i386/hvf/trace-events b/target/i386/hvf/trace-events
new file mode 100644
index 0000000000..7d0230fb37
--- /dev/null
+++ b/target/i386/hvf/trace-events
@@ -0,0 +1,9 @@ 
+# See docs/devel/tracing.rst for syntax documentation.
+
+# hvf.c
+hvf_x86_vcpu_exec_apic_access_slowpath(const char *access_type, uint32_t apic_register_idx, uint32_t ins_len, uint8_t ins_byte_0, uint8_t ins_byte_1, uint8_t ins_byte_2, uint8_t ins_byte_3, uint8_t ins_byte_4, uint8_t ins_byte_5) "xAPIC %s register 0x%" PRIx32" taking slow path; instruction length: %" PRIu32 ", bytes: %02x %02x %02x %02x  %02x %02x ..."
+
+# x86_emu.c
+hvf_x86_emu_mmio_load_instruction_fastpath(int cmd, int operand_size, int opcode_len, uint8_t opcode_byte_0, uint8_t opcode_byte_1, uint8_t opcode_byte_2) "slow path apic load: cmd = %d, operand_size = %u, opcode_len = %u, opcode = [ %02x %02x %02x ... ]"
+hvf_x86_emu_mmio_store_instruction_fastpath(int cmd, int operand_size, int opcode_len, uint8_t opcode_byte_0, uint8_t opcode_byte_1, uint8_t opcode_byte_2) "slow path apic store: cmd = %d, operand_size = %u, opcode_len = %u, opcode = [ %02x %02x %02x ... ]"
+hvf_x86_fast_path_apic_mmio_failed(const char *access_type, uint32_t apic_register_idx, uint64_t value, int result) "xAPIC %s register 0x%"PRIx32", value 0x%"PRIx64" returned error %d from APIC"
diff --git a/target/i386/hvf/trace.h b/target/i386/hvf/trace.h
new file mode 100644
index 0000000000..14f15a752a
--- /dev/null
+++ b/target/i386/hvf/trace.h
@@ -0,0 +1 @@ 
+#include "trace/trace-target_i386_hvf.h"
diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
index 015f760acb..197fa155a0 100644
--- a/target/i386/hvf/x86_emu.c
+++ b/target/i386/hvf/x86_emu.c
@@ -44,6 +44,7 @@ 
 #include "x86_flags.h"
 #include "vmcs.h"
 #include "vmx.h"
+#include "trace.h"
 
 void hvf_handle_io(CPUState *cs, uint16_t port, void *data,
                    int direction, int size, uint32_t count);
@@ -897,6 +898,89 @@  static void exec_wrmsr(CPUX86State *env, struct x86_decode *decode)
     env->eip += decode->len;
 }
 
+static bool mmio_load_instruction_fastpath(x86_decode *decode, CPUX86State *env,
+                                           int *load_dest_reg)
+{
+    if (decode->cmd == X86_DECODE_CMD_MOV && decode->operand_size == 4
+        && decode->opcode_len == 1) {
+        if (decode->opcode[0] == 0x8b) {
+            g_assert(decode->op[0].type == X86_VAR_REG);
+            g_assert(decode->op[1].type == X86_VAR_RM);
+
+            *load_dest_reg = decode->op[0].reg | (decode->rex.r ? R_R8 : 0);
+            return true;
+        } else if (decode->opcode[0] == 0xa1) {
+            *load_dest_reg = R_EAX;
+            return true;
+        }
+    }
+
+    trace_hvf_x86_emu_mmio_load_instruction_fastpath(
+        decode->cmd, decode->operand_size, decode->opcode_len,
+        decode->opcode[0], decode->opcode[1], decode->opcode[2]);
+
+    return false;
+}
+
+static bool mmio_store_instruction_fastpath(x86_decode *decode, CPUX86State *env,
+                                            uint64_t *store_val)
+{
+    if (decode->cmd == X86_DECODE_CMD_MOV && decode->operand_size == 4 &&
+        decode->opcode_len == 1) {
+        if (decode->opcode[0] == 0x89) { /* mov DWORD PTR [reg0+off],reg1 */
+            g_assert(decode->op[1].type == X86_VAR_REG);
+            g_assert(decode->op[0].type == X86_VAR_RM);
+
+            *store_val = RRX(env, decode->op[1].reg | (decode->rex.r ? R_R8 : 0));
+            return true;
+        } else if (decode->opcode[0] == 0xc7) { /* mov DWORD PTR [reg0+off],imm*/
+            g_assert(decode->op[0].type == X86_VAR_RM);
+            g_assert(decode->op[1].type == X86_VAR_IMMEDIATE);
+            *store_val = decode->op[1].val;
+            return true;
+        } else if (decode->opcode[0] == 0xa3) { /* movabs ds:immaddr,eax */
+            *store_val = RRX(env, R_EAX);
+            return true;
+        }
+    }
+
+    trace_hvf_x86_emu_mmio_store_instruction_fastpath(
+        decode->cmd, decode->operand_size, decode->opcode_len,
+        decode->opcode[0], decode->opcode[1], decode->opcode[2]);
+    return false;
+}
+
+
+bool simulate_fast_path_apic_mmio(bool is_load, uint32_t apic_register_idx,
+                                  CPUX86State *env, x86_decode* decode)
+{
+    uint64_t value;
+    int load_dest_reg;
+    int res;
+
+    if (is_load) {
+        if (!mmio_load_instruction_fastpath(decode, env, &load_dest_reg)) {
+            return false;
+        }
+        res = apic_register_read(apic_register_idx, &value);
+        if (res == 0) {
+            RRX(env, load_dest_reg) = value;
+        }
+    } else {
+        if (!mmio_store_instruction_fastpath(decode, env, &value)) {
+            return false;
+        }
+        res = apic_register_write(apic_register_idx, value);
+    }
+
+    if (res != 0) {
+        trace_hvf_x86_fast_path_apic_mmio_failed(
+            is_load ? "load from" : "store to", apic_register_idx, value, res);
+        raise_exception(env, EXCP0D_GPF, 0);
+    }
+    return true;
+}
+
 /*
  * flag:
  * 0 - bt, 1 - btc, 2 - bts, 3 - btr
diff --git a/target/i386/hvf/x86_emu.h b/target/i386/hvf/x86_emu.h
index 8bd97608c4..6726ca2240 100644
--- a/target/i386/hvf/x86_emu.h
+++ b/target/i386/hvf/x86_emu.h
@@ -31,6 +31,8 @@  void store_regs(CPUState *cpu);
 
 void simulate_rdmsr(CPUX86State *env);
 void simulate_wrmsr(CPUX86State *env);
+bool simulate_fast_path_apic_mmio(bool is_load, uint32_t apic_register_idx,
+                                  CPUX86State *env, x86_decode* decode);
 
 target_ulong read_reg(CPUX86State *env, int reg, int size);
 void write_reg(CPUX86State *env, int reg, target_ulong val, int size);