From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KJS6h-00065K-SZ for qemu-devel@nongnu.org; Thu, 17 Jul 2008 07:57:11 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KJS6h-00064a-5S for qemu-devel@nongnu.org; Thu, 17 Jul 2008 07:57:11 -0400 Received: from [199.232.76.173] (port=55472 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KJS6g-00064J-Pp for qemu-devel@nongnu.org; Thu, 17 Jul 2008 07:57:10 -0400 Received: from gecko.sbs.de ([194.138.37.40]:20656) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1KJS6g-0004O1-Cj for qemu-devel@nongnu.org; Thu, 17 Jul 2008 07:57:10 -0400 Received: from mail1.sbs.de (localhost [127.0.0.1]) by gecko.sbs.de (8.12.11.20060308/8.12.11) with ESMTP id m6HBv7Na032444 for ; Thu, 17 Jul 2008 13:57:08 +0200 Received: from [139.25.109.167] (mchn012c.mchp.siemens.de [139.25.109.167] (may be forged)) by mail1.sbs.de (8.12.11.20060308/8.12.11) with ESMTP id m6HBv7Um019301 for ; Thu, 17 Jul 2008 13:57:07 +0200 Message-ID: <487F3393.3040609@siemens.com> Date: Thu, 17 Jul 2008 13:57:07 +0200 From: Jan Kiszka MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] [RFC][PATCH] x86: CS limit checks Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Here is a proposal for adding code segment limit checks to x86. This patch should not need the -seg-checks switch as its tests are mostly performed during translation time. Moreover, I tried to confine the small additional overhead in the TB lookup procedure to x86 and Sparc. Note that this patch depends on my debugging series, namely [1], as that one reduces the x86-specific code passages for TB generation. Also note that this patch is early and only lightly tested so far, not yet intended for inclusion, but definitely for commenting on! Jan [1] http://permalink.gmane.org/gmane.comp.emulators.qemu/26993 --- cpu-exec.c | 23 ++++++++++++++--------- exec-all.h | 29 +++++++++++++++++++++++++++-- exec.c | 8 +++++--- target-i386/translate.c | 15 ++++++++++++--- 4 files changed, 58 insertions(+), 17 deletions(-) Index: b/exec-all.h =================================================================== --- a/exec-all.h +++ b/exec-all.h @@ -78,8 +78,8 @@ int cpu_restore_state_copy(struct Transl void cpu_resume_from_signal(CPUState *env1, void *puc); void cpu_io_recompile(CPUState *env, void *retaddr); TranslationBlock *tb_gen_code(CPUState *env, - target_ulong pc, target_ulong cs_base, int flags, - int cflags); + target_ulong pc, target_ulong cs_base, + target_ulong cs_limit, int flags, int cflags); void cpu_exec_init(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, @@ -125,6 +125,7 @@ static inline int tlb_set_page(CPUState struct TranslationBlock { target_ulong pc; /* simulated PC corresponding to this block (EIP + CS base) */ target_ulong cs_base; /* CS base for this block */ + target_ulong cs_limit; /* CS limit for this block */ 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) */ @@ -157,6 +158,30 @@ struct TranslationBlock { uint32_t icount; }; +#if defined(TARGET_I386) || defined(TARGET_SPARC) +static inline int tb_cs_base_equal(TranslationBlock *tb, target_ulong cs_base) +{ + return (tb->cs_base == cs_base); +} +#else +static inline int tb_cs_base_equal(TranslationBlock *tb, target_ulong cs_base) +{ + return 1; +} +#endif + +#if defined(TARGET_I386) +static inline int tb_cs_limit_equal(TranslationBlock *tb, target_ulong cs_limit) +{ + return (tb->cs_limit == cs_limit); +} +#else +static inline int tb_cs_limit_equal(TranslationBlock *tb, target_ulong cs_limit) +{ + return 1; +} +#endif + static inline unsigned int tb_jmp_cache_hash_page(target_ulong pc) { target_ulong tmp; Index: b/exec.c =================================================================== --- a/exec.c +++ b/exec.c @@ -810,7 +810,7 @@ static void build_page_bitmap(PageDesc * TranslationBlock *tb_gen_code(CPUState *env, target_ulong pc, target_ulong cs_base, - int flags, int cflags) + target_ulong cs_limit, int flags, int cflags) { TranslationBlock *tb; uint8_t *tc_ptr; @@ -830,6 +830,7 @@ TranslationBlock *tb_gen_code(CPUState * tc_ptr = code_gen_ptr; tb->tc_ptr = tc_ptr; tb->cs_base = cs_base; + tb->cs_limit = cs_limit; tb->flags = flags; tb->cflags = cflags; cpu_gen_code(env, tb, &code_gen_size); @@ -3133,7 +3134,7 @@ void cpu_io_recompile(CPUState *env, voi { TranslationBlock *tb; uint32_t n, cflags; - target_ulong pc, cs_base; + target_ulong pc, cs_base, cs_limit; uint64_t flags; tb = tb_find_pc((unsigned long)retaddr); @@ -3173,11 +3174,12 @@ void cpu_io_recompile(CPUState *env, voi cflags = n | CF_LAST_IO; pc = tb->pc; cs_base = tb->cs_base; + cs_limit = tb->cs_limit; flags = tb->flags; tb_phys_invalidate(tb, -1); /* FIXME: In theory this could raise an exception. In practice we have already translated the block once so it's probably ok. */ - tb_gen_code(env, pc, cs_base, flags, cflags); + tb_gen_code(env, pc, cs_base, cs_limit, flags, cflags); /* TODO: If env->pc != tb->pc (i.e. the faulting instruction was not the first in the TB) then we end up generating a whole new TB and repeating the fault, which is horribly inefficient. Index: b/cpu-exec.c =================================================================== --- a/cpu-exec.c +++ b/cpu-exec.c @@ -94,8 +94,8 @@ static void cpu_exec_nocache(int max_cyc if (max_cycles > CF_COUNT_MASK) max_cycles = CF_COUNT_MASK; - tb = tb_gen_code(env, orig_tb->pc, orig_tb->cs_base, orig_tb->flags, - max_cycles); + tb = tb_gen_code(env, orig_tb->pc, orig_tb->cs_base, orig_tb->cs_limit, + orig_tb->flags, max_cycles); env->current_tb = tb; /* execute the generated code */ next_tb = tcg_qemu_tb_exec(tb->tc_ptr); @@ -109,9 +109,8 @@ static void cpu_exec_nocache(int max_cyc tb_free(tb); } -static TranslationBlock *tb_find_slow(target_ulong pc, - target_ulong cs_base, - uint64_t flags) +static TranslationBlock *tb_find_slow(target_ulong pc, target_ulong cs_base, + target_ulong cs_limit, uint64_t flags) { TranslationBlock *tb, **ptb1; unsigned int h; @@ -133,7 +132,8 @@ static TranslationBlock *tb_find_slow(ta goto not_found; if (tb->pc == pc && tb->page_addr[0] == phys_page1 && - tb->cs_base == cs_base && + tb_cs_base_equal(tb, cs_base) && + tb_cs_limit_equal(tb, cs_limit) && tb->flags == flags) { /* check next page if needed */ if (tb->page_addr[1] != -1) { @@ -150,7 +150,7 @@ static TranslationBlock *tb_find_slow(ta } not_found: /* if no translated code available, then translate it now */ - tb = tb_gen_code(env, pc, cs_base, flags, env->next_cflags); + tb = tb_gen_code(env, pc, cs_base, cs_limit, flags, env->next_cflags); env->next_cflags = 0; found: @@ -163,6 +163,7 @@ static inline TranslationBlock *tb_find_ { TranslationBlock *tb; target_ulong cs_base, pc; + target_ulong cs_limit = (target_ulong)-1; uint64_t flags; /* we record a subset of the CPU state. It will @@ -172,6 +173,8 @@ static inline TranslationBlock *tb_find_ flags = env->hflags; flags |= (env->eflags & (IOPL_MASK | TF_MASK | VM_MASK)); cs_base = env->segs[R_CS].base; + if ((env->hflags & (HF_PE_MASK | HF_CS64_MASK)) == HF_PE_MASK) + cs_limit = env->segs[R_CS].limit; pc = cs_base + env->eip; #elif defined(TARGET_ARM) flags = env->thumb | (env->vfp.vec_len << 1) @@ -225,9 +228,11 @@ static inline TranslationBlock *tb_find_ #error unsupported CPU #endif tb = env->tb_jmp_cache[tb_jmp_cache_hash_func(pc)]; - if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base || + if (unlikely(!tb || tb->pc != pc || + !tb_cs_base_equal(tb, cs_base) || + !tb_cs_limit_equal(tb, cs_limit) || tb->flags != flags)) { - tb = tb_find_slow(pc, cs_base, flags); + tb = tb_find_slow(pc, cs_base, cs_limit, flags); } return tb; } Index: b/target-i386/translate.c =================================================================== --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -81,6 +81,7 @@ typedef struct DisasContext { static state change (stop translation) */ /* current block context */ target_ulong cs_base; /* base of CS segment */ + target_ulong cs_limit; /* limit of the CS segment */ int pe; /* protected mode */ int code32; /* 32 bit code segment */ #ifdef TARGET_X86_64 @@ -3687,6 +3688,8 @@ static target_ulong disas_insn(DisasCont int modrm, reg, rm, mod, reg_addr, op, opreg, offset_addr, val, seg_reg; target_ulong next_eip, tval; int rex_w, rex_r; + uint16_t *gen_opc_start = gen_opc_ptr; + TCGArg *gen_opparam_start = gen_opparam_ptr; if (unlikely(loglevel & CPU_LOG_TB_OP)) tcg_gen_debug_insn_start(pc_start); @@ -7270,6 +7273,13 @@ static target_ulong disas_insn(DisasCont /* lock generation */ if (s->prefix & PREFIX_LOCK) tcg_gen_helper_0_0(helper_unlock); + if (s->pc < s->cs_base || s->pc - s->cs_base > s->cs_limit) { + /* At least some of the opcode fetches violate the CS limit. + Overwrite the generated code with a GPF raising one. */ + gen_opc_ptr = gen_opc_start; + gen_opparam_ptr = gen_opparam_start; + gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base); + } return s->pc; illegal_op: if (s->prefix & PREFIX_LOCK) @@ -7316,13 +7326,11 @@ static inline int gen_intermediate_code_ int j, lj, cflags; uint64_t flags; target_ulong pc_start; - target_ulong cs_base; int num_insns; int max_insns; /* generate intermediate code */ pc_start = tb->pc; - cs_base = tb->cs_base; flags = tb->flags; cflags = tb->cflags; @@ -7337,7 +7345,8 @@ static inline int gen_intermediate_code_ dc->tf = (flags >> TF_SHIFT) & 1; dc->singlestep_enabled = env->singlestep_enabled; dc->cc_op = CC_OP_DYNAMIC; - dc->cs_base = cs_base; + dc->cs_base = tb->cs_base; + dc->cs_limit = tb->cs_limit; dc->tb = tb; dc->popl_esp_hack = 0; /* select memory access functions */