Re: [Qemu-devel] [PATCH 20/22] target/i386: add the tcg_enabled() in target/i386/

2017-07-04 Thread Richard Henderson 

On 07/04/2017 01:12 AM, Paolo Bonzini wrote:

From: Yang Zhong

Add the tcg_enabled() where the x86 target needs to disable
TCG-specific code.

Signed-off-by: Yang Zhong
Signed-off-by: Paolo Bonzini
---
v2: do not touch bpt_helper.c, adjust caller in machine.c [Richard]


Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PATCH 20/22] target/i386: add the tcg_enabled() in target/i386/

2017-07-04 Thread Paolo Bonzini 
From: Yang Zhong 

Add the tcg_enabled() where the x86 target needs to disable
TCG-specific code.

Signed-off-by: Yang Zhong 
Signed-off-by: Paolo Bonzini 
---
v2: do not touch bpt_helper.c, adjust caller in machine.c [Richard]

 target/i386/cpu.c |  4 +++-
 target/i386/cpu.h |  8 +++-
 target/i386/helper.c  |  2 +-
 target/i386/machine.c | 10 +-
 4 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 642519a..c571772 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4040,8 +4040,10 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->class_by_name = x86_cpu_class_by_name;
 cc->parse_features = x86_cpu_parse_featurestr;
 cc->has_work = x86_cpu_has_work;
+#ifdef CONFIG_TCG
 cc->do_interrupt = x86_cpu_do_interrupt;
 cc->cpu_exec_interrupt = x86_cpu_exec_interrupt;
+#endif
 cc->dump_state = x86_cpu_dump_state;
 cc->get_crash_info = x86_cpu_get_crash_info;
 cc->set_pc = x86_cpu_set_pc;
@@ -4070,7 +4072,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->gdb_core_xml_file = "i386-32bit.xml";
 cc->gdb_num_core_regs = 41;
 #endif
-#ifndef CONFIG_USER_ONLY
+#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY)
 cc->debug_excp_handler = breakpoint_handler;
 #endif
 cc->cpu_exec_enter = x86_cpu_exec_enter;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 66a363f..cef7dbe 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -52,7 +52,9 @@
 
 #include "exec/cpu-defs.h"
 
+#ifdef CONFIG_TCG
 #include "fpu/softfloat.h"
+#endif
 
 #define R_EAX 0
 #define R_ECX 1
@@ -1597,7 +1599,11 @@ uint32_t cpu_cc_compute_all(CPUX86State *env1, int op);
 
 static inline uint32_t cpu_compute_eflags(CPUX86State *env)
 {
-return env->eflags | cpu_cc_compute_all(env, CC_OP) | (env->df & DF_MASK);
+uint32_t eflags = env->eflags;
+if (tcg_enabled()) {
+eflags |= cpu_cc_compute_all(env, CC_OP) | (env->df & DF_MASK);
+}
+return eflags;
 }
 
 /* NOTE: the translator must set DisasContext.cc_op to CC_OP_EFLAGS
diff --git a/target/i386/helper.c b/target/i386/helper.c
index bcf9b22..f63eb3d 100644
--- a/target/i386/helper.c
+++ b/target/i386/helper.c
@@ -990,7 +990,7 @@ void cpu_report_tpr_access(CPUX86State *env, TPRAccess 
access)
 env->tpr_access_type = access;
 
 cpu_interrupt(cs, CPU_INTERRUPT_TPR);
-} else {
+} else if (tcg_enabled()) {
 cpu_restore_state(cs, cs->mem_io_pc);
 
 apic_handle_tpr_access_report(cpu->apic_state, env->eip, access);
diff --git a/target/i386/machine.c b/target/i386/machine.c
index e0417fe..eab3372 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -281,16 +281,16 @@ static int cpu_post_load(void *opaque, int version_id)
 env->fptags[i] = (env->fptag_vmstate >> i) & 1;
 }
 if (tcg_enabled()) {
+target_ulong dr7;
 update_fp_status(env);
 update_mxcsr_status(env);
-}
 
-cpu_breakpoint_remove_all(cs, BP_CPU);
-cpu_watchpoint_remove_all(cs, BP_CPU);
-{
+cpu_breakpoint_remove_all(cs, BP_CPU);
+cpu_watchpoint_remove_all(cs, BP_CPU);
+
 /* Indicate all breakpoints disabled, as they are, then
let the helper re-enable them.  */
-target_ulong dr7 = env->dr[7];
+dr7 = env->dr[7];
 env->dr[7] = dr7 & ~(DR7_GLOBAL_BP_MASK | DR7_LOCAL_BP_MASK);
 cpu_x86_update_dr7(env, dr7);
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 20/22] target/i386: add the tcg_enabled() in target/i386/

2017-07-04 Thread Paolo Bonzini 


On 03/07/2017 22:19, Richard Henderson wrote:
> On 07/03/2017 09:34 AM, Paolo Bonzini wrote:
>> @@ -215,10 +215,12 @@ void breakpoint_handler(CPUState *cs)
>>   if (cs->watchpoint_hit) {
>>   if (cs->watchpoint_hit->flags & BP_CPU) {
>>   cs->watchpoint_hit = NULL;
>> -if (check_hw_breakpoints(env, false)) {
>> -raise_exception(env, EXCP01_DB);
>> -} else {
>> -cpu_loop_exit_noexc(cs);
>> +if (tcg_enabled()) {
>> +if (check_hw_breakpoints(env, false)) {
>> +raise_exception(env, EXCP01_DB);
>> +} else {
>> +cpu_loop_exit_noexc(cs);
>> +}
> 
> This seems like an odd place for the tcg_enabled check.  It seems like
> it should be much higher in the if/call chain.
> 
> Why are we doing all these bp checks only to disable the final raising
> of an exception?
> 
> Indeed, what in bpt_helper.c needs to be compiled in when !tcg_enabled?

Only cpu_x86_update_dr7, but it's not used by KVM (I was worried about
guest debug aka gdbstub mode).  So that's my own brain fart; I'll wrap
the cpu_x86_update_dr7 with tcg_enabled and drop the file altogether.

Paolo

Re: [Qemu-devel] [PATCH 20/22] target/i386: add the tcg_enabled() in target/i386/

2017-07-03 Thread Richard Henderson 

On 07/03/2017 09:34 AM, Paolo Bonzini wrote:

@@ -215,10 +215,12 @@ void breakpoint_handler(CPUState *cs)
  if (cs->watchpoint_hit) {
  if (cs->watchpoint_hit->flags & BP_CPU) {
  cs->watchpoint_hit = NULL;
-if (check_hw_breakpoints(env, false)) {
-raise_exception(env, EXCP01_DB);
-} else {
-cpu_loop_exit_noexc(cs);
+if (tcg_enabled()) {
+if (check_hw_breakpoints(env, false)) {
+raise_exception(env, EXCP01_DB);
+} else {
+cpu_loop_exit_noexc(cs);
+}


This seems like an odd place for the tcg_enabled check.  It seems like it 
should be much higher in the if/call chain.


Why are we doing all these bp checks only to disable the final raising of an 
exception?


Indeed, what in bpt_helper.c needs to be compiled in when !tcg_enabled?


r~

[Qemu-devel] [PATCH 20/22] target/i386: add the tcg_enabled() in target/i386/

2017-07-03 Thread Paolo Bonzini 
From: Yang Zhong 

Add the tcg_enabled() where the x86 target needs to disable
TCG-specific code.

Signed-off-by: Yang Zhong 
Signed-off-by: Paolo Bonzini 
---
 target/i386/bpt_helper.c | 32 +++-
 target/i386/cpu.c|  4 +++-
 target/i386/cpu.h|  8 +++-
 target/i386/machine.c|  5 +++--
 4 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/target/i386/bpt_helper.c b/target/i386/bpt_helper.c
index b3efdc7..e0fa54c 100644
--- a/target/i386/bpt_helper.c
+++ b/target/i386/bpt_helper.c
@@ -215,10 +215,12 @@ void breakpoint_handler(CPUState *cs)
 if (cs->watchpoint_hit) {
 if (cs->watchpoint_hit->flags & BP_CPU) {
 cs->watchpoint_hit = NULL;
-if (check_hw_breakpoints(env, false)) {
-raise_exception(env, EXCP01_DB);
-} else {
-cpu_loop_exit_noexc(cs);
+if (tcg_enabled()) {
+if (check_hw_breakpoints(env, false)) {
+raise_exception(env, EXCP01_DB);
+} else {
+cpu_loop_exit_noexc(cs);
+}
 }
 }
 } else {
@@ -226,7 +228,9 @@ void breakpoint_handler(CPUState *cs)
 if (bp->pc == env->eip) {
 if (bp->flags & BP_CPU) {
 check_hw_breakpoints(env, true);
-raise_exception(env, EXCP01_DB);
+if (tcg_enabled()) {
+raise_exception(env, EXCP01_DB);
+}
 }
 break;
 }
@@ -241,7 +245,9 @@ void helper_single_step(CPUX86State *env)
 check_hw_breakpoints(env, true);
 env->dr[6] |= DR6_BS;
 #endif
-raise_exception(env, EXCP01_DB);
+if (tcg_enabled()) {
+raise_exception(env, EXCP01_DB);
+}
 }
 
 void helper_rechecking_single_step(CPUX86State *env)
@@ -282,7 +288,9 @@ void helper_set_dr(CPUX86State *env, int reg, target_ulong 
t0)
 cpu_x86_update_dr7(env, t0);
 return;
 }
-raise_exception_err_ra(env, EXCP06_ILLOP, 0, GETPC());
+if (tcg_enabled()) {
+raise_exception_err_ra(env, EXCP06_ILLOP, 0, GETPC());
+}
 #endif
 }
 
@@ -304,7 +312,11 @@ target_ulong helper_get_dr(CPUX86State *env, int reg)
 return env->dr[7];
 }
 }
-raise_exception_err_ra(env, EXCP06_ILLOP, 0, GETPC());
+if (tcg_enabled()) {
+raise_exception_err_ra(env, EXCP06_ILLOP, 0, GETPC());
+} else {
+return 0;
+}
 }
 
 /* Check if Port I/O is trapped by a breakpoint.  */
@@ -329,7 +341,9 @@ void helper_bpt_io(CPUX86State *env, uint32_t port,
 if (hit) {
 env->dr[6] = (env->dr[6] & ~0xf) | hit;
 env->eip = next_eip;
-raise_exception(env, EXCP01_DB);
+if (tcg_enabled()) {
+raise_exception(env, EXCP01_DB);
+}
 }
 #endif
 }
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 642519a..c571772 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4040,8 +4040,10 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->class_by_name = x86_cpu_class_by_name;
 cc->parse_features = x86_cpu_parse_featurestr;
 cc->has_work = x86_cpu_has_work;
+#ifdef CONFIG_TCG
 cc->do_interrupt = x86_cpu_do_interrupt;
 cc->cpu_exec_interrupt = x86_cpu_exec_interrupt;
+#endif
 cc->dump_state = x86_cpu_dump_state;
 cc->get_crash_info = x86_cpu_get_crash_info;
 cc->set_pc = x86_cpu_set_pc;
@@ -4070,7 +4072,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->gdb_core_xml_file = "i386-32bit.xml";
 cc->gdb_num_core_regs = 41;
 #endif
-#ifndef CONFIG_USER_ONLY
+#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY)
 cc->debug_excp_handler = breakpoint_handler;
 #endif
 cc->cpu_exec_enter = x86_cpu_exec_enter;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 67a6091..27732ad 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -52,7 +52,9 @@
 
 #include "exec/cpu-defs.h"
 
+#ifdef CONFIG_TCG
 #include "fpu/softfloat.h"
+#endif
 
 #define R_EAX 0
 #define R_ECX 1
@@ -1598,7 +1600,11 @@ void update_fp_status(CPUX86State *env);
 
 static inline uint32_t cpu_compute_eflags(CPUX86State *env)
 {
-return env->eflags | cpu_cc_compute_all(env, CC_OP) | (env->df & DF_MASK);
+uint32_t eflags = env->eflags;
+if (tcg_enabled()) {
+eflags |= cpu_cc_compute_all(env, CC_OP) | (env->df & DF_MASK);
+}
+return eflags;
 }
 
 /* NOTE: the translator must set DisasContext.cc_op to CC_OP_EFLAGS
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 53587ae..b78b36c 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -280,8 +280,9 @@ static int cpu_post_load(void *opaque, int version_id)
 for(i = 0; i < 8; i++) {
 env->fptags[i] = (env->fptag_vmstate >> i) & 1;
 }
-