* [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState
@ 2020-05-28 19:37 Roman Bolshakov
2020-05-28 19:37 ` [PATCH 01/13] i386: hvf: Move HVFState definition into hvf Roman Bolshakov
` (14 more replies)
0 siblings, 15 replies; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Roman Bolshakov, Cameron Esfahani
Hi,
This is a cleanup series for HVF accel.
HVF is using two emulator states CPUX86State and HVFX86EmulatorState
simultaneously. HVFX86EmulatorState is used for instruction emulation.
CPUX86State is used in all other places. Sometimes the states are in
sync, sometimes they're not. It complicates reasoning about emulator
behaviour given that there's a third state - VMCS.
The series tries to leverage CPUX86State for instruction decoding and
removes HVFX86EmulatorState. I had to add two new hvf-specific fields to
CPUX86State: lazy_flags and mmio_buf. It's likely that cc_op, cc_dst,
etc could be reused for lazy_flags but it'd require major rework of flag
processing during instruction emulation. Hopefully that'll happen too in
the future.
I tried to include sysemu/hvf.h into target/i386/cpu.h to add definition
of hvf lazy flags but couldn't do that at first it because it introduced
circular dependency between existing sysemu/hvf.h and cpu.h. The first
three patches untangle and prune sysemu/hvf.h to the bare minimum to
allow inclusion of sysemu/hvf.h into target/i386/cpu.h.
This might conflict with [1], but merge/rebase should be trivial.
1. https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg07449.html
Thanks,
Roman
Roman Bolshakov (13):
i386: hvf: Move HVFState definition into hvf
i386: hvf: Drop useless declarations in sysemu
i386: hvf: Clean stray includes in sysemu
i386: hvf: Drop unused variable
i386: hvf: Use ins_len to advance IP
i386: hvf: Use IP from CPUX86State
i386: hvf: Drop fetch_rip from HVFX86EmulatorState
i386: hvf: Drop rflags from HVFX86EmulatorState
i386: hvf: Drop copy of RFLAGS defines
i386: hvf: Drop regs in HVFX86EmulatorState
i386: hvf: Move lazy_flags into CPUX86State
i386: hvf: Move mmio_buf into CPUX86State
i386: hvf: Drop HVFX86EmulatorState
include/qemu/typedefs.h | 1 -
include/sysemu/hvf.h | 73 ++-------------------
target/i386/cpu.h | 4 +-
target/i386/hvf/hvf-i386.h | 35 ++++++++++
target/i386/hvf/hvf.c | 30 ++++-----
target/i386/hvf/x86.c | 2 +-
target/i386/hvf/x86.h | 89 ++-----------------------
target/i386/hvf/x86_decode.c | 25 ++++---
target/i386/hvf/x86_emu.c | 122 +++++++++++++++++------------------
target/i386/hvf/x86_flags.c | 81 ++++++++++++-----------
target/i386/hvf/x86_task.c | 10 +--
target/i386/hvf/x86hvf.c | 6 +-
12 files changed, 186 insertions(+), 292 deletions(-)
--
2.26.1
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 01/13] i386: hvf: Move HVFState definition into hvf
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
@ 2020-05-28 19:37 ` Roman Bolshakov
2020-06-05 14:19 ` Claudio Fontana
2020-05-28 19:37 ` [PATCH 02/13] i386: hvf: Drop useless declarations in sysemu Roman Bolshakov
` (13 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Roman Bolshakov, Eduardo Habkost,
Cameron Esfahani, Richard Henderson
"sysemu/hvf.h" is intended for inclusion in generic code. However it
also contains several hvf definitions and declarations, including
HVFState that are used only inside "hvf.c". "hvf-i386.h" would be more
appropriate place to define HVFState as it's only included by "hvf.c"
and "x86_task.c".
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
include/sysemu/hvf.h | 37 -------------------------------------
target/i386/hvf/hvf-i386.h | 35 +++++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+), 37 deletions(-)
diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index d211e808e9..30a565ab73 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -15,8 +15,6 @@
#include "cpu.h"
#include "qemu/bitops.h"
-#include "exec/memory.h"
-#include "sysemu/accel.h"
extern bool hvf_allowed;
#ifdef CONFIG_HVF
@@ -32,41 +30,6 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx,
#define hvf_get_supported_cpuid(func, idx, reg) 0
#endif
-/* hvf_slot flags */
-#define HVF_SLOT_LOG (1 << 0)
-
-typedef struct hvf_slot {
- uint64_t start;
- uint64_t size;
- uint8_t *mem;
- int slot_id;
- uint32_t flags;
- MemoryRegion *region;
-} hvf_slot;
-
-typedef struct hvf_vcpu_caps {
- uint64_t vmx_cap_pinbased;
- uint64_t vmx_cap_procbased;
- uint64_t vmx_cap_procbased2;
- uint64_t vmx_cap_entry;
- uint64_t vmx_cap_exit;
- uint64_t vmx_cap_preemption_timer;
-} hvf_vcpu_caps;
-
-typedef struct HVFState {
- AccelState parent;
- hvf_slot slots[32];
- int num_slots;
-
- hvf_vcpu_caps *hvf_caps;
-} HVFState;
-extern HVFState *hvf_state;
-
-void hvf_set_phys_mem(MemoryRegionSection *, bool);
-void hvf_handle_io(CPUArchState *, uint16_t, void *,
- int, int, int);
-hvf_slot *hvf_find_overlap_slot(uint64_t, uint64_t);
-
/* Disable HVF if |disable| is 1, otherwise, enable it iff it is supported by
* the host CPU. Use hvf_enabled() after this to get the result. */
void hvf_disable(int disable);
diff --git a/target/i386/hvf/hvf-i386.h b/target/i386/hvf/hvf-i386.h
index 15ee4835cf..7cdf256649 100644
--- a/target/i386/hvf/hvf-i386.h
+++ b/target/i386/hvf/hvf-i386.h
@@ -16,6 +16,7 @@
#ifndef HVF_I386_H
#define HVF_I386_H
+#include "sysemu/accel.h"
#include "sysemu/hvf.h"
#include "cpu.h"
#include "x86.h"
@@ -37,6 +38,40 @@ struct hvf_state {
uint64_t mem_quota;
};
+/* hvf_slot flags */
+#define HVF_SLOT_LOG (1 << 0)
+
+typedef struct hvf_slot {
+ uint64_t start;
+ uint64_t size;
+ uint8_t *mem;
+ int slot_id;
+ uint32_t flags;
+ MemoryRegion *region;
+} hvf_slot;
+
+typedef struct hvf_vcpu_caps {
+ uint64_t vmx_cap_pinbased;
+ uint64_t vmx_cap_procbased;
+ uint64_t vmx_cap_procbased2;
+ uint64_t vmx_cap_entry;
+ uint64_t vmx_cap_exit;
+ uint64_t vmx_cap_preemption_timer;
+} hvf_vcpu_caps;
+
+typedef struct HVFState {
+ AccelState parent;
+ hvf_slot slots[32];
+ int num_slots;
+
+ hvf_vcpu_caps *hvf_caps;
+} HVFState;
+extern HVFState *hvf_state;
+
+void hvf_set_phys_mem(MemoryRegionSection *, bool);
+void hvf_handle_io(CPUArchState *, uint16_t, void *, int, int, int);
+hvf_slot *hvf_find_overlap_slot(uint64_t, uint64_t);
+
#ifdef NEED_CPU_H
/* Functions exported to host specific mode */
--
2.26.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 02/13] i386: hvf: Drop useless declarations in sysemu
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
2020-05-28 19:37 ` [PATCH 01/13] i386: hvf: Move HVFState definition into hvf Roman Bolshakov
@ 2020-05-28 19:37 ` Roman Bolshakov
2020-06-04 6:35 ` Philippe Mathieu-Daudé
2020-06-04 9:53 ` Claudio Fontana
2020-05-28 19:37 ` [PATCH 03/13] i386: hvf: Clean stray includes " Roman Bolshakov
` (12 subsequent siblings)
14 siblings, 2 replies; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Roman Bolshakov, Eduardo Habkost,
Cameron Esfahani, Richard Henderson
They're either declared elsewhere or have no use.
While at it, rename _hvf_cpu_synchronize_post_init() to
do_hvf_cpu_synchronize_post_init().
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
include/sysemu/hvf.h | 22 ----------------------
target/i386/hvf/hvf.c | 7 ++++---
2 files changed, 4 insertions(+), 25 deletions(-)
diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index 30a565ab73..03f3cd7db3 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -30,35 +30,13 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx,
#define hvf_get_supported_cpuid(func, idx, reg) 0
#endif
-/* Disable HVF if |disable| is 1, otherwise, enable it iff it is supported by
- * the host CPU. Use hvf_enabled() after this to get the result. */
-void hvf_disable(int disable);
-
-/* Returns non-0 if the host CPU supports the VMX "unrestricted guest" feature
- * which allows the virtual CPU to directly run in "real mode". If true, this
- * allows QEMU to run several vCPU threads in parallel (see cpus.c). Otherwise,
- * only a a single TCG thread can run, and it will call HVF to run the current
- * instructions, except in case of "real mode" (paging disabled, typically at
- * boot time), or MMIO operations. */
-
-int hvf_sync_vcpus(void);
-
int hvf_init_vcpu(CPUState *);
int hvf_vcpu_exec(CPUState *);
-int hvf_smp_cpu_exec(CPUState *);
void hvf_cpu_synchronize_state(CPUState *);
void hvf_cpu_synchronize_post_reset(CPUState *);
void hvf_cpu_synchronize_post_init(CPUState *);
-void _hvf_cpu_synchronize_post_init(CPUState *, run_on_cpu_data);
-
void hvf_vcpu_destroy(CPUState *);
-void hvf_raise_event(CPUState *);
-/* void hvf_reset_vcpu_state(void *opaque); */
void hvf_reset_vcpu(CPUState *);
-void vmx_update_tpr(CPUState *);
-void update_apic_tpr(CPUState *);
-int hvf_put_registers(CPUState *);
-void vmx_clear_int_window_exiting(CPUState *cpu);
#define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf")
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index d72543dc31..9ccdb7e7c7 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -251,7 +251,7 @@ void vmx_update_tpr(CPUState *cpu)
}
}
-void update_apic_tpr(CPUState *cpu)
+static void update_apic_tpr(CPUState *cpu)
{
X86CPU *x86_cpu = X86_CPU(cpu);
int tpr = rreg(cpu->hvf_fd, HV_X86_TPR) >> 4;
@@ -312,7 +312,8 @@ void hvf_cpu_synchronize_post_reset(CPUState *cpu_state)
run_on_cpu(cpu_state, do_hvf_cpu_synchronize_post_reset, RUN_ON_CPU_NULL);
}
-void _hvf_cpu_synchronize_post_init(CPUState *cpu, run_on_cpu_data arg)
+static void do_hvf_cpu_synchronize_post_init(CPUState *cpu,
+ run_on_cpu_data arg)
{
CPUState *cpu_state = cpu;
hvf_put_registers(cpu_state);
@@ -321,7 +322,7 @@ void _hvf_cpu_synchronize_post_init(CPUState *cpu, run_on_cpu_data arg)
void hvf_cpu_synchronize_post_init(CPUState *cpu_state)
{
- run_on_cpu(cpu_state, _hvf_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
+ run_on_cpu(cpu_state, do_hvf_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
}
static bool ept_emulation_fault(hvf_slot *slot, uint64_t gpa, uint64_t ept_qual)
--
2.26.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 03/13] i386: hvf: Clean stray includes in sysemu
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
2020-05-28 19:37 ` [PATCH 01/13] i386: hvf: Move HVFState definition into hvf Roman Bolshakov
2020-05-28 19:37 ` [PATCH 02/13] i386: hvf: Drop useless declarations in sysemu Roman Bolshakov
@ 2020-05-28 19:37 ` Roman Bolshakov
2020-06-05 14:20 ` Claudio Fontana
2020-05-28 19:37 ` [PATCH 04/13] i386: hvf: Drop unused variable Roman Bolshakov
` (11 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Roman Bolshakov, Cameron Esfahani
They have no use.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
include/sysemu/hvf.h | 7 -------
1 file changed, 7 deletions(-)
diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index 03f3cd7db3..cf579e1592 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -13,15 +13,8 @@
#ifndef HVF_H
#define HVF_H
-#include "cpu.h"
-#include "qemu/bitops.h"
-
extern bool hvf_allowed;
#ifdef CONFIG_HVF
-#include <Hypervisor/hv.h>
-#include <Hypervisor/hv_vmx.h>
-#include <Hypervisor/hv_error.h>
-#include "target/i386/cpu.h"
uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx,
int reg);
#define hvf_enabled() (hvf_allowed)
--
2.26.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 04/13] i386: hvf: Drop unused variable
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
` (2 preceding siblings ...)
2020-05-28 19:37 ` [PATCH 03/13] i386: hvf: Clean stray includes " Roman Bolshakov
@ 2020-05-28 19:37 ` Roman Bolshakov
2020-06-04 6:35 ` Philippe Mathieu-Daudé
2020-05-28 19:37 ` [PATCH 05/13] i386: hvf: Use ins_len to advance IP Roman Bolshakov
` (10 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Roman Bolshakov, Eduardo Habkost,
Cameron Esfahani, Richard Henderson
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
target/i386/hvf/x86.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
index c95d5b2116..56fcde13c6 100644
--- a/target/i386/hvf/x86.h
+++ b/target/i386/hvf/x86.h
@@ -293,7 +293,6 @@ typedef struct lazy_flags {
/* Definition of hvf_x86_state is here */
struct HVFX86EmulatorState {
- int interruptable;
uint64_t fetch_rip;
uint64_t rip;
struct x86_register regs[16];
--
2.26.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 05/13] i386: hvf: Use ins_len to advance IP
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
` (3 preceding siblings ...)
2020-05-28 19:37 ` [PATCH 04/13] i386: hvf: Drop unused variable Roman Bolshakov
@ 2020-05-28 19:37 ` Roman Bolshakov
2020-06-04 6:39 ` Philippe Mathieu-Daudé
2020-05-28 19:37 ` [PATCH 06/13] i386: hvf: Use IP from CPUX86State Roman Bolshakov
` (9 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Roman Bolshakov, Eduardo Habkost,
Cameron Esfahani, Richard Henderson
There's no need to read VMCS twice, instruction length is already
available in ins_len.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
target/i386/hvf/hvf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 9ccdb7e7c7..8ff1d25521 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -871,7 +871,7 @@ int hvf_vcpu_exec(CPUState *cpu)
} else {
simulate_wrmsr(cpu);
}
- RIP(env) += rvmcs(cpu->hvf_fd, VMCS_EXIT_INSTRUCTION_LENGTH);
+ RIP(env) += ins_len;
store_regs(cpu);
break;
}
--
2.26.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 06/13] i386: hvf: Use IP from CPUX86State
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
` (4 preceding siblings ...)
2020-05-28 19:37 ` [PATCH 05/13] i386: hvf: Use ins_len to advance IP Roman Bolshakov
@ 2020-05-28 19:37 ` Roman Bolshakov
2020-06-04 6:47 ` Philippe Mathieu-Daudé
2020-06-04 9:06 ` Claudio Fontana
2020-05-28 19:37 ` [PATCH 07/13] i386: hvf: Drop fetch_rip from HVFX86EmulatorState Roman Bolshakov
` (8 subsequent siblings)
14 siblings, 2 replies; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Roman Bolshakov, Eduardo Habkost,
Cameron Esfahani, Richard Henderson
Drop and replace rip field from HVFX86EmulatorState in favor of eip from
common CPUX86State.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
target/i386/hvf/hvf.c | 6 +--
target/i386/hvf/x86.h | 3 --
target/i386/hvf/x86_decode.c | 6 +--
target/i386/hvf/x86_emu.c | 86 ++++++++++++++++++------------------
target/i386/hvf/x86_task.c | 4 +-
5 files changed, 51 insertions(+), 54 deletions(-)
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 8ff1d25521..45ae55dd27 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -797,7 +797,7 @@ int hvf_vcpu_exec(CPUState *cpu)
} else {
RAX(env) = (uint64_t)val;
}
- RIP(env) += ins_len;
+ env->eip += ins_len;
store_regs(cpu);
break;
} else if (!string && !in) {
@@ -871,7 +871,7 @@ int hvf_vcpu_exec(CPUState *cpu)
} else {
simulate_wrmsr(cpu);
}
- RIP(env) += ins_len;
+ env->eip += ins_len;
store_regs(cpu);
break;
}
@@ -907,7 +907,7 @@ int hvf_vcpu_exec(CPUState *cpu)
error_report("Unrecognized CR %d", cr);
abort();
}
- RIP(env) += ins_len;
+ env->eip += ins_len;
store_regs(cpu);
break;
}
diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
index 56fcde13c6..e3ab7c5137 100644
--- a/target/i386/hvf/x86.h
+++ b/target/i386/hvf/x86.h
@@ -294,7 +294,6 @@ typedef struct lazy_flags {
/* Definition of hvf_x86_state is here */
struct HVFX86EmulatorState {
uint64_t fetch_rip;
- uint64_t rip;
struct x86_register regs[16];
struct x86_reg_flags rflags;
struct lazy_flags lflags;
@@ -302,8 +301,6 @@ struct HVFX86EmulatorState {
};
/* useful register access macros */
-#define RIP(cpu) (cpu->hvf_emul->rip)
-#define EIP(cpu) ((uint32_t)cpu->hvf_emul->rip)
#define RFLAGS(cpu) (cpu->hvf_emul->rflags.rflags)
#define EFLAGS(cpu) (cpu->hvf_emul->rflags.eflags)
diff --git a/target/i386/hvf/x86_decode.c b/target/i386/hvf/x86_decode.c
index 77c346605f..a590088f54 100644
--- a/target/i386/hvf/x86_decode.c
+++ b/target/i386/hvf/x86_decode.c
@@ -75,7 +75,7 @@ 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), RIP(env)) + decode->len;
+ target_ulong va = linear_rip(env_cpu(env), env->eip) + decode->len;
vmx_read_mem(env_cpu(env), &val, va, size);
decode->len += size;
@@ -1771,7 +1771,7 @@ void calc_modrm_operand32(CPUX86State *env, struct x86_decode *decode,
ptr += get_sib_val(env, decode, &seg);
} else if (!decode->modrm.mod && 5 == decode->modrm.rm) {
if (x86_is_long_mode(env_cpu(env))) {
- ptr += RIP(env) + decode->len;
+ ptr += env->eip + decode->len;
} else {
ptr = decode->displacement;
}
@@ -1807,7 +1807,7 @@ void calc_modrm_operand64(CPUX86State *env, struct x86_decode *decode,
if (4 == rm) {
ptr = get_sib_val(env, decode, &seg) + offset;
} else if (0 == mod && 5 == rm) {
- ptr = RIP(env) + decode->len + (int32_t) offset;
+ ptr = env->eip + decode->len + (int32_t) offset;
} else {
ptr = get_reg_val(env, src, decode->rex.rex, decode->rex.b, 8) +
(int64_t) offset;
diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
index 92ab815f5d..0efd9f9928 100644
--- a/target/i386/hvf/x86_emu.c
+++ b/target/i386/hvf/x86_emu.c
@@ -267,49 +267,49 @@ static void exec_mov(struct CPUX86State *env, struct x86_decode *decode)
write_val_ext(env, decode->op[0].ptr, decode->op[1].val,
decode->operand_size);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_add(struct CPUX86State *env, struct x86_decode *decode)
{
EXEC_2OP_FLAGS_CMD(env, decode, +, SET_FLAGS_OSZAPC_ADD, true);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_or(struct CPUX86State *env, struct x86_decode *decode)
{
EXEC_2OP_FLAGS_CMD(env, decode, |, SET_FLAGS_OSZAPC_LOGIC, true);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_adc(struct CPUX86State *env, struct x86_decode *decode)
{
EXEC_2OP_FLAGS_CMD(env, decode, +get_CF(env)+, SET_FLAGS_OSZAPC_ADD, true);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_sbb(struct CPUX86State *env, struct x86_decode *decode)
{
EXEC_2OP_FLAGS_CMD(env, decode, -get_CF(env)-, SET_FLAGS_OSZAPC_SUB, true);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_and(struct CPUX86State *env, struct x86_decode *decode)
{
EXEC_2OP_FLAGS_CMD(env, decode, &, SET_FLAGS_OSZAPC_LOGIC, true);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_sub(struct CPUX86State *env, struct x86_decode *decode)
{
EXEC_2OP_FLAGS_CMD(env, decode, -, SET_FLAGS_OSZAPC_SUB, true);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_xor(struct CPUX86State *env, struct x86_decode *decode)
{
EXEC_2OP_FLAGS_CMD(env, decode, ^, SET_FLAGS_OSZAPC_LOGIC, true);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_neg(struct CPUX86State *env, struct x86_decode *decode)
@@ -332,13 +332,13 @@ static void exec_neg(struct CPUX86State *env, struct x86_decode *decode)
}
/*lflags_to_rflags(env);*/
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_cmp(struct CPUX86State *env, struct x86_decode *decode)
{
EXEC_2OP_FLAGS_CMD(env, decode, -, SET_FLAGS_OSZAPC_SUB, false);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_inc(struct CPUX86State *env, struct x86_decode *decode)
@@ -348,7 +348,7 @@ static void exec_inc(struct CPUX86State *env, struct x86_decode *decode)
EXEC_2OP_FLAGS_CMD(env, decode, +1+, SET_FLAGS_OSZAP_ADD, true);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_dec(struct CPUX86State *env, struct x86_decode *decode)
@@ -357,13 +357,13 @@ static void exec_dec(struct CPUX86State *env, struct x86_decode *decode)
decode->op[1].val = 0;
EXEC_2OP_FLAGS_CMD(env, decode, -1-, SET_FLAGS_OSZAP_SUB, true);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_tst(struct CPUX86State *env, struct x86_decode *decode)
{
EXEC_2OP_FLAGS_CMD(env, decode, &, SET_FLAGS_OSZAPC_LOGIC, false);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_not(struct CPUX86State *env, struct x86_decode *decode)
@@ -372,7 +372,7 @@ static void exec_not(struct CPUX86State *env, struct x86_decode *decode)
write_val_ext(env, decode->op[0].ptr, ~decode->op[0].val,
decode->operand_size);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
void exec_movzx(struct CPUX86State *env, struct x86_decode *decode)
@@ -392,7 +392,7 @@ void exec_movzx(struct CPUX86State *env, struct x86_decode *decode)
decode->op[1].val = read_val_ext(env, decode->op[1].ptr, src_op_size);
write_val_ext(env, decode->op[0].ptr, decode->op[1].val, op_size);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_out(struct CPUX86State *env, struct x86_decode *decode)
@@ -416,7 +416,7 @@ static void exec_out(struct CPUX86State *env, struct x86_decode *decode)
VM_PANIC("Bad out opcode\n");
break;
}
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_in(struct CPUX86State *env, struct x86_decode *decode)
@@ -452,7 +452,7 @@ static void exec_in(struct CPUX86State *env, struct x86_decode *decode)
break;
}
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static inline void string_increment_reg(struct CPUX86State *env, int reg,
@@ -505,7 +505,7 @@ static void exec_ins(struct CPUX86State *env, struct x86_decode *decode)
exec_ins_single(env, decode);
}
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_outs_single(struct CPUX86State *env, struct x86_decode *decode)
@@ -528,7 +528,7 @@ static void exec_outs(struct CPUX86State *env, struct x86_decode *decode)
exec_outs_single(env, decode);
}
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_movs_single(struct CPUX86State *env, struct x86_decode *decode)
@@ -556,7 +556,7 @@ static void exec_movs(struct CPUX86State *env, struct x86_decode *decode)
exec_movs_single(env, decode);
}
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_cmps_single(struct CPUX86State *env, struct x86_decode *decode)
@@ -586,7 +586,7 @@ static void exec_cmps(struct CPUX86State *env, struct x86_decode *decode)
} else {
exec_cmps_single(env, decode);
}
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
@@ -612,7 +612,7 @@ static void exec_stos(struct CPUX86State *env, struct x86_decode *decode)
exec_stos_single(env, decode);
}
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_scas_single(struct CPUX86State *env, struct x86_decode *decode)
@@ -638,7 +638,7 @@ static void exec_scas(struct CPUX86State *env, struct x86_decode *decode)
exec_scas_single(env, decode);
}
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_lods_single(struct CPUX86State *env, struct x86_decode *decode)
@@ -661,7 +661,7 @@ static void exec_lods(struct CPUX86State *env, struct x86_decode *decode)
exec_lods_single(env, decode);
}
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
void simulate_rdmsr(struct CPUState *cpu)
@@ -758,7 +758,7 @@ void simulate_rdmsr(struct CPUState *cpu)
static void exec_rdmsr(struct CPUX86State *env, struct x86_decode *decode)
{
simulate_rdmsr(env_cpu(env));
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
void simulate_wrmsr(struct CPUState *cpu)
@@ -853,7 +853,7 @@ void simulate_wrmsr(struct CPUState *cpu)
static void exec_wrmsr(struct CPUX86State *env, struct x86_decode *decode)
{
simulate_wrmsr(env_cpu(env));
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
/*
@@ -909,25 +909,25 @@ static void do_bt(struct CPUX86State *env, struct x86_decode *decode, int flag)
static void exec_bt(struct CPUX86State *env, struct x86_decode *decode)
{
do_bt(env, decode, 0);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_btc(struct CPUX86State *env, struct x86_decode *decode)
{
do_bt(env, decode, 1);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_btr(struct CPUX86State *env, struct x86_decode *decode)
{
do_bt(env, decode, 3);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_bts(struct CPUX86State *env, struct x86_decode *decode)
{
do_bt(env, decode, 2);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
void exec_shl(struct CPUX86State *env, struct x86_decode *decode)
@@ -991,7 +991,7 @@ void exec_shl(struct CPUX86State *env, struct x86_decode *decode)
exit:
/* lflags_to_rflags(env); */
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
void exec_movsx(CPUX86State *env, struct x86_decode *decode)
@@ -1014,7 +1014,7 @@ void exec_movsx(CPUX86State *env, struct x86_decode *decode)
write_val_ext(env, decode->op[0].ptr, decode->op[1].val, op_size);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
void exec_ror(struct CPUX86State *env, struct x86_decode *decode)
@@ -1092,7 +1092,7 @@ void exec_ror(struct CPUX86State *env, struct x86_decode *decode)
break;
}
}
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
void exec_rol(struct CPUX86State *env, struct x86_decode *decode)
@@ -1173,7 +1173,7 @@ void exec_rol(struct CPUX86State *env, struct x86_decode *decode)
break;
}
}
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
@@ -1259,7 +1259,7 @@ void exec_rcl(struct CPUX86State *env, struct x86_decode *decode)
break;
}
}
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
void exec_rcr(struct CPUX86State *env, struct x86_decode *decode)
@@ -1334,7 +1334,7 @@ void exec_rcr(struct CPUX86State *env, struct x86_decode *decode)
break;
}
}
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_xchg(struct CPUX86State *env, struct x86_decode *decode)
@@ -1346,7 +1346,7 @@ static void exec_xchg(struct CPUX86State *env, struct x86_decode *decode)
write_val_ext(env, decode->op[1].ptr, decode->op[0].val,
decode->operand_size);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static void exec_xadd(struct CPUX86State *env, struct x86_decode *decode)
@@ -1355,7 +1355,7 @@ static void exec_xadd(struct CPUX86State *env, struct x86_decode *decode)
write_val_ext(env, decode->op[1].ptr, decode->op[0].val,
decode->operand_size);
- RIP(env) += decode->len;
+ env->eip += decode->len;
}
static struct cmd_handler {
@@ -1434,7 +1434,7 @@ void load_regs(struct CPUState *cpu)
RFLAGS(env) = rreg(cpu->hvf_fd, HV_X86_RFLAGS);
rflags_to_lflags(env);
- RIP(env) = rreg(cpu->hvf_fd, HV_X86_RIP);
+ env->eip = rreg(cpu->hvf_fd, HV_X86_RIP);
}
void store_regs(struct CPUState *cpu)
@@ -1457,20 +1457,20 @@ void store_regs(struct CPUState *cpu)
lflags_to_rflags(env);
wreg(cpu->hvf_fd, HV_X86_RFLAGS, RFLAGS(env));
- macvm_set_rip(cpu, RIP(env));
+ macvm_set_rip(cpu, env->eip);
}
bool exec_instruction(struct CPUX86State *env, struct x86_decode *ins)
{
/*if (hvf_vcpu_id(cpu))
- printf("%d, %llx: exec_instruction %s\n", hvf_vcpu_id(cpu), RIP(cpu),
+ printf("%d, %llx: exec_instruction %s\n", hvf_vcpu_id(cpu), env->eip,
decode_cmd_to_string(ins->cmd));*/
if (!_cmd_handler[ins->cmd].handler) {
- printf("Unimplemented handler (%llx) for %d (%x %x) \n", RIP(env),
+ printf("Unimplemented handler (%llx) for %d (%x %x) \n", env->eip,
ins->cmd, ins->opcode[0],
ins->opcode_len > 1 ? ins->opcode[1] : 0);
- RIP(env) += ins->len;
+ env->eip += ins->len;
return true;
}
diff --git a/target/i386/hvf/x86_task.c b/target/i386/hvf/x86_task.c
index 1daac6cc2b..834baec3ea 100644
--- a/target/i386/hvf/x86_task.c
+++ b/target/i386/hvf/x86_task.c
@@ -38,7 +38,7 @@ static void save_state_to_tss32(CPUState *cpu, struct x86_tss_segment32 *tss)
CPUX86State *env = &x86_cpu->env;
/* CR3 and ldt selector are not saved intentionally */
- tss->eip = EIP(env);
+ tss->eip = (uint32_t)env->eip;
tss->eflags = EFLAGS(env);
tss->eax = EAX(env);
tss->ecx = ECX(env);
@@ -64,7 +64,7 @@ static void load_state_from_tss32(CPUState *cpu, struct x86_tss_segment32 *tss)
wvmcs(cpu->hvf_fd, VMCS_GUEST_CR3, tss->cr3);
- RIP(env) = tss->eip;
+ env->eip = tss->eip;
EFLAGS(env) = tss->eflags | 2;
/* General purpose registers */
--
2.26.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 07/13] i386: hvf: Drop fetch_rip from HVFX86EmulatorState
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
` (5 preceding siblings ...)
2020-05-28 19:37 ` [PATCH 06/13] i386: hvf: Use IP from CPUX86State Roman Bolshakov
@ 2020-05-28 19:37 ` Roman Bolshakov
2020-05-28 19:37 ` [PATCH 08/13] i386: hvf: Drop rflags " Roman Bolshakov
` (7 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Roman Bolshakov, Eduardo Habkost,
Cameron Esfahani, Richard Henderson
The field is used to print address of instructions that have no parser
in decode_invalid(). RIP from VMCS is saved into fetch_rip before
decoding starts but it's also saved into env->eip in load_regs().
Therefore env->eip can be used instead of fetch_rip.
While at it, correct address printed in decode_invalid(). It prints an
address before the unknown instruction.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
target/i386/hvf/hvf.c | 6 ------
target/i386/hvf/x86.h | 1 -
target/i386/hvf/x86_decode.c | 3 +--
3 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 45ae55dd27..416a6fae7c 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -767,8 +767,6 @@ int hvf_vcpu_exec(CPUState *cpu)
struct x86_decode decode;
load_regs(cpu);
- env->hvf_emul->fetch_rip = rip;
-
decode_instruction(env, &decode);
exec_instruction(env, &decode);
store_regs(cpu);
@@ -809,8 +807,6 @@ int hvf_vcpu_exec(CPUState *cpu)
struct x86_decode decode;
load_regs(cpu);
- env->hvf_emul->fetch_rip = rip;
-
decode_instruction(env, &decode);
assert(ins_len == decode.len);
exec_instruction(env, &decode);
@@ -915,8 +911,6 @@ int hvf_vcpu_exec(CPUState *cpu)
struct x86_decode decode;
load_regs(cpu);
- env->hvf_emul->fetch_rip = rip;
-
decode_instruction(env, &decode);
exec_instruction(env, &decode);
store_regs(cpu);
diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
index e3ab7c5137..411e4b6599 100644
--- a/target/i386/hvf/x86.h
+++ b/target/i386/hvf/x86.h
@@ -293,7 +293,6 @@ typedef struct lazy_flags {
/* Definition of hvf_x86_state is here */
struct HVFX86EmulatorState {
- uint64_t fetch_rip;
struct x86_register regs[16];
struct x86_reg_flags rflags;
struct lazy_flags lflags;
diff --git a/target/i386/hvf/x86_decode.c b/target/i386/hvf/x86_decode.c
index a590088f54..d881542181 100644
--- a/target/i386/hvf/x86_decode.c
+++ b/target/i386/hvf/x86_decode.c
@@ -29,8 +29,7 @@
static void decode_invalid(CPUX86State *env, struct x86_decode *decode)
{
- printf("%llx: failed to decode instruction ", env->hvf_emul->fetch_rip -
- decode->len);
+ printf("%llx: failed to decode instruction ", env->eip);
for (int i = 0; i < decode->opcode_len; i++) {
printf("%x ", decode->opcode[i]);
}
--
2.26.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 08/13] i386: hvf: Drop rflags from HVFX86EmulatorState
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
` (6 preceding siblings ...)
2020-05-28 19:37 ` [PATCH 07/13] i386: hvf: Drop fetch_rip from HVFX86EmulatorState Roman Bolshakov
@ 2020-05-28 19:37 ` Roman Bolshakov
2020-05-28 19:37 ` [PATCH 09/13] i386: hvf: Drop copy of RFLAGS defines Roman Bolshakov
` (6 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Roman Bolshakov, Eduardo Habkost,
Cameron Esfahani, Richard Henderson
HVFX86EmulatorState carries it's own copy of x86 flags. It can be
dropped in favor of eflags in generic CPUX86State.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
target/i386/hvf/hvf.c | 5 ++---
target/i386/hvf/x86.c | 2 +-
target/i386/hvf/x86.h | 42 -------------------------------------
target/i386/hvf/x86_emu.c | 6 +++---
target/i386/hvf/x86_flags.c | 24 ++++++++++-----------
target/i386/hvf/x86_task.c | 6 +++---
target/i386/hvf/x86hvf.c | 6 +++---
7 files changed, 24 insertions(+), 67 deletions(-)
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 416a6fae7c..4cee496d71 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -723,8 +723,7 @@ int hvf_vcpu_exec(CPUState *cpu)
hvf_store_events(cpu, ins_len, idtvec_info);
rip = rreg(cpu->hvf_fd, HV_X86_RIP);
- RFLAGS(env) = rreg(cpu->hvf_fd, HV_X86_RFLAGS);
- env->eflags = RFLAGS(env);
+ env->eflags = rreg(cpu->hvf_fd, HV_X86_RFLAGS);
qemu_mutex_lock_iothread();
@@ -736,7 +735,7 @@ int hvf_vcpu_exec(CPUState *cpu)
case EXIT_REASON_HLT: {
macvm_set_rip(cpu, rip + ins_len);
if (!((cpu->interrupt_request & CPU_INTERRUPT_HARD) &&
- (EFLAGS(env) & IF_MASK))
+ (env->eflags & IF_MASK))
&& !(cpu->interrupt_request & CPU_INTERRUPT_NMI) &&
!(idtvec_info & VMCS_IDT_VEC_VALID)) {
cpu->halted = 1;
diff --git a/target/i386/hvf/x86.c b/target/i386/hvf/x86.c
index 3afcedc7fc..7ebb5b45bd 100644
--- a/target/i386/hvf/x86.c
+++ b/target/i386/hvf/x86.c
@@ -131,7 +131,7 @@ bool x86_is_v8086(struct CPUState *cpu)
{
X86CPU *x86_cpu = X86_CPU(cpu);
CPUX86State *env = &x86_cpu->env;
- return x86_is_protected(cpu) && (RFLAGS(env) & RFLAGS_VM);
+ return x86_is_protected(cpu) && (env->eflags & RFLAGS_VM);
}
bool x86_is_long_mode(struct CPUState *cpu)
diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
index 411e4b6599..e309b8f203 100644
--- a/target/i386/hvf/x86.h
+++ b/target/i386/hvf/x86.h
@@ -62,44 +62,6 @@ typedef enum x86_rflags {
RFLAGS_ID = (1L << 21),
} x86_rflags;
-/* rflags register */
-typedef struct x86_reg_flags {
- union {
- struct {
- uint64_t rflags;
- };
- struct {
- uint32_t eflags;
- uint32_t hi32_unused1;
- };
- struct {
- uint32_t cf:1;
- uint32_t unused1:1;
- uint32_t pf:1;
- uint32_t unused2:1;
- uint32_t af:1;
- uint32_t unused3:1;
- uint32_t zf:1;
- uint32_t sf:1;
- uint32_t tf:1;
- uint32_t ief:1;
- uint32_t df:1;
- uint32_t of:1;
- uint32_t iopl:2;
- uint32_t nt:1;
- uint32_t unused4:1;
- uint32_t rf:1;
- uint32_t vm:1;
- uint32_t ac:1;
- uint32_t vif:1;
- uint32_t vip:1;
- uint32_t id:1;
- uint32_t unused5:10;
- uint32_t hi32_unused2;
- };
- };
-} __attribute__ ((__packed__)) x86_reg_flags;
-
typedef enum x86_reg_cr0 {
CR0_PE = (1L << 0),
CR0_MP = (1L << 1),
@@ -294,15 +256,11 @@ typedef struct lazy_flags {
/* Definition of hvf_x86_state is here */
struct HVFX86EmulatorState {
struct x86_register regs[16];
- struct x86_reg_flags rflags;
struct lazy_flags lflags;
uint8_t mmio_buf[4096];
};
/* useful register access macros */
-#define RFLAGS(cpu) (cpu->hvf_emul->rflags.rflags)
-#define EFLAGS(cpu) (cpu->hvf_emul->rflags.eflags)
-
#define RRX(cpu, reg) (cpu->hvf_emul->regs[reg].rrx)
#define RAX(cpu) RRX(cpu, R_EAX)
#define RCX(cpu) RRX(cpu, R_ECX)
diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
index 0efd9f9928..04fac64e72 100644
--- a/target/i386/hvf/x86_emu.c
+++ b/target/i386/hvf/x86_emu.c
@@ -459,7 +459,7 @@ static inline void string_increment_reg(struct CPUX86State *env, int reg,
struct x86_decode *decode)
{
target_ulong val = read_reg(env, reg, decode->addressing_size);
- if (env->hvf_emul->rflags.df) {
+ if (env->eflags & DF_MASK) {
val -= decode->operand_size;
} else {
val += decode->operand_size;
@@ -1432,7 +1432,7 @@ void load_regs(struct CPUState *cpu)
RRX(env, i) = rreg(cpu->hvf_fd, HV_X86_RAX + i);
}
- RFLAGS(env) = rreg(cpu->hvf_fd, HV_X86_RFLAGS);
+ env->eflags = rreg(cpu->hvf_fd, HV_X86_RFLAGS);
rflags_to_lflags(env);
env->eip = rreg(cpu->hvf_fd, HV_X86_RIP);
}
@@ -1456,7 +1456,7 @@ void store_regs(struct CPUState *cpu)
}
lflags_to_rflags(env);
- wreg(cpu->hvf_fd, HV_X86_RFLAGS, RFLAGS(env));
+ wreg(cpu->hvf_fd, HV_X86_RFLAGS, env->eflags);
macvm_set_rip(cpu, env->eip);
}
diff --git a/target/i386/hvf/x86_flags.c b/target/i386/hvf/x86_flags.c
index ee6d33f861..1152cd7234 100644
--- a/target/i386/hvf/x86_flags.c
+++ b/target/i386/hvf/x86_flags.c
@@ -295,21 +295,21 @@ void set_SF(CPUX86State *env, bool val)
void lflags_to_rflags(CPUX86State *env)
{
- env->hvf_emul->rflags.cf = get_CF(env);
- env->hvf_emul->rflags.pf = get_PF(env);
- env->hvf_emul->rflags.af = get_AF(env);
- env->hvf_emul->rflags.zf = get_ZF(env);
- env->hvf_emul->rflags.sf = get_SF(env);
- env->hvf_emul->rflags.of = get_OF(env);
+ env->eflags |= get_CF(env) ? CC_C : 0;
+ env->eflags |= get_PF(env) ? CC_P : 0;
+ env->eflags |= get_AF(env) ? CC_A : 0;
+ env->eflags |= get_ZF(env) ? CC_Z : 0;
+ env->eflags |= get_SF(env) ? CC_S : 0;
+ env->eflags |= get_OF(env) ? CC_O : 0;
}
void rflags_to_lflags(CPUX86State *env)
{
env->hvf_emul->lflags.auxbits = env->hvf_emul->lflags.result = 0;
- set_OF(env, env->hvf_emul->rflags.of);
- set_SF(env, env->hvf_emul->rflags.sf);
- set_ZF(env, env->hvf_emul->rflags.zf);
- set_AF(env, env->hvf_emul->rflags.af);
- set_PF(env, env->hvf_emul->rflags.pf);
- set_CF(env, env->hvf_emul->rflags.cf);
+ set_OF(env, env->eflags & CC_O);
+ set_SF(env, env->eflags & CC_S);
+ set_ZF(env, env->eflags & CC_Z);
+ set_AF(env, env->eflags & CC_A);
+ set_PF(env, env->eflags & CC_P);
+ set_CF(env, env->eflags & CC_C);
}
diff --git a/target/i386/hvf/x86_task.c b/target/i386/hvf/x86_task.c
index 834baec3ea..6ea8508946 100644
--- a/target/i386/hvf/x86_task.c
+++ b/target/i386/hvf/x86_task.c
@@ -39,7 +39,7 @@ static void save_state_to_tss32(CPUState *cpu, struct x86_tss_segment32 *tss)
/* CR3 and ldt selector are not saved intentionally */
tss->eip = (uint32_t)env->eip;
- tss->eflags = EFLAGS(env);
+ tss->eflags = (uint32_t)env->eflags;
tss->eax = EAX(env);
tss->ecx = ECX(env);
tss->edx = EDX(env);
@@ -65,7 +65,7 @@ static void load_state_from_tss32(CPUState *cpu, struct x86_tss_segment32 *tss)
wvmcs(cpu->hvf_fd, VMCS_GUEST_CR3, tss->cr3);
env->eip = tss->eip;
- EFLAGS(env) = tss->eflags | 2;
+ env->eflags = tss->eflags | 2;
/* General purpose registers */
RAX(env) = tss->eax;
@@ -158,7 +158,7 @@ void vmx_handle_task_switch(CPUState *cpu, x68_segment_selector tss_sel, int rea
}
if (reason == TSR_IRET)
- EFLAGS(env) &= ~RFLAGS_NT;
+ env->eflags &= ~RFLAGS_NT;
if (reason != TSR_CALL && reason != TSR_IDT_GATE)
old_tss_sel.sel = 0xffff;
diff --git a/target/i386/hvf/x86hvf.c b/target/i386/hvf/x86hvf.c
index edefe5319a..5cbcb32ab6 100644
--- a/target/i386/hvf/x86hvf.c
+++ b/target/i386/hvf/x86hvf.c
@@ -412,7 +412,7 @@ bool hvf_inject_interrupts(CPUState *cpu_state)
if (!(env->hflags & HF_INHIBIT_IRQ_MASK) &&
(cpu_state->interrupt_request & CPU_INTERRUPT_HARD) &&
- (EFLAGS(env) & IF_MASK) && !(info & VMCS_INTR_VALID)) {
+ (env->eflags & IF_MASK) && !(info & VMCS_INTR_VALID)) {
int line = cpu_get_pic_interrupt(&x86cpu->env);
cpu_state->interrupt_request &= ~CPU_INTERRUPT_HARD;
if (line >= 0) {
@@ -432,7 +432,7 @@ int hvf_process_events(CPUState *cpu_state)
X86CPU *cpu = X86_CPU(cpu_state);
CPUX86State *env = &cpu->env;
- EFLAGS(env) = rreg(cpu_state->hvf_fd, HV_X86_RFLAGS);
+ env->eflags = rreg(cpu_state->hvf_fd, HV_X86_RFLAGS);
if (cpu_state->interrupt_request & CPU_INTERRUPT_INIT) {
hvf_cpu_synchronize_state(cpu_state);
@@ -444,7 +444,7 @@ int hvf_process_events(CPUState *cpu_state)
apic_poll_irq(cpu->apic_state);
}
if (((cpu_state->interrupt_request & CPU_INTERRUPT_HARD) &&
- (EFLAGS(env) & IF_MASK)) ||
+ (env->eflags & IF_MASK)) ||
(cpu_state->interrupt_request & CPU_INTERRUPT_NMI)) {
cpu_state->halted = 0;
}
--
2.26.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 09/13] i386: hvf: Drop copy of RFLAGS defines
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
` (7 preceding siblings ...)
2020-05-28 19:37 ` [PATCH 08/13] i386: hvf: Drop rflags " Roman Bolshakov
@ 2020-05-28 19:37 ` Roman Bolshakov
2020-05-28 19:37 ` [PATCH 10/13] i386: hvf: Drop regs in HVFX86EmulatorState Roman Bolshakov
` (5 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Roman Bolshakov, Eduardo Habkost,
Cameron Esfahani, Richard Henderson
Use the ones provided in target/i386/cpu.h instead.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
target/i386/hvf/x86.c | 2 +-
target/i386/hvf/x86.h | 20 --------------------
target/i386/hvf/x86_decode.c | 16 +++++++---------
target/i386/hvf/x86_task.c | 2 +-
4 files changed, 9 insertions(+), 31 deletions(-)
diff --git a/target/i386/hvf/x86.c b/target/i386/hvf/x86.c
index 7ebb5b45bd..fdb11c8db9 100644
--- a/target/i386/hvf/x86.c
+++ b/target/i386/hvf/x86.c
@@ -131,7 +131,7 @@ bool x86_is_v8086(struct CPUState *cpu)
{
X86CPU *x86_cpu = X86_CPU(cpu);
CPUX86State *env = &x86_cpu->env;
- return x86_is_protected(cpu) && (env->eflags & RFLAGS_VM);
+ return x86_is_protected(cpu) && (env->eflags & VM_MASK);
}
bool x86_is_long_mode(struct CPUState *cpu)
diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
index e309b8f203..f0d03faff9 100644
--- a/target/i386/hvf/x86.h
+++ b/target/i386/hvf/x86.h
@@ -42,26 +42,6 @@ typedef struct x86_register {
};
} __attribute__ ((__packed__)) x86_register;
-typedef enum x86_rflags {
- RFLAGS_CF = (1L << 0),
- RFLAGS_PF = (1L << 2),
- RFLAGS_AF = (1L << 4),
- RFLAGS_ZF = (1L << 6),
- RFLAGS_SF = (1L << 7),
- RFLAGS_TF = (1L << 8),
- RFLAGS_IF = (1L << 9),
- RFLAGS_DF = (1L << 10),
- RFLAGS_OF = (1L << 11),
- RFLAGS_IOPL = (3L << 12),
- RFLAGS_NT = (1L << 14),
- RFLAGS_RF = (1L << 16),
- RFLAGS_VM = (1L << 17),
- RFLAGS_AC = (1L << 18),
- RFLAGS_VIF = (1L << 19),
- RFLAGS_VIP = (1L << 20),
- RFLAGS_ID = (1L << 21),
-} x86_rflags;
-
typedef enum x86_reg_cr0 {
CR0_PE = (1L << 0),
CR0_MP = (1L << 1),
diff --git a/target/i386/hvf/x86_decode.c b/target/i386/hvf/x86_decode.c
index d881542181..34c5e3006c 100644
--- a/target/i386/hvf/x86_decode.c
+++ b/target/i386/hvf/x86_decode.c
@@ -697,15 +697,13 @@ static void decode_db_4(CPUX86State *env, struct x86_decode *decode)
#define RFLAGS_MASK_NONE 0
-#define RFLAGS_MASK_OSZAPC (RFLAGS_OF | RFLAGS_SF | RFLAGS_ZF | RFLAGS_AF | \
- RFLAGS_PF | RFLAGS_CF)
-#define RFLAGS_MASK_LAHF (RFLAGS_SF | RFLAGS_ZF | RFLAGS_AF | RFLAGS_PF | \
- RFLAGS_CF)
-#define RFLAGS_MASK_CF (RFLAGS_CF)
-#define RFLAGS_MASK_IF (RFLAGS_IF)
-#define RFLAGS_MASK_TF (RFLAGS_TF)
-#define RFLAGS_MASK_DF (RFLAGS_DF)
-#define RFLAGS_MASK_ZF (RFLAGS_ZF)
+#define RFLAGS_MASK_OSZAPC (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C)
+#define RFLAGS_MASK_LAHF (CC_S | CC_Z | CC_A | CC_P | CC_C)
+#define RFLAGS_MASK_CF (CC_C)
+#define RFLAGS_MASK_IF (IF_MASK)
+#define RFLAGS_MASK_TF (TF_MASK)
+#define RFLAGS_MASK_DF (DF_MASK)
+#define RFLAGS_MASK_ZF (CC_Z)
struct decode_tbl _1op_inst[] = {
{0x0, X86_DECODE_CMD_ADD, 1, true, decode_modrm_rm, decode_modrm_reg, NULL,
diff --git a/target/i386/hvf/x86_task.c b/target/i386/hvf/x86_task.c
index 6ea8508946..6f04478b3a 100644
--- a/target/i386/hvf/x86_task.c
+++ b/target/i386/hvf/x86_task.c
@@ -158,7 +158,7 @@ void vmx_handle_task_switch(CPUState *cpu, x68_segment_selector tss_sel, int rea
}
if (reason == TSR_IRET)
- env->eflags &= ~RFLAGS_NT;
+ env->eflags &= ~NT_MASK;
if (reason != TSR_CALL && reason != TSR_IDT_GATE)
old_tss_sel.sel = 0xffff;
--
2.26.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 10/13] i386: hvf: Drop regs in HVFX86EmulatorState
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
` (8 preceding siblings ...)
2020-05-28 19:37 ` [PATCH 09/13] i386: hvf: Drop copy of RFLAGS defines Roman Bolshakov
@ 2020-05-28 19:37 ` Roman Bolshakov
2020-05-28 19:37 ` [PATCH 11/13] i386: hvf: Move lazy_flags into CPUX86State Roman Bolshakov
` (4 subsequent siblings)
14 siblings, 0 replies; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Roman Bolshakov, Eduardo Habkost,
Cameron Esfahani, Richard Henderson
HVFX86EmulatorState carries it's own copy of x86 registers. It can be
dropped in favor of regs in generic CPUX86State.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
target/i386/hvf/x86.h | 13 +++++++------
target/i386/hvf/x86_emu.c | 18 +++++++++---------
2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
index f0d03faff9..6048b5cc74 100644
--- a/target/i386/hvf/x86.h
+++ b/target/i386/hvf/x86.h
@@ -235,13 +235,14 @@ typedef struct lazy_flags {
/* Definition of hvf_x86_state is here */
struct HVFX86EmulatorState {
- struct x86_register regs[16];
struct lazy_flags lflags;
uint8_t mmio_buf[4096];
};
/* useful register access macros */
-#define RRX(cpu, reg) (cpu->hvf_emul->regs[reg].rrx)
+#define x86_reg(cpu, reg) ((x86_register *) &cpu->regs[reg])
+
+#define RRX(cpu, reg) (x86_reg(cpu, reg)->rrx)
#define RAX(cpu) RRX(cpu, R_EAX)
#define RCX(cpu) RRX(cpu, R_ECX)
#define RDX(cpu) RRX(cpu, R_EDX)
@@ -259,7 +260,7 @@ struct HVFX86EmulatorState {
#define R14(cpu) RRX(cpu, R_R14)
#define R15(cpu) RRX(cpu, R_R15)
-#define ERX(cpu, reg) (cpu->hvf_emul->regs[reg].erx)
+#define ERX(cpu, reg) (x86_reg(cpu, reg)->erx)
#define EAX(cpu) ERX(cpu, R_EAX)
#define ECX(cpu) ERX(cpu, R_ECX)
#define EDX(cpu) ERX(cpu, R_EDX)
@@ -269,7 +270,7 @@ struct HVFX86EmulatorState {
#define ESI(cpu) ERX(cpu, R_ESI)
#define EDI(cpu) ERX(cpu, R_EDI)
-#define RX(cpu, reg) (cpu->hvf_emul->regs[reg].rx)
+#define RX(cpu, reg) (x86_reg(cpu, reg)->rx)
#define AX(cpu) RX(cpu, R_EAX)
#define CX(cpu) RX(cpu, R_ECX)
#define DX(cpu) RX(cpu, R_EDX)
@@ -279,13 +280,13 @@ struct HVFX86EmulatorState {
#define SI(cpu) RX(cpu, R_ESI)
#define DI(cpu) RX(cpu, R_EDI)
-#define RL(cpu, reg) (cpu->hvf_emul->regs[reg].lx)
+#define RL(cpu, reg) (x86_reg(cpu, reg)->lx)
#define AL(cpu) RL(cpu, R_EAX)
#define CL(cpu) RL(cpu, R_ECX)
#define DL(cpu) RL(cpu, R_EDX)
#define BL(cpu) RL(cpu, R_EBX)
-#define RH(cpu, reg) (cpu->hvf_emul->regs[reg].hx)
+#define RH(cpu, reg) (x86_reg(cpu, reg)->hx)
#define AH(cpu) RH(cpu, R_EAX)
#define CH(cpu) RH(cpu, R_ECX)
#define DH(cpu) RH(cpu, R_EDX)
diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
index 04fac64e72..1ad2c30e16 100644
--- a/target/i386/hvf/x86_emu.c
+++ b/target/i386/hvf/x86_emu.c
@@ -95,13 +95,13 @@ target_ulong read_reg(CPUX86State *env, int reg, int size)
{
switch (size) {
case 1:
- return env->hvf_emul->regs[reg].lx;
+ return x86_reg(env, reg)->lx;
case 2:
- return env->hvf_emul->regs[reg].rx;
+ return x86_reg(env, reg)->rx;
case 4:
- return env->hvf_emul->regs[reg].erx;
+ return x86_reg(env, reg)->erx;
case 8:
- return env->hvf_emul->regs[reg].rrx;
+ return x86_reg(env, reg)->rrx;
default:
abort();
}
@@ -112,16 +112,16 @@ void write_reg(CPUX86State *env, int reg, target_ulong val, int size)
{
switch (size) {
case 1:
- env->hvf_emul->regs[reg].lx = val;
+ x86_reg(env, reg)->lx = val;
break;
case 2:
- env->hvf_emul->regs[reg].rx = val;
+ x86_reg(env, reg)->rx = val;
break;
case 4:
- env->hvf_emul->regs[reg].rrx = (uint32_t)val;
+ x86_reg(env, reg)->rrx = (uint32_t)val;
break;
case 8:
- env->hvf_emul->regs[reg].rrx = val;
+ x86_reg(env, reg)->rrx = val;
break;
default:
abort();
@@ -173,7 +173,7 @@ void write_val_to_reg(target_ulong reg_ptr, target_ulong val, int size)
static bool is_host_reg(struct CPUX86State *env, target_ulong ptr)
{
- return (ptr - (target_ulong)&env->hvf_emul->regs[0]) < sizeof(env->hvf_emul->regs);
+ return (ptr - (target_ulong)&env->regs[0]) < sizeof(env->regs);
}
void write_val_ext(struct CPUX86State *env, target_ulong ptr, target_ulong val, int size)
--
2.26.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 11/13] i386: hvf: Move lazy_flags into CPUX86State
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
` (9 preceding siblings ...)
2020-05-28 19:37 ` [PATCH 10/13] i386: hvf: Drop regs in HVFX86EmulatorState Roman Bolshakov
@ 2020-05-28 19:37 ` Roman Bolshakov
2020-06-04 6:43 ` Philippe Mathieu-Daudé
2020-05-28 19:37 ` [PATCH 12/13] i386: hvf: Move mmio_buf " Roman Bolshakov
` (3 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Roman Bolshakov, Eduardo Habkost,
Cameron Esfahani, Richard Henderson
The lazy flags are still needed for instruction decoder.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
include/sysemu/hvf.h | 7 +++++
target/i386/cpu.h | 2 ++
target/i386/hvf/x86.h | 6 ----
target/i386/hvf/x86_flags.c | 57 ++++++++++++++++++-------------------
4 files changed, 37 insertions(+), 35 deletions(-)
diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index cf579e1592..41f5470c96 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -15,9 +15,16 @@
extern bool hvf_allowed;
#ifdef CONFIG_HVF
+#include "exec/cpu-defs.h"
+
uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx,
int reg);
#define hvf_enabled() (hvf_allowed)
+
+typedef struct hvf_lazy_flags {
+ target_ulong result;
+ target_ulong auxbits;
+} hvf_lazy_flags;
#else
#define hvf_enabled() 0
#define hvf_get_supported_cpuid(func, idx, reg) 0
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 408392dbf6..7e6566565a 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -20,6 +20,7 @@
#ifndef I386_CPU_H
#define I386_CPU_H
+#include "sysemu/hvf.h"
#include "sysemu/tcg.h"
#include "cpu-qom.h"
#include "hyperv-proto.h"
@@ -1591,6 +1592,7 @@ typedef struct CPUX86State {
struct kvm_nested_state *nested_state;
#endif
#if defined(CONFIG_HVF)
+ hvf_lazy_flags hvf_lflags;
HVFX86EmulatorState *hvf_emul;
#endif
diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
index 6048b5cc74..2363616c07 100644
--- a/target/i386/hvf/x86.h
+++ b/target/i386/hvf/x86.h
@@ -228,14 +228,8 @@ typedef struct x68_segment_selector {
};
} __attribute__ ((__packed__)) x68_segment_selector;
-typedef struct lazy_flags {
- target_ulong result;
- target_ulong auxbits;
-} lazy_flags;
-
/* Definition of hvf_x86_state is here */
struct HVFX86EmulatorState {
- struct lazy_flags lflags;
uint8_t mmio_buf[4096];
};
diff --git a/target/i386/hvf/x86_flags.c b/target/i386/hvf/x86_flags.c
index 1152cd7234..5ca4f41f5c 100644
--- a/target/i386/hvf/x86_flags.c
+++ b/target/i386/hvf/x86_flags.c
@@ -63,7 +63,7 @@
#define SET_FLAGS_OSZAPC_SIZE(size, lf_carries, lf_result) { \
target_ulong temp = ((lf_carries) & (LF_MASK_AF)) | \
(((lf_carries) >> (size - 2)) << LF_BIT_PO); \
- env->hvf_emul->lflags.result = (target_ulong)(int##size##_t)(lf_result); \
+ env->hvf_lflags.result = (target_ulong)(int##size##_t)(lf_result); \
if ((size) == 32) { \
temp = ((lf_carries) & ~(LF_MASK_PDB | LF_MASK_SD)); \
} else if ((size) == 16) { \
@@ -73,7 +73,7 @@
} else { \
VM_PANIC("unimplemented"); \
} \
- env->hvf_emul->lflags.auxbits = (target_ulong)(uint32_t)temp; \
+ env->hvf_lflags.auxbits = (target_ulong)(uint32_t)temp; \
}
/* carries, result */
@@ -100,10 +100,10 @@
} else { \
VM_PANIC("unimplemented"); \
} \
- env->hvf_emul->lflags.result = (target_ulong)(int##size##_t)(lf_result); \
- target_ulong delta_c = (env->hvf_emul->lflags.auxbits ^ temp) & LF_MASK_CF; \
+ env->hvf_lflags.result = (target_ulong)(int##size##_t)(lf_result); \
+ target_ulong delta_c = (env->hvf_lflags.auxbits ^ temp) & LF_MASK_CF; \
delta_c ^= (delta_c >> 1); \
- env->hvf_emul->lflags.auxbits = (target_ulong)(uint32_t)(temp ^ delta_c); \
+ env->hvf_lflags.auxbits = (target_ulong)(uint32_t)(temp ^ delta_c); \
}
/* carries, result */
@@ -117,9 +117,8 @@
void SET_FLAGS_OxxxxC(CPUX86State *env, uint32_t new_of, uint32_t new_cf)
{
uint32_t temp_po = new_of ^ new_cf;
- env->hvf_emul->lflags.auxbits &= ~(LF_MASK_PO | LF_MASK_CF);
- env->hvf_emul->lflags.auxbits |= (temp_po << LF_BIT_PO) |
- (new_cf << LF_BIT_CF);
+ env->hvf_lflags.auxbits &= ~(LF_MASK_PO | LF_MASK_CF);
+ env->hvf_lflags.auxbits |= (temp_po << LF_BIT_PO) | (new_cf << LF_BIT_CF);
}
void SET_FLAGS_OSZAPC_SUB32(CPUX86State *env, uint32_t v1, uint32_t v2,
@@ -215,27 +214,27 @@ void SET_FLAGS_OSZAPC_LOGIC8(CPUX86State *env, uint8_t v1, uint8_t v2,
bool get_PF(CPUX86State *env)
{
- uint32_t temp = (255 & env->hvf_emul->lflags.result);
- temp = temp ^ (255 & (env->hvf_emul->lflags.auxbits >> LF_BIT_PDB));
+ uint32_t temp = (255 & env->hvf_lflags.result);
+ temp = temp ^ (255 & (env->hvf_lflags.auxbits >> LF_BIT_PDB));
temp = (temp ^ (temp >> 4)) & 0x0F;
return (0x9669U >> temp) & 1;
}
void set_PF(CPUX86State *env, bool val)
{
- uint32_t temp = (255 & env->hvf_emul->lflags.result) ^ (!val);
- env->hvf_emul->lflags.auxbits &= ~(LF_MASK_PDB);
- env->hvf_emul->lflags.auxbits |= (temp << LF_BIT_PDB);
+ uint32_t temp = (255 & env->hvf_lflags.result) ^ (!val);
+ env->hvf_lflags.auxbits &= ~(LF_MASK_PDB);
+ env->hvf_lflags.auxbits |= (temp << LF_BIT_PDB);
}
bool get_OF(CPUX86State *env)
{
- return ((env->hvf_emul->lflags.auxbits + (1U << LF_BIT_PO)) >> LF_BIT_CF) & 1;
+ return ((env->hvf_lflags.auxbits + (1U << LF_BIT_PO)) >> LF_BIT_CF) & 1;
}
bool get_CF(CPUX86State *env)
{
- return (env->hvf_emul->lflags.auxbits >> LF_BIT_CF) & 1;
+ return (env->hvf_lflags.auxbits >> LF_BIT_CF) & 1;
}
void set_OF(CPUX86State *env, bool val)
@@ -252,45 +251,45 @@ void set_CF(CPUX86State *env, bool val)
bool get_AF(CPUX86State *env)
{
- return (env->hvf_emul->lflags.auxbits >> LF_BIT_AF) & 1;
+ return (env->hvf_lflags.auxbits >> LF_BIT_AF) & 1;
}
void set_AF(CPUX86State *env, bool val)
{
- env->hvf_emul->lflags.auxbits &= ~(LF_MASK_AF);
- env->hvf_emul->lflags.auxbits |= val << LF_BIT_AF;
+ env->hvf_lflags.auxbits &= ~(LF_MASK_AF);
+ env->hvf_lflags.auxbits |= val << LF_BIT_AF;
}
bool get_ZF(CPUX86State *env)
{
- return !env->hvf_emul->lflags.result;
+ return !env->hvf_lflags.result;
}
void set_ZF(CPUX86State *env, bool val)
{
if (val) {
- env->hvf_emul->lflags.auxbits ^=
- (((env->hvf_emul->lflags.result >> LF_SIGN_BIT) & 1) << LF_BIT_SD);
+ env->hvf_lflags.auxbits ^=
+ (((env->hvf_lflags.result >> LF_SIGN_BIT) & 1) << LF_BIT_SD);
/* merge the parity bits into the Parity Delta Byte */
- uint32_t temp_pdb = (255 & env->hvf_emul->lflags.result);
- env->hvf_emul->lflags.auxbits ^= (temp_pdb << LF_BIT_PDB);
+ uint32_t temp_pdb = (255 & env->hvf_lflags.result);
+ env->hvf_lflags.auxbits ^= (temp_pdb << LF_BIT_PDB);
/* now zero the .result value */
- env->hvf_emul->lflags.result = 0;
+ env->hvf_lflags.result = 0;
} else {
- env->hvf_emul->lflags.result |= (1 << 8);
+ env->hvf_lflags.result |= (1 << 8);
}
}
bool get_SF(CPUX86State *env)
{
- return ((env->hvf_emul->lflags.result >> LF_SIGN_BIT) ^
- (env->hvf_emul->lflags.auxbits >> LF_BIT_SD)) & 1;
+ return ((env->hvf_lflags.result >> LF_SIGN_BIT) ^
+ (env->hvf_lflags.auxbits >> LF_BIT_SD)) & 1;
}
void set_SF(CPUX86State *env, bool val)
{
bool temp_sf = get_SF(env);
- env->hvf_emul->lflags.auxbits ^= (temp_sf ^ val) << LF_BIT_SD;
+ env->hvf_lflags.auxbits ^= (temp_sf ^ val) << LF_BIT_SD;
}
void lflags_to_rflags(CPUX86State *env)
@@ -305,7 +304,7 @@ void lflags_to_rflags(CPUX86State *env)
void rflags_to_lflags(CPUX86State *env)
{
- env->hvf_emul->lflags.auxbits = env->hvf_emul->lflags.result = 0;
+ env->hvf_lflags.auxbits = env->hvf_lflags.result = 0;
set_OF(env, env->eflags & CC_O);
set_SF(env, env->eflags & CC_S);
set_ZF(env, env->eflags & CC_Z);
--
2.26.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 12/13] i386: hvf: Move mmio_buf into CPUX86State
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
` (10 preceding siblings ...)
2020-05-28 19:37 ` [PATCH 11/13] i386: hvf: Move lazy_flags into CPUX86State Roman Bolshakov
@ 2020-05-28 19:37 ` Roman Bolshakov
2020-06-04 18:27 ` Paolo Bonzini
2020-05-28 19:37 ` [PATCH 13/13] i386: hvf: Drop HVFX86EmulatorState Roman Bolshakov
` (2 subsequent siblings)
14 siblings, 1 reply; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Roman Bolshakov, Eduardo Habkost,
Cameron Esfahani, Richard Henderson
There's no similar field in CPUX86State, but it's needed for MMIO traps.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
target/i386/cpu.h | 1 +
target/i386/hvf/hvf.c | 5 +++++
target/i386/hvf/x86.h | 1 -
target/i386/hvf/x86_emu.c | 12 ++++++------
4 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 7e6566565a..be44e19154 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1593,6 +1593,7 @@ typedef struct CPUX86State {
#endif
#if defined(CONFIG_HVF)
hvf_lazy_flags hvf_lflags;
+ void *hvf_mmio_buf;
HVFX86EmulatorState *hvf_emul;
#endif
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 4cee496d71..57696c46c7 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -533,7 +533,11 @@ void hvf_reset_vcpu(CPUState *cpu) {
void hvf_vcpu_destroy(CPUState *cpu)
{
+ X86CPU *x86_cpu = X86_CPU(cpu);
+ CPUX86State *env = &x86_cpu->env;
+
hv_return_t ret = hv_vcpu_destroy((hv_vcpuid_t)cpu->hvf_fd);
+ g_free(env->hvf_mmio_buf);
assert_hvf_ok(ret);
}
@@ -563,6 +567,7 @@ int hvf_init_vcpu(CPUState *cpu)
init_decoder();
hvf_state->hvf_caps = g_new0(struct hvf_vcpu_caps, 1);
+ env->hvf_mmio_buf = g_new(char, 4096);
env->hvf_emul = g_new0(HVFX86EmulatorState, 1);
r = hv_vcpu_create((hv_vcpuid_t *)&cpu->hvf_fd, HV_VCPU_DEFAULT);
diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
index 2363616c07..483fcea762 100644
--- a/target/i386/hvf/x86.h
+++ b/target/i386/hvf/x86.h
@@ -230,7 +230,6 @@ typedef struct x68_segment_selector {
/* Definition of hvf_x86_state is here */
struct HVFX86EmulatorState {
- uint8_t mmio_buf[4096];
};
/* useful register access macros */
diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
index 1ad2c30e16..d3e289ed87 100644
--- a/target/i386/hvf/x86_emu.c
+++ b/target/i386/hvf/x86_emu.c
@@ -187,8 +187,8 @@ void write_val_ext(struct CPUX86State *env, target_ulong ptr, target_ulong val,
uint8_t *read_mmio(struct CPUX86State *env, target_ulong ptr, int bytes)
{
- vmx_read_mem(env_cpu(env), env->hvf_emul->mmio_buf, ptr, bytes);
- return env->hvf_emul->mmio_buf;
+ vmx_read_mem(env_cpu(env), env->hvf_mmio_buf, ptr, bytes);
+ return env->hvf_mmio_buf;
}
@@ -489,9 +489,9 @@ static void exec_ins_single(struct CPUX86State *env, struct x86_decode *decode)
target_ulong addr = linear_addr_size(env_cpu(env), RDI(env),
decode->addressing_size, R_ES);
- hvf_handle_io(env_cpu(env), DX(env), env->hvf_emul->mmio_buf, 0,
+ hvf_handle_io(env_cpu(env), DX(env), env->hvf_mmio_buf, 0,
decode->operand_size, 1);
- vmx_write_mem(env_cpu(env), addr, env->hvf_emul->mmio_buf,
+ vmx_write_mem(env_cpu(env), addr, env->hvf_mmio_buf,
decode->operand_size);
string_increment_reg(env, R_EDI, decode);
@@ -512,9 +512,9 @@ static void exec_outs_single(struct CPUX86State *env, struct x86_decode *decode)
{
target_ulong addr = decode_linear_addr(env, decode, RSI(env), R_DS);
- vmx_read_mem(env_cpu(env), env->hvf_emul->mmio_buf, addr,
+ vmx_read_mem(env_cpu(env), env->hvf_mmio_buf, addr,
decode->operand_size);
- hvf_handle_io(env_cpu(env), DX(env), env->hvf_emul->mmio_buf, 1,
+ hvf_handle_io(env_cpu(env), DX(env), env->hvf_mmio_buf, 1,
decode->operand_size, 1);
string_increment_reg(env, R_ESI, decode);
--
2.26.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 13/13] i386: hvf: Drop HVFX86EmulatorState
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
` (11 preceding siblings ...)
2020-05-28 19:37 ` [PATCH 12/13] i386: hvf: Move mmio_buf " Roman Bolshakov
@ 2020-05-28 19:37 ` Roman Bolshakov
2020-05-29 2:57 ` [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState no-reply
2020-06-04 1:53 ` Cameron Esfahani
14 siblings, 0 replies; 32+ messages in thread
From: Roman Bolshakov @ 2020-05-28 19:37 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Roman Bolshakov, Eduardo Habkost,
Cameron Esfahani, Richard Henderson
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
include/qemu/typedefs.h | 1 -
target/i386/cpu.h | 1 -
target/i386/hvf/hvf.c | 1 -
target/i386/hvf/x86.h | 4 ----
4 files changed, 7 deletions(-)
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index ecf3cde26c..6ce0356f2c 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -51,7 +51,6 @@ typedef struct FWCfgIoState FWCfgIoState;
typedef struct FWCfgMemState FWCfgMemState;
typedef struct FWCfgState FWCfgState;
typedef struct HostMemoryBackend HostMemoryBackend;
-typedef struct HVFX86EmulatorState HVFX86EmulatorState;
typedef struct I2CBus I2CBus;
typedef struct I2SCodec I2SCodec;
typedef struct IOMMUMemoryRegion IOMMUMemoryRegion;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index be44e19154..abf9d10d86 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1594,7 +1594,6 @@ typedef struct CPUX86State {
#if defined(CONFIG_HVF)
hvf_lazy_flags hvf_lflags;
void *hvf_mmio_buf;
- HVFX86EmulatorState *hvf_emul;
#endif
uint64_t mcg_cap;
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 57696c46c7..be016b951a 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -568,7 +568,6 @@ int hvf_init_vcpu(CPUState *cpu)
hvf_state->hvf_caps = g_new0(struct hvf_vcpu_caps, 1);
env->hvf_mmio_buf = g_new(char, 4096);
- env->hvf_emul = g_new0(HVFX86EmulatorState, 1);
r = hv_vcpu_create((hv_vcpuid_t *)&cpu->hvf_fd, HV_VCPU_DEFAULT);
cpu->vcpu_dirty = 1;
diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
index 483fcea762..bacade7b65 100644
--- a/target/i386/hvf/x86.h
+++ b/target/i386/hvf/x86.h
@@ -228,10 +228,6 @@ typedef struct x68_segment_selector {
};
} __attribute__ ((__packed__)) x68_segment_selector;
-/* Definition of hvf_x86_state is here */
-struct HVFX86EmulatorState {
-};
-
/* useful register access macros */
#define x86_reg(cpu, reg) ((x86_register *) &cpu->regs[reg])
--
2.26.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
` (12 preceding siblings ...)
2020-05-28 19:37 ` [PATCH 13/13] i386: hvf: Drop HVFX86EmulatorState Roman Bolshakov
@ 2020-05-29 2:57 ` no-reply
2020-06-04 1:53 ` Cameron Esfahani
14 siblings, 0 replies; 32+ messages in thread
From: no-reply @ 2020-05-29 2:57 UTC (permalink / raw)
To: r.bolshakov; +Cc: r.bolshakov, qemu-devel, dirty
Patchew URL: https://patchew.org/QEMU/20200528193758.51454-1-r.bolshakov@yadro.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20200528193758.51454-1-r.bolshakov@yadro.com
Subject: [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Switched to a new branch 'test'
babfd75 i386: hvf: Drop HVFX86EmulatorState
57557be i386: hvf: Move mmio_buf into CPUX86State
53b9354 i386: hvf: Move lazy_flags into CPUX86State
ba6c821 i386: hvf: Drop regs in HVFX86EmulatorState
70b2839 i386: hvf: Drop copy of RFLAGS defines
aef7278 i386: hvf: Drop rflags from HVFX86EmulatorState
3aa57aa i386: hvf: Drop fetch_rip from HVFX86EmulatorState
44a94ed i386: hvf: Use IP from CPUX86State
ef6fe79 i386: hvf: Use ins_len to advance IP
ec88b12 i386: hvf: Drop unused variable
de8d999 i386: hvf: Clean stray includes in sysemu
ad061bc i386: hvf: Drop useless declarations in sysemu
0da6fba i386: hvf: Move HVFState definition into hvf
=== OUTPUT BEGIN ===
1/13 Checking commit 0da6fbafda5f (i386: hvf: Move HVFState definition into hvf)
2/13 Checking commit ad061bc7f025 (i386: hvf: Drop useless declarations in sysemu)
3/13 Checking commit de8d9997e911 (i386: hvf: Clean stray includes in sysemu)
4/13 Checking commit ec88b12c4ae7 (i386: hvf: Drop unused variable)
5/13 Checking commit ef6fe796978e (i386: hvf: Use ins_len to advance IP)
6/13 Checking commit 44a94ed21d06 (i386: hvf: Use IP from CPUX86State)
ERROR: unnecessary whitespace before a quoted newline
#444: FILE: target/i386/hvf/x86_emu.c:1470:
+ printf("Unimplemented handler (%llx) for %d (%x %x) \n", env->eip,
total: 1 errors, 0 warnings, 403 lines checked
Patch 6/13 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/13 Checking commit 3aa57aab9271 (i386: hvf: Drop fetch_rip from HVFX86EmulatorState)
8/13 Checking commit aef72785da68 (i386: hvf: Drop rflags from HVFX86EmulatorState)
9/13 Checking commit 70b2839d8d2e (i386: hvf: Drop copy of RFLAGS defines)
10/13 Checking commit ba6c821f8a7e (i386: hvf: Drop regs in HVFX86EmulatorState)
11/13 Checking commit 53b935424444 (i386: hvf: Move lazy_flags into CPUX86State)
12/13 Checking commit 57557be2d13c (i386: hvf: Move mmio_buf into CPUX86State)
13/13 Checking commit babfd7578724 (i386: hvf: Drop HVFX86EmulatorState)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200528193758.51454-1-r.bolshakov@yadro.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
` (13 preceding siblings ...)
2020-05-29 2:57 ` [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState no-reply
@ 2020-06-04 1:53 ` Cameron Esfahani
14 siblings, 0 replies; 32+ messages in thread
From: Cameron Esfahani @ 2020-06-04 1:53 UTC (permalink / raw)
To: Roman Bolshakov; +Cc: qemu-devel
Reviewed-by: Cameron Esfahani <dirty@apple.com>
Cameron Esfahani
dirty@apple.com
"The cake is a lie."
Common wisdom
> On May 28, 2020, at 12:37 PM, Roman Bolshakov <r.bolshakov@yadro.com> wrote:
>
> Hi,
>
> This is a cleanup series for HVF accel.
>
> HVF is using two emulator states CPUX86State and HVFX86EmulatorState
> simultaneously. HVFX86EmulatorState is used for instruction emulation.
> CPUX86State is used in all other places. Sometimes the states are in
> sync, sometimes they're not. It complicates reasoning about emulator
> behaviour given that there's a third state - VMCS.
>
> The series tries to leverage CPUX86State for instruction decoding and
> removes HVFX86EmulatorState. I had to add two new hvf-specific fields to
> CPUX86State: lazy_flags and mmio_buf. It's likely that cc_op, cc_dst,
> etc could be reused for lazy_flags but it'd require major rework of flag
> processing during instruction emulation. Hopefully that'll happen too in
> the future.
>
> I tried to include sysemu/hvf.h into target/i386/cpu.h to add definition
> of hvf lazy flags but couldn't do that at first it because it introduced
> circular dependency between existing sysemu/hvf.h and cpu.h. The first
> three patches untangle and prune sysemu/hvf.h to the bare minimum to
> allow inclusion of sysemu/hvf.h into target/i386/cpu.h.
>
> This might conflict with [1], but merge/rebase should be trivial.
>
> 1. https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg07449.html
>
> Thanks,
> Roman
>
> Roman Bolshakov (13):
> i386: hvf: Move HVFState definition into hvf
> i386: hvf: Drop useless declarations in sysemu
> i386: hvf: Clean stray includes in sysemu
> i386: hvf: Drop unused variable
> i386: hvf: Use ins_len to advance IP
> i386: hvf: Use IP from CPUX86State
> i386: hvf: Drop fetch_rip from HVFX86EmulatorState
> i386: hvf: Drop rflags from HVFX86EmulatorState
> i386: hvf: Drop copy of RFLAGS defines
> i386: hvf: Drop regs in HVFX86EmulatorState
> i386: hvf: Move lazy_flags into CPUX86State
> i386: hvf: Move mmio_buf into CPUX86State
> i386: hvf: Drop HVFX86EmulatorState
>
> include/qemu/typedefs.h | 1 -
> include/sysemu/hvf.h | 73 ++-------------------
> target/i386/cpu.h | 4 +-
> target/i386/hvf/hvf-i386.h | 35 ++++++++++
> target/i386/hvf/hvf.c | 30 ++++-----
> target/i386/hvf/x86.c | 2 +-
> target/i386/hvf/x86.h | 89 ++-----------------------
> target/i386/hvf/x86_decode.c | 25 ++++---
> target/i386/hvf/x86_emu.c | 122 +++++++++++++++++------------------
> target/i386/hvf/x86_flags.c | 81 ++++++++++++-----------
> target/i386/hvf/x86_task.c | 10 +--
> target/i386/hvf/x86hvf.c | 6 +-
> 12 files changed, 186 insertions(+), 292 deletions(-)
>
> --
> 2.26.1
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 04/13] i386: hvf: Drop unused variable
2020-05-28 19:37 ` [PATCH 04/13] i386: hvf: Drop unused variable Roman Bolshakov
@ 2020-06-04 6:35 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-04 6:35 UTC (permalink / raw)
To: Roman Bolshakov, qemu-devel
Cc: Paolo Bonzini, Eduardo Habkost, Cameron Esfahani, Richard Henderson
On 5/28/20 9:37 PM, Roman Bolshakov wrote:
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> target/i386/hvf/x86.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
> index c95d5b2116..56fcde13c6 100644
> --- a/target/i386/hvf/x86.h
> +++ b/target/i386/hvf/x86.h
> @@ -293,7 +293,6 @@ typedef struct lazy_flags {
>
> /* Definition of hvf_x86_state is here */
> struct HVFX86EmulatorState {
> - int interruptable;
> uint64_t fetch_rip;
> uint64_t rip;
> struct x86_register regs[16];
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 02/13] i386: hvf: Drop useless declarations in sysemu
2020-05-28 19:37 ` [PATCH 02/13] i386: hvf: Drop useless declarations in sysemu Roman Bolshakov
@ 2020-06-04 6:35 ` Philippe Mathieu-Daudé
2020-06-04 9:53 ` Claudio Fontana
1 sibling, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-04 6:35 UTC (permalink / raw)
To: Roman Bolshakov, qemu-devel
Cc: Paolo Bonzini, Eduardo Habkost, Cameron Esfahani, Richard Henderson
On 5/28/20 9:37 PM, Roman Bolshakov wrote:
> They're either declared elsewhere or have no use.
>
> While at it, rename _hvf_cpu_synchronize_post_init() to
> do_hvf_cpu_synchronize_post_init().
>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> include/sysemu/hvf.h | 22 ----------------------
> target/i386/hvf/hvf.c | 7 ++++---
> 2 files changed, 4 insertions(+), 25 deletions(-)
>
> diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
> index 30a565ab73..03f3cd7db3 100644
> --- a/include/sysemu/hvf.h
> +++ b/include/sysemu/hvf.h
> @@ -30,35 +30,13 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx,
> #define hvf_get_supported_cpuid(func, idx, reg) 0
> #endif
>
> -/* Disable HVF if |disable| is 1, otherwise, enable it iff it is supported by
> - * the host CPU. Use hvf_enabled() after this to get the result. */
> -void hvf_disable(int disable);
> -
> -/* Returns non-0 if the host CPU supports the VMX "unrestricted guest" feature
> - * which allows the virtual CPU to directly run in "real mode". If true, this
> - * allows QEMU to run several vCPU threads in parallel (see cpus.c). Otherwise,
> - * only a a single TCG thread can run, and it will call HVF to run the current
> - * instructions, except in case of "real mode" (paging disabled, typically at
> - * boot time), or MMIO operations. */
> -
> -int hvf_sync_vcpus(void);
> -
> int hvf_init_vcpu(CPUState *);
> int hvf_vcpu_exec(CPUState *);
> -int hvf_smp_cpu_exec(CPUState *);
> void hvf_cpu_synchronize_state(CPUState *);
> void hvf_cpu_synchronize_post_reset(CPUState *);
> void hvf_cpu_synchronize_post_init(CPUState *);
> -void _hvf_cpu_synchronize_post_init(CPUState *, run_on_cpu_data);
> -
> void hvf_vcpu_destroy(CPUState *);
> -void hvf_raise_event(CPUState *);
> -/* void hvf_reset_vcpu_state(void *opaque); */
> void hvf_reset_vcpu(CPUState *);
> -void vmx_update_tpr(CPUState *);
> -void update_apic_tpr(CPUState *);
> -int hvf_put_registers(CPUState *);
> -void vmx_clear_int_window_exiting(CPUState *cpu);
>
> #define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf")
>
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index d72543dc31..9ccdb7e7c7 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -251,7 +251,7 @@ void vmx_update_tpr(CPUState *cpu)
> }
> }
>
> -void update_apic_tpr(CPUState *cpu)
> +static void update_apic_tpr(CPUState *cpu)
> {
> X86CPU *x86_cpu = X86_CPU(cpu);
> int tpr = rreg(cpu->hvf_fd, HV_X86_TPR) >> 4;
> @@ -312,7 +312,8 @@ void hvf_cpu_synchronize_post_reset(CPUState *cpu_state)
> run_on_cpu(cpu_state, do_hvf_cpu_synchronize_post_reset, RUN_ON_CPU_NULL);
> }
>
> -void _hvf_cpu_synchronize_post_init(CPUState *cpu, run_on_cpu_data arg)
> +static void do_hvf_cpu_synchronize_post_init(CPUState *cpu,
> + run_on_cpu_data arg)
> {
> CPUState *cpu_state = cpu;
> hvf_put_registers(cpu_state);
> @@ -321,7 +322,7 @@ void _hvf_cpu_synchronize_post_init(CPUState *cpu, run_on_cpu_data arg)
>
> void hvf_cpu_synchronize_post_init(CPUState *cpu_state)
> {
> - run_on_cpu(cpu_state, _hvf_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
> + run_on_cpu(cpu_state, do_hvf_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
> }
>
> static bool ept_emulation_fault(hvf_slot *slot, uint64_t gpa, uint64_t ept_qual)
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 05/13] i386: hvf: Use ins_len to advance IP
2020-05-28 19:37 ` [PATCH 05/13] i386: hvf: Use ins_len to advance IP Roman Bolshakov
@ 2020-06-04 6:39 ` Philippe Mathieu-Daudé
2020-06-04 18:15 ` Paolo Bonzini
0 siblings, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-04 6:39 UTC (permalink / raw)
To: Roman Bolshakov, qemu-devel
Cc: Paolo Bonzini, Eduardo Habkost, Cameron Esfahani, Richard Henderson
On 5/28/20 9:37 PM, Roman Bolshakov wrote:
> There's no need to read VMCS twice, instruction length is already
> available in ins_len.
>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> target/i386/hvf/hvf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index 9ccdb7e7c7..8ff1d25521 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -871,7 +871,7 @@ int hvf_vcpu_exec(CPUState *cpu)
> } else {
> simulate_wrmsr(cpu);
> }
> - RIP(env) += rvmcs(cpu->hvf_fd, VMCS_EXIT_INSTRUCTION_LENGTH);
> + RIP(env) += ins_len;
I'd feel safer if you change ins_len to uint64_t first.
> store_regs(cpu);
> break;
> }
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 11/13] i386: hvf: Move lazy_flags into CPUX86State
2020-05-28 19:37 ` [PATCH 11/13] i386: hvf: Move lazy_flags into CPUX86State Roman Bolshakov
@ 2020-06-04 6:43 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-04 6:43 UTC (permalink / raw)
To: Roman Bolshakov, qemu-devel
Cc: Paolo Bonzini, Eduardo Habkost, Cameron Esfahani, Richard Henderson
On 5/28/20 9:37 PM, Roman Bolshakov wrote:
> The lazy flags are still needed for instruction decoder.
>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> include/sysemu/hvf.h | 7 +++++
> target/i386/cpu.h | 2 ++
> target/i386/hvf/x86.h | 6 ----
> target/i386/hvf/x86_flags.c | 57 ++++++++++++++++++-------------------
> 4 files changed, 37 insertions(+), 35 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 06/13] i386: hvf: Use IP from CPUX86State
2020-05-28 19:37 ` [PATCH 06/13] i386: hvf: Use IP from CPUX86State Roman Bolshakov
@ 2020-06-04 6:47 ` Philippe Mathieu-Daudé
2020-06-04 9:06 ` Claudio Fontana
1 sibling, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-04 6:47 UTC (permalink / raw)
To: Roman Bolshakov, qemu-devel
Cc: Paolo Bonzini, Eduardo Habkost, Cameron Esfahani, Richard Henderson
On 5/28/20 9:37 PM, Roman Bolshakov wrote:
> Drop and replace rip field from HVFX86EmulatorState in favor of eip from
> common CPUX86State.
>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> target/i386/hvf/hvf.c | 6 +--
> target/i386/hvf/x86.h | 3 --
> target/i386/hvf/x86_decode.c | 6 +--
> target/i386/hvf/x86_emu.c | 86 ++++++++++++++++++------------------
> target/i386/hvf/x86_task.c | 4 +-
> 5 files changed, 51 insertions(+), 54 deletions(-)
>
[...]
> diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
> index 56fcde13c6..e3ab7c5137 100644
> --- a/target/i386/hvf/x86.h
> +++ b/target/i386/hvf/x86.h
> @@ -294,7 +294,6 @@ typedef struct lazy_flags {
> /* Definition of hvf_x86_state is here */
> struct HVFX86EmulatorState {
> uint64_t fetch_rip;
> - uint64_t rip;
> struct x86_register regs[16];
> struct x86_reg_flags rflags;
> struct lazy_flags lflags;
> @@ -302,8 +301,6 @@ struct HVFX86EmulatorState {
> };
>
> /* useful register access macros */
> -#define RIP(cpu) (cpu->hvf_emul->rip)
> -#define EIP(cpu) ((uint32_t)cpu->hvf_emul->rip)
> #define RFLAGS(cpu) (cpu->hvf_emul->rflags.rflags)
> #define EFLAGS(cpu) (cpu->hvf_emul->rflags.eflags)
>
[...]
> diff --git a/target/i386/hvf/x86_task.c b/target/i386/hvf/x86_task.c
> index 1daac6cc2b..834baec3ea 100644
> --- a/target/i386/hvf/x86_task.c
> +++ b/target/i386/hvf/x86_task.c
> @@ -38,7 +38,7 @@ static void save_state_to_tss32(CPUState *cpu, struct x86_tss_segment32 *tss)
> CPUX86State *env = &x86_cpu->env;
>
> /* CR3 and ldt selector are not saved intentionally */
> - tss->eip = EIP(env);
> + tss->eip = (uint32_t)env->eip;
> tss->eflags = EFLAGS(env);
> tss->eax = EAX(env);
> tss->ecx = ECX(env);
> @@ -64,7 +64,7 @@ static void load_state_from_tss32(CPUState *cpu, struct x86_tss_segment32 *tss)
>
> wvmcs(cpu->hvf_fd, VMCS_GUEST_CR3, tss->cr3);
>
> - RIP(env) = tss->eip;
> + env->eip = tss->eip;
> EFLAGS(env) = tss->eflags | 2;
>
> /* General purpose registers */
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 06/13] i386: hvf: Use IP from CPUX86State
2020-05-28 19:37 ` [PATCH 06/13] i386: hvf: Use IP from CPUX86State Roman Bolshakov
2020-06-04 6:47 ` Philippe Mathieu-Daudé
@ 2020-06-04 9:06 ` Claudio Fontana
1 sibling, 0 replies; 32+ messages in thread
From: Claudio Fontana @ 2020-06-04 9:06 UTC (permalink / raw)
To: Roman Bolshakov, qemu-devel
Cc: Paolo Bonzini, Eduardo Habkost, Cameron Esfahani, Richard Henderson
Hi Roman,
On 5/28/20 9:37 PM, Roman Bolshakov wrote:
> Drop and replace rip field from HVFX86EmulatorState in favor of eip from
> common CPUX86State.
>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> target/i386/hvf/hvf.c | 6 +--
> target/i386/hvf/x86.h | 3 --
> target/i386/hvf/x86_decode.c | 6 +--
> target/i386/hvf/x86_emu.c | 86 ++++++++++++++++++------------------
> target/i386/hvf/x86_task.c | 4 +-
> 5 files changed, 51 insertions(+), 54 deletions(-)
>
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index 8ff1d25521..45ae55dd27 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -797,7 +797,7 @@ int hvf_vcpu_exec(CPUState *cpu)
> } else {
> RAX(env) = (uint64_t)val;
> }
> - RIP(env) += ins_len;
> + env->eip += ins_len;
> store_regs(cpu);
> break;
> } else if (!string && !in) {
> @@ -871,7 +871,7 @@ int hvf_vcpu_exec(CPUState *cpu)
> } else {
> simulate_wrmsr(cpu);
> }
> - RIP(env) += ins_len;
> + env->eip += ins_len;
> store_regs(cpu);
> break;
> }
> @@ -907,7 +907,7 @@ int hvf_vcpu_exec(CPUState *cpu)
> error_report("Unrecognized CR %d", cr);
> abort();
> }
> - RIP(env) += ins_len;
> + env->eip += ins_len;
> store_regs(cpu);
> break;
> }
> diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
> index 56fcde13c6..e3ab7c5137 100644
> --- a/target/i386/hvf/x86.h
> +++ b/target/i386/hvf/x86.h
> @@ -294,7 +294,6 @@ typedef struct lazy_flags {
> /* Definition of hvf_x86_state is here */
> struct HVFX86EmulatorState {
> uint64_t fetch_rip;
> - uint64_t rip;
> struct x86_register regs[16];
> struct x86_reg_flags rflags;
> struct lazy_flags lflags;
> @@ -302,8 +301,6 @@ struct HVFX86EmulatorState {
> };
>
> /* useful register access macros */
> -#define RIP(cpu) (cpu->hvf_emul->rip)
> -#define EIP(cpu) ((uint32_t)cpu->hvf_emul->rip)
> #define RFLAGS(cpu) (cpu->hvf_emul->rflags.rflags)
> #define EFLAGS(cpu) (cpu->hvf_emul->rflags.eflags)
>
> diff --git a/target/i386/hvf/x86_decode.c b/target/i386/hvf/x86_decode.c
> index 77c346605f..a590088f54 100644
> --- a/target/i386/hvf/x86_decode.c
> +++ b/target/i386/hvf/x86_decode.c
> @@ -75,7 +75,7 @@ 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), RIP(env)) + decode->len;
> + target_ulong va = linear_rip(env_cpu(env), env->eip) + decode->len;
> vmx_read_mem(env_cpu(env), &val, va, size);
> decode->len += size;
>
> @@ -1771,7 +1771,7 @@ void calc_modrm_operand32(CPUX86State *env, struct x86_decode *decode,
> ptr += get_sib_val(env, decode, &seg);
> } else if (!decode->modrm.mod && 5 == decode->modrm.rm) {
> if (x86_is_long_mode(env_cpu(env))) {
> - ptr += RIP(env) + decode->len;
> + ptr += env->eip + decode->len;
> } else {
> ptr = decode->displacement;
> }
> @@ -1807,7 +1807,7 @@ void calc_modrm_operand64(CPUX86State *env, struct x86_decode *decode,
> if (4 == rm) {
> ptr = get_sib_val(env, decode, &seg) + offset;
> } else if (0 == mod && 5 == rm) {
> - ptr = RIP(env) + decode->len + (int32_t) offset;
> + ptr = env->eip + decode->len + (int32_t) offset;
> } else {
> ptr = get_reg_val(env, src, decode->rex.rex, decode->rex.b, 8) +
> (int64_t) offset;
> diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
> index 92ab815f5d..0efd9f9928 100644
> --- a/target/i386/hvf/x86_emu.c
> +++ b/target/i386/hvf/x86_emu.c
> @@ -267,49 +267,49 @@ static void exec_mov(struct CPUX86State *env, struct x86_decode *decode)
> write_val_ext(env, decode->op[0].ptr, decode->op[1].val,
> decode->operand_size);
>
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_add(struct CPUX86State *env, struct x86_decode *decode)
> {
> EXEC_2OP_FLAGS_CMD(env, decode, +, SET_FLAGS_OSZAPC_ADD, true);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_or(struct CPUX86State *env, struct x86_decode *decode)
> {
> EXEC_2OP_FLAGS_CMD(env, decode, |, SET_FLAGS_OSZAPC_LOGIC, true);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_adc(struct CPUX86State *env, struct x86_decode *decode)
> {
> EXEC_2OP_FLAGS_CMD(env, decode, +get_CF(env)+, SET_FLAGS_OSZAPC_ADD, true);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_sbb(struct CPUX86State *env, struct x86_decode *decode)
> {
> EXEC_2OP_FLAGS_CMD(env, decode, -get_CF(env)-, SET_FLAGS_OSZAPC_SUB, true);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_and(struct CPUX86State *env, struct x86_decode *decode)
> {
> EXEC_2OP_FLAGS_CMD(env, decode, &, SET_FLAGS_OSZAPC_LOGIC, true);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_sub(struct CPUX86State *env, struct x86_decode *decode)
> {
> EXEC_2OP_FLAGS_CMD(env, decode, -, SET_FLAGS_OSZAPC_SUB, true);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_xor(struct CPUX86State *env, struct x86_decode *decode)
> {
> EXEC_2OP_FLAGS_CMD(env, decode, ^, SET_FLAGS_OSZAPC_LOGIC, true);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_neg(struct CPUX86State *env, struct x86_decode *decode)
> @@ -332,13 +332,13 @@ static void exec_neg(struct CPUX86State *env, struct x86_decode *decode)
> }
>
> /*lflags_to_rflags(env);*/
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_cmp(struct CPUX86State *env, struct x86_decode *decode)
> {
> EXEC_2OP_FLAGS_CMD(env, decode, -, SET_FLAGS_OSZAPC_SUB, false);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_inc(struct CPUX86State *env, struct x86_decode *decode)
> @@ -348,7 +348,7 @@ static void exec_inc(struct CPUX86State *env, struct x86_decode *decode)
>
> EXEC_2OP_FLAGS_CMD(env, decode, +1+, SET_FLAGS_OSZAP_ADD, true);
>
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_dec(struct CPUX86State *env, struct x86_decode *decode)
> @@ -357,13 +357,13 @@ static void exec_dec(struct CPUX86State *env, struct x86_decode *decode)
> decode->op[1].val = 0;
>
> EXEC_2OP_FLAGS_CMD(env, decode, -1-, SET_FLAGS_OSZAP_SUB, true);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_tst(struct CPUX86State *env, struct x86_decode *decode)
> {
> EXEC_2OP_FLAGS_CMD(env, decode, &, SET_FLAGS_OSZAPC_LOGIC, false);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_not(struct CPUX86State *env, struct x86_decode *decode)
> @@ -372,7 +372,7 @@ static void exec_not(struct CPUX86State *env, struct x86_decode *decode)
>
> write_val_ext(env, decode->op[0].ptr, ~decode->op[0].val,
> decode->operand_size);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> void exec_movzx(struct CPUX86State *env, struct x86_decode *decode)
> @@ -392,7 +392,7 @@ void exec_movzx(struct CPUX86State *env, struct x86_decode *decode)
> decode->op[1].val = read_val_ext(env, decode->op[1].ptr, src_op_size);
> write_val_ext(env, decode->op[0].ptr, decode->op[1].val, op_size);
>
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_out(struct CPUX86State *env, struct x86_decode *decode)
> @@ -416,7 +416,7 @@ static void exec_out(struct CPUX86State *env, struct x86_decode *decode)
> VM_PANIC("Bad out opcode\n");
> break;
> }
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_in(struct CPUX86State *env, struct x86_decode *decode)
> @@ -452,7 +452,7 @@ static void exec_in(struct CPUX86State *env, struct x86_decode *decode)
> break;
> }
>
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static inline void string_increment_reg(struct CPUX86State *env, int reg,
> @@ -505,7 +505,7 @@ static void exec_ins(struct CPUX86State *env, struct x86_decode *decode)
> exec_ins_single(env, decode);
> }
>
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_outs_single(struct CPUX86State *env, struct x86_decode *decode)
> @@ -528,7 +528,7 @@ static void exec_outs(struct CPUX86State *env, struct x86_decode *decode)
> exec_outs_single(env, decode);
> }
>
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_movs_single(struct CPUX86State *env, struct x86_decode *decode)
> @@ -556,7 +556,7 @@ static void exec_movs(struct CPUX86State *env, struct x86_decode *decode)
> exec_movs_single(env, decode);
> }
>
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_cmps_single(struct CPUX86State *env, struct x86_decode *decode)
> @@ -586,7 +586,7 @@ static void exec_cmps(struct CPUX86State *env, struct x86_decode *decode)
> } else {
> exec_cmps_single(env, decode);
> }
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
>
> @@ -612,7 +612,7 @@ static void exec_stos(struct CPUX86State *env, struct x86_decode *decode)
> exec_stos_single(env, decode);
> }
>
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_scas_single(struct CPUX86State *env, struct x86_decode *decode)
> @@ -638,7 +638,7 @@ static void exec_scas(struct CPUX86State *env, struct x86_decode *decode)
> exec_scas_single(env, decode);
> }
>
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_lods_single(struct CPUX86State *env, struct x86_decode *decode)
> @@ -661,7 +661,7 @@ static void exec_lods(struct CPUX86State *env, struct x86_decode *decode)
> exec_lods_single(env, decode);
> }
>
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> void simulate_rdmsr(struct CPUState *cpu)
> @@ -758,7 +758,7 @@ void simulate_rdmsr(struct CPUState *cpu)
> static void exec_rdmsr(struct CPUX86State *env, struct x86_decode *decode)
> {
> simulate_rdmsr(env_cpu(env));
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> void simulate_wrmsr(struct CPUState *cpu)
> @@ -853,7 +853,7 @@ void simulate_wrmsr(struct CPUState *cpu)
> static void exec_wrmsr(struct CPUX86State *env, struct x86_decode *decode)
> {
> simulate_wrmsr(env_cpu(env));
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> /*
> @@ -909,25 +909,25 @@ static void do_bt(struct CPUX86State *env, struct x86_decode *decode, int flag)
> static void exec_bt(struct CPUX86State *env, struct x86_decode *decode)
> {
> do_bt(env, decode, 0);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_btc(struct CPUX86State *env, struct x86_decode *decode)
> {
> do_bt(env, decode, 1);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_btr(struct CPUX86State *env, struct x86_decode *decode)
> {
> do_bt(env, decode, 3);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_bts(struct CPUX86State *env, struct x86_decode *decode)
> {
> do_bt(env, decode, 2);
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> void exec_shl(struct CPUX86State *env, struct x86_decode *decode)
> @@ -991,7 +991,7 @@ void exec_shl(struct CPUX86State *env, struct x86_decode *decode)
>
> exit:
> /* lflags_to_rflags(env); */
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> void exec_movsx(CPUX86State *env, struct x86_decode *decode)
> @@ -1014,7 +1014,7 @@ void exec_movsx(CPUX86State *env, struct x86_decode *decode)
>
> write_val_ext(env, decode->op[0].ptr, decode->op[1].val, op_size);
>
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> void exec_ror(struct CPUX86State *env, struct x86_decode *decode)
> @@ -1092,7 +1092,7 @@ void exec_ror(struct CPUX86State *env, struct x86_decode *decode)
> break;
> }
> }
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> void exec_rol(struct CPUX86State *env, struct x86_decode *decode)
> @@ -1173,7 +1173,7 @@ void exec_rol(struct CPUX86State *env, struct x86_decode *decode)
> break;
> }
> }
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
>
> @@ -1259,7 +1259,7 @@ void exec_rcl(struct CPUX86State *env, struct x86_decode *decode)
> break;
> }
> }
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> void exec_rcr(struct CPUX86State *env, struct x86_decode *decode)
> @@ -1334,7 +1334,7 @@ void exec_rcr(struct CPUX86State *env, struct x86_decode *decode)
> break;
> }
> }
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_xchg(struct CPUX86State *env, struct x86_decode *decode)
> @@ -1346,7 +1346,7 @@ static void exec_xchg(struct CPUX86State *env, struct x86_decode *decode)
> write_val_ext(env, decode->op[1].ptr, decode->op[0].val,
> decode->operand_size);
>
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static void exec_xadd(struct CPUX86State *env, struct x86_decode *decode)
> @@ -1355,7 +1355,7 @@ static void exec_xadd(struct CPUX86State *env, struct x86_decode *decode)
> write_val_ext(env, decode->op[1].ptr, decode->op[0].val,
> decode->operand_size);
>
> - RIP(env) += decode->len;
> + env->eip += decode->len;
> }
>
> static struct cmd_handler {
> @@ -1434,7 +1434,7 @@ void load_regs(struct CPUState *cpu)
>
> RFLAGS(env) = rreg(cpu->hvf_fd, HV_X86_RFLAGS);
> rflags_to_lflags(env);
> - RIP(env) = rreg(cpu->hvf_fd, HV_X86_RIP);
> + env->eip = rreg(cpu->hvf_fd, HV_X86_RIP);
> }
>
> void store_regs(struct CPUState *cpu)
> @@ -1457,20 +1457,20 @@ void store_regs(struct CPUState *cpu)
>
> lflags_to_rflags(env);
> wreg(cpu->hvf_fd, HV_X86_RFLAGS, RFLAGS(env));
> - macvm_set_rip(cpu, RIP(env));
> + macvm_set_rip(cpu, env->eip);
> }
>
> bool exec_instruction(struct CPUX86State *env, struct x86_decode *ins)
> {
> /*if (hvf_vcpu_id(cpu))
> - printf("%d, %llx: exec_instruction %s\n", hvf_vcpu_id(cpu), RIP(cpu),
> + printf("%d, %llx: exec_instruction %s\n", hvf_vcpu_id(cpu), env->eip,
> decode_cmd_to_string(ins->cmd));*/
>
> if (!_cmd_handler[ins->cmd].handler) {
> - printf("Unimplemented handler (%llx) for %d (%x %x) \n", RIP(env),
> + printf("Unimplemented handler (%llx) for %d (%x %x) \n", env->eip,
this space before \n is what checkpatch is complaining about.
Ciao,
CLaudio
> ins->cmd, ins->opcode[0],
> ins->opcode_len > 1 ? ins->opcode[1] : 0);
> - RIP(env) += ins->len;
> + env->eip += ins->len;
> return true;
> }
>
> diff --git a/target/i386/hvf/x86_task.c b/target/i386/hvf/x86_task.c
> index 1daac6cc2b..834baec3ea 100644
> --- a/target/i386/hvf/x86_task.c
> +++ b/target/i386/hvf/x86_task.c
> @@ -38,7 +38,7 @@ static void save_state_to_tss32(CPUState *cpu, struct x86_tss_segment32 *tss)
> CPUX86State *env = &x86_cpu->env;
>
> /* CR3 and ldt selector are not saved intentionally */
> - tss->eip = EIP(env);
> + tss->eip = (uint32_t)env->eip;
> tss->eflags = EFLAGS(env);
> tss->eax = EAX(env);
> tss->ecx = ECX(env);
> @@ -64,7 +64,7 @@ static void load_state_from_tss32(CPUState *cpu, struct x86_tss_segment32 *tss)
>
> wvmcs(cpu->hvf_fd, VMCS_GUEST_CR3, tss->cr3);
>
> - RIP(env) = tss->eip;
> + env->eip = tss->eip;
> EFLAGS(env) = tss->eflags | 2;
>
> /* General purpose registers */
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 02/13] i386: hvf: Drop useless declarations in sysemu
2020-05-28 19:37 ` [PATCH 02/13] i386: hvf: Drop useless declarations in sysemu Roman Bolshakov
2020-06-04 6:35 ` Philippe Mathieu-Daudé
@ 2020-06-04 9:53 ` Claudio Fontana
2020-06-05 16:30 ` Roman Bolshakov
1 sibling, 1 reply; 32+ messages in thread
From: Claudio Fontana @ 2020-06-04 9:53 UTC (permalink / raw)
To: Roman Bolshakov, qemu-devel
Cc: Paolo Bonzini, Eduardo Habkost, Cameron Esfahani, Richard Henderson
On 5/28/20 9:37 PM, Roman Bolshakov wrote:
> They're either declared elsewhere or have no use.
>
> While at it, rename _hvf_cpu_synchronize_post_init() to
> do_hvf_cpu_synchronize_post_init().
>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> include/sysemu/hvf.h | 22 ----------------------
> target/i386/hvf/hvf.c | 7 ++++---
> 2 files changed, 4 insertions(+), 25 deletions(-)
>
> diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
> index 30a565ab73..03f3cd7db3 100644
> --- a/include/sysemu/hvf.h
> +++ b/include/sysemu/hvf.h
> @@ -30,35 +30,13 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx,
> #define hvf_get_supported_cpuid(func, idx, reg) 0
> #endif
>
> -/* Disable HVF if |disable| is 1, otherwise, enable it iff it is supported by
> - * the host CPU. Use hvf_enabled() after this to get the result. */
> -void hvf_disable(int disable);
> -
> -/* Returns non-0 if the host CPU supports the VMX "unrestricted guest" feature
> - * which allows the virtual CPU to directly run in "real mode". If true, this
> - * allows QEMU to run several vCPU threads in parallel (see cpus.c). Otherwise,
> - * only a a single TCG thread can run, and it will call HVF to run the current
> - * instructions, except in case of "real mode" (paging disabled, typically at
> - * boot time), or MMIO operations. */
> -
> -int hvf_sync_vcpus(void);
> -
> int hvf_init_vcpu(CPUState *);
> int hvf_vcpu_exec(CPUState *);
> -int hvf_smp_cpu_exec(CPUState *);
> void hvf_cpu_synchronize_state(CPUState *);
> void hvf_cpu_synchronize_post_reset(CPUState *);
> void hvf_cpu_synchronize_post_init(CPUState *);
> -void _hvf_cpu_synchronize_post_init(CPUState *, run_on_cpu_data);
> -
> void hvf_vcpu_destroy(CPUState *);
> -void hvf_raise_event(CPUState *);
> -/* void hvf_reset_vcpu_state(void *opaque); */
> void hvf_reset_vcpu(CPUState *);
> -void vmx_update_tpr(CPUState *);
> -void update_apic_tpr(CPUState *);
> -int hvf_put_registers(CPUState *);
> -void vmx_clear_int_window_exiting(CPUState *cpu);
>
> #define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf")
>
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index d72543dc31..9ccdb7e7c7 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -251,7 +251,7 @@ void vmx_update_tpr(CPUState *cpu)
> }
> }
>
> -void update_apic_tpr(CPUState *cpu)
> +static void update_apic_tpr(CPUState *cpu)
> {
> X86CPU *x86_cpu = X86_CPU(cpu);
> int tpr = rreg(cpu->hvf_fd, HV_X86_TPR) >> 4;
> @@ -312,7 +312,8 @@ void hvf_cpu_synchronize_post_reset(CPUState *cpu_state)
> run_on_cpu(cpu_state, do_hvf_cpu_synchronize_post_reset, RUN_ON_CPU_NULL);
> }
>
> -void _hvf_cpu_synchronize_post_init(CPUState *cpu, run_on_cpu_data arg)
> +static void do_hvf_cpu_synchronize_post_init(CPUState *cpu,
> + run_on_cpu_data arg)
> {
> CPUState *cpu_state = cpu;
> hvf_put_registers(cpu_state);
> @@ -321,7 +322,7 @@ void _hvf_cpu_synchronize_post_init(CPUState *cpu, run_on_cpu_data arg)
>
> void hvf_cpu_synchronize_post_init(CPUState *cpu_state)
> {
> - run_on_cpu(cpu_state, _hvf_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
> + run_on_cpu(cpu_state, do_hvf_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
> }
>
> static bool ept_emulation_fault(hvf_slot *slot, uint64_t gpa, uint64_t ept_qual)
>
in this file (hvf.c) there is a comment:
/* TODO: synchronize vcpu state */
is the TODO still valid after this change? Or should the TODO be eliminated?
Thanks,
Claudio
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 05/13] i386: hvf: Use ins_len to advance IP
2020-06-04 6:39 ` Philippe Mathieu-Daudé
@ 2020-06-04 18:15 ` Paolo Bonzini
2020-06-05 14:29 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2020-06-04 18:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Roman Bolshakov, qemu-devel
Cc: Eduardo Habkost, Cameron Esfahani, Richard Henderson
On 04/06/20 08:39, Philippe Mathieu-Daudé wrote:
>> simulate_wrmsr(cpu);
>> }
>> - RIP(env) += rvmcs(cpu->hvf_fd, VMCS_EXIT_INSTRUCTION_LENGTH);
>> + RIP(env) += ins_len;
> I'd feel safer if you change ins_len to uint64_t first.
>
Why? It will never be more than 15 (it's also a 32-bit field in the VMCS).
Paolo
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 12/13] i386: hvf: Move mmio_buf into CPUX86State
2020-05-28 19:37 ` [PATCH 12/13] i386: hvf: Move mmio_buf " Roman Bolshakov
@ 2020-06-04 18:27 ` Paolo Bonzini
2020-06-05 17:24 ` Roman Bolshakov
2020-06-11 13:24 ` Claudio Fontana
0 siblings, 2 replies; 32+ messages in thread
From: Paolo Bonzini @ 2020-06-04 18:27 UTC (permalink / raw)
To: Roman Bolshakov, qemu-devel
Cc: Eduardo Habkost, Cameron Esfahani, Richard Henderson
On 28/05/20 21:37, Roman Bolshakov wrote:
> There's no similar field in CPUX86State, but it's needed for MMIO traps.
>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> target/i386/cpu.h | 1 +
> target/i386/hvf/hvf.c | 5 +++++
> target/i386/hvf/x86.h | 1 -
> target/i386/hvf/x86_emu.c | 12 ++++++------
> 4 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 7e6566565a..be44e19154 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1593,6 +1593,7 @@ typedef struct CPUX86State {
> #endif
> #if defined(CONFIG_HVF)
> hvf_lazy_flags hvf_lflags;
> + void *hvf_mmio_buf;
> HVFX86EmulatorState *hvf_emul;
> #endif
>
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index 4cee496d71..57696c46c7 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -533,7 +533,11 @@ void hvf_reset_vcpu(CPUState *cpu) {
>
> void hvf_vcpu_destroy(CPUState *cpu)
> {
> + X86CPU *x86_cpu = X86_CPU(cpu);
> + CPUX86State *env = &x86_cpu->env;
> +
> hv_return_t ret = hv_vcpu_destroy((hv_vcpuid_t)cpu->hvf_fd);
> + g_free(env->hvf_mmio_buf);
> assert_hvf_ok(ret);
> }
>
> @@ -563,6 +567,7 @@ int hvf_init_vcpu(CPUState *cpu)
> init_decoder();
>
> hvf_state->hvf_caps = g_new0(struct hvf_vcpu_caps, 1);
> + env->hvf_mmio_buf = g_new(char, 4096);
> env->hvf_emul = g_new0(HVFX86EmulatorState, 1);
>
> r = hv_vcpu_create((hv_vcpuid_t *)&cpu->hvf_fd, HV_VCPU_DEFAULT);
> diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
> index 2363616c07..483fcea762 100644
> --- a/target/i386/hvf/x86.h
> +++ b/target/i386/hvf/x86.h
> @@ -230,7 +230,6 @@ typedef struct x68_segment_selector {
>
> /* Definition of hvf_x86_state is here */
> struct HVFX86EmulatorState {
> - uint8_t mmio_buf[4096];
> };
>
> /* useful register access macros */
> diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
> index 1ad2c30e16..d3e289ed87 100644
> --- a/target/i386/hvf/x86_emu.c
> +++ b/target/i386/hvf/x86_emu.c
> @@ -187,8 +187,8 @@ void write_val_ext(struct CPUX86State *env, target_ulong ptr, target_ulong val,
>
> uint8_t *read_mmio(struct CPUX86State *env, target_ulong ptr, int bytes)
> {
> - vmx_read_mem(env_cpu(env), env->hvf_emul->mmio_buf, ptr, bytes);
> - return env->hvf_emul->mmio_buf;
> + vmx_read_mem(env_cpu(env), env->hvf_mmio_buf, ptr, bytes);
> + return env->hvf_mmio_buf;
> }
>
>
> @@ -489,9 +489,9 @@ static void exec_ins_single(struct CPUX86State *env, struct x86_decode *decode)
> target_ulong addr = linear_addr_size(env_cpu(env), RDI(env),
> decode->addressing_size, R_ES);
>
> - hvf_handle_io(env_cpu(env), DX(env), env->hvf_emul->mmio_buf, 0,
> + hvf_handle_io(env_cpu(env), DX(env), env->hvf_mmio_buf, 0,
> decode->operand_size, 1);
> - vmx_write_mem(env_cpu(env), addr, env->hvf_emul->mmio_buf,
> + vmx_write_mem(env_cpu(env), addr, env->hvf_mmio_buf,
> decode->operand_size);
>
> string_increment_reg(env, R_EDI, decode);
> @@ -512,9 +512,9 @@ static void exec_outs_single(struct CPUX86State *env, struct x86_decode *decode)
> {
> target_ulong addr = decode_linear_addr(env, decode, RSI(env), R_DS);
>
> - vmx_read_mem(env_cpu(env), env->hvf_emul->mmio_buf, addr,
> + vmx_read_mem(env_cpu(env), env->hvf_mmio_buf, addr,
> decode->operand_size);
> - hvf_handle_io(env_cpu(env), DX(env), env->hvf_emul->mmio_buf, 1,
> + hvf_handle_io(env_cpu(env), DX(env), env->hvf_mmio_buf, 1,
> decode->operand_size, 1);
>
> string_increment_reg(env, R_ESI, decode);
>
It should be possible to get rid of the buffer altogether, but it's ok
to do it separately.
I queued the series, thanks.
Paolo
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 01/13] i386: hvf: Move HVFState definition into hvf
2020-05-28 19:37 ` [PATCH 01/13] i386: hvf: Move HVFState definition into hvf Roman Bolshakov
@ 2020-06-05 14:19 ` Claudio Fontana
0 siblings, 0 replies; 32+ messages in thread
From: Claudio Fontana @ 2020-06-05 14:19 UTC (permalink / raw)
To: Roman Bolshakov, qemu-devel
Cc: Paolo Bonzini, Eduardo Habkost, Cameron Esfahani, Richard Henderson
On 5/28/20 9:37 PM, Roman Bolshakov wrote:
> "sysemu/hvf.h" is intended for inclusion in generic code. However it
> also contains several hvf definitions and declarations, including
> HVFState that are used only inside "hvf.c". "hvf-i386.h" would be more
> appropriate place to define HVFState as it's only included by "hvf.c"
> and "x86_task.c".
>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> include/sysemu/hvf.h | 37 -------------------------------------
> target/i386/hvf/hvf-i386.h | 35 +++++++++++++++++++++++++++++++++++
> 2 files changed, 35 insertions(+), 37 deletions(-)
>
> diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
> index d211e808e9..30a565ab73 100644
> --- a/include/sysemu/hvf.h
> +++ b/include/sysemu/hvf.h
> @@ -15,8 +15,6 @@
>
> #include "cpu.h"
> #include "qemu/bitops.h"
> -#include "exec/memory.h"
> -#include "sysemu/accel.h"
>
> extern bool hvf_allowed;
> #ifdef CONFIG_HVF
> @@ -32,41 +30,6 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx,
> #define hvf_get_supported_cpuid(func, idx, reg) 0
> #endif
>
> -/* hvf_slot flags */
> -#define HVF_SLOT_LOG (1 << 0)
> -
> -typedef struct hvf_slot {
> - uint64_t start;
> - uint64_t size;
> - uint8_t *mem;
> - int slot_id;
> - uint32_t flags;
> - MemoryRegion *region;
> -} hvf_slot;
> -
> -typedef struct hvf_vcpu_caps {
> - uint64_t vmx_cap_pinbased;
> - uint64_t vmx_cap_procbased;
> - uint64_t vmx_cap_procbased2;
> - uint64_t vmx_cap_entry;
> - uint64_t vmx_cap_exit;
> - uint64_t vmx_cap_preemption_timer;
> -} hvf_vcpu_caps;
> -
> -typedef struct HVFState {
> - AccelState parent;
> - hvf_slot slots[32];
> - int num_slots;
> -
> - hvf_vcpu_caps *hvf_caps;
> -} HVFState;
> -extern HVFState *hvf_state;
> -
> -void hvf_set_phys_mem(MemoryRegionSection *, bool);
> -void hvf_handle_io(CPUArchState *, uint16_t, void *,
> - int, int, int);
> -hvf_slot *hvf_find_overlap_slot(uint64_t, uint64_t);
> -
> /* Disable HVF if |disable| is 1, otherwise, enable it iff it is supported by
> * the host CPU. Use hvf_enabled() after this to get the result. */
> void hvf_disable(int disable);
> diff --git a/target/i386/hvf/hvf-i386.h b/target/i386/hvf/hvf-i386.h
> index 15ee4835cf..7cdf256649 100644
> --- a/target/i386/hvf/hvf-i386.h
> +++ b/target/i386/hvf/hvf-i386.h
> @@ -16,6 +16,7 @@
> #ifndef HVF_I386_H
> #define HVF_I386_H
>
> +#include "sysemu/accel.h"
> #include "sysemu/hvf.h"
> #include "cpu.h"
> #include "x86.h"
> @@ -37,6 +38,40 @@ struct hvf_state {
> uint64_t mem_quota;
> };
>
> +/* hvf_slot flags */
> +#define HVF_SLOT_LOG (1 << 0)
> +
> +typedef struct hvf_slot {
> + uint64_t start;
> + uint64_t size;
> + uint8_t *mem;
> + int slot_id;
> + uint32_t flags;
> + MemoryRegion *region;
> +} hvf_slot;
> +
> +typedef struct hvf_vcpu_caps {
> + uint64_t vmx_cap_pinbased;
> + uint64_t vmx_cap_procbased;
> + uint64_t vmx_cap_procbased2;
> + uint64_t vmx_cap_entry;
> + uint64_t vmx_cap_exit;
> + uint64_t vmx_cap_preemption_timer;
> +} hvf_vcpu_caps;
> +
> +typedef struct HVFState {
> + AccelState parent;
> + hvf_slot slots[32];
> + int num_slots;
> +
> + hvf_vcpu_caps *hvf_caps;
> +} HVFState;
> +extern HVFState *hvf_state;
> +
> +void hvf_set_phys_mem(MemoryRegionSection *, bool);
> +void hvf_handle_io(CPUArchState *, uint16_t, void *, int, int, int);
> +hvf_slot *hvf_find_overlap_slot(uint64_t, uint64_t);
> +
> #ifdef NEED_CPU_H
> /* Functions exported to host specific mode */
>
>
Reviewed-by: Claudio Fontana <cfontana@suse.de>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 03/13] i386: hvf: Clean stray includes in sysemu
2020-05-28 19:37 ` [PATCH 03/13] i386: hvf: Clean stray includes " Roman Bolshakov
@ 2020-06-05 14:20 ` Claudio Fontana
0 siblings, 0 replies; 32+ messages in thread
From: Claudio Fontana @ 2020-06-05 14:20 UTC (permalink / raw)
To: Roman Bolshakov, qemu-devel; +Cc: Cameron Esfahani
On 5/28/20 9:37 PM, Roman Bolshakov wrote:
> They have no use.
>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> include/sysemu/hvf.h | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
> index 03f3cd7db3..cf579e1592 100644
> --- a/include/sysemu/hvf.h
> +++ b/include/sysemu/hvf.h
> @@ -13,15 +13,8 @@
> #ifndef HVF_H
> #define HVF_H
>
> -#include "cpu.h"
> -#include "qemu/bitops.h"
> -
> extern bool hvf_allowed;
> #ifdef CONFIG_HVF
> -#include <Hypervisor/hv.h>
> -#include <Hypervisor/hv_vmx.h>
> -#include <Hypervisor/hv_error.h>
> -#include "target/i386/cpu.h"
> uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx,
> int reg);
> #define hvf_enabled() (hvf_allowed)
>
[I will include then cpu.h from hvf-cpus.c in the cpus-refactoring series.]
Reviewed-by: Claudio Fontana <cfontana@suse.de>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 05/13] i386: hvf: Use ins_len to advance IP
2020-06-04 18:15 ` Paolo Bonzini
@ 2020-06-05 14:29 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-05 14:29 UTC (permalink / raw)
To: Paolo Bonzini, Roman Bolshakov, qemu-devel
Cc: Eduardo Habkost, Cameron Esfahani, Richard Henderson
On 6/4/20 8:15 PM, Paolo Bonzini wrote:
> On 04/06/20 08:39, Philippe Mathieu-Daudé wrote:
>>> simulate_wrmsr(cpu);
>>> }
>>> - RIP(env) += rvmcs(cpu->hvf_fd, VMCS_EXIT_INSTRUCTION_LENGTH);
>>> + RIP(env) += ins_len;
>> I'd feel safer if you change ins_len to uint64_t first.
>>
>
> Why? It will never be more than 15 (it's also a 32-bit field in the VMCS).
Indeed, I am now seeing the comment in target/i386/hvf/vmcs.h:132
/* 32-bit read-only data fields */
#define VMCS_EXIT_INSTRUCTION_LENGTH 0x0000440C
So:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Thanks Paolo.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 02/13] i386: hvf: Drop useless declarations in sysemu
2020-06-04 9:53 ` Claudio Fontana
@ 2020-06-05 16:30 ` Roman Bolshakov
0 siblings, 0 replies; 32+ messages in thread
From: Roman Bolshakov @ 2020-06-05 16:30 UTC (permalink / raw)
To: Claudio Fontana
Cc: Paolo Bonzini, Richard Henderson, qemu-devel, Cameron Esfahani,
Eduardo Habkost
On Thu, Jun 04, 2020 at 11:53:53AM +0200, Claudio Fontana wrote:
> On 5/28/20 9:37 PM, Roman Bolshakov wrote:
> > They're either declared elsewhere or have no use.
> >
> > While at it, rename _hvf_cpu_synchronize_post_init() to
> > do_hvf_cpu_synchronize_post_init().
> >
> > Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> > ---
> > include/sysemu/hvf.h | 22 ----------------------
> > target/i386/hvf/hvf.c | 7 ++++---
> > 2 files changed, 4 insertions(+), 25 deletions(-)
> >
> > diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
> > index 30a565ab73..03f3cd7db3 100644
> > --- a/include/sysemu/hvf.h
> > +++ b/include/sysemu/hvf.h
> > @@ -30,35 +30,13 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx,
> > #define hvf_get_supported_cpuid(func, idx, reg) 0
> > #endif
> >
> > -/* Disable HVF if |disable| is 1, otherwise, enable it iff it is supported by
> > - * the host CPU. Use hvf_enabled() after this to get the result. */
> > -void hvf_disable(int disable);
> > -
> > -/* Returns non-0 if the host CPU supports the VMX "unrestricted guest" feature
> > - * which allows the virtual CPU to directly run in "real mode". If true, this
> > - * allows QEMU to run several vCPU threads in parallel (see cpus.c). Otherwise,
> > - * only a a single TCG thread can run, and it will call HVF to run the current
> > - * instructions, except in case of "real mode" (paging disabled, typically at
> > - * boot time), or MMIO operations. */
> > -
> > -int hvf_sync_vcpus(void);
> > -
> > int hvf_init_vcpu(CPUState *);
> > int hvf_vcpu_exec(CPUState *);
> > -int hvf_smp_cpu_exec(CPUState *);
> > void hvf_cpu_synchronize_state(CPUState *);
> > void hvf_cpu_synchronize_post_reset(CPUState *);
> > void hvf_cpu_synchronize_post_init(CPUState *);
> > -void _hvf_cpu_synchronize_post_init(CPUState *, run_on_cpu_data);
> > -
> > void hvf_vcpu_destroy(CPUState *);
> > -void hvf_raise_event(CPUState *);
> > -/* void hvf_reset_vcpu_state(void *opaque); */
> > void hvf_reset_vcpu(CPUState *);
> > -void vmx_update_tpr(CPUState *);
> > -void update_apic_tpr(CPUState *);
> > -int hvf_put_registers(CPUState *);
> > -void vmx_clear_int_window_exiting(CPUState *cpu);
> >
> > #define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf")
> >
> > diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> > index d72543dc31..9ccdb7e7c7 100644
> > --- a/target/i386/hvf/hvf.c
> > +++ b/target/i386/hvf/hvf.c
> > @@ -251,7 +251,7 @@ void vmx_update_tpr(CPUState *cpu)
> > }
> > }
> >
> > -void update_apic_tpr(CPUState *cpu)
> > +static void update_apic_tpr(CPUState *cpu)
> > {
> > X86CPU *x86_cpu = X86_CPU(cpu);
> > int tpr = rreg(cpu->hvf_fd, HV_X86_TPR) >> 4;
> > @@ -312,7 +312,8 @@ void hvf_cpu_synchronize_post_reset(CPUState *cpu_state)
> > run_on_cpu(cpu_state, do_hvf_cpu_synchronize_post_reset, RUN_ON_CPU_NULL);
> > }
> >
> > -void _hvf_cpu_synchronize_post_init(CPUState *cpu, run_on_cpu_data arg)
> > +static void do_hvf_cpu_synchronize_post_init(CPUState *cpu,
> > + run_on_cpu_data arg)
> > {
> > CPUState *cpu_state = cpu;
> > hvf_put_registers(cpu_state);
> > @@ -321,7 +322,7 @@ void _hvf_cpu_synchronize_post_init(CPUState *cpu, run_on_cpu_data arg)
> >
> > void hvf_cpu_synchronize_post_init(CPUState *cpu_state)
> > {
> > - run_on_cpu(cpu_state, _hvf_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
> > + run_on_cpu(cpu_state, do_hvf_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
> > }
> >
> > static bool ept_emulation_fault(hvf_slot *slot, uint64_t gpa, uint64_t ept_qual)
> >
>
> in this file (hvf.c) there is a comment:
>
> /* TODO: synchronize vcpu state */
>
> is the TODO still valid after this change? Or should the TODO be eliminated?
>
Hi Claudio,
Yeah, it's still valid. There will be another series to have only one
function where emulator state is synchronized.
Thanks,
Roman
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 12/13] i386: hvf: Move mmio_buf into CPUX86State
2020-06-04 18:27 ` Paolo Bonzini
@ 2020-06-05 17:24 ` Roman Bolshakov
2020-06-11 13:24 ` Claudio Fontana
1 sibling, 0 replies; 32+ messages in thread
From: Roman Bolshakov @ 2020-06-05 17:24 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Eduardo Habkost, qemu-devel, Cameron Esfahani, Richard Henderson
On Thu, Jun 04, 2020 at 08:27:37PM +0200, Paolo Bonzini wrote:
> On 28/05/20 21:37, Roman Bolshakov wrote:
> > There's no similar field in CPUX86State, but it's needed for MMIO traps.
> >
>
> It should be possible to get rid of the buffer altogether, but it's ok
> to do it separately.
>
Hi Paolo,
I will look into that.
Thanks,
Roman
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 12/13] i386: hvf: Move mmio_buf into CPUX86State
2020-06-04 18:27 ` Paolo Bonzini
2020-06-05 17:24 ` Roman Bolshakov
@ 2020-06-11 13:24 ` Claudio Fontana
2020-06-11 15:02 ` Roman Bolshakov
1 sibling, 1 reply; 32+ messages in thread
From: Claudio Fontana @ 2020-06-11 13:24 UTC (permalink / raw)
To: Paolo Bonzini, Roman Bolshakov, qemu-devel
Cc: Eduardo Habkost, Cameron Esfahani, Richard Henderson
On 6/4/20 8:27 PM, Paolo Bonzini wrote:
> On 28/05/20 21:37, Roman Bolshakov wrote:
>> There's no similar field in CPUX86State, but it's needed for MMIO traps.
>>
>> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
>> ---
>> target/i386/cpu.h | 1 +
>> target/i386/hvf/hvf.c | 5 +++++
>> target/i386/hvf/x86.h | 1 -
>> target/i386/hvf/x86_emu.c | 12 ++++++------
>> 4 files changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>> index 7e6566565a..be44e19154 100644
>> --- a/target/i386/cpu.h
>> +++ b/target/i386/cpu.h
>> @@ -1593,6 +1593,7 @@ typedef struct CPUX86State {
>> #endif
>> #if defined(CONFIG_HVF)
>> hvf_lazy_flags hvf_lflags;
>> + void *hvf_mmio_buf;
>> HVFX86EmulatorState *hvf_emul;
>> #endif
>>
>> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
>> index 4cee496d71..57696c46c7 100644
>> --- a/target/i386/hvf/hvf.c
>> +++ b/target/i386/hvf/hvf.c
>> @@ -533,7 +533,11 @@ void hvf_reset_vcpu(CPUState *cpu) {
>>
>> void hvf_vcpu_destroy(CPUState *cpu)
>> {
>> + X86CPU *x86_cpu = X86_CPU(cpu);
>> + CPUX86State *env = &x86_cpu->env;
>> +
>> hv_return_t ret = hv_vcpu_destroy((hv_vcpuid_t)cpu->hvf_fd);
>> + g_free(env->hvf_mmio_buf);
>> assert_hvf_ok(ret);
>> }
>>
>> @@ -563,6 +567,7 @@ int hvf_init_vcpu(CPUState *cpu)
>> init_decoder();
>>
>> hvf_state->hvf_caps = g_new0(struct hvf_vcpu_caps, 1);
>> + env->hvf_mmio_buf = g_new(char, 4096);
>> env->hvf_emul = g_new0(HVFX86EmulatorState, 1);
>>
>> r = hv_vcpu_create((hv_vcpuid_t *)&cpu->hvf_fd, HV_VCPU_DEFAULT);
>> diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
>> index 2363616c07..483fcea762 100644
>> --- a/target/i386/hvf/x86.h
>> +++ b/target/i386/hvf/x86.h
>> @@ -230,7 +230,6 @@ typedef struct x68_segment_selector {
>>
>> /* Definition of hvf_x86_state is here */
>> struct HVFX86EmulatorState {
>> - uint8_t mmio_buf[4096];
>> };
>>
>> /* useful register access macros */
>> diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
>> index 1ad2c30e16..d3e289ed87 100644
>> --- a/target/i386/hvf/x86_emu.c
>> +++ b/target/i386/hvf/x86_emu.c
>> @@ -187,8 +187,8 @@ void write_val_ext(struct CPUX86State *env, target_ulong ptr, target_ulong val,
>>
>> uint8_t *read_mmio(struct CPUX86State *env, target_ulong ptr, int bytes)
>> {
>> - vmx_read_mem(env_cpu(env), env->hvf_emul->mmio_buf, ptr, bytes);
>> - return env->hvf_emul->mmio_buf;
>> + vmx_read_mem(env_cpu(env), env->hvf_mmio_buf, ptr, bytes);
>> + return env->hvf_mmio_buf;
>> }
>>
>>
>> @@ -489,9 +489,9 @@ static void exec_ins_single(struct CPUX86State *env, struct x86_decode *decode)
>> target_ulong addr = linear_addr_size(env_cpu(env), RDI(env),
>> decode->addressing_size, R_ES);
>>
>> - hvf_handle_io(env_cpu(env), DX(env), env->hvf_emul->mmio_buf, 0,
>> + hvf_handle_io(env_cpu(env), DX(env), env->hvf_mmio_buf, 0,
>> decode->operand_size, 1);
>> - vmx_write_mem(env_cpu(env), addr, env->hvf_emul->mmio_buf,
>> + vmx_write_mem(env_cpu(env), addr, env->hvf_mmio_buf,
>> decode->operand_size);
>>
>> string_increment_reg(env, R_EDI, decode);
>> @@ -512,9 +512,9 @@ static void exec_outs_single(struct CPUX86State *env, struct x86_decode *decode)
>> {
>> target_ulong addr = decode_linear_addr(env, decode, RSI(env), R_DS);
>>
>> - vmx_read_mem(env_cpu(env), env->hvf_emul->mmio_buf, addr,
>> + vmx_read_mem(env_cpu(env), env->hvf_mmio_buf, addr,
>> decode->operand_size);
>> - hvf_handle_io(env_cpu(env), DX(env), env->hvf_emul->mmio_buf, 1,
>> + hvf_handle_io(env_cpu(env), DX(env), env->hvf_mmio_buf, 1,
>> decode->operand_size, 1);
>>
>> string_increment_reg(env, R_ESI, decode);
>>
>
> It should be possible to get rid of the buffer altogether, but it's ok
> to do it separately.
>
> I queued the series, thanks.
>
> Paolo
>
>
Thanks Paolo, I am waiting for this (and maybe another series from Roman) to be able to do the cpus refactoring.
Incidentally, would it not be great to have some machinery that automatically tracks which series is already queued where?
Maybe there is already a mechanism I am unaware of?
Ciao,
Claudio
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 12/13] i386: hvf: Move mmio_buf into CPUX86State
2020-06-11 13:24 ` Claudio Fontana
@ 2020-06-11 15:02 ` Roman Bolshakov
0 siblings, 0 replies; 32+ messages in thread
From: Roman Bolshakov @ 2020-06-11 15:02 UTC (permalink / raw)
To: Claudio Fontana
Cc: Paolo Bonzini, Richard Henderson, qemu-devel, Cameron Esfahani,
Eduardo Habkost
On Thu, Jun 11, 2020 at 03:24:31PM +0200, Claudio Fontana wrote:
> On 6/4/20 8:27 PM, Paolo Bonzini wrote:
> > On 28/05/20 21:37, Roman Bolshakov wrote:
> >> There's no similar field in CPUX86State, but it's needed for MMIO traps.
> >>
> >> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> >> ---
> >> target/i386/cpu.h | 1 +
> >> target/i386/hvf/hvf.c | 5 +++++
> >> target/i386/hvf/x86.h | 1 -
> >> target/i386/hvf/x86_emu.c | 12 ++++++------
> >> 4 files changed, 12 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> >> index 7e6566565a..be44e19154 100644
> >> --- a/target/i386/cpu.h
> >> +++ b/target/i386/cpu.h
> >> @@ -1593,6 +1593,7 @@ typedef struct CPUX86State {
> >> #endif
> >> #if defined(CONFIG_HVF)
> >> hvf_lazy_flags hvf_lflags;
> >> + void *hvf_mmio_buf;
> >> HVFX86EmulatorState *hvf_emul;
> >> #endif
> >>
> >> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> >> index 4cee496d71..57696c46c7 100644
> >> --- a/target/i386/hvf/hvf.c
> >> +++ b/target/i386/hvf/hvf.c
> >> @@ -533,7 +533,11 @@ void hvf_reset_vcpu(CPUState *cpu) {
> >>
> >> void hvf_vcpu_destroy(CPUState *cpu)
> >> {
> >> + X86CPU *x86_cpu = X86_CPU(cpu);
> >> + CPUX86State *env = &x86_cpu->env;
> >> +
> >> hv_return_t ret = hv_vcpu_destroy((hv_vcpuid_t)cpu->hvf_fd);
> >> + g_free(env->hvf_mmio_buf);
> >> assert_hvf_ok(ret);
> >> }
> >>
> >> @@ -563,6 +567,7 @@ int hvf_init_vcpu(CPUState *cpu)
> >> init_decoder();
> >>
> >> hvf_state->hvf_caps = g_new0(struct hvf_vcpu_caps, 1);
> >> + env->hvf_mmio_buf = g_new(char, 4096);
> >> env->hvf_emul = g_new0(HVFX86EmulatorState, 1);
> >>
> >> r = hv_vcpu_create((hv_vcpuid_t *)&cpu->hvf_fd, HV_VCPU_DEFAULT);
> >> diff --git a/target/i386/hvf/x86.h b/target/i386/hvf/x86.h
> >> index 2363616c07..483fcea762 100644
> >> --- a/target/i386/hvf/x86.h
> >> +++ b/target/i386/hvf/x86.h
> >> @@ -230,7 +230,6 @@ typedef struct x68_segment_selector {
> >>
> >> /* Definition of hvf_x86_state is here */
> >> struct HVFX86EmulatorState {
> >> - uint8_t mmio_buf[4096];
> >> };
> >>
> >> /* useful register access macros */
> >> diff --git a/target/i386/hvf/x86_emu.c b/target/i386/hvf/x86_emu.c
> >> index 1ad2c30e16..d3e289ed87 100644
> >> --- a/target/i386/hvf/x86_emu.c
> >> +++ b/target/i386/hvf/x86_emu.c
> >> @@ -187,8 +187,8 @@ void write_val_ext(struct CPUX86State *env, target_ulong ptr, target_ulong val,
> >>
> >> uint8_t *read_mmio(struct CPUX86State *env, target_ulong ptr, int bytes)
> >> {
> >> - vmx_read_mem(env_cpu(env), env->hvf_emul->mmio_buf, ptr, bytes);
> >> - return env->hvf_emul->mmio_buf;
> >> + vmx_read_mem(env_cpu(env), env->hvf_mmio_buf, ptr, bytes);
> >> + return env->hvf_mmio_buf;
> >> }
> >>
> >>
> >> @@ -489,9 +489,9 @@ static void exec_ins_single(struct CPUX86State *env, struct x86_decode *decode)
> >> target_ulong addr = linear_addr_size(env_cpu(env), RDI(env),
> >> decode->addressing_size, R_ES);
> >>
> >> - hvf_handle_io(env_cpu(env), DX(env), env->hvf_emul->mmio_buf, 0,
> >> + hvf_handle_io(env_cpu(env), DX(env), env->hvf_mmio_buf, 0,
> >> decode->operand_size, 1);
> >> - vmx_write_mem(env_cpu(env), addr, env->hvf_emul->mmio_buf,
> >> + vmx_write_mem(env_cpu(env), addr, env->hvf_mmio_buf,
> >> decode->operand_size);
> >>
> >> string_increment_reg(env, R_EDI, decode);
> >> @@ -512,9 +512,9 @@ static void exec_outs_single(struct CPUX86State *env, struct x86_decode *decode)
> >> {
> >> target_ulong addr = decode_linear_addr(env, decode, RSI(env), R_DS);
> >>
> >> - vmx_read_mem(env_cpu(env), env->hvf_emul->mmio_buf, addr,
> >> + vmx_read_mem(env_cpu(env), env->hvf_mmio_buf, addr,
> >> decode->operand_size);
> >> - hvf_handle_io(env_cpu(env), DX(env), env->hvf_emul->mmio_buf, 1,
> >> + hvf_handle_io(env_cpu(env), DX(env), env->hvf_mmio_buf, 1,
> >> decode->operand_size, 1);
> >>
> >> string_increment_reg(env, R_ESI, decode);
> >>
> >
> > It should be possible to get rid of the buffer altogether, but it's ok
> > to do it separately.
> >
> > I queued the series, thanks.
> >
> > Paolo
> >
> >
>
> Thanks Paolo, I am waiting for this (and maybe another series from Roman) to be able to do the cpus refactoring.
>
> Incidentally, would it not be great to have some machinery that automatically tracks which series is already queued where?
> Maybe there is already a mechanism I am unaware of?
>
> Ciao,
>
> Claudio
Hi Claudio,
I also had the same question earlier today on IRC but I've just recalled
that PULL requests typically have a reference to the queue repo/branch:
https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg06825.html
I'll rebase on it and prepare the series.
Regards,
Roman
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2020-06-11 15:06 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-28 19:37 [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState Roman Bolshakov
2020-05-28 19:37 ` [PATCH 01/13] i386: hvf: Move HVFState definition into hvf Roman Bolshakov
2020-06-05 14:19 ` Claudio Fontana
2020-05-28 19:37 ` [PATCH 02/13] i386: hvf: Drop useless declarations in sysemu Roman Bolshakov
2020-06-04 6:35 ` Philippe Mathieu-Daudé
2020-06-04 9:53 ` Claudio Fontana
2020-06-05 16:30 ` Roman Bolshakov
2020-05-28 19:37 ` [PATCH 03/13] i386: hvf: Clean stray includes " Roman Bolshakov
2020-06-05 14:20 ` Claudio Fontana
2020-05-28 19:37 ` [PATCH 04/13] i386: hvf: Drop unused variable Roman Bolshakov
2020-06-04 6:35 ` Philippe Mathieu-Daudé
2020-05-28 19:37 ` [PATCH 05/13] i386: hvf: Use ins_len to advance IP Roman Bolshakov
2020-06-04 6:39 ` Philippe Mathieu-Daudé
2020-06-04 18:15 ` Paolo Bonzini
2020-06-05 14:29 ` Philippe Mathieu-Daudé
2020-05-28 19:37 ` [PATCH 06/13] i386: hvf: Use IP from CPUX86State Roman Bolshakov
2020-06-04 6:47 ` Philippe Mathieu-Daudé
2020-06-04 9:06 ` Claudio Fontana
2020-05-28 19:37 ` [PATCH 07/13] i386: hvf: Drop fetch_rip from HVFX86EmulatorState Roman Bolshakov
2020-05-28 19:37 ` [PATCH 08/13] i386: hvf: Drop rflags " Roman Bolshakov
2020-05-28 19:37 ` [PATCH 09/13] i386: hvf: Drop copy of RFLAGS defines Roman Bolshakov
2020-05-28 19:37 ` [PATCH 10/13] i386: hvf: Drop regs in HVFX86EmulatorState Roman Bolshakov
2020-05-28 19:37 ` [PATCH 11/13] i386: hvf: Move lazy_flags into CPUX86State Roman Bolshakov
2020-06-04 6:43 ` Philippe Mathieu-Daudé
2020-05-28 19:37 ` [PATCH 12/13] i386: hvf: Move mmio_buf " Roman Bolshakov
2020-06-04 18:27 ` Paolo Bonzini
2020-06-05 17:24 ` Roman Bolshakov
2020-06-11 13:24 ` Claudio Fontana
2020-06-11 15:02 ` Roman Bolshakov
2020-05-28 19:37 ` [PATCH 13/13] i386: hvf: Drop HVFX86EmulatorState Roman Bolshakov
2020-05-29 2:57 ` [PATCH 00/13] i386: hvf: Remove HVFX86EmulatorState no-reply
2020-06-04 1:53 ` Cameron Esfahani
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.