* [Qemu-devel] [PATCH 0/6] simplify cpu-exec new spin @ 2008-05-27 15:18 Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 1/6] simplify cpu_exec Glauber Costa 0 siblings, 1 reply; 22+ messages in thread From: Glauber Costa @ 2008-05-27 15:18 UTC (permalink / raw) To: qemu-devel; +Cc: kvm Hi, This is an update on the previous series I sent that aims at simplifying cpu_exec. It fixes some bugs I've found myself, merge some comments from anthony, and goes a little bit further in enclosing functions. Comments welcome ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 1/6] simplify cpu_exec 2008-05-27 15:18 [Qemu-devel] [PATCH 0/6] simplify cpu-exec new spin Glauber Costa @ 2008-05-27 15:18 ` Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 2/6] Push common interrupt variables to cpu-defs.h Glauber Costa ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: Glauber Costa @ 2008-05-27 15:18 UTC (permalink / raw) To: qemu-devel; +Cc: kvm This is a first attempt to simplify cpu_exec(): it has simply too many ifdefs, which is not a very good practice at all. Following some work I've already posted in the past, I'm moving the target-b ifdefs to target-xxx/helper.c, encapsuled into relevant functions. Signed-off-by: Glauber Costa <gcosta@redhat.com> --- cpu-exec.c | 54 +----------------------------------------------- exec-all.h | 4 +++ target-alpha/helper.c | 3 ++ target-arm/helper.c | 4 +++ target-cris/helper.c | 3 ++ target-i386/helper.c | 14 ++++++++++++ target-m68k/helper.c | 16 ++++++++++++++ target-mips/helper.c | 3 ++ target-ppc/helper.c | 3 ++ target-sh4/helper.c | 3 ++ target-sparc/helper.c | 17 +++++++++++++++ 11 files changed, 72 insertions(+), 52 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index 65af9a6..a829b7b 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -247,11 +247,6 @@ int cpu_exec(CPUState *env1) { #define DECLARE_HOST_REGS 1 #include "hostregs_helper.h" -#if defined(TARGET_SPARC) -#if defined(reg_REGWPTR) - uint32_t *saved_regwptr; -#endif -#endif int ret, interrupt_request; TranslationBlock *tb; uint8_t *tc_ptr; @@ -267,30 +262,8 @@ int cpu_exec(CPUState *env1) env = env1; env_to_regs(); -#if defined(TARGET_I386) - /* put eflags in CPU temporary format */ - CC_SRC = env->eflags & (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C); - DF = 1 - (2 * ((env->eflags >> 10) & 1)); - CC_OP = CC_OP_EFLAGS; - env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C); -#elif defined(TARGET_SPARC) -#if defined(reg_REGWPTR) - saved_regwptr = REGWPTR; -#endif -#elif defined(TARGET_M68K) - env->cc_op = CC_OP_FLAGS; - env->cc_dest = env->sr & 0xf; - env->cc_x = (env->sr >> 4) & 1; -#elif defined(TARGET_ALPHA) -#elif defined(TARGET_ARM) -#elif defined(TARGET_PPC) -#elif defined(TARGET_MIPS) -#elif defined(TARGET_SH4) -#elif defined(TARGET_CRIS) + cpu_load_flags(env); /* XXXXX */ -#else -#error unsupported target CPU -#endif env->exception_index = -1; /* prepare setjmp context for exception handling */ @@ -640,30 +613,7 @@ int cpu_exec(CPUState *env1) } } /* for(;;) */ - -#if defined(TARGET_I386) - /* restore flags in standard format */ - env->eflags = env->eflags | cc_table[CC_OP].compute_all() | (DF & DF_MASK); -#elif defined(TARGET_ARM) - /* XXX: Save/restore host fpu exception state?. */ -#elif defined(TARGET_SPARC) -#if defined(reg_REGWPTR) - REGWPTR = saved_regwptr; -#endif -#elif defined(TARGET_PPC) -#elif defined(TARGET_M68K) - cpu_m68k_flush_flags(env, env->cc_op); - env->cc_op = CC_OP_FLAGS; - env->sr = (env->sr & 0xffe0) - | env->cc_dest | (env->cc_x << 4); -#elif defined(TARGET_MIPS) -#elif defined(TARGET_SH4) -#elif defined(TARGET_ALPHA) -#elif defined(TARGET_CRIS) - /* XXXXX */ -#else -#error unsupported target CPU -#endif + cpu_put_flags(env); /* restore global registers */ #include "hostregs_helper.h" diff --git a/exec-all.h b/exec-all.h index 51b27b5..fd2164f 100644 --- a/exec-all.h +++ b/exec-all.h @@ -82,6 +82,10 @@ int cpu_restore_state_copy(struct TranslationBlock *tb, void *puc); void cpu_resume_from_signal(CPUState *env1, void *puc); void cpu_exec_init(CPUState *env); + +void cpu_load_flags(CPUState *env); +void cpu_put_flags(CPUState *env); + int page_unprotect(target_ulong address, unsigned long pc, void *puc); void tb_invalidate_phys_page_range(target_phys_addr_t start, target_phys_addr_t end, int is_cpu_write_access); diff --git a/target-alpha/helper.c b/target-alpha/helper.c index fd39f5f..73d565f 100644 --- a/target-alpha/helper.c +++ b/target-alpha/helper.c @@ -452,3 +452,6 @@ void cpu_dump_EA (target_ulong EA) f = stdout; fprintf(f, "Memory access at address " TARGET_FMT_lx "\n", EA); } + +void cpu_load_flags(CPUState *env) {} +void cpu_put_flags(CPUState *env) {} diff --git a/target-arm/helper.c b/target-arm/helper.c index 8e85435..496f326 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2522,3 +2522,7 @@ uint32_t HELPER(rsqrte_u32)(uint32_t a, CPUState *env) tmp = float32_scalbn(tmp, 31, s); return float32_to_int32(tmp, s); } + +void cpu_load_flags(CPUState *env) { } +/* XXX: Save/restore host fpu exception state?. */ +void cpu_put_flags(CPUState *env) { } diff --git a/target-cris/helper.c b/target-cris/helper.c index 5548b76..1e216aa 100644 --- a/target-cris/helper.c +++ b/target-cris/helper.c @@ -183,3 +183,6 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUState * env, target_ulong addr) return phy; } #endif + +void cpu_load_flags(CPUState *env) {} +void cpu_put_flags(CPUState *env) {} diff --git a/target-i386/helper.c b/target-i386/helper.c index 5322491..1d88a9e 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -4668,6 +4668,20 @@ void helper_clgi(void) { env->hflags &= ~HF_GIF_MASK; } +void cpu_load_flags(CPUState *env) +{ + /* put eflags in CPU temporary format */ + CC_SRC = env->eflags & (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C); + DF = 1 - (2 * ((env->eflags >> 10) & 1)); + CC_OP = CC_OP_EFLAGS; + env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C); +} + +void cpu_put_flags(CPUState *env) +{ + /* restore flags in standard format */ + env->eflags = env->eflags | cc_table[CC_OP].compute_all() | (DF & DF_MASK); +} #if defined(CONFIG_USER_ONLY) diff --git a/target-m68k/helper.c b/target-m68k/helper.c index 848c589..73e93bc 100644 --- a/target-m68k/helper.c +++ b/target-m68k/helper.c @@ -339,6 +339,21 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUState *env, target_ulong addr) return addr; } +void cpu_load_flags(CPUState *env) +{ + env->cc_op = CC_OP_FLAGS; + env->cc_dest = env->sr & 0xf; + env->cc_x = (env->sr >> 4) & 1; +} + +void cpu_put_flags(CPUState *env) +{ + cpu_m68k_flush_flags(env, env->cc_op); + env->cc_op = CC_OP_FLAGS; + env->sr = (env->sr & 0xffe0) + | env->cc_dest | (env->cc_x << 4); +} + #if defined(CONFIG_USER_ONLY) int cpu_m68k_handle_mmu_fault (CPUState *env, target_ulong address, int rw, @@ -376,3 +391,4 @@ void m68k_set_irq_level(CPUM68KState *env, int level, uint8_t vector) } #endif + diff --git a/target-mips/helper.c b/target-mips/helper.c index b962295..90e6dbe 100644 --- a/target-mips/helper.c +++ b/target-mips/helper.c @@ -636,3 +636,6 @@ void r4k_invalidate_tlb (CPUState *env, int idx, int use_extra) } } } + +void cpu_put_flags(CPUState *env) { } +void cpu_load_flags(CPUState *env) { } diff --git a/target-ppc/helper.c b/target-ppc/helper.c index 2a52dc6..300532f 100644 --- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -2988,3 +2988,6 @@ void cpu_ppc_close (CPUPPCState *env) /* Should also remove all opcode tables... */ qemu_free(env); } + +void cpu_load_flags(CPUState *env) { } +void cpu_put_flags(CPUState *env) { } diff --git a/target-sh4/helper.c b/target-sh4/helper.c index 917f02f..ebc16ac 100644 --- a/target-sh4/helper.c +++ b/target-sh4/helper.c @@ -534,3 +534,6 @@ void cpu_load_tlb(CPUState * env) } #endif + +void cpu_load_flags(CPUState *env) {} +void cpu_put_flags(CPUState *env) {} diff --git a/target-sparc/helper.c b/target-sparc/helper.c index 1bc2fed..76c68fa 100644 --- a/target-sparc/helper.c +++ b/target-sparc/helper.c @@ -1316,6 +1316,23 @@ void cpu_dump_state(CPUState *env, FILE *f, cpu_fprintf(f, "fsr: 0x%08x\n", GET_FSR32(env)); } +#if defined(reg_REGWPTR) +static uint32_t *saved_regwptr; +#endif + +void cpu_load_flags(CPUState *env) +{ +#if defined(reg_REGWPTR) + saved_regwptr = REGWPTR; +#endif +} + +void cpu_put_flags(CPUState *env) +{ +#if defined(reg_REGWPTR) + REGWPTR = saved_regwptr; +#endif +} #ifdef TARGET_SPARC64 #if !defined(CONFIG_USER_ONLY) #include "qemu-common.h" -- 1.5.4.5 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 2/6] Push common interrupt variables to cpu-defs.h 2008-05-27 15:18 ` [Qemu-devel] [PATCH 1/6] simplify cpu_exec Glauber Costa @ 2008-05-27 15:18 ` Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too Glauber Costa 2008-05-27 15:39 ` [Qemu-devel] [PATCH 2/6] Push common interrupt variables to cpu-defs.h Fabrice Bellard 2008-05-27 15:33 ` [Qemu-devel] [PATCH 1/6] simplify cpu_exec Paul Brook 2008-05-27 15:37 ` Fabrice Bellard 2 siblings, 2 replies; 22+ messages in thread From: Glauber Costa @ 2008-05-27 15:18 UTC (permalink / raw) To: qemu-devel; +Cc: kvm Some interrupt-related attributes, which includes the jmp_buf, are present in all, or almost all, architectures. So move them to common code in cpu-defs.h instead of replicating them everywhere Signed-off-by: Glauber Costa <gcosta@redhat.com> --- cpu-defs.h | 5 +++++ target-alpha/cpu.h | 3 --- target-arm/cpu.h | 2 -- target-cris/cpu.h | 2 -- target-i386/cpu.h | 3 --- target-m68k/cpu.h | 2 -- target-mips/cpu.h | 3 --- target-ppc/cpu.h | 3 --- target-sh4/cpu.h | 2 -- target-sparc/cpu.h | 2 -- 10 files changed, 5 insertions(+), 22 deletions(-) diff --git a/cpu-defs.h b/cpu-defs.h index c4389ed..bb00c80 100644 --- a/cpu-defs.h +++ b/cpu-defs.h @@ -159,6 +159,11 @@ typedef struct CPUTLBEntry { int nb_watchpoints; \ int watchpoint_hit; \ \ + /* Core interrupt code */ \ + jmp_buf jmp_env; \ + int exception_index; \ + int error_code; \ + \ void *next_cpu; /* next CPU sharing TB cache */ \ int cpu_index; /* CPU index (informative) */ \ /* user data */ \ diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h index f8bbc70..b3c10d7 100644 --- a/target-alpha/cpu.h +++ b/target-alpha/cpu.h @@ -282,13 +282,10 @@ struct CPUAlphaState { /* Those resources are used only in Qemu core */ CPU_COMMON - jmp_buf jmp_env; int user_mode_only; /* user mode only simulation */ uint32_t hflags; int halted; - int exception_index; - int error_code; int interrupt_request; uint32_t features; diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 2ff25a5..60a7a64 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -156,8 +156,6 @@ typedef struct CPUARMState { void *irq_opaque; /* exception/interrupt handling */ - jmp_buf jmp_env; - int exception_index; int interrupt_request; int user_mode_only; int halted; diff --git a/target-cris/cpu.h b/target-cris/cpu.h index a26dd80..d45abea 100644 --- a/target-cris/cpu.h +++ b/target-cris/cpu.h @@ -124,7 +124,6 @@ typedef struct CPUCRISState { int cc_x_live; int cc_x; - int exception_index; int interrupt_request; int interrupt_vector; int fault_vector; @@ -164,7 +163,6 @@ typedef struct CPUCRISState { int user_mode_only; int halted; - jmp_buf jmp_env; CPU_COMMON } CPUCRISState; diff --git a/target-i386/cpu.h b/target-i386/cpu.h index eb784b4..c8a4559 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -553,9 +553,6 @@ typedef struct CPUX86State { uint64_t pat; /* exception/interrupt handling */ - jmp_buf jmp_env; - int exception_index; - int error_code; int exception_is_int; target_ulong exception_next_eip; target_ulong dr[8]; /* debug registers */ diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h index 732929b..ba36387 100644 --- a/target-m68k/cpu.h +++ b/target-m68k/cpu.h @@ -104,8 +104,6 @@ typedef struct CPUM68KState { uint32_t t1; /* exception/interrupt handling */ - jmp_buf jmp_env; - int exception_index; int interrupt_request; int user_mode_only; int halted; diff --git a/target-mips/cpu.h b/target-mips/cpu.h index 49b7e63..08aad3e 100644 --- a/target-mips/cpu.h +++ b/target-mips/cpu.h @@ -411,9 +411,6 @@ struct CPUMIPSState { int32_t CP0_DESAVE; /* Qemu */ int interrupt_request; - jmp_buf jmp_env; - int exception_index; - int error_code; int user_mode_only; /* user mode only simulation */ uint32_t hflags; /* CPU State */ /* TMASK defines different execution modes */ diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h index 7f7916f..e9f9287 100644 --- a/target-ppc/cpu.h +++ b/target-ppc/cpu.h @@ -648,8 +648,6 @@ struct CPUPPCState { int bfd_mach; uint32_t flags; - int exception_index; - int error_code; int interrupt_request; uint32_t pending_interrupts; #if !defined(CONFIG_USER_ONLY) @@ -674,7 +672,6 @@ struct CPUPPCState { opc_handler_t *opcodes[0x40]; /* Those resources are used only in Qemu core */ - jmp_buf jmp_env; int user_mode_only; /* user mode only simulation */ target_ulong hflags; /* hflags is a MSR & HFLAGS_MASK */ target_ulong hflags_nmsr; /* specific hflags, not comming from MSR */ diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h index 72ac82f..534ada3 100644 --- a/target-sh4/cpu.h +++ b/target-sh4/cpu.h @@ -114,11 +114,9 @@ typedef struct CPUSH4State { uint32_t expevt; /* exception event register */ uint32_t intevt; /* interrupt event register */ - jmp_buf jmp_env; int user_mode_only; int interrupt_request; int halted; - int exception_index; CPU_COMMON tlb_t utlb[UTLB_SIZE]; /* unified translation table */ tlb_t itlb[ITLB_SIZE]; /* instruction translation table */ void *intc_handle; diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h index af0ebd1..b663fe2 100644 --- a/target-sparc/cpu.h +++ b/target-sparc/cpu.h @@ -214,9 +214,7 @@ typedef struct CPUSPARCState { uint32_t pil_in; /* incoming interrupt level bitmap */ int psref; /* enable fpu */ target_ulong version; - jmp_buf jmp_env; int user_mode_only; - int exception_index; int interrupt_index; int interrupt_request; int halted; -- 1.5.4.5 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too. 2008-05-27 15:18 ` [Qemu-devel] [PATCH 2/6] Push common interrupt variables to cpu-defs.h Glauber Costa @ 2008-05-27 15:18 ` Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 4/6] simply cpu_exec further Glauber Costa 2008-05-27 15:34 ` [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too Fabrice Bellard 2008-05-27 15:39 ` [Qemu-devel] [PATCH 2/6] Push common interrupt variables to cpu-defs.h Fabrice Bellard 1 sibling, 2 replies; 22+ messages in thread From: Glauber Costa @ 2008-05-27 15:18 UTC (permalink / raw) To: qemu-devel; +Cc: kvm Unlike other architectures, i386 lacked a "halted" attribute, going with a flag into hflags. By using the halted attribute, we can make the code look like more other architectures, and simplify the code in some instances. In this commit, we make the code for info_cpus simpler in monitor.c Signed-off-by: Glauber Costa <gcosta@redhat.com> --- cpu-defs.h | 2 ++ exec-all.h | 1 + hw/apic.c | 4 ++-- hw/pc.c | 2 +- monitor.c | 23 +++++------------------ target-alpha/cpu.h | 1 - target-alpha/helper.c | 1 + target-arm/cpu.h | 1 - target-arm/helper.c | 1 + target-cris/cpu.h | 1 - target-cris/helper.c | 1 + target-i386/exec.h | 4 ++-- target-i386/helper.c | 9 ++++++++- target-m68k/cpu.h | 1 - target-m68k/helper.c | 1 + target-mips/cpu.h | 2 -- target-mips/helper.c | 6 ++++++ target-ppc/cpu.h | 2 -- target-ppc/helper.c | 6 ++++++ target-sh4/cpu.h | 1 - target-sh4/helper.c | 1 + target-sparc/cpu.h | 1 - target-sparc/helper.c | 6 ++++++ 23 files changed, 44 insertions(+), 34 deletions(-) diff --git a/cpu-defs.h b/cpu-defs.h index bb00c80..5bcd1ee 100644 --- a/cpu-defs.h +++ b/cpu-defs.h @@ -164,6 +164,8 @@ typedef struct CPUTLBEntry { int exception_index; \ int error_code; \ \ + int halted; \ + \ void *next_cpu; /* next CPU sharing TB cache */ \ int cpu_index; /* CPU index (informative) */ \ /* user data */ \ diff --git a/exec-all.h b/exec-all.h index fd2164f..fd1ea6a 100644 --- a/exec-all.h +++ b/exec-all.h @@ -83,6 +83,7 @@ int cpu_restore_state_copy(struct TranslationBlock *tb, void cpu_resume_from_signal(CPUState *env1, void *puc); void cpu_exec_init(CPUState *env); +extern int cpu_info_ip(CPUState *env, char *buf); void cpu_load_flags(CPUState *env); void cpu_put_flags(CPUState *env); diff --git a/hw/apic.c b/hw/apic.c index a1ebf21..0dc9f8e 100644 --- a/hw/apic.c +++ b/hw/apic.c @@ -443,12 +443,12 @@ static void apic_init_ipi(APICState *s) static void apic_startup(APICState *s, int vector_num) { CPUState *env = s->cpu_env; - if (!(env->hflags & HF_HALTED_MASK)) + if (!(env->halted)) return; env->eip = 0; cpu_x86_load_seg_cache(env, R_CS, vector_num << 8, vector_num << 12, 0xffff, 0); - env->hflags &= ~HF_HALTED_MASK; + env->halted = 0; } static void apic_deliver(APICState *s, uint8_t dest, uint8_t dest_mode, diff --git a/hw/pc.c b/hw/pc.c index c92384c..c68323b 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -761,7 +761,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size, exit(1); } if (i != 0) - env->hflags |= HF_HALTED_MASK; + env->halted = 1; if (smp_cpus > 1) { /* XXX: enable it in all cases */ env->cpuid_features |= CPUID_APIC; diff --git a/monitor.c b/monitor.c index 236b827..f34d147 100644 --- a/monitor.c +++ b/monitor.c @@ -36,6 +36,8 @@ #include "disas.h" #include <dirent.h> +#include "exec-all.h" + #ifdef CONFIG_PROFILER #include "qemu-timer.h" /* for ticks_per_sec */ #endif @@ -304,6 +306,7 @@ static void do_info_registers(void) static void do_info_cpus(void) { CPUState *env; + char buf[1024]; /* just to set the default cpu if not already done */ mon_get_cpu(); @@ -312,24 +315,8 @@ static void do_info_cpus(void) term_printf("%c CPU #%d:", (env == mon_cpu) ? '*' : ' ', env->cpu_index); -#if defined(TARGET_I386) - term_printf(" pc=0x" TARGET_FMT_lx, env->eip + env->segs[R_CS].base); - if (env->hflags & HF_HALTED_MASK) - term_printf(" (halted)"); -#elif defined(TARGET_PPC) - term_printf(" nip=0x" TARGET_FMT_lx, env->nip); - if (env->halted) - term_printf(" (halted)"); -#elif defined(TARGET_SPARC) - term_printf(" pc=0x" TARGET_FMT_lx " npc=0x" TARGET_FMT_lx, env->pc, env->npc); - if (env->halted) - term_printf(" (halted)"); -#elif defined(TARGET_MIPS) - term_printf(" PC=0x" TARGET_FMT_lx, env->PC[env->current_tc]); - if (env->halted) - term_printf(" (halted)"); -#endif - term_printf("\n"); + if (cpu_info_ip(env, buf)) + term_printf("%s %s\n", buf, env->halted ? "(halted)" : ""); } } diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h index b3c10d7..dc2460a 100644 --- a/target-alpha/cpu.h +++ b/target-alpha/cpu.h @@ -284,7 +284,6 @@ struct CPUAlphaState { int user_mode_only; /* user mode only simulation */ uint32_t hflags; - int halted; int interrupt_request; diff --git a/target-alpha/helper.c b/target-alpha/helper.c index 73d565f..6056d6c 100644 --- a/target-alpha/helper.c +++ b/target-alpha/helper.c @@ -453,5 +453,6 @@ void cpu_dump_EA (target_ulong EA) fprintf(f, "Memory access at address " TARGET_FMT_lx "\n", EA); } +int cpu_info_ip(CPUState *env, char *buf) { return 0; } void cpu_load_flags(CPUState *env) {} void cpu_put_flags(CPUState *env) {} diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 60a7a64..8d0558c 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -158,7 +158,6 @@ typedef struct CPUARMState { /* exception/interrupt handling */ int interrupt_request; int user_mode_only; - int halted; /* VFP coprocessor state. */ struct { diff --git a/target-arm/helper.c b/target-arm/helper.c index 496f326..6724909 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2523,6 +2523,7 @@ uint32_t HELPER(rsqrte_u32)(uint32_t a, CPUState *env) return float32_to_int32(tmp, s); } +int cpu_info_ip(CPUState *env, char *buf) { return 0; } void cpu_load_flags(CPUState *env) { } /* XXX: Save/restore host fpu exception state?. */ void cpu_put_flags(CPUState *env) { } diff --git a/target-cris/cpu.h b/target-cris/cpu.h index d45abea..acc1b5d 100644 --- a/target-cris/cpu.h +++ b/target-cris/cpu.h @@ -161,7 +161,6 @@ typedef struct CPUCRISState { int features; int user_mode_only; - int halted; CPU_COMMON } CPUCRISState; diff --git a/target-cris/helper.c b/target-cris/helper.c index 1e216aa..0ed169c 100644 --- a/target-cris/helper.c +++ b/target-cris/helper.c @@ -184,5 +184,6 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUState * env, target_ulong addr) } #endif +int cpu_info_ip(CPUState *env, char *buf) { return 0; } void cpu_load_flags(CPUState *env) {} void cpu_put_flags(CPUState *env) {} diff --git a/target-i386/exec.h b/target-i386/exec.h index 8597e79..f6517ce 100644 --- a/target-i386/exec.h +++ b/target-i386/exec.h @@ -425,13 +425,13 @@ static inline void regs_to_env(void) static inline int cpu_halted(CPUState *env) { /* handle exit of HALTED state */ - if (!(env->hflags & HF_HALTED_MASK)) + if (!(env->halted)) return 0; /* disable halt condition */ if (((env->interrupt_request & CPU_INTERRUPT_HARD) && (env->eflags & IF_MASK)) || (env->interrupt_request & CPU_INTERRUPT_NMI)) { - env->hflags &= ~HF_HALTED_MASK; + env->halted = 0; return 0; } return EXCP_HALTED; diff --git a/target-i386/helper.c b/target-i386/helper.c index 1d88a9e..7e690de 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -4487,7 +4487,7 @@ void helper_idivq_EAX(target_ulong t0) void helper_hlt(void) { env->hflags &= ~HF_INHIBIT_IRQ_MASK; /* needed if sti is just before */ - env->hflags |= HF_HALTED_MASK; + env->halted = 1; env->exception_index = EXCP_HLT; cpu_loop_exit(); } @@ -4668,6 +4668,13 @@ void helper_clgi(void) { env->hflags &= ~HF_GIF_MASK; } + +int cpu_info_ip(CPUState *env, char *buf) +{ + sprintf(buf, " pc=0x" TARGET_FMT_lx, env->eip + env->segs[R_CS].base); + return 1; +} + void cpu_load_flags(CPUState *env) { /* put eflags in CPU temporary format */ diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h index ba36387..e8c1dfc 100644 --- a/target-m68k/cpu.h +++ b/target-m68k/cpu.h @@ -106,7 +106,6 @@ typedef struct CPUM68KState { /* exception/interrupt handling */ int interrupt_request; int user_mode_only; - int halted; int pending_vector; int pending_level; diff --git a/target-m68k/helper.c b/target-m68k/helper.c index 73e93bc..82b5576 100644 --- a/target-m68k/helper.c +++ b/target-m68k/helper.c @@ -339,6 +339,7 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUState *env, target_ulong addr) return addr; } +int cpu_info_ip(CPUState *env, char *buf) { return 0; } void cpu_load_flags(CPUState *env) { env->cc_op = CC_OP_FLAGS; diff --git a/target-mips/cpu.h b/target-mips/cpu.h index 08aad3e..38ae0b4 100644 --- a/target-mips/cpu.h +++ b/target-mips/cpu.h @@ -445,8 +445,6 @@ struct CPUMIPSState { target_ulong btarget; /* Jump / branch target */ int bcond; /* Branch condition (if needed) */ - int halted; /* TRUE if the CPU is in suspend state */ - int SYNCI_Step; /* Address step size for SYNCI */ int CCRes; /* Cycle count resolution/divisor */ uint32_t CP0_Status_rw_bitmask; /* Read/write bits in CP0_Status */ diff --git a/target-mips/helper.c b/target-mips/helper.c index 90e6dbe..81a1a6b 100644 --- a/target-mips/helper.c +++ b/target-mips/helper.c @@ -637,5 +637,11 @@ void r4k_invalidate_tlb (CPUState *env, int idx, int use_extra) } } +int cpu_info_ip(CPUState *env, char *buf) +{ + sprintf(buf, " PC=0x" TARGET_FMT_lx, env->PC[env->current_tc]); + return 1; +} + void cpu_put_flags(CPUState *env) { } void cpu_load_flags(CPUState *env) { } diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h index e9f9287..a1f90e2 100644 --- a/target-ppc/cpu.h +++ b/target-ppc/cpu.h @@ -586,8 +586,6 @@ struct CPUPPCState { CPU_COMMON - int halted; /* TRUE if the CPU is in suspend state */ - int access_type; /* when a memory exception occurs, the access type is stored here */ diff --git a/target-ppc/helper.c b/target-ppc/helper.c index 300532f..2858780 100644 --- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -2989,5 +2989,11 @@ void cpu_ppc_close (CPUPPCState *env) qemu_free(env); } +int cpu_info_ip(CPUState *env, char *buf) +{ + sprintf(buf, " nip=0x" TARGET_FMT_lx, env->nip); + return 1; +} + void cpu_load_flags(CPUState *env) { } void cpu_put_flags(CPUState *env) { } diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h index 534ada3..c03cdb1 100644 --- a/target-sh4/cpu.h +++ b/target-sh4/cpu.h @@ -116,7 +116,6 @@ typedef struct CPUSH4State { int user_mode_only; int interrupt_request; - int halted; CPU_COMMON tlb_t utlb[UTLB_SIZE]; /* unified translation table */ tlb_t itlb[ITLB_SIZE]; /* instruction translation table */ void *intc_handle; diff --git a/target-sh4/helper.c b/target-sh4/helper.c index ebc16ac..df64d28 100644 --- a/target-sh4/helper.c +++ b/target-sh4/helper.c @@ -535,5 +535,6 @@ void cpu_load_tlb(CPUState * env) #endif +int cpu_info_ip(CPUState *env, char *buf) { return 0; } void cpu_load_flags(CPUState *env) {} void cpu_put_flags(CPUState *env) {} diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h index b663fe2..ba3ee01 100644 --- a/target-sparc/cpu.h +++ b/target-sparc/cpu.h @@ -217,7 +217,6 @@ typedef struct CPUSPARCState { int user_mode_only; int interrupt_index; int interrupt_request; - int halted; uint32_t mmu_bm; uint32_t mmu_ctpr_mask; uint32_t mmu_cxr_mask; diff --git a/target-sparc/helper.c b/target-sparc/helper.c index 76c68fa..0a300fc 100644 --- a/target-sparc/helper.c +++ b/target-sparc/helper.c @@ -1320,6 +1320,12 @@ void cpu_dump_state(CPUState *env, FILE *f, static uint32_t *saved_regwptr; #endif +int cpu_info_ip(CPUState *env, char *buf) +{ + sprintf(buf, " pc=0x" TARGET_FMT_lx " npc=0x" TARGET_FMT_lx, env->pc, env->npc); + return 1; +} + void cpu_load_flags(CPUState *env) { #if defined(reg_REGWPTR) -- 1.5.4.5 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 4/6] simply cpu_exec further 2008-05-27 15:18 ` [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too Glauber Costa @ 2008-05-27 15:18 ` Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function Glauber Costa 2008-05-27 16:18 ` [Qemu-devel] [PATCH 4/6] simply cpu_exec further Blue Swirl 2008-05-27 15:34 ` [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too Fabrice Bellard 1 sibling, 2 replies; 22+ messages in thread From: Glauber Costa @ 2008-05-27 15:18 UTC (permalink / raw) To: qemu-devel; +Cc: kvm We change interrupt functions so they have the same signature, getting only an env parameter. When necessary, some more attributed were added to the relevant CPUState to make it possible. Signed-off-by: Glauber Costa <gcosta@redhat.com> --- cpu-defs.h | 1 + cpu-exec.c | 197 +++++----------------------------------------- exec-all.h | 7 ++ target-alpha/helper.c | 9 ++ target-arm/helper.c | 27 ++++++ target-cris/helper.c | 9 ++ target-i386/exec.h | 5 +- target-i386/helper.c | 69 ++++++++++++++++- target-m68k/cpu.h | 2 +- target-m68k/helper.c | 16 ++++ target-m68k/op_helper.c | 8 +- target-mips/helper.c | 17 ++++ target-ppc/helper.c | 16 ++++ target-sh4/helper.c | 8 ++ target-sparc/exec.h | 3 +- target-sparc/helper.c | 1 + target-sparc/op_helper.c | 29 ++++++- 17 files changed, 232 insertions(+), 192 deletions(-) diff --git a/cpu-defs.h b/cpu-defs.h index 5bcd1ee..5f3f9df 100644 --- a/cpu-defs.h +++ b/cpu-defs.h @@ -162,6 +162,7 @@ typedef struct CPUTLBEntry { /* Core interrupt code */ \ jmp_buf jmp_env; \ int exception_index; \ + int exception_is_hw; \ int error_code; \ \ int halted; \ diff --git a/cpu-exec.c b/cpu-exec.c index a829b7b..e798b03 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -241,6 +241,21 @@ static inline TranslationBlock *tb_find_fast(void) return tb; } +void handle_interrupt_common(CPUState *env) +{ + if (env->interrupt_request & CPU_INTERRUPT_HALT) { + env->interrupt_request &= ~CPU_INTERRUPT_HALT; + env->halted = 1; + env->exception_index = EXCP_HLT; + cpu_loop_exit(); + } +} + +void reset_tb(void) +{ + next_tb = 0; +} + /* main execution loop */ int cpu_exec(CPUState *env1) @@ -281,43 +296,16 @@ int cpu_exec(CPUState *env1) which will be handled outside the cpu execution loop */ #if defined(TARGET_I386) - do_interrupt_user(env->exception_index, - env->exception_is_int, - env->error_code, - env->exception_next_eip); - /* successfully delivered */ - env->old_exception = -1; + do_interrupt(env); #endif ret = env->exception_index; break; } else { -#if defined(TARGET_I386) /* simulate a real cpu exception. On i386, it can trigger new exceptions, but we do not handle double or triple faults yet. */ - do_interrupt(env->exception_index, - env->exception_is_int, - env->error_code, - env->exception_next_eip, 0); - /* successfully delivered */ - env->old_exception = -1; -#elif defined(TARGET_PPC) - do_interrupt(env); -#elif defined(TARGET_MIPS) - do_interrupt(env); -#elif defined(TARGET_SPARC) - do_interrupt(env->exception_index); -#elif defined(TARGET_ARM) + env->exception_is_hw = 0; do_interrupt(env); -#elif defined(TARGET_SH4) - do_interrupt(env); -#elif defined(TARGET_ALPHA) - do_interrupt(env); -#elif defined(TARGET_CRIS) - do_interrupt(env); -#elif defined(TARGET_M68K) - do_interrupt(0); -#endif } env->exception_index = -1; } @@ -360,156 +348,9 @@ int cpu_exec(CPUState *env1) env->exception_index = EXCP_DEBUG; cpu_loop_exit(); } -#if defined(TARGET_ARM) || defined(TARGET_SPARC) || defined(TARGET_MIPS) || \ - defined(TARGET_PPC) || defined(TARGET_ALPHA) || defined(TARGET_CRIS) - if (interrupt_request & CPU_INTERRUPT_HALT) { - env->interrupt_request &= ~CPU_INTERRUPT_HALT; - env->halted = 1; - env->exception_index = EXCP_HLT; - cpu_loop_exit(); - } -#endif -#if defined(TARGET_I386) - if ((interrupt_request & CPU_INTERRUPT_SMI) && - !(env->hflags & HF_SMM_MASK)) { - svm_check_intercept(SVM_EXIT_SMI); - env->interrupt_request &= ~CPU_INTERRUPT_SMI; - do_smm_enter(); - next_tb = 0; - } else if ((interrupt_request & CPU_INTERRUPT_NMI) && - !(env->hflags & HF_NMI_MASK)) { - env->interrupt_request &= ~CPU_INTERRUPT_NMI; - env->hflags |= HF_NMI_MASK; - do_interrupt(EXCP02_NMI, 0, 0, 0, 1); - next_tb = 0; - } else if ((interrupt_request & CPU_INTERRUPT_HARD) && - (env->eflags & IF_MASK || env->hflags & HF_HIF_MASK) && - !(env->hflags & HF_INHIBIT_IRQ_MASK)) { - int intno; - svm_check_intercept(SVM_EXIT_INTR); - env->interrupt_request &= ~(CPU_INTERRUPT_HARD | CPU_INTERRUPT_VIRQ); - intno = cpu_get_pic_interrupt(env); - if (loglevel & CPU_LOG_TB_IN_ASM) { - fprintf(logfile, "Servicing hardware INT=0x%02x\n", intno); - } - do_interrupt(intno, 0, 0, 0, 1); - /* ensure that no TB jump will be modified as - the program flow was changed */ - next_tb = 0; -#if !defined(CONFIG_USER_ONLY) - } else if ((interrupt_request & CPU_INTERRUPT_VIRQ) && - (env->eflags & IF_MASK) && !(env->hflags & HF_INHIBIT_IRQ_MASK)) { - int intno; - /* FIXME: this should respect TPR */ - env->interrupt_request &= ~CPU_INTERRUPT_VIRQ; - svm_check_intercept(SVM_EXIT_VINTR); - intno = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.int_vector)); - if (loglevel & CPU_LOG_TB_IN_ASM) - fprintf(logfile, "Servicing virtual hardware INT=0x%02x\n", intno); - do_interrupt(intno, 0, 0, -1, 1); - stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.int_ctl), - ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.int_ctl)) & ~V_IRQ_MASK); - next_tb = 0; -#endif - } -#elif defined(TARGET_PPC) -#if 0 - if ((interrupt_request & CPU_INTERRUPT_RESET)) { - cpu_ppc_reset(env); - } -#endif - if (interrupt_request & CPU_INTERRUPT_HARD) { - ppc_hw_interrupt(env); - if (env->pending_interrupts == 0) - env->interrupt_request &= ~CPU_INTERRUPT_HARD; - next_tb = 0; - } -#elif defined(TARGET_MIPS) - if ((interrupt_request & CPU_INTERRUPT_HARD) && - (env->CP0_Status & env->CP0_Cause & CP0Ca_IP_mask) && - (env->CP0_Status & (1 << CP0St_IE)) && - !(env->CP0_Status & (1 << CP0St_EXL)) && - !(env->CP0_Status & (1 << CP0St_ERL)) && - !(env->hflags & MIPS_HFLAG_DM)) { - /* Raise it */ - env->exception_index = EXCP_EXT_INTERRUPT; - env->error_code = 0; - do_interrupt(env); - next_tb = 0; - } -#elif defined(TARGET_SPARC) - if ((interrupt_request & CPU_INTERRUPT_HARD) && - (env->psret != 0)) { - int pil = env->interrupt_index & 15; - int type = env->interrupt_index & 0xf0; - - if (((type == TT_EXTINT) && - (pil == 15 || pil > env->psrpil)) || - type != TT_EXTINT) { - env->interrupt_request &= ~CPU_INTERRUPT_HARD; - do_interrupt(env->interrupt_index); - env->interrupt_index = 0; -#if !defined(TARGET_SPARC64) && !defined(CONFIG_USER_ONLY) - cpu_check_irqs(env); -#endif - next_tb = 0; - } - } else if (interrupt_request & CPU_INTERRUPT_TIMER) { - //do_interrupt(0, 0, 0, 0, 0); - env->interrupt_request &= ~CPU_INTERRUPT_TIMER; - } -#elif defined(TARGET_ARM) - if (interrupt_request & CPU_INTERRUPT_FIQ - && !(env->uncached_cpsr & CPSR_F)) { - env->exception_index = EXCP_FIQ; - do_interrupt(env); - next_tb = 0; - } - /* ARMv7-M interrupt return works by loading a magic value - into the PC. On real hardware the load causes the - return to occur. The qemu implementation performs the - jump normally, then does the exception return when the - CPU tries to execute code at the magic address. - This will cause the magic PC value to be pushed to - the stack if an interrupt occured at the wrong time. - We avoid this by disabling interrupts when - pc contains a magic address. */ - if (interrupt_request & CPU_INTERRUPT_HARD - && ((IS_M(env) && env->regs[15] < 0xfffffff0) - || !(env->uncached_cpsr & CPSR_I))) { - env->exception_index = EXCP_IRQ; - do_interrupt(env); - next_tb = 0; - } -#elif defined(TARGET_SH4) - if (interrupt_request & CPU_INTERRUPT_HARD) { - do_interrupt(env); - next_tb = 0; - } -#elif defined(TARGET_ALPHA) - if (interrupt_request & CPU_INTERRUPT_HARD) { - do_interrupt(env); - next_tb = 0; - } -#elif defined(TARGET_CRIS) - if (interrupt_request & CPU_INTERRUPT_HARD) { - do_interrupt(env); - next_tb = 0; - } -#elif defined(TARGET_M68K) - if (interrupt_request & CPU_INTERRUPT_HARD - && ((env->sr & SR_I) >> SR_I_SHIFT) - < env->pending_level) { - /* Real hardware gets the interrupt vector via an - IACK cycle at this point. Current emulated - hardware doesn't rely on this, so we - provide/save the vector when the interrupt is - first signalled. */ - env->exception_index = env->pending_vector; - do_interrupt(1); - next_tb = 0; - } -#endif + + arch_handle_interrupt_request(env); + /* Don't use the cached interupt_request value, do_interrupt may have updated the EXITTB flag. */ if (env->interrupt_request & CPU_INTERRUPT_EXITTB) { diff --git a/exec-all.h b/exec-all.h index fd1ea6a..b43526c 100644 --- a/exec-all.h +++ b/exec-all.h @@ -84,6 +84,13 @@ void cpu_resume_from_signal(CPUState *env1, void *puc); void cpu_exec_init(CPUState *env); extern int cpu_info_ip(CPUState *env, char *buf); + +/* implemented by the targets */ +void arch_handle_interrupt_request(CPUState *env); +/* implemented by cpu-exec.c */ +void handle_interrupt_common(CPUState *env); +void reset_tb(void); + void cpu_load_flags(CPUState *env); void cpu_put_flags(CPUState *env); diff --git a/target-alpha/helper.c b/target-alpha/helper.c index 6056d6c..003601a 100644 --- a/target-alpha/helper.c +++ b/target-alpha/helper.c @@ -456,3 +456,12 @@ void cpu_dump_EA (target_ulong EA) int cpu_info_ip(CPUState *env, char *buf) { return 0; } void cpu_load_flags(CPUState *env) {} void cpu_put_flags(CPUState *env) {} + +void arch_handle_interrupt_request(CPUState *env) +{ + handle_interrupt_common(env); + if (env->interrupt_request & CPU_INTERRUPT_HARD) { + do_interrupt(env); + reset_tb(); + } +} diff --git a/target-arm/helper.c b/target-arm/helper.c index 6724909..c0b3e84 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -2527,3 +2527,30 @@ int cpu_info_ip(CPUState *env, char *buf) { return 0; } void cpu_load_flags(CPUState *env) { } /* XXX: Save/restore host fpu exception state?. */ void cpu_put_flags(CPUState *env) { } + +void arch_handle_interrupt_request(CPUState *env) +{ + handle_interrupt_common(env); + if (env->interrupt_request & CPU_INTERRUPT_FIQ + && !(env->uncached_cpsr & CPSR_F)) { + env->exception_index = EXCP_FIQ; + do_interrupt(env); + reset_tb(); + } + /* ARMv7-M interrupt return works by loading a magic value + into the PC. On real hardware the load causes the + return to occur. The qemu implementation performs the + jump normally, then does the exception return when the + CPU tries to execute code at the magic address. + This will cause the magic PC value to be pushed to + the stack if an interrupt occured at the wrong time. + We avoid this by disabling interrupts when + pc contains a magic address. */ + if (env->interrupt_request & CPU_INTERRUPT_HARD + && ((IS_M(env) && env->regs[15] < 0xfffffff0) + || !(env->uncached_cpsr & CPSR_I))) { + env->exception_index = EXCP_IRQ; + do_interrupt(env); + reset_tb(); + } +} diff --git a/target-cris/helper.c b/target-cris/helper.c index 0ed169c..7acba95 100644 --- a/target-cris/helper.c +++ b/target-cris/helper.c @@ -187,3 +187,12 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUState * env, target_ulong addr) int cpu_info_ip(CPUState *env, char *buf) { return 0; } void cpu_load_flags(CPUState *env) {} void cpu_put_flags(CPUState *env) {} + +void arch_handle_interrupt_request(CPUState *env) +{ + handle_interrupt_common(env); + if (env->interrupt_request & CPU_INTERRUPT_HARD) { + do_interrupt(env); + reset_tb(); + } +} diff --git a/target-i386/exec.h b/target-i386/exec.h index f6517ce..b4adabf 100644 --- a/target-i386/exec.h +++ b/target-i386/exec.h @@ -108,10 +108,7 @@ void tlb_fill(target_ulong addr, int is_write, int mmu_idx, void *retaddr); void __hidden cpu_lock(void); void __hidden cpu_unlock(void); -void do_interrupt(int intno, int is_int, int error_code, - target_ulong next_eip, int is_hw); -void do_interrupt_user(int intno, int is_int, int error_code, - target_ulong next_eip); +void do_interrupt(CPUState *env); void raise_interrupt(int intno, int is_int, int error_code, int next_eip_addend); void raise_exception_err(int exception_index, int error_code); diff --git a/target-i386/helper.c b/target-i386/helper.c index 7e690de..7682d8e 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -1199,8 +1199,8 @@ void do_interrupt_user(int intno, int is_int, int error_code, * the int instruction. next_eip is the EIP value AFTER the interrupt * instruction. It is only relevant if is_int is TRUE. */ -void do_interrupt(int intno, int is_int, int error_code, - target_ulong next_eip, int is_hw) +void do_interrupt_system(int intno, int is_int, int error_code, + target_ulong next_eip, int is_hw) { if (loglevel & CPU_LOG_INT) { if ((env->cr[0] & CR0_PE_MASK)) { @@ -1247,6 +1247,71 @@ void do_interrupt(int intno, int is_int, int error_code, } } +void do_interrupt(CPUState *env) +{ + if (env->user_mode_only) + do_interrupt_user(env->exception_index, + env->exception_is_int, + env->error_code, + env->exception_next_eip); + else + do_interrupt_system(env->exception_index, + env->exception_is_int, + env->error_code, + env->exception_next_eip, + env->exception_is_hw); + + env->old_exception = -1; +} + +void arch_handle_interrupt_request(CPUState *env) +{ + int interrupt_request = env->interrupt_request; + int intno; + + if ((interrupt_request & CPU_INTERRUPT_SMI) && + !(env->hflags & HF_SMM_MASK)) { + svm_check_intercept(SVM_EXIT_SMI); + env->interrupt_request &= ~CPU_INTERRUPT_SMI; + do_smm_enter(); + reset_tb(); + } else if ((interrupt_request & CPU_INTERRUPT_NMI) && + !(env->hflags & HF_NMI_MASK)) { + env->interrupt_request &= ~CPU_INTERRUPT_NMI; + env->hflags |= HF_NMI_MASK; + do_interrupt_system(EXCP02_NMI, 0, 0, 0, 1); + reset_tb(); + } else if ((interrupt_request & CPU_INTERRUPT_HARD) && + (env->eflags & IF_MASK || env->hflags & HF_HIF_MASK) && + !(env->hflags & HF_INHIBIT_IRQ_MASK)) { + svm_check_intercept(SVM_EXIT_INTR); + env->interrupt_request &= ~(CPU_INTERRUPT_HARD | CPU_INTERRUPT_VIRQ); + intno = cpu_get_pic_interrupt(env); + if (loglevel & CPU_LOG_TB_IN_ASM) { + fprintf(logfile, "Servicing hardware INT=0x%02x\n", intno); + } + + do_interrupt_system(intno, 0, 0, 0, 1); + /* ensure that no TB jump will be modified as + the program flow was changed */ + reset_tb(); +#if !defined(CONFIG_USER_ONLY) + } else if ((interrupt_request & CPU_INTERRUPT_VIRQ) && + (env->eflags & IF_MASK) && !(env->hflags & HF_INHIBIT_IRQ_MASK)) { + /* FIXME: this should respect TPR */ + env->interrupt_request &= ~CPU_INTERRUPT_VIRQ; + svm_check_intercept(SVM_EXIT_VINTR); + intno = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.int_vector)); + if (loglevel & CPU_LOG_TB_IN_ASM) + fprintf(logfile, "Servicing virtual hardware INT=0x%02x\n", intno); + + do_interrupt_system(intno, 0, 0, -1, 1); + stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.int_ctl), + ldl_phys(env->vm_vmcb + offsetof(struct vmcb, control.int_ctl)) & ~V_IRQ_MASK); + reset_tb(); +#endif + } +} /* * Check nested exceptions and change to double or triple fault if * needed. It should only be called, if this is not an interrupt. diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h index e8c1dfc..1acb4a3 100644 --- a/target-m68k/cpu.h +++ b/target-m68k/cpu.h @@ -120,7 +120,7 @@ typedef struct CPUM68KState { CPUM68KState *cpu_m68k_init(const char *cpu_model); int cpu_m68k_exec(CPUM68KState *s); void cpu_m68k_close(CPUM68KState *s); -void do_interrupt(int is_hw); +void do_interrupt(CPUM68KState *s); /* you can call this signal handler from your SIGBUS and SIGSEGV signal handlers to inform the virtual CPU of exceptions. non zero is returned if the signal was handled by the virtual CPU. */ diff --git a/target-m68k/helper.c b/target-m68k/helper.c index 82b5576..65d68c9 100644 --- a/target-m68k/helper.c +++ b/target-m68k/helper.c @@ -355,6 +355,22 @@ void cpu_put_flags(CPUState *env) | env->cc_dest | (env->cc_x << 4); } +void arch_handle_interrupt_request(CPUState *env) +{ + if (env->interrupt_request & CPU_INTERRUPT_HARD + && ((env->sr & SR_I) >> SR_I_SHIFT) + < env->pending_level) { + /* Real hardware gets the interrupt vector via an + IACK cycle at this point. Current emulated + hardware doesn't rely on this, so we + provide/save the vector when the interrupt is + signalled. */ + env->exception_index = env->pending_vector; + env->exception_is_hw = 1; + do_interrupt(env); + reset_tb(); + } +} #if defined(CONFIG_USER_ONLY) int cpu_m68k_handle_mmu_fault (CPUState *env, target_ulong address, int rw, diff --git a/target-m68k/op_helper.c b/target-m68k/op_helper.c index 547f13d..2b0fc1c 100644 --- a/target-m68k/op_helper.c +++ b/target-m68k/op_helper.c @@ -21,7 +21,7 @@ #if defined(CONFIG_USER_ONLY) -void do_interrupt(int is_hw) +void do_interrupt(CPUState *env) { env->exception_index = -1; } @@ -95,7 +95,7 @@ static void do_rte(void) env->aregs[7] = sp + 8; } -void do_interrupt(int is_hw) +void do_interrupt(CPUState *env) { uint32_t sp; uint32_t fmt; @@ -105,7 +105,7 @@ void do_interrupt(int is_hw) fmt = 0; retaddr = env->pc; - if (!is_hw) { + if (!env->exception_is_hw) { switch (env->exception_index) { case EXCP_RTE: /* Return from an exception. */ @@ -143,7 +143,7 @@ void do_interrupt(int is_hw) fmt |= env->sr; env->sr |= SR_S; - if (is_hw) { + if (env->exception_is_hw) { env->sr = (env->sr & ~SR_I) | (env->pending_level << SR_I_SHIFT); env->sr &= ~SR_M; } diff --git a/target-mips/helper.c b/target-mips/helper.c index 81a1a6b..f10930a 100644 --- a/target-mips/helper.c +++ b/target-mips/helper.c @@ -645,3 +645,20 @@ int cpu_info_ip(CPUState *env, char *buf) void cpu_put_flags(CPUState *env) { } void cpu_load_flags(CPUState *env) { } + +void arch_handle_interrupt_request(CPUState *env) +{ + handle_interrupt_common(env); + if ((env->interrupt_request & CPU_INTERRUPT_HARD) && + (env->CP0_Status & env->CP0_Cause & CP0Ca_IP_mask) && + (env->CP0_Status & (1 << CP0St_IE)) && + !(env->CP0_Status & (1 << CP0St_EXL)) && + !(env->CP0_Status & (1 << CP0St_ERL)) && + !(env->hflags & MIPS_HFLAG_DM)) { + /* Raise it */ + env->exception_index = EXCP_EXT_INTERRUPT; + env->error_code = 0; + do_interrupt(env); + reset_tb(); + } +} diff --git a/target-ppc/helper.c b/target-ppc/helper.c index 2858780..ff8398f 100644 --- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -2997,3 +2997,19 @@ int cpu_info_ip(CPUState *env, char *buf) void cpu_load_flags(CPUState *env) { } void cpu_put_flags(CPUState *env) { } + +void arch_handle_interrupt_request(CPUState *env) +{ +#if 0 + if ((interrupt_request & CPU_INTERRUPT_RESET)) { + cpu_ppc_reset(env); + } +#endif + if (env->interrupt_request & CPU_INTERRUPT_HARD) { + ppc_hw_interrupt(env); + if (env->pending_interrupts == 0) + env->interrupt_request &= ~CPU_INTERRUPT_HARD; + reset_tb(); + } +} + diff --git a/target-sh4/helper.c b/target-sh4/helper.c index df64d28..9d475ba 100644 --- a/target-sh4/helper.c +++ b/target-sh4/helper.c @@ -538,3 +538,11 @@ void cpu_load_tlb(CPUState * env) int cpu_info_ip(CPUState *env, char *buf) { return 0; } void cpu_load_flags(CPUState *env) {} void cpu_put_flags(CPUState *env) {} + +void arch_handle_interrupt_request(CPUState *env) +{ + if (env->interrupt_request & CPU_INTERRUPT_HARD) { + do_interrupt(env); + reset_tb(); + } +} diff --git a/target-sparc/exec.h b/target-sparc/exec.h index 19d1732..33c5267 100644 --- a/target-sparc/exec.h +++ b/target-sparc/exec.h @@ -30,7 +30,8 @@ static inline void regs_to_env(void) int cpu_sparc_handle_mmu_fault(CPUState *env1, target_ulong address, int rw, int mmu_idx, int is_softmmu); -void do_interrupt(int intno); + +void do_interrupt(CPUState *env); static inline int cpu_halted(CPUState *env1) { if (!env1->halted) diff --git a/target-sparc/helper.c b/target-sparc/helper.c index 0a300fc..d63a91d 100644 --- a/target-sparc/helper.c +++ b/target-sparc/helper.c @@ -1339,6 +1339,7 @@ void cpu_put_flags(CPUState *env) REGWPTR = saved_regwptr; #endif } + #ifdef TARGET_SPARC64 #if !defined(CONFIG_USER_ONLY) #include "qemu-common.h" diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c index 2d85560..a22e752 100644 --- a/target-sparc/op_helper.c +++ b/target-sparc/op_helper.c @@ -2689,8 +2689,9 @@ static const char * const excp_names[0x50] = { }; #endif -void do_interrupt(int intno) +void do_interrupt(CPUState *cpu) { + int intno = cpu->exception_index; #ifdef DEBUG_PCALL if (loglevel & CPU_LOG_INT) { static int count; @@ -2803,9 +2804,10 @@ static const char * const excp_names[0x80] = { }; #endif -void do_interrupt(int intno) +void do_interrupt(CPUState *env) { int cwp; + int intno = env->exception_index; #ifdef DEBUG_PCALL if (loglevel & CPU_LOG_INT) { @@ -2864,6 +2866,29 @@ void do_interrupt(int intno) } #endif +void arch_handle_interrupt_request(CPUState *env) +{ + handle_interrupt_common(env); + if ((env->interrupt_request & CPU_INTERRUPT_HARD) && (env->psret != 0)) { + int pil = env->interrupt_index & 15; + int type = env->interrupt_index & 0xf0; + + if (((type == TT_EXTINT) && + (pil == 15 || pil > env->psrpil)) || type != TT_EXTINT) { + env->interrupt_request &= ~CPU_INTERRUPT_HARD; + do_interrupt(env); + env->interrupt_index = 0; +#if !defined(TARGET_SPARC64) && !defined(CONFIG_USER_ONLY) + cpu_check_irqs(env); +#endif + reset_tb(); + } + } else if (env->interrupt_request & CPU_INTERRUPT_TIMER) { + //do_interrupt(0, 0, 0, 0, 0); + env->interrupt_request &= ~CPU_INTERRUPT_TIMER; + } +} + #if !defined(CONFIG_USER_ONLY) static void do_unaligned_access(target_ulong addr, int is_write, int is_user, -- 1.5.4.5 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function 2008-05-27 15:18 ` [Qemu-devel] [PATCH 4/6] simply cpu_exec further Glauber Costa @ 2008-05-27 15:18 ` Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 6/6] cpu-exec-dump Glauber Costa ` (2 more replies) 2008-05-27 16:18 ` [Qemu-devel] [PATCH 4/6] simply cpu_exec further Blue Swirl 1 sibling, 3 replies; 22+ messages in thread From: Glauber Costa @ 2008-05-27 15:18 UTC (permalink / raw) To: qemu-devel; +Cc: kvm Simplify cpu_exec by removing a TARGET_I386 block, responsible for conditionally resetting the mmu, and enclosing it inside a function. --- cpu-exec.c | 17 +---------------- exec-all.h | 1 + target-alpha/helper.c | 1 + target-arm/helper.c | 1 + target-cris/helper.c | 1 + target-i386/helper.c | 19 +++++++++++++++++++ target-m68k/helper.c | 2 ++ target-mips/helper.c | 1 + target-ppc/helper.c | 1 + target-sh4/helper.c | 1 + target-sparc/helper.c | 2 ++ 11 files changed, 31 insertions(+), 16 deletions(-) Index: qemu/cpu-exec.c =================================================================== --- qemu.orig/cpu-exec.c +++ qemu/cpu-exec.c @@ -432,15 +432,7 @@ int cpu_exec(CPUState *env1) #endif next_tb = tcg_qemu_tb_exec(tc_ptr); env->current_tb = NULL; - /* reset soft MMU for next block (it can currently - only be set by a memory fault) */ -#if defined(TARGET_I386) && !defined(CONFIG_SOFTMMU) - if (env->hflags & HF_SOFTMMU_MASK) { - env->hflags &= ~HF_SOFTMMU_MASK; - /* do not allow linking to another block */ - next_tb = 0; - } -#endif + cpu_reset_mmu(env); #if defined(USE_KQEMU) #define MIN_CYCLE_BEFORE_SWITCH (100 * 1000) if (kqemu_is_ok(env) && @@ -448,6 +440,7 @@ int cpu_exec(CPUState *env1) cpu_loop_exit(); } #endif + } /* for(;;) */ } else { env_to_regs(); Index: qemu/exec-all.h =================================================================== --- qemu.orig/exec-all.h +++ qemu/exec-all.h @@ -93,6 +93,7 @@ void reset_tb(void); void cpu_load_flags(CPUState *env); void cpu_put_flags(CPUState *env); +void cpu_reset_mmu(CPUState *env); int page_unprotect(target_ulong address, unsigned long pc, void *puc); void tb_invalidate_phys_page_range(target_phys_addr_t start, target_phys_addr_t end, Index: qemu/target-alpha/helper.c =================================================================== --- qemu.orig/target-alpha/helper.c +++ qemu/target-alpha/helper.c @@ -456,6 +456,7 @@ void cpu_dump_EA (target_ulong EA) int cpu_info_ip(CPUState *env, char *buf) { return 0; } void cpu_load_flags(CPUState *env) {} void cpu_put_flags(CPUState *env) {} +void cpu_reset_mmu(CPUState *env) {} void arch_handle_interrupt_request(CPUState *env) { Index: qemu/target-arm/helper.c =================================================================== --- qemu.orig/target-arm/helper.c +++ qemu/target-arm/helper.c @@ -2527,6 +2527,7 @@ int cpu_info_ip(CPUState *env, char *buf void cpu_load_flags(CPUState *env) { } /* XXX: Save/restore host fpu exception state?. */ void cpu_put_flags(CPUState *env) { } +void cpu_reset_mmu(CPUState *env) {} void arch_handle_interrupt_request(CPUState *env) { Index: qemu/target-cris/helper.c =================================================================== --- qemu.orig/target-cris/helper.c +++ qemu/target-cris/helper.c @@ -187,6 +187,7 @@ target_phys_addr_t cpu_get_phys_page_deb int cpu_info_ip(CPUState *env, char *buf) { return 0; } void cpu_load_flags(CPUState *env) {} void cpu_put_flags(CPUState *env) {} +void cpu_reset_mmu(CPUState *env) {} void arch_handle_interrupt_request(CPUState *env) { Index: qemu/target-i386/helper.c =================================================================== --- qemu.orig/target-i386/helper.c +++ qemu/target-i386/helper.c @@ -4755,6 +4755,18 @@ void cpu_put_flags(CPUState *env) env->eflags = env->eflags | cc_table[CC_OP].compute_all() | (DF & DF_MASK); } +void cpu_reset_mmu(CPUState *env) +{ + /* reset soft MMU for next block (it can currently + only be set by a memory fault) */ +#if !defined(CONFIG_SOFTMMU) + if (env->hflags & HF_SOFTMMU_MASK) { + env->hflags &= ~HF_SOFTMMU_MASK; + /* do not allow linking to another block */ + reset_tb(); + } +#endif +} #if defined(CONFIG_USER_ONLY) void helper_vmrun(void) Index: qemu/target-m68k/helper.c =================================================================== --- qemu.orig/target-m68k/helper.c +++ qemu/target-m68k/helper.c @@ -355,6 +355,8 @@ void cpu_put_flags(CPUState *env) | env->cc_dest | (env->cc_x << 4); } +void cpu_reset_mmu(CPUState *env) {} + void arch_handle_interrupt_request(CPUState *env) { if (env->interrupt_request & CPU_INTERRUPT_HARD Index: qemu/target-mips/helper.c =================================================================== --- qemu.orig/target-mips/helper.c +++ qemu/target-mips/helper.c @@ -645,6 +645,7 @@ int cpu_info_ip(CPUState *env, char *buf void cpu_put_flags(CPUState *env) { } void cpu_load_flags(CPUState *env) { } +void cpu_reset_mmu(CPUState *env) { } void arch_handle_interrupt_request(CPUState *env) { Index: qemu/target-ppc/helper.c =================================================================== --- qemu.orig/target-ppc/helper.c +++ qemu/target-ppc/helper.c @@ -2997,6 +2997,7 @@ int cpu_info_ip(CPUState *env, char *buf void cpu_load_flags(CPUState *env) { } void cpu_put_flags(CPUState *env) { } +void cpu_reset_mmu(CPUState *env) { } void arch_handle_interrupt_request(CPUState *env) { Index: qemu/target-sh4/helper.c =================================================================== --- qemu.orig/target-sh4/helper.c +++ qemu/target-sh4/helper.c @@ -538,6 +538,7 @@ void cpu_load_tlb(CPUState * env) int cpu_info_ip(CPUState *env, char *buf) { return 0; } void cpu_load_flags(CPUState *env) {} void cpu_put_flags(CPUState *env) {} +void cpu_reset_mmu(CPUState *env) {} void arch_handle_interrupt_request(CPUState *env) { Index: qemu/target-sparc/helper.c =================================================================== --- qemu.orig/target-sparc/helper.c +++ qemu/target-sparc/helper.c @@ -1340,6 +1340,8 @@ void cpu_put_flags(CPUState *env) #endif } +void cpu_reset_mmu(CPUState *env) {} + #ifdef TARGET_SPARC64 #if !defined(CONFIG_USER_ONLY) #include "qemu-common.h" ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 6/6] cpu-exec-dump 2008-05-27 15:18 ` [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function Glauber Costa @ 2008-05-27 15:18 ` Glauber Costa 2008-05-27 15:38 ` Paul Brook 2008-05-27 15:30 ` [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function Fabrice Bellard 2008-05-27 15:36 ` Paul Brook 2 siblings, 1 reply; 22+ messages in thread From: Glauber Costa @ 2008-05-27 15:18 UTC (permalink / raw) To: qemu-devel; +Cc: kvm enclose the cpu dumping logic in cpu_exec() inside cpu_exec_dump() --- cpu-exec.c | 30 +----------------------------- target-alpha/helper.c | 5 +++++ target-arm/helper.c | 5 +++++ target-cris/helper.c | 5 +++++ target-i386/helper.c | 8 ++++++++ target-m68k/helper.c | 10 ++++++++++ target-mips/helper.c | 5 +++++ target-ppc/helper.c | 4 ++++ target-sh4/helper.c | 5 +++++ target-sparc/helper.c | 9 +++++++++ 10 files changed, 57 insertions(+), 29 deletions(-) Index: qemu/cpu-exec.c =================================================================== --- qemu.orig/cpu-exec.c +++ qemu/cpu-exec.c @@ -369,35 +369,7 @@ int cpu_exec(CPUState *env1) if ((loglevel & CPU_LOG_TB_CPU)) { /* restore flags in standard format */ regs_to_env(); -#if defined(TARGET_I386) - env->eflags = env->eflags | cc_table[CC_OP].compute_all() | (DF & DF_MASK); - cpu_dump_state(env, logfile, fprintf, X86_DUMP_CCOP); - env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C); -#elif defined(TARGET_ARM) - cpu_dump_state(env, logfile, fprintf, 0); -#elif defined(TARGET_SPARC) - REGWPTR = env->regbase + (env->cwp * 16); - env->regwptr = REGWPTR; - cpu_dump_state(env, logfile, fprintf, 0); -#elif defined(TARGET_PPC) - cpu_dump_state(env, logfile, fprintf, 0); -#elif defined(TARGET_M68K) - cpu_m68k_flush_flags(env, env->cc_op); - env->cc_op = CC_OP_FLAGS; - env->sr = (env->sr & 0xffe0) - | env->cc_dest | (env->cc_x << 4); - cpu_dump_state(env, logfile, fprintf, 0); -#elif defined(TARGET_MIPS) - cpu_dump_state(env, logfile, fprintf, 0); -#elif defined(TARGET_SH4) - cpu_dump_state(env, logfile, fprintf, 0); -#elif defined(TARGET_ALPHA) - cpu_dump_state(env, logfile, fprintf, 0); -#elif defined(TARGET_CRIS) - cpu_dump_state(env, logfile, fprintf, 0); -#else -#error unsupported target CPU -#endif + cpu_exec_dump(env); } #endif tb = tb_find_fast(); Index: qemu/target-alpha/helper.c =================================================================== --- qemu.orig/target-alpha/helper.c +++ qemu/target-alpha/helper.c @@ -466,3 +466,8 @@ void arch_handle_interrupt_request(CPUSt reset_tb(); } } + +void cpu_exec_dump(CPUState *env) +{ + cpu_dump_state(env, logfile, fprintf, 0); +} Index: qemu/target-arm/helper.c =================================================================== --- qemu.orig/target-arm/helper.c +++ qemu/target-arm/helper.c @@ -2555,3 +2555,8 @@ void arch_handle_interrupt_request(CPUSt reset_tb(); } } + +void cpu_exec_dump(CPUState *env) +{ + cpu_dump_state(env, logfile, fprintf, 0); +} Index: qemu/target-cris/helper.c =================================================================== --- qemu.orig/target-cris/helper.c +++ qemu/target-cris/helper.c @@ -197,3 +197,8 @@ void arch_handle_interrupt_request(CPUSt reset_tb(); } } + +void cpu_exec_dump(CPUState *env) +{ + cpu_dump_state(env, logfile, fprintf, 0); +} Index: qemu/target-i386/helper.c =================================================================== --- qemu.orig/target-i386/helper.c +++ qemu/target-i386/helper.c @@ -4767,6 +4767,14 @@ void cpu_reset_mmu(CPUState *env) } #endif } + +void cpu_exec_dump(CPUState *env) +{ + env->eflags = env->eflags | cc_table[CC_OP].compute_all() | (DF & DF_MASK); + cpu_dump_state(env, logfile, fprintf, X86_DUMP_CCOP); + env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C); +} + #if defined(CONFIG_USER_ONLY) void helper_vmrun(void) Index: qemu/target-m68k/helper.c =================================================================== --- qemu.orig/target-m68k/helper.c +++ qemu/target-m68k/helper.c @@ -373,6 +373,16 @@ void arch_handle_interrupt_request(CPUSt reset_tb(); } } + +void cpu_exec_dump(CPUState *env) +{ + cpu_m68k_flush_flags(env, env->cc_op); + env->cc_op = CC_OP_FLAGS; + env->sr = (env->sr & 0xffe0) + | env->cc_dest | (env->cc_x << 4); + cpu_dump_state(env, logfile, fprintf, 0); +} + #if defined(CONFIG_USER_ONLY) int cpu_m68k_handle_mmu_fault (CPUState *env, target_ulong address, int rw, Index: qemu/target-mips/helper.c =================================================================== --- qemu.orig/target-mips/helper.c +++ qemu/target-mips/helper.c @@ -663,3 +663,8 @@ void arch_handle_interrupt_request(CPUSt reset_tb(); } } + +void cpu_exec_dump(CPUState *env) +{ + cpu_dump_state(env, logfile, fprintf, 0); +} Index: qemu/target-ppc/helper.c =================================================================== --- qemu.orig/target-ppc/helper.c +++ qemu/target-ppc/helper.c @@ -3014,3 +3014,7 @@ void arch_handle_interrupt_request(CPUSt } } +void cpu_exec_dump(CPUState *env) +{ + cpu_dump_state(env, logfile, fprintf, 0); +} Index: qemu/target-sh4/helper.c =================================================================== --- qemu.orig/target-sh4/helper.c +++ qemu/target-sh4/helper.c @@ -547,3 +547,8 @@ void arch_handle_interrupt_request(CPUSt reset_tb(); } } + +void cpu_exec_dump(CPUState *env) +{ + cpu_dump_state(env, logfile, fprintf, 0); +} Index: qemu/target-sparc/helper.c =================================================================== --- qemu.orig/target-sparc/helper.c +++ qemu/target-sparc/helper.c @@ -1342,6 +1342,15 @@ void cpu_put_flags(CPUState *env) void cpu_reset_mmu(CPUState *env) {} +void cpu_exec_dump(CPUState *env) +{ +#if defined(reg_REGWPTR) + REGWPTR = env->regbase + (env->cwp * 16); + env->regwptr = REGWPTR; +#endif + cpu_dump_state(env, logfile, fprintf, 0); +} + #ifdef TARGET_SPARC64 #if !defined(CONFIG_USER_ONLY) #include "qemu-common.h" ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] cpu-exec-dump 2008-05-27 15:18 ` [Qemu-devel] [PATCH 6/6] cpu-exec-dump Glauber Costa @ 2008-05-27 15:38 ` Paul Brook 0 siblings, 0 replies; 22+ messages in thread From: Paul Brook @ 2008-05-27 15:38 UTC (permalink / raw) To: qemu-devel; +Cc: kvm, Glauber Costa On Tuesday 27 May 2008, Glauber Costa wrote: > - cpu_m68k_flush_flags(env, env->cc_op); > - env->cc_op = CC_OP_FLAGS; > - env->sr = (env->sr & 0xffe0) > - | env->cc_dest | (env->cc_x << 4); This should have been part of your first patch (cpu_put_flags). Paul ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function 2008-05-27 15:18 ` [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 6/6] cpu-exec-dump Glauber Costa @ 2008-05-27 15:30 ` Fabrice Bellard 2008-05-27 15:42 ` Glauber Costa 2008-05-27 15:36 ` Paul Brook 2 siblings, 1 reply; 22+ messages in thread From: Fabrice Bellard @ 2008-05-27 15:30 UTC (permalink / raw) To: qemu-devel This is not the right solution for two reasons: - This part of the code is speed critical, so the function must be at least static inline. - The related code is never used. So just removing the piece of code is the right solution :-) Fabrice. Glauber Costa wrote: > Simplify cpu_exec by removing a TARGET_I386 block, responsible > for conditionally resetting the mmu, and enclosing it inside > a function. > --- > cpu-exec.c | 17 +---------------- > exec-all.h | 1 + > target-alpha/helper.c | 1 + > target-arm/helper.c | 1 + > target-cris/helper.c | 1 + > target-i386/helper.c | 19 +++++++++++++++++++ > target-m68k/helper.c | 2 ++ > target-mips/helper.c | 1 + > target-ppc/helper.c | 1 + > target-sh4/helper.c | 1 + > target-sparc/helper.c | 2 ++ > 11 files changed, 31 insertions(+), 16 deletions(-) > > Index: qemu/cpu-exec.c > =================================================================== > --- qemu.orig/cpu-exec.c > +++ qemu/cpu-exec.c > @@ -432,15 +432,7 @@ int cpu_exec(CPUState *env1) > #endif > next_tb = tcg_qemu_tb_exec(tc_ptr); > env->current_tb = NULL; > - /* reset soft MMU for next block (it can currently > - only be set by a memory fault) */ > -#if defined(TARGET_I386) && !defined(CONFIG_SOFTMMU) > - if (env->hflags & HF_SOFTMMU_MASK) { > - env->hflags &= ~HF_SOFTMMU_MASK; > - /* do not allow linking to another block */ > - next_tb = 0; > - } > -#endif > + cpu_reset_mmu(env); > #if defined(USE_KQEMU) > #define MIN_CYCLE_BEFORE_SWITCH (100 * 1000) > if (kqemu_is_ok(env) && > @@ -448,6 +440,7 @@ int cpu_exec(CPUState *env1) > cpu_loop_exit(); > } > #endif > + > } /* for(;;) */ > } else { > env_to_regs(); > Index: qemu/exec-all.h > =================================================================== > --- qemu.orig/exec-all.h > +++ qemu/exec-all.h > @@ -93,6 +93,7 @@ void reset_tb(void); > > void cpu_load_flags(CPUState *env); > void cpu_put_flags(CPUState *env); > +void cpu_reset_mmu(CPUState *env); > > int page_unprotect(target_ulong address, unsigned long pc, void *puc); > void tb_invalidate_phys_page_range(target_phys_addr_t start, target_phys_addr_t end, > Index: qemu/target-alpha/helper.c > =================================================================== > --- qemu.orig/target-alpha/helper.c > +++ qemu/target-alpha/helper.c > @@ -456,6 +456,7 @@ void cpu_dump_EA (target_ulong EA) > int cpu_info_ip(CPUState *env, char *buf) { return 0; } > void cpu_load_flags(CPUState *env) {} > void cpu_put_flags(CPUState *env) {} > +void cpu_reset_mmu(CPUState *env) {} > > void arch_handle_interrupt_request(CPUState *env) > { > Index: qemu/target-arm/helper.c > =================================================================== > --- qemu.orig/target-arm/helper.c > +++ qemu/target-arm/helper.c > @@ -2527,6 +2527,7 @@ int cpu_info_ip(CPUState *env, char *buf > void cpu_load_flags(CPUState *env) { } > /* XXX: Save/restore host fpu exception state?. */ > void cpu_put_flags(CPUState *env) { } > +void cpu_reset_mmu(CPUState *env) {} > > void arch_handle_interrupt_request(CPUState *env) > { > Index: qemu/target-cris/helper.c > =================================================================== > --- qemu.orig/target-cris/helper.c > +++ qemu/target-cris/helper.c > @@ -187,6 +187,7 @@ target_phys_addr_t cpu_get_phys_page_deb > int cpu_info_ip(CPUState *env, char *buf) { return 0; } > void cpu_load_flags(CPUState *env) {} > void cpu_put_flags(CPUState *env) {} > +void cpu_reset_mmu(CPUState *env) {} > > void arch_handle_interrupt_request(CPUState *env) > { > Index: qemu/target-i386/helper.c > =================================================================== > --- qemu.orig/target-i386/helper.c > +++ qemu/target-i386/helper.c > @@ -4755,6 +4755,18 @@ void cpu_put_flags(CPUState *env) > env->eflags = env->eflags | cc_table[CC_OP].compute_all() | (DF & DF_MASK); > } > > +void cpu_reset_mmu(CPUState *env) > +{ > + /* reset soft MMU for next block (it can currently > + only be set by a memory fault) */ > +#if !defined(CONFIG_SOFTMMU) > + if (env->hflags & HF_SOFTMMU_MASK) { > + env->hflags &= ~HF_SOFTMMU_MASK; > + /* do not allow linking to another block */ > + reset_tb(); > + } > +#endif > +} > #if defined(CONFIG_USER_ONLY) > > void helper_vmrun(void) > Index: qemu/target-m68k/helper.c > =================================================================== > --- qemu.orig/target-m68k/helper.c > +++ qemu/target-m68k/helper.c > @@ -355,6 +355,8 @@ void cpu_put_flags(CPUState *env) > | env->cc_dest | (env->cc_x << 4); > } > > +void cpu_reset_mmu(CPUState *env) {} > + > void arch_handle_interrupt_request(CPUState *env) > { > if (env->interrupt_request & CPU_INTERRUPT_HARD > Index: qemu/target-mips/helper.c > =================================================================== > --- qemu.orig/target-mips/helper.c > +++ qemu/target-mips/helper.c > @@ -645,6 +645,7 @@ int cpu_info_ip(CPUState *env, char *buf > > void cpu_put_flags(CPUState *env) { } > void cpu_load_flags(CPUState *env) { } > +void cpu_reset_mmu(CPUState *env) { } > > void arch_handle_interrupt_request(CPUState *env) > { > Index: qemu/target-ppc/helper.c > =================================================================== > --- qemu.orig/target-ppc/helper.c > +++ qemu/target-ppc/helper.c > @@ -2997,6 +2997,7 @@ int cpu_info_ip(CPUState *env, char *buf > > void cpu_load_flags(CPUState *env) { } > void cpu_put_flags(CPUState *env) { } > +void cpu_reset_mmu(CPUState *env) { } > > void arch_handle_interrupt_request(CPUState *env) > { > Index: qemu/target-sh4/helper.c > =================================================================== > --- qemu.orig/target-sh4/helper.c > +++ qemu/target-sh4/helper.c > @@ -538,6 +538,7 @@ void cpu_load_tlb(CPUState * env) > int cpu_info_ip(CPUState *env, char *buf) { return 0; } > void cpu_load_flags(CPUState *env) {} > void cpu_put_flags(CPUState *env) {} > +void cpu_reset_mmu(CPUState *env) {} > > void arch_handle_interrupt_request(CPUState *env) > { > Index: qemu/target-sparc/helper.c > =================================================================== > --- qemu.orig/target-sparc/helper.c > +++ qemu/target-sparc/helper.c > @@ -1340,6 +1340,8 @@ void cpu_put_flags(CPUState *env) > #endif > } > > +void cpu_reset_mmu(CPUState *env) {} > + > #ifdef TARGET_SPARC64 > #if !defined(CONFIG_USER_ONLY) > #include "qemu-common.h" > > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function 2008-05-27 15:30 ` [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function Fabrice Bellard @ 2008-05-27 15:42 ` Glauber Costa 0 siblings, 0 replies; 22+ messages in thread From: Glauber Costa @ 2008-05-27 15:42 UTC (permalink / raw) To: qemu-devel On Tue, May 27, 2008 at 12:30 PM, Fabrice Bellard <fabrice@bellard.org> wrote: > This is not the right solution for two reasons: > - This part of the code is speed critical, so the function must be at least > static inline. > - The related code is never used. > > So just removing the piece of code is the right solution :-) If it can really be safely removed, this sounds like the best solution to me by far. > Fabrice. > > Glauber Costa wrote: >> >> Simplify cpu_exec by removing a TARGET_I386 block, responsible >> for conditionally resetting the mmu, and enclosing it inside >> a function. >> --- >> cpu-exec.c | 17 +---------------- >> exec-all.h | 1 + >> target-alpha/helper.c | 1 + >> target-arm/helper.c | 1 + >> target-cris/helper.c | 1 + >> target-i386/helper.c | 19 +++++++++++++++++++ >> target-m68k/helper.c | 2 ++ >> target-mips/helper.c | 1 + >> target-ppc/helper.c | 1 + >> target-sh4/helper.c | 1 + >> target-sparc/helper.c | 2 ++ >> 11 files changed, 31 insertions(+), 16 deletions(-) >> >> Index: qemu/cpu-exec.c >> =================================================================== >> --- qemu.orig/cpu-exec.c >> +++ qemu/cpu-exec.c >> @@ -432,15 +432,7 @@ int cpu_exec(CPUState *env1) >> #endif >> next_tb = tcg_qemu_tb_exec(tc_ptr); >> env->current_tb = NULL; >> - /* reset soft MMU for next block (it can currently >> - only be set by a memory fault) */ >> -#if defined(TARGET_I386) && !defined(CONFIG_SOFTMMU) >> - if (env->hflags & HF_SOFTMMU_MASK) { >> - env->hflags &= ~HF_SOFTMMU_MASK; >> - /* do not allow linking to another block */ >> - next_tb = 0; >> - } >> -#endif >> + cpu_reset_mmu(env); >> #if defined(USE_KQEMU) >> #define MIN_CYCLE_BEFORE_SWITCH (100 * 1000) >> if (kqemu_is_ok(env) && >> @@ -448,6 +440,7 @@ int cpu_exec(CPUState *env1) >> cpu_loop_exit(); >> } >> #endif >> + >> } /* for(;;) */ >> } else { >> env_to_regs(); >> Index: qemu/exec-all.h >> =================================================================== >> --- qemu.orig/exec-all.h >> +++ qemu/exec-all.h >> @@ -93,6 +93,7 @@ void reset_tb(void); >> void cpu_load_flags(CPUState *env); >> void cpu_put_flags(CPUState *env); >> +void cpu_reset_mmu(CPUState *env); >> int page_unprotect(target_ulong address, unsigned long pc, void *puc); >> void tb_invalidate_phys_page_range(target_phys_addr_t start, >> target_phys_addr_t end, >> Index: qemu/target-alpha/helper.c >> =================================================================== >> --- qemu.orig/target-alpha/helper.c >> +++ qemu/target-alpha/helper.c >> @@ -456,6 +456,7 @@ void cpu_dump_EA (target_ulong EA) >> int cpu_info_ip(CPUState *env, char *buf) { return 0; } >> void cpu_load_flags(CPUState *env) {} >> void cpu_put_flags(CPUState *env) {} >> +void cpu_reset_mmu(CPUState *env) {} >> void arch_handle_interrupt_request(CPUState *env) >> { >> Index: qemu/target-arm/helper.c >> =================================================================== >> --- qemu.orig/target-arm/helper.c >> +++ qemu/target-arm/helper.c >> @@ -2527,6 +2527,7 @@ int cpu_info_ip(CPUState *env, char *buf >> void cpu_load_flags(CPUState *env) { } >> /* XXX: Save/restore host fpu exception state?. */ >> void cpu_put_flags(CPUState *env) { } >> +void cpu_reset_mmu(CPUState *env) {} >> void arch_handle_interrupt_request(CPUState *env) >> { >> Index: qemu/target-cris/helper.c >> =================================================================== >> --- qemu.orig/target-cris/helper.c >> +++ qemu/target-cris/helper.c >> @@ -187,6 +187,7 @@ target_phys_addr_t cpu_get_phys_page_deb >> int cpu_info_ip(CPUState *env, char *buf) { return 0; } >> void cpu_load_flags(CPUState *env) {} >> void cpu_put_flags(CPUState *env) {} >> +void cpu_reset_mmu(CPUState *env) {} >> void arch_handle_interrupt_request(CPUState *env) >> { >> Index: qemu/target-i386/helper.c >> =================================================================== >> --- qemu.orig/target-i386/helper.c >> +++ qemu/target-i386/helper.c >> @@ -4755,6 +4755,18 @@ void cpu_put_flags(CPUState *env) >> env->eflags = env->eflags | cc_table[CC_OP].compute_all() | (DF & >> DF_MASK); >> } >> +void cpu_reset_mmu(CPUState *env) >> +{ >> + /* reset soft MMU for next block (it can currently >> + only be set by a memory fault) */ >> +#if !defined(CONFIG_SOFTMMU) >> + if (env->hflags & HF_SOFTMMU_MASK) { >> + env->hflags &= ~HF_SOFTMMU_MASK; >> + /* do not allow linking to another block */ >> + reset_tb(); >> + } >> +#endif >> +} >> #if defined(CONFIG_USER_ONLY) >> void helper_vmrun(void) Index: qemu/target-m68k/helper.c >> =================================================================== >> --- qemu.orig/target-m68k/helper.c >> +++ qemu/target-m68k/helper.c >> @@ -355,6 +355,8 @@ void cpu_put_flags(CPUState *env) >> | env->cc_dest | (env->cc_x << 4); >> } >> +void cpu_reset_mmu(CPUState *env) {} >> + >> void arch_handle_interrupt_request(CPUState *env) >> { >> if (env->interrupt_request & CPU_INTERRUPT_HARD >> Index: qemu/target-mips/helper.c >> =================================================================== >> --- qemu.orig/target-mips/helper.c >> +++ qemu/target-mips/helper.c >> @@ -645,6 +645,7 @@ int cpu_info_ip(CPUState *env, char *buf >> void cpu_put_flags(CPUState *env) { } >> void cpu_load_flags(CPUState *env) { } >> +void cpu_reset_mmu(CPUState *env) { } >> void arch_handle_interrupt_request(CPUState *env) >> { >> Index: qemu/target-ppc/helper.c >> =================================================================== >> --- qemu.orig/target-ppc/helper.c >> +++ qemu/target-ppc/helper.c >> @@ -2997,6 +2997,7 @@ int cpu_info_ip(CPUState *env, char *buf >> void cpu_load_flags(CPUState *env) { } >> void cpu_put_flags(CPUState *env) { } >> +void cpu_reset_mmu(CPUState *env) { } >> void arch_handle_interrupt_request(CPUState *env) >> { >> Index: qemu/target-sh4/helper.c >> =================================================================== >> --- qemu.orig/target-sh4/helper.c >> +++ qemu/target-sh4/helper.c >> @@ -538,6 +538,7 @@ void cpu_load_tlb(CPUState * env) >> int cpu_info_ip(CPUState *env, char *buf) { return 0; } >> void cpu_load_flags(CPUState *env) {} >> void cpu_put_flags(CPUState *env) {} >> +void cpu_reset_mmu(CPUState *env) {} >> void arch_handle_interrupt_request(CPUState *env) >> { >> Index: qemu/target-sparc/helper.c >> =================================================================== >> --- qemu.orig/target-sparc/helper.c >> +++ qemu/target-sparc/helper.c >> @@ -1340,6 +1340,8 @@ void cpu_put_flags(CPUState *env) >> #endif >> } >> +void cpu_reset_mmu(CPUState *env) {} >> + >> #ifdef TARGET_SPARC64 >> #if !defined(CONFIG_USER_ONLY) >> #include "qemu-common.h" >> >> >> > > > > -- Glauber Costa. "Free as in Freedom" http://glommer.net "The less confident you are, the more serious you have to act." ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function 2008-05-27 15:18 ` [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 6/6] cpu-exec-dump Glauber Costa 2008-05-27 15:30 ` [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function Fabrice Bellard @ 2008-05-27 15:36 ` Paul Brook 2 siblings, 0 replies; 22+ messages in thread From: Paul Brook @ 2008-05-27 15:36 UTC (permalink / raw) To: qemu-devel; +Cc: kvm, Glauber Costa On Tuesday 27 May 2008, Glauber Costa wrote: > Simplify cpu_exec by removing a TARGET_I386 block, responsible > for conditionally resetting the mmu, and enclosing it inside > a function. I suspect this doesn't do what you think it does. My suggestion is to remove the !CONFIG_SOFTMMU bits altogether. I expect they're fairly bitrotten. Paul ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 4/6] simply cpu_exec further 2008-05-27 15:18 ` [Qemu-devel] [PATCH 4/6] simply cpu_exec further Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function Glauber Costa @ 2008-05-27 16:18 ` Blue Swirl 2008-05-27 16:27 ` Glauber Costa 1 sibling, 1 reply; 22+ messages in thread From: Blue Swirl @ 2008-05-27 16:18 UTC (permalink / raw) To: qemu-devel; +Cc: kvm On 5/27/08, Glauber Costa <gcosta@redhat.com> wrote: > We change interrupt functions so they have the same > signature, getting only an env parameter. When necessary, > some more attributed were added to the relevant CPUState to > make it possible. > +void arch_handle_interrupt_request(CPUState *env); I'd use cpu_handle_interrupt_request(). > --- a/target-sparc/op_helper.c > +++ b/target-sparc/op_helper.c > @@ -2689,8 +2689,9 @@ static const char * const excp_names[0x50] = { > }; > #endif > > -void do_interrupt(int intno) > +void do_interrupt(CPUState *cpu) env1 would be better. > { > + int intno = cpu->exception_index; > #ifdef DEBUG_PCALL > if (loglevel & CPU_LOG_INT) { > static int count; > @@ -2803,9 +2804,10 @@ static const char * const excp_names[0x80] = { > }; > #endif > > -void do_interrupt(int intno) > +void do_interrupt(CPUState *env) Same here. Otherwise you shadow the AREG0 env. > +void arch_handle_interrupt_request(CPUState *env) And here. I think this function and do_interrupt should belong to helper.c, they are not called from ops. I'll do some rearranging. After that my env comment does not matter. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 4/6] simply cpu_exec further 2008-05-27 16:18 ` [Qemu-devel] [PATCH 4/6] simply cpu_exec further Blue Swirl @ 2008-05-27 16:27 ` Glauber Costa 0 siblings, 0 replies; 22+ messages in thread From: Glauber Costa @ 2008-05-27 16:27 UTC (permalink / raw) To: Blue Swirl; +Cc: qemu-devel, kvm On Tue, May 27, 2008 at 1:18 PM, Blue Swirl <blauwirbel@gmail.com> wrote: > On 5/27/08, Glauber Costa <gcosta@redhat.com> wrote: >> We change interrupt functions so they have the same >> signature, getting only an env parameter. When necessary, >> some more attributed were added to the relevant CPUState to >> make it possible. > >> +void arch_handle_interrupt_request(CPUState *env); > > I'd use cpu_handle_interrupt_request(). fair enough. It does not follow my own naming scheme. > >> --- a/target-sparc/op_helper.c >> +++ b/target-sparc/op_helper.c >> @@ -2689,8 +2689,9 @@ static const char * const excp_names[0x50] = { >> }; >> #endif >> >> -void do_interrupt(int intno) >> +void do_interrupt(CPUState *cpu) > > env1 would be better. > >> { >> + int intno = cpu->exception_index; >> #ifdef DEBUG_PCALL >> if (loglevel & CPU_LOG_INT) { >> static int count; >> @@ -2803,9 +2804,10 @@ static const char * const excp_names[0x80] = { >> }; >> #endif >> >> -void do_interrupt(int intno) >> +void do_interrupt(CPUState *env) > > Same here. Otherwise you shadow the AREG0 env. > >> +void arch_handle_interrupt_request(CPUState *env) > > And here. > > I think this function and do_interrupt should belong to helper.c, they > are not called from ops. I'll do some rearranging. After that my env > comment does not matter. Agreed. I had problems compiling this in helper.c, but if you can move the code so to better acomodate those, I'd happily change > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Glauber Costa. "Free as in Freedom" http://glommer.net "The less confident you are, the more serious you have to act." ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too. 2008-05-27 15:18 ` [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 4/6] simply cpu_exec further Glauber Costa @ 2008-05-27 15:34 ` Fabrice Bellard 2008-05-27 16:10 ` Blue Swirl 2008-05-27 16:25 ` Glauber Costa 1 sibling, 2 replies; 22+ messages in thread From: Fabrice Bellard @ 2008-05-27 15:34 UTC (permalink / raw) To: qemu-devel Glauber Costa wrote: > Unlike other architectures, i386 lacked a "halted" attribute, going > with a flag into hflags. By using the halted attribute, we can make > the code look like more other architectures, and simplify the code in > some instances. In this commit, we make the code for info_cpus simpler > in monitor.c Good for the I386 halted attribute, as it was a mistake to put in in the hflags. For the memory, hflags should contain only parts of the CPU state known at translation time and should be equal to tb->flags. Most CPUs (including x86 !) do not follow this sane rule. For cpu_info_ip, it would be simpler to return an uint64_t containing the PC. I don't think the sparc case with npc really matters. Fabrice. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too. 2008-05-27 15:34 ` [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too Fabrice Bellard @ 2008-05-27 16:10 ` Blue Swirl 2008-05-27 21:49 ` Glauber Costa 2008-05-27 16:25 ` Glauber Costa 1 sibling, 1 reply; 22+ messages in thread From: Blue Swirl @ 2008-05-27 16:10 UTC (permalink / raw) To: qemu-devel On 5/27/08, Fabrice Bellard <fabrice@bellard.org> wrote: > Glauber Costa wrote: > > > Unlike other architectures, i386 lacked a "halted" attribute, going > > with a flag into hflags. By using the halted attribute, we can make > > the code look like more other architectures, and simplify the code in > > some instances. In this commit, we make the code for info_cpus simpler > > in monitor.c > > > > Good for the I386 halted attribute, as it was a mistake to put in in the > hflags. For the memory, hflags should contain only parts of the CPU state > known at translation time and should be equal to tb->flags. Most CPUs > (including x86 !) do not follow this sane rule. > > For cpu_info_ip, it would be simpler to return an uint64_t containing the > PC. I don't think the sparc case with npc really matters. It's not very likely that the NPC would be different for some value of PC when halted, but it's still nice. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too. 2008-05-27 16:10 ` Blue Swirl @ 2008-05-27 21:49 ` Glauber Costa 0 siblings, 0 replies; 22+ messages in thread From: Glauber Costa @ 2008-05-27 21:49 UTC (permalink / raw) To: qemu-devel On Tue, May 27, 2008 at 1:10 PM, Blue Swirl <blauwirbel@gmail.com> wrote: > On 5/27/08, Fabrice Bellard <fabrice@bellard.org> wrote: >> Glauber Costa wrote: >> >> > Unlike other architectures, i386 lacked a "halted" attribute, going >> > with a flag into hflags. By using the halted attribute, we can make >> > the code look like more other architectures, and simplify the code in >> > some instances. In this commit, we make the code for info_cpus simpler >> > in monitor.c >> > >> >> Good for the I386 halted attribute, as it was a mistake to put in in the >> hflags. For the memory, hflags should contain only parts of the CPU state >> known at translation time and should be equal to tb->flags. Most CPUs >> (including x86 !) do not follow this sane rule. >> >> For cpu_info_ip, it would be simpler to return an uint64_t containing the >> PC. I don't think the sparc case with npc really matters. > > It's not very likely that the NPC would be different for some value of > PC when halted, but it's still nice. BS, do you have another suggestion besides keeping th current code as is? > -- Glauber Costa. "Free as in Freedom" http://glommer.net "The less confident you are, the more serious you have to act." ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too. 2008-05-27 15:34 ` [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too Fabrice Bellard 2008-05-27 16:10 ` Blue Swirl @ 2008-05-27 16:25 ` Glauber Costa 2008-05-27 21:21 ` Fabrice Bellard 1 sibling, 1 reply; 22+ messages in thread From: Glauber Costa @ 2008-05-27 16:25 UTC (permalink / raw) To: qemu-devel On Tue, May 27, 2008 at 12:34 PM, Fabrice Bellard <fabrice@bellard.org> wrote: > Glauber Costa wrote: >> >> Unlike other architectures, i386 lacked a "halted" attribute, going >> with a flag into hflags. By using the halted attribute, we can make >> the code look like more other architectures, and simplify the code in >> some instances. In this commit, we make the code for info_cpus simpler >> in monitor.c > > Good for the I386 halted attribute, as it was a mistake to put in in the > hflags. For the memory, hflags should contain only parts of the CPU state > known at translation time and should be equal to tb->flags. Most CPUs > (including x86 !) do not follow this sane rule. What about HF_GIF_MASK? It looks like another case of something that is mistakenly put into hflags. What do you say? > For cpu_info_ip, it would be simpler to return an uint64_t containing the > PC. I don't think the sparc case with npc really matters. > > Fabrice. > > > > -- Glauber Costa. "Free as in Freedom" http://glommer.net "The less confident you are, the more serious you have to act." ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too. 2008-05-27 16:25 ` Glauber Costa @ 2008-05-27 21:21 ` Fabrice Bellard 0 siblings, 0 replies; 22+ messages in thread From: Fabrice Bellard @ 2008-05-27 21:21 UTC (permalink / raw) To: qemu-devel Glauber Costa wrote: > On Tue, May 27, 2008 at 12:34 PM, Fabrice Bellard <fabrice@bellard.org> wrote: >> Glauber Costa wrote: >>> Unlike other architectures, i386 lacked a "halted" attribute, going >>> with a flag into hflags. By using the halted attribute, we can make >>> the code look like more other architectures, and simplify the code in >>> some instances. In this commit, we make the code for info_cpus simpler >>> in monitor.c >> Good for the I386 halted attribute, as it was a mistake to put in in the >> hflags. For the memory, hflags should contain only parts of the CPU state >> known at translation time and should be equal to tb->flags. Most CPUs >> (including x86 !) do not follow this sane rule. > > What about HF_GIF_MASK? It looks like another case of something that > is mistakenly put into hflags. What do you say? This is true for: HF_GIF_MASK, HF_HIF_MASK, HF_NMI_MASK. Fabrice. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 2/6] Push common interrupt variables to cpu-defs.h 2008-05-27 15:18 ` [Qemu-devel] [PATCH 2/6] Push common interrupt variables to cpu-defs.h Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too Glauber Costa @ 2008-05-27 15:39 ` Fabrice Bellard 1 sibling, 0 replies; 22+ messages in thread From: Fabrice Bellard @ 2008-05-27 15:39 UTC (permalink / raw) To: qemu-devel Glauber Costa wrote: > Some interrupt-related attributes, which includes the jmp_buf, > are present in all, or almost all, architectures. So move them > to common code in cpu-defs.h instead of replicating them everywhere OK for jmp_buf and exception_index, but not for error_code as it is CPU specific (it is present on other CPUs because the authors choose to use the same naming as on x86). Fabrice. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] simplify cpu_exec 2008-05-27 15:18 ` [Qemu-devel] [PATCH 1/6] simplify cpu_exec Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 2/6] Push common interrupt variables to cpu-defs.h Glauber Costa @ 2008-05-27 15:33 ` Paul Brook 2008-05-27 16:05 ` Blue Swirl 2008-05-27 15:37 ` Fabrice Bellard 2 siblings, 1 reply; 22+ messages in thread From: Paul Brook @ 2008-05-27 15:33 UTC (permalink / raw) To: qemu-devel; +Cc: kvm, Glauber Costa > +void cpu_load_flags(CPUState *env); > +void cpu_put_flags(CPUState *env); These really need a comment saying what they do. The names are not at all intuitive. Also, why load/put? I'd expect load/save/store or get/put. > +#if defined(reg_REGWPTR) > +static uint32_t *saved_regwptr; > +#endif Eww. You've replaced reasonably self contained local code with a global variable hidden deep in target specific code. I plan on adding multithreaded qemu support soon, so this is a non-starter. Paul ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] simplify cpu_exec 2008-05-27 15:33 ` [Qemu-devel] [PATCH 1/6] simplify cpu_exec Paul Brook @ 2008-05-27 16:05 ` Blue Swirl 0 siblings, 0 replies; 22+ messages in thread From: Blue Swirl @ 2008-05-27 16:05 UTC (permalink / raw) To: qemu-devel; +Cc: kvm, Glauber Costa On 5/27/08, Paul Brook <paul@codesourcery.com> wrote: > > +void cpu_load_flags(CPUState *env); > > +void cpu_put_flags(CPUState *env); > > > These really need a comment saying what they do. The names are not at all > intuitive. Also, why load/put? I'd expect load/save/store or get/put. > > > > +#if defined(reg_REGWPTR) > > +static uint32_t *saved_regwptr; > > +#endif > > > Eww. You've replaced reasonably self contained local code with a global > variable hidden deep in target specific code. I plan on adding multithreaded > qemu support soon, so this is a non-starter. REGWPTR code can be removed, it's not used now. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] simplify cpu_exec 2008-05-27 15:18 ` [Qemu-devel] [PATCH 1/6] simplify cpu_exec Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 2/6] Push common interrupt variables to cpu-defs.h Glauber Costa 2008-05-27 15:33 ` [Qemu-devel] [PATCH 1/6] simplify cpu_exec Paul Brook @ 2008-05-27 15:37 ` Fabrice Bellard 2 siblings, 0 replies; 22+ messages in thread From: Fabrice Bellard @ 2008-05-27 15:37 UTC (permalink / raw) To: qemu-devel Glauber Costa wrote: > This is a first attempt to simplify cpu_exec(): it has simply > too many ifdefs, which is not a very good practice at all. > > Following some work I've already posted in the past, I'm moving the target-b > ifdefs to target-xxx/helper.c, encapsuled into relevant functions. OK for that, except that the naming is confusing because the related functions may not deal with the "flags". cpu_env_to_exec() and cpu_exec_to_env() might be better. Moreover, using inline functions (in exec.h) should be better. Fabrice. ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2008-05-27 21:49 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-05-27 15:18 [Qemu-devel] [PATCH 0/6] simplify cpu-exec new spin Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 1/6] simplify cpu_exec Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 2/6] Push common interrupt variables to cpu-defs.h Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 4/6] simply cpu_exec further Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function Glauber Costa 2008-05-27 15:18 ` [Qemu-devel] [PATCH 6/6] cpu-exec-dump Glauber Costa 2008-05-27 15:38 ` Paul Brook 2008-05-27 15:30 ` [Qemu-devel] [PATCH 5/6] isolate mmu code in arch-specific function Fabrice Bellard 2008-05-27 15:42 ` Glauber Costa 2008-05-27 15:36 ` Paul Brook 2008-05-27 16:18 ` [Qemu-devel] [PATCH 4/6] simply cpu_exec further Blue Swirl 2008-05-27 16:27 ` Glauber Costa 2008-05-27 15:34 ` [Qemu-devel] [PATCH 3/6] use halted attribute for i386 too Fabrice Bellard 2008-05-27 16:10 ` Blue Swirl 2008-05-27 21:49 ` Glauber Costa 2008-05-27 16:25 ` Glauber Costa 2008-05-27 21:21 ` Fabrice Bellard 2008-05-27 15:39 ` [Qemu-devel] [PATCH 2/6] Push common interrupt variables to cpu-defs.h Fabrice Bellard 2008-05-27 15:33 ` [Qemu-devel] [PATCH 1/6] simplify cpu_exec Paul Brook 2008-05-27 16:05 ` Blue Swirl 2008-05-27 15:37 ` Fabrice Bellard
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).