* [Qemu-devel] [PATCH 0/3] Alternative post-instruction early TB termination
@ 2008-06-04 18:47 Jan Kiszka
2008-06-04 18:48 ` [Qemu-devel] [PATCH 3/3] remove unused TB cflags Jan Kiszka
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Jan Kiszka @ 2008-06-04 18:47 UTC (permalink / raw)
To: qemu-devel
Originally I was hoping to get my whole debugger patch series on the
track, but I'm still stuck with a bug in the x86 debug register support
(weird single step race, triggered by dr-usage). So let's start smaller
with a friction of that series.
These patches introduce a new single step mode that allows the emulator
to generate and execute only a single-instruction TB, but without
triggering a debug event afterwards. This is exploited by
tb_invalidate_phys_page[_range] and later on by the watchpoint subsystem
(patch to be posted). This should also allow to remove cflags
from TranslationBlock, as done by the third patch.
Jan
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 3/3] remove unused TB cflags
2008-06-04 18:47 [Qemu-devel] [PATCH 0/3] Alternative post-instruction early TB termination Jan Kiszka
@ 2008-06-04 18:48 ` Jan Kiszka
2008-06-05 19:52 ` [Qemu-devel] [PATCH 3/3] remove unused TB cflags - v2 Jan Kiszka
2008-06-04 18:53 ` [Qemu-devel] [PATCH 1/3] Introduce SSTEP_INTERNAL Jan Kiszka
2008-06-04 18:56 ` [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL Jan Kiszka
2 siblings, 1 reply; 8+ messages in thread
From: Jan Kiszka @ 2008-06-04 18:48 UTC (permalink / raw)
To: qemu-devel
TranslationBlock.cflags is unused, now that no one is interested in
CF_SINGLE_INSN anymore. Also the other CF_* flags have no users, so
let's clean this up.
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
exec-all.h | 4 ----
exec.c | 7 ++-----
target-i386/translate.c | 6 ++----
3 files changed, 4 insertions(+), 13 deletions(-)
Index: b/exec-all.h
===================================================================
--- a/exec-all.h
+++ b/exec-all.h
@@ -123,10 +123,6 @@ typedef struct TranslationBlock {
uint64_t flags; /* flags defining in which context the code was generated */
uint16_t size; /* size of target code for this block (1 <=
size <= TARGET_PAGE_SIZE) */
- uint16_t cflags; /* compile flags */
-#define CF_TB_FP_USED 0x0002 /* fp ops are used in the TB */
-#define CF_FP_USED 0x0004 /* fp ops are used in the TB or in a chained TB */
-#define CF_SINGLE_INSN 0x0008 /* compile only a single instruction */
uint8_t *tc_ptr; /* pointer to the translated code */
/* next matching tb for physical address. */
Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -781,8 +781,7 @@ void tb_invalidate_phys_page_range(targe
current_tb = tb_find_pc(env->mem_write_pc);
}
}
- if (current_tb == tb &&
- !(current_tb->cflags & CF_SINGLE_INSN)) {
+ if (current_tb == tb) {
/* If we are modifying the current TB, we must stop
its execution. We could be more precise by checking
that the modification is after the current PC, but it
@@ -899,8 +898,7 @@ static void tb_invalidate_phys_page(targ
n = (long)tb & 3;
tb = (TranslationBlock *)((long)tb & ~3);
#ifdef TARGET_HAS_PRECISE_SMC
- if (current_tb == tb &&
- !(current_tb->cflags & CF_SINGLE_INSN)) {
+ if (current_tb == tb) {
/* If we are modifying the current TB, we must stop
its execution. We could be more precise by checking
that the modification is after the current PC, but it
@@ -1002,7 +1000,6 @@ TranslationBlock *tb_alloc(target_ulong
return NULL;
tb = &tbs[nb_tbs++];
tb->pc = pc;
- tb->cflags = 0;
return tb;
}
Index: b/target-i386/translate.c
===================================================================
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7101,7 +7101,7 @@ static inline int gen_intermediate_code_
DisasContext dc1, *dc = &dc1;
target_ulong pc_ptr;
uint16_t *gen_opc_end;
- int j, lj, cflags;
+ int j, lj;
uint64_t flags;
target_ulong pc_start;
target_ulong cs_base;
@@ -7110,7 +7110,6 @@ static inline int gen_intermediate_code_
pc_start = tb->pc;
cs_base = tb->cs_base;
flags = tb->flags;
- cflags = tb->cflags;
dc->pe = (flags >> HF_PE_SHIFT) & 1;
dc->code32 = (flags >> HF_CS32_SHIFT) & 1;
@@ -7206,8 +7205,7 @@ static inline int gen_intermediate_code_
the flag and abort the translation to give the irqs a
change to be happen */
if (dc->tf || dc->singlestep_enabled ||
- (flags & HF_INHIBIT_IRQ_MASK) ||
- (cflags & CF_SINGLE_INSN)) {
+ (flags & HF_INHIBIT_IRQ_MASK)) {
gen_jmp_im(pc_ptr - dc->cs_base);
gen_eob(dc);
break;
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 1/3] Introduce SSTEP_INTERNAL
2008-06-04 18:47 [Qemu-devel] [PATCH 0/3] Alternative post-instruction early TB termination Jan Kiszka
2008-06-04 18:48 ` [Qemu-devel] [PATCH 3/3] remove unused TB cflags Jan Kiszka
@ 2008-06-04 18:53 ` Jan Kiszka
2008-06-04 18:56 ` [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL Jan Kiszka
2 siblings, 0 replies; 8+ messages in thread
From: Jan Kiszka @ 2008-06-04 18:53 UTC (permalink / raw)
To: qemu-devel
Introducing SSTEP_INTERNAL, this patch allows to reuse the
(host-injected) single-step infrastructure to let the emulator generate
and execute TBs that only include one instruction.
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
cpu-all.h | 7 ++++---
cpu-exec.c | 4 +++-
exec.c | 2 ++
gdbstub.c | 4 ++--
target-arm/translate.c | 2 +-
target-cris/translate.c | 2 +-
target-i386/translate.c | 2 +-
target-m68k/translate.c | 4 ++--
target-mips/translate.c | 2 +-
target-ppc/translate.c | 2 +-
target-sh4/translate.c | 6 +++---
vl.c | 7 ++++---
12 files changed, 25 insertions(+), 19 deletions(-)
Index: b/cpu-all.h
===================================================================
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -804,9 +804,10 @@ int cpu_breakpoint_insert(CPUState *env,
int cpu_breakpoint_remove(CPUState *env, target_ulong pc);
void cpu_breakpoint_remove_all(CPUState *env);
-#define SSTEP_ENABLE 0x1 /* Enable simulated HW single stepping */
-#define SSTEP_NOIRQ 0x2 /* Do not use IRQ while single stepping */
-#define SSTEP_NOTIMER 0x4 /* Do not Timers while single stepping */
+#define SSTEP_DEBUG 0x1 /* Enable simulated HW single stepping */
+#define SSTEP_NOIRQ 0x2 /* Do not use IRQ while single stepping */
+#define SSTEP_NOTIMER 0x4 /* Do not Timers while single stepping */
+#define SSTEP_INTERNAL 0x8 /* QEMU internal, do not generate breakpoint */
void cpu_single_step(CPUState *env, int enabled);
void cpu_reset(CPUState *s);
Index: b/cpu-exec.c
===================================================================
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -369,7 +369,8 @@ int cpu_exec(CPUState *env1)
for(;;) {
interrupt_request = env->interrupt_request;
if (__builtin_expect(interrupt_request, 0) &&
- likely(!(env->singlestep_enabled & SSTEP_NOIRQ))) {
+ likely(!(env->singlestep_enabled &
+ (SSTEP_NOIRQ | SSTEP_INTERNAL)))) {
if (interrupt_request & CPU_INTERRUPT_DEBUG) {
env->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
env->exception_index = EXCP_DEBUG;
@@ -609,6 +610,7 @@ int cpu_exec(CPUState *env1)
#endif
next_tb = tcg_qemu_tb_exec(tc_ptr);
env->current_tb = NULL;
+ env->singlestep_enabled &= ~SSTEP_INTERNAL;
/* reset soft MMU for next block (it can currently
only be set by a memory fault) */
#if defined(USE_KQEMU)
Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -1292,6 +1292,8 @@ int cpu_breakpoint_remove(CPUState *env,
void cpu_single_step(CPUState *env, int enabled)
{
#if defined(TARGET_HAS_ICE)
+ enabled &= SSTEP_DEBUG | SSTEP_NOIRQ | SSTEP_NOTIMER;
+ enabled |= env->singlestep_enabled & SSTEP_INTERNAL;
if (env->singlestep_enabled != enabled) {
env->singlestep_enabled = enabled;
/* must flush all the translated code to avoid inconsistancies */
Index: b/target-arm/translate.c
===================================================================
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -8666,7 +8666,7 @@ static inline int gen_intermediate_code_
/* At this stage dc->condjmp will only be set when the skipped
instruction was a conditional branch or trap, and the PC has
already been written. */
- if (__builtin_expect(env->singlestep_enabled, 0)) {
+ if (__builtin_expect(env->singlestep_enabled & SSTEP_DEBUG, 0)) {
/* Make sure the pc is updated, and raise a debug exception. */
if (dc->condjmp) {
gen_set_condexec(dc);
Index: b/target-cris/translate.c
===================================================================
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3067,7 +3067,7 @@ gen_intermediate_code_internal(CPUState
cris_evaluate_flags (dc);
done:
- if (__builtin_expect(env->singlestep_enabled, 0)) {
+ if (__builtin_expect(env->singlestep_enabled & SSTEP_DEBUG, 0)) {
t_gen_raise_exception(EXCP_DEBUG);
} else {
switch(dc->is_jmp) {
Index: b/target-i386/translate.c
===================================================================
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -2618,7 +2618,7 @@ static void gen_eob(DisasContext *s)
if (s->tb->flags & HF_INHIBIT_IRQ_MASK) {
tcg_gen_helper_0_0(helper_reset_inhibit_irq);
}
- if (s->singlestep_enabled) {
+ if (s->singlestep_enabled & SSTEP_DEBUG) {
tcg_gen_helper_0_0(helper_debug);
} else if (s->tf) {
tcg_gen_helper_0_0(helper_single_step);
Index: b/target-m68k/translate.c
===================================================================
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -871,7 +871,7 @@ static void gen_jmp_tb(DisasContext *s,
TranslationBlock *tb;
tb = s->tb;
- if (__builtin_expect (s->singlestep_enabled, 0)) {
+ if (__builtin_expect (s->singlestep_enabled & SSTEP_DEBUG, 0)) {
gen_exception(s, dest, EXCP_DEBUG);
} else if ((tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK) ||
(s->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK)) {
@@ -2974,7 +2974,7 @@ gen_intermediate_code_internal(CPUState
!env->singlestep_enabled &&
(pc_offset) < (TARGET_PAGE_SIZE - 32));
- if (__builtin_expect(env->singlestep_enabled, 0)) {
+ if (__builtin_expect(env->singlestep_enabled & SSTEP_DEBUG, 0)) {
/* Make sure the pc is updated, and raise a debug exception. */
if (!dc->is_jmp) {
gen_flush_cc_op(dc);
Index: b/target-mips/translate.c
===================================================================
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -7259,7 +7259,7 @@ gen_intermediate_code_internal (CPUState
break;
#endif
}
- if (env->singlestep_enabled) {
+ if (env->singlestep_enabled & SSTEP_DEBUG) {
save_cpu_state(&ctx, ctx.bstate == BS_NONE);
gen_op_debug();
} else {
Index: b/target-ppc/translate.c
===================================================================
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -6319,7 +6319,7 @@ static always_inline int gen_intermediat
if (ctx.exception == POWERPC_EXCP_NONE) {
gen_goto_tb(&ctx, 0, ctx.nip);
} else if (ctx.exception != POWERPC_EXCP_BRANCH) {
- if (unlikely(env->singlestep_enabled)) {
+ if (unlikely(env->singlestep_enabled & SSTEP_DEBUG)) {
gen_update_nip(&ctx, ctx.nip);
gen_op_debug();
}
Index: b/target-sh4/translate.c
===================================================================
--- a/target-sh4/translate.c
+++ b/target-sh4/translate.c
@@ -161,7 +161,7 @@ static void gen_goto_tb(DisasContext * c
tcg_gen_exit_tb((long) tb + n);
} else {
gen_op_movl_imm_PC(dest);
- if (ctx->singlestep_enabled)
+ if (ctx->singlestep_enabled & SSTEP_DEBUG)
gen_op_debug();
tcg_gen_exit_tb(0);
}
@@ -173,7 +173,7 @@ static void gen_jump(DisasContext * ctx)
/* Target is not statically known, it comes necessarily from a
delayed jump as immediate jump are conditinal jumps */
gen_op_movl_delayed_pc_PC();
- if (ctx->singlestep_enabled)
+ if (ctx->singlestep_enabled & SSTEP_DEBUG)
gen_op_debug();
tcg_gen_exit_tb(0);
} else {
@@ -1251,7 +1251,7 @@ gen_intermediate_code_internal(CPUState
break;
#endif
}
- if (env->singlestep_enabled) {
+ if (env->singlestep_enabled & SSTEP_DEBUG) {
gen_op_debug();
} else {
switch (ctx.bstate) {
Index: b/vl.c
===================================================================
--- a/vl.c
+++ b/vl.c
@@ -7032,9 +7032,10 @@ void main_loop_wait(int timeout)
qemu_aio_poll();
if (vm_running) {
- if (likely(!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER)))
- qemu_run_timers(&active_timers[QEMU_TIMER_VIRTUAL],
- qemu_get_clock(vm_clock));
+ if (likely(!(cur_cpu->singlestep_enabled &
+ (SSTEP_NOTIMER | SSTEP_INTERNAL))))
+ qemu_run_timers(&active_timers[QEMU_TIMER_VIRTUAL],
+ qemu_get_clock(vm_clock));
/* run dma transfers, if any */
DMA_run();
}
Index: b/gdbstub.c
===================================================================
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -77,7 +77,7 @@ typedef struct GDBState {
/* By default use no IRQs and no timers while single stepping so as to
* make single stepping like an ICE HW step.
*/
-static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
+static int sstep_flags = SSTEP_DEBUG | SSTEP_NOIRQ | SSTEP_NOTIMER;
#ifdef CONFIG_USER_ONLY
/* XXX: This is not thread safe. Do we care? */
@@ -1144,7 +1144,7 @@ static int gdb_handle_packet(GDBState *s
if (!strcmp(p,"qemu.sstepbits")) {
/* Query Breakpoint bit definitions */
sprintf(buf,"ENABLE=%x,NOIRQ=%x,NOTIMER=%x",
- SSTEP_ENABLE,
+ SSTEP_DEBUG,
SSTEP_NOIRQ,
SSTEP_NOTIMER);
put_packet(s, buf);
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL
2008-06-04 18:47 [Qemu-devel] [PATCH 0/3] Alternative post-instruction early TB termination Jan Kiszka
2008-06-04 18:48 ` [Qemu-devel] [PATCH 3/3] remove unused TB cflags Jan Kiszka
2008-06-04 18:53 ` [Qemu-devel] [PATCH 1/3] Introduce SSTEP_INTERNAL Jan Kiszka
@ 2008-06-04 18:56 ` Jan Kiszka
2008-06-04 21:43 ` [Qemu-devel] Re: [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2 Jan Kiszka
2008-06-05 8:36 ` [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL Fabrice Bellard
2 siblings, 2 replies; 8+ messages in thread
From: Jan Kiszka @ 2008-06-04 18:56 UTC (permalink / raw)
To: qemu-devel
With the help of SSTEP_INTERNAL, we can overcome CF_SINGLE_INSN and,
thus, tb_gen_code.
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
exec.c | 43 ++-----------------------------------------
1 file changed, 2 insertions(+), 41 deletions(-)
Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -723,43 +723,6 @@ static void build_page_bitmap(PageDesc *
}
}
-#ifdef TARGET_HAS_PRECISE_SMC
-
-static void tb_gen_code(CPUState *env,
- target_ulong pc, target_ulong cs_base, int flags,
- int cflags)
-{
- TranslationBlock *tb;
- uint8_t *tc_ptr;
- target_ulong phys_pc, phys_page2, virt_page2;
- int code_gen_size;
-
- phys_pc = get_phys_addr_code(env, pc);
- tb = tb_alloc(pc);
- if (!tb) {
- /* flush must be done */
- tb_flush(env);
- /* cannot fail at this point */
- tb = tb_alloc(pc);
- }
- tc_ptr = code_gen_ptr;
- tb->tc_ptr = tc_ptr;
- tb->cs_base = cs_base;
- tb->flags = flags;
- tb->cflags = cflags;
- cpu_gen_code(env, tb, &code_gen_size);
- code_gen_ptr = (void *)(((unsigned long)code_gen_ptr + code_gen_size + CODE_GEN_ALIGN - 1) & ~(CODE_GEN_ALIGN - 1));
-
- /* check next page if needed */
- virt_page2 = (pc + tb->size - 1) & TARGET_PAGE_MASK;
- phys_page2 = -1;
- if ((pc & TARGET_PAGE_MASK) != virt_page2) {
- phys_page2 = get_phys_addr_code(env, virt_page2);
- }
- tb_link_phys(tb, phys_pc, phys_page2);
-}
-#endif
-
/* invalidate all TBs which intersect with the target physical page
starting in range [start;end[. NOTE: start and end must refer to
the same physical page. 'is_cpu_write_access' should be true if called
@@ -870,8 +833,7 @@ void tb_invalidate_phys_page_range(targe
modifying the memory. It will ensure that it cannot modify
itself */
env->current_tb = NULL;
- tb_gen_code(env, current_pc, current_cs_base, current_flags,
- CF_SINGLE_INSN);
+ env->singlestep_enabled |= SSTEP_INTERNAL;
cpu_resume_from_signal(env, NULL);
}
#endif
@@ -967,8 +929,7 @@ static void tb_invalidate_phys_page(targ
modifying the memory. It will ensure that it cannot modify
itself */
env->current_tb = NULL;
- tb_gen_code(env, current_pc, current_cs_base, current_flags,
- CF_SINGLE_INSN);
+ env->singlestep_enabled |= SSTEP_INTERNAL;
cpu_resume_from_signal(env, puc);
}
#endif
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2
2008-06-04 18:56 ` [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL Jan Kiszka
@ 2008-06-04 21:43 ` Jan Kiszka
2008-06-05 8:36 ` [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL Fabrice Bellard
1 sibling, 0 replies; 8+ messages in thread
From: Jan Kiszka @ 2008-06-04 21:43 UTC (permalink / raw)
To: qemu-devel
[ Missed even more cleanup possibilities. ]
With the help of SSTEP_INTERNAL, we can overcome CF_SINGLE_INSN and,
thus, tb_gen_code with its setup code.
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
exec.c | 71 +++--------------------------------------------------------------
1 file changed, 4 insertions(+), 67 deletions(-)
Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -723,43 +723,6 @@ static void build_page_bitmap(PageDesc *
}
}
-#ifdef TARGET_HAS_PRECISE_SMC
-
-static void tb_gen_code(CPUState *env,
- target_ulong pc, target_ulong cs_base, int flags,
- int cflags)
-{
- TranslationBlock *tb;
- uint8_t *tc_ptr;
- target_ulong phys_pc, phys_page2, virt_page2;
- int code_gen_size;
-
- phys_pc = get_phys_addr_code(env, pc);
- tb = tb_alloc(pc);
- if (!tb) {
- /* flush must be done */
- tb_flush(env);
- /* cannot fail at this point */
- tb = tb_alloc(pc);
- }
- tc_ptr = code_gen_ptr;
- tb->tc_ptr = tc_ptr;
- tb->cs_base = cs_base;
- tb->flags = flags;
- tb->cflags = cflags;
- cpu_gen_code(env, tb, &code_gen_size);
- code_gen_ptr = (void *)(((unsigned long)code_gen_ptr + code_gen_size + CODE_GEN_ALIGN - 1) & ~(CODE_GEN_ALIGN - 1));
-
- /* check next page if needed */
- virt_page2 = (pc + tb->size - 1) & TARGET_PAGE_MASK;
- phys_page2 = -1;
- if ((pc & TARGET_PAGE_MASK) != virt_page2) {
- phys_page2 = get_phys_addr_code(env, virt_page2);
- }
- tb_link_phys(tb, phys_pc, phys_page2);
-}
-#endif
-
/* invalidate all TBs which intersect with the target physical page
starting in range [start;end[. NOTE: start and end must refer to
the same physical page. 'is_cpu_write_access' should be true if called
@@ -768,12 +731,11 @@ static void tb_gen_code(CPUState *env,
void tb_invalidate_phys_page_range(target_phys_addr_t start, target_phys_addr_t end,
int is_cpu_write_access)
{
- int n, current_tb_modified, current_tb_not_found, current_flags;
+ int n, current_tb_modified, current_tb_not_found;
CPUState *env = cpu_single_env;
PageDesc *p;
TranslationBlock *tb, *tb_next, *current_tb, *saved_tb;
target_ulong tb_start, tb_end;
- target_ulong current_pc, current_cs_base;
p = page_find(start >> TARGET_PAGE_BITS);
if (!p)
@@ -790,9 +752,6 @@ void tb_invalidate_phys_page_range(targe
current_tb_not_found = is_cpu_write_access;
current_tb_modified = 0;
current_tb = NULL; /* avoid warning */
- current_pc = 0; /* avoid warning */
- current_cs_base = 0; /* avoid warning */
- current_flags = 0; /* avoid warning */
tb = p->first_tb;
while (tb != NULL) {
n = (long)tb & 3;
@@ -829,14 +788,6 @@ void tb_invalidate_phys_page_range(targe
current_tb_modified = 1;
cpu_restore_state(current_tb, env,
env->mem_write_pc, NULL);
-#if defined(TARGET_I386)
- current_flags = env->hflags;
- current_flags |= (env->eflags & (IOPL_MASK | TF_MASK | VM_MASK));
- current_cs_base = (target_ulong)env->segs[R_CS].base;
- current_pc = current_cs_base + env->eip;
-#else
-#error unsupported CPU
-#endif
}
#endif /* TARGET_HAS_PRECISE_SMC */
/* we need to do that to handle the case where a signal
@@ -870,8 +821,7 @@ void tb_invalidate_phys_page_range(targe
modifying the memory. It will ensure that it cannot modify
itself */
env->current_tb = NULL;
- tb_gen_code(env, current_pc, current_cs_base, current_flags,
- CF_SINGLE_INSN);
+ env->singlestep_enabled |= SSTEP_INTERNAL;
cpu_resume_from_signal(env, NULL);
}
#endif
@@ -910,8 +860,7 @@ static inline void tb_invalidate_phys_pa
static void tb_invalidate_phys_page(target_phys_addr_t addr,
unsigned long pc, void *puc)
{
- int n, current_flags, current_tb_modified;
- target_ulong current_pc, current_cs_base;
+ int n, current_tb_modified;
PageDesc *p;
TranslationBlock *tb, *current_tb;
#ifdef TARGET_HAS_PRECISE_SMC
@@ -925,9 +874,6 @@ static void tb_invalidate_phys_page(targ
tb = p->first_tb;
current_tb_modified = 0;
current_tb = NULL;
- current_pc = 0; /* avoid warning */
- current_cs_base = 0; /* avoid warning */
- current_flags = 0; /* avoid warning */
#ifdef TARGET_HAS_PRECISE_SMC
if (tb && pc != 0) {
current_tb = tb_find_pc(pc);
@@ -947,14 +893,6 @@ static void tb_invalidate_phys_page(targ
current_tb_modified = 1;
cpu_restore_state(current_tb, env, pc, puc);
-#if defined(TARGET_I386)
- current_flags = env->hflags;
- current_flags |= (env->eflags & (IOPL_MASK | TF_MASK | VM_MASK));
- current_cs_base = (target_ulong)env->segs[R_CS].base;
- current_pc = current_cs_base + env->eip;
-#else
-#error unsupported CPU
-#endif
}
#endif /* TARGET_HAS_PRECISE_SMC */
tb_phys_invalidate(tb, addr);
@@ -967,8 +905,7 @@ static void tb_invalidate_phys_page(targ
modifying the memory. It will ensure that it cannot modify
itself */
env->current_tb = NULL;
- tb_gen_code(env, current_pc, current_cs_base, current_flags,
- CF_SINGLE_INSN);
+ env->singlestep_enabled |= SSTEP_INTERNAL;
cpu_resume_from_signal(env, puc);
}
#endif
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL
2008-06-04 18:56 ` [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL Jan Kiszka
2008-06-04 21:43 ` [Qemu-devel] Re: [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2 Jan Kiszka
@ 2008-06-05 8:36 ` Fabrice Bellard
2008-06-05 19:52 ` [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2 Jan Kiszka
1 sibling, 1 reply; 8+ messages in thread
From: Fabrice Bellard @ 2008-06-05 8:36 UTC (permalink / raw)
To: qemu-devel
Jan Kiszka wrote:
> With the help of SSTEP_INTERNAL, we can overcome CF_SINGLE_INSN and,
> thus, tb_gen_code.
Does your code avoid looping if an instruction modifies itself ?
Fabrice.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2
2008-06-05 8:36 ` [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL Fabrice Bellard
@ 2008-06-05 19:52 ` Jan Kiszka
0 siblings, 0 replies; 8+ messages in thread
From: Jan Kiszka @ 2008-06-05 19:52 UTC (permalink / raw)
To: qemu-devel
Fabrice Bellard wrote:
> Jan Kiszka wrote:
>> With the help of SSTEP_INTERNAL, we can overcome CF_SINGLE_INSN and,
>> thus, tb_gen_code.
>
> Does your code avoid looping if an instruction modifies itself ?
Err, well, of course not. Not completely understanding what I patched, I
missed the meaning of those two CF_SINGLE_INSN checks. Version below
gets this right. Successfully tested with qemu-x86_64 system&userspace.
An updated patch 3 will follow, too. Thanks!
------------
With the help of SSTEP_INTERNAL, we can overcome CF_SINGLE_INSN and,
thus, tb_gen_code with its setup code.
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
exec.c | 75 +++++------------------------------------------------------------
1 file changed, 6 insertions(+), 69 deletions(-)
Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -723,43 +723,6 @@ static void build_page_bitmap(PageDesc *
}
}
-#ifdef TARGET_HAS_PRECISE_SMC
-
-static void tb_gen_code(CPUState *env,
- target_ulong pc, target_ulong cs_base, int flags,
- int cflags)
-{
- TranslationBlock *tb;
- uint8_t *tc_ptr;
- target_ulong phys_pc, phys_page2, virt_page2;
- int code_gen_size;
-
- phys_pc = get_phys_addr_code(env, pc);
- tb = tb_alloc(pc);
- if (!tb) {
- /* flush must be done */
- tb_flush(env);
- /* cannot fail at this point */
- tb = tb_alloc(pc);
- }
- tc_ptr = code_gen_ptr;
- tb->tc_ptr = tc_ptr;
- tb->cs_base = cs_base;
- tb->flags = flags;
- tb->cflags = cflags;
- cpu_gen_code(env, tb, &code_gen_size);
- code_gen_ptr = (void *)(((unsigned long)code_gen_ptr + code_gen_size + CODE_GEN_ALIGN - 1) & ~(CODE_GEN_ALIGN - 1));
-
- /* check next page if needed */
- virt_page2 = (pc + tb->size - 1) & TARGET_PAGE_MASK;
- phys_page2 = -1;
- if ((pc & TARGET_PAGE_MASK) != virt_page2) {
- phys_page2 = get_phys_addr_code(env, virt_page2);
- }
- tb_link_phys(tb, phys_pc, phys_page2);
-}
-#endif
-
/* invalidate all TBs which intersect with the target physical page
starting in range [start;end[. NOTE: start and end must refer to
the same physical page. 'is_cpu_write_access' should be true if called
@@ -768,12 +731,11 @@ static void tb_gen_code(CPUState *env,
void tb_invalidate_phys_page_range(target_phys_addr_t start, target_phys_addr_t end,
int is_cpu_write_access)
{
- int n, current_tb_modified, current_tb_not_found, current_flags;
+ int n, current_tb_modified, current_tb_not_found;
CPUState *env = cpu_single_env;
PageDesc *p;
TranslationBlock *tb, *tb_next, *current_tb, *saved_tb;
target_ulong tb_start, tb_end;
- target_ulong current_pc, current_cs_base;
p = page_find(start >> TARGET_PAGE_BITS);
if (!p)
@@ -790,9 +752,6 @@ void tb_invalidate_phys_page_range(targe
current_tb_not_found = is_cpu_write_access;
current_tb_modified = 0;
current_tb = NULL; /* avoid warning */
- current_pc = 0; /* avoid warning */
- current_cs_base = 0; /* avoid warning */
- current_flags = 0; /* avoid warning */
tb = p->first_tb;
while (tb != NULL) {
n = (long)tb & 3;
@@ -819,7 +778,7 @@ void tb_invalidate_phys_page_range(targe
}
}
if (current_tb == tb &&
- !(current_tb->cflags & CF_SINGLE_INSN)) {
+ !(env->singlestep_enabled & SSTEP_INTERNAL)) {
/* If we are modifying the current TB, we must stop
its execution. We could be more precise by checking
that the modification is after the current PC, but it
@@ -829,14 +788,6 @@ void tb_invalidate_phys_page_range(targe
current_tb_modified = 1;
cpu_restore_state(current_tb, env,
env->mem_write_pc, NULL);
-#if defined(TARGET_I386)
- current_flags = env->hflags;
- current_flags |= (env->eflags & (IOPL_MASK | TF_MASK | VM_MASK));
- current_cs_base = (target_ulong)env->segs[R_CS].base;
- current_pc = current_cs_base + env->eip;
-#else
-#error unsupported CPU
-#endif
}
#endif /* TARGET_HAS_PRECISE_SMC */
/* we need to do that to handle the case where a signal
@@ -870,8 +821,7 @@ void tb_invalidate_phys_page_range(targe
modifying the memory. It will ensure that it cannot modify
itself */
env->current_tb = NULL;
- tb_gen_code(env, current_pc, current_cs_base, current_flags,
- CF_SINGLE_INSN);
+ env->singlestep_enabled |= SSTEP_INTERNAL;
cpu_resume_from_signal(env, NULL);
}
#endif
@@ -910,8 +860,7 @@ static inline void tb_invalidate_phys_pa
static void tb_invalidate_phys_page(target_phys_addr_t addr,
unsigned long pc, void *puc)
{
- int n, current_flags, current_tb_modified;
- target_ulong current_pc, current_cs_base;
+ int n, current_tb_modified;
PageDesc *p;
TranslationBlock *tb, *current_tb;
#ifdef TARGET_HAS_PRECISE_SMC
@@ -925,9 +874,6 @@ static void tb_invalidate_phys_page(targ
tb = p->first_tb;
current_tb_modified = 0;
current_tb = NULL;
- current_pc = 0; /* avoid warning */
- current_cs_base = 0; /* avoid warning */
- current_flags = 0; /* avoid warning */
#ifdef TARGET_HAS_PRECISE_SMC
if (tb && pc != 0) {
current_tb = tb_find_pc(pc);
@@ -938,7 +884,7 @@ static void tb_invalidate_phys_page(targ
tb = (TranslationBlock *)((long)tb & ~3);
#ifdef TARGET_HAS_PRECISE_SMC
if (current_tb == tb &&
- !(current_tb->cflags & CF_SINGLE_INSN)) {
+ !(env->singlestep_enabled & SSTEP_INTERNAL)) {
/* If we are modifying the current TB, we must stop
its execution. We could be more precise by checking
that the modification is after the current PC, but it
@@ -947,14 +893,6 @@ static void tb_invalidate_phys_page(targ
current_tb_modified = 1;
cpu_restore_state(current_tb, env, pc, puc);
-#if defined(TARGET_I386)
- current_flags = env->hflags;
- current_flags |= (env->eflags & (IOPL_MASK | TF_MASK | VM_MASK));
- current_cs_base = (target_ulong)env->segs[R_CS].base;
- current_pc = current_cs_base + env->eip;
-#else
-#error unsupported CPU
-#endif
}
#endif /* TARGET_HAS_PRECISE_SMC */
tb_phys_invalidate(tb, addr);
@@ -967,8 +905,7 @@ static void tb_invalidate_phys_page(targ
modifying the memory. It will ensure that it cannot modify
itself */
env->current_tb = NULL;
- tb_gen_code(env, current_pc, current_cs_base, current_flags,
- CF_SINGLE_INSN);
+ env->singlestep_enabled |= SSTEP_INTERNAL;
cpu_resume_from_signal(env, puc);
}
#endif
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 3/3] remove unused TB cflags - v2
2008-06-04 18:48 ` [Qemu-devel] [PATCH 3/3] remove unused TB cflags Jan Kiszka
@ 2008-06-05 19:52 ` Jan Kiszka
0 siblings, 0 replies; 8+ messages in thread
From: Jan Kiszka @ 2008-06-05 19:52 UTC (permalink / raw)
To: qemu-devel
[ updated after fixing patch 2 ]
TranslationBlock.cflags is unused, now that no one is interested in
CF_SINGLE_INSN anymore. Also the other CF_* flags have no users, so
let's clean this up.
Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
exec-all.h | 4 ----
exec.c | 1 -
target-i386/translate.c | 6 ++----
3 files changed, 2 insertions(+), 9 deletions(-)
Index: b/exec-all.h
===================================================================
--- a/exec-all.h
+++ b/exec-all.h
@@ -123,10 +123,6 @@ typedef struct TranslationBlock {
uint64_t flags; /* flags defining in which context the code was generated */
uint16_t size; /* size of target code for this block (1 <=
size <= TARGET_PAGE_SIZE) */
- uint16_t cflags; /* compile flags */
-#define CF_TB_FP_USED 0x0002 /* fp ops are used in the TB */
-#define CF_FP_USED 0x0004 /* fp ops are used in the TB or in a chained TB */
-#define CF_SINGLE_INSN 0x0008 /* compile only a single instruction */
uint8_t *tc_ptr; /* pointer to the translated code */
/* next matching tb for physical address. */
Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -978,7 +978,6 @@ TranslationBlock *tb_alloc(target_ulong
return NULL;
tb = &tbs[nb_tbs++];
tb->pc = pc;
- tb->cflags = 0;
return tb;
}
Index: b/target-i386/translate.c
===================================================================
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7101,7 +7101,7 @@ static inline int gen_intermediate_code_
DisasContext dc1, *dc = &dc1;
target_ulong pc_ptr;
uint16_t *gen_opc_end;
- int j, lj, cflags;
+ int j, lj;
uint64_t flags;
target_ulong pc_start;
target_ulong cs_base;
@@ -7110,7 +7110,6 @@ static inline int gen_intermediate_code_
pc_start = tb->pc;
cs_base = tb->cs_base;
flags = tb->flags;
- cflags = tb->cflags;
dc->pe = (flags >> HF_PE_SHIFT) & 1;
dc->code32 = (flags >> HF_CS32_SHIFT) & 1;
@@ -7206,8 +7205,7 @@ static inline int gen_intermediate_code_
the flag and abort the translation to give the irqs a
change to be happen */
if (dc->tf || dc->singlestep_enabled ||
- (flags & HF_INHIBIT_IRQ_MASK) ||
- (cflags & CF_SINGLE_INSN)) {
+ (flags & HF_INHIBIT_IRQ_MASK)) {
gen_jmp_im(pc_ptr - dc->cs_base);
gen_eob(dc);
break;
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-06-05 19:53 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-04 18:47 [Qemu-devel] [PATCH 0/3] Alternative post-instruction early TB termination Jan Kiszka
2008-06-04 18:48 ` [Qemu-devel] [PATCH 3/3] remove unused TB cflags Jan Kiszka
2008-06-05 19:52 ` [Qemu-devel] [PATCH 3/3] remove unused TB cflags - v2 Jan Kiszka
2008-06-04 18:53 ` [Qemu-devel] [PATCH 1/3] Introduce SSTEP_INTERNAL Jan Kiszka
2008-06-04 18:56 ` [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL Jan Kiszka
2008-06-04 21:43 ` [Qemu-devel] Re: [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2 Jan Kiszka
2008-06-05 8:36 ` [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL Fabrice Bellard
2008-06-05 19:52 ` [Qemu-devel] [PATCH 2/3] Replace CF_SINGLE_INSN with SSTEP_INTERNAL - v2 Jan Kiszka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).