From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40440) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SqJfZ-0004SJ-9Q for qemu-devel@nongnu.org; Sun, 15 Jul 2012 03:55:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SqJfY-00057D-4i for qemu-devel@nongnu.org; Sun, 15 Jul 2012 03:55:09 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:50102) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SqJfX-00056x-UD for qemu-devel@nongnu.org; Sun, 15 Jul 2012 03:55:08 -0400 Received: by obbta14 with SMTP id ta14so7760071obb.4 for ; Sun, 15 Jul 2012 00:55:07 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1342154108-798-1-git-send-email-proljc@gmail.com> <1342154108-798-12-git-send-email-proljc@gmail.com> From: Blue Swirl Date: Sun, 15 Jul 2012 07:54:45 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v9 11/15] target-or32: Add system instructions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jia Liu Cc: Max Filippov , qemu-devel@nongnu.org On Sun, Jul 15, 2012 at 12:39 AM, Jia Liu wrote: > Hi Blue > > On Sat, Jul 14, 2012 at 10:42 PM, Blue Swirl wrote: >> On Sat, Jul 14, 2012 at 2:08 PM, Jia Liu wrote: >>> Hi Max >>> >>> On Sat, Jul 14, 2012 at 9:49 PM, Max Filippov wrote: >>>>>> I don't think so, please check for example target-ppc/translate.c:4192 >>>>>> on how supervisor only mfsr is handled there. >>>>>> >>>>> >>>>> Thank you for comment, Blue. >>>>> >>>>> is this code OK? >>>> >>>> Shouldn't there also be an exception in softmmu mode >>>> if the CPU is not in supervisor mode? >>>> >>> >>> Sorry, I... >>> May you give me more comment? I'm not sure about this. >> >> If a user tries to execute a supervisor instruction (only allowed for >> kernel level code, not applications), the instruction won't be >> executed but an exception will be raised. >> >> This is the PPC mfsr instruction part of target-ppc/translate.c: >> /* mfsr */ >> static void gen_mfsr(DisasContext *ctx) >> { >> #if defined(CONFIG_USER_ONLY) >> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG); >> #else >> TCGv t0; >> if (unlikely(!ctx->mem_idx)) { >> >> Here the MMU mode is checked for user mode. I'd use more explicit >> check ctx->mmu_idx == MMU_USER_IDX. >> >> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG); >> return; >> } >> t0 = tcg_const_tl(SR(ctx->opcode)); >> gen_helper_load_sr(cpu_gpr[rD(ctx->opcode)], cpu_env, t0); >> tcg_temp_free(t0); >> #endif >> } >> > > Thank you very much for this nice example. > Is this code OK? > case 0x2d: /* l.mfspr */ > LOG_DIS("l.mfspr r%d, r%d, %d\n", rd, ra, I16); > { > #if defined(CONFIG_USER_ONLY) > gen_illegal_exception(dc); > #else > TCGv_i32 ti = tcg_const_i32(I16); > if (dc->mem_idx == MMU_USER_IDX) { > gen_illegal_exception(dc); > return; > } > gen_helper_mfspr(cpu_R[rd], cpu_env, cpu_R[rd], cpu_R[ra], ti); > tcg_temp_free_i32(ti); > #endif > } > break; > > case 0x30: /* l.mtspr */ > LOG_DIS("l.mtspr %d, r%d, r%d, %d\n", I5, ra, rb, I11); > { > #if defined(CONFIG_USER_ONLY) > gen_illegal_exception(dc); Maybe return? > #else > TCGv_i32 im = tcg_const_i32(tmp); > if (dc->mem_idx == MMU_USER_IDX) { > gen_illegal_exception(dc); > return; > } > gen_helper_mtspr(cpu_env, cpu_R[ra], cpu_R[rb], im); > tcg_temp_free_i32(im); > #endif > } > break; Otherwise looks OK. Perhaps there are other supervisor mode only instructions which need this kind of checks? > > >>> >>>>> >>>>> case 0x2d: /* l.mfspr */ >>>>> LOG_DIS("l.mfspr r%d, r%d, %d\n", rd, ra, I16); >>>>> { >>>>> #if defined(CONFIG_USER_ONLY) >>>>> gen_illegal_exception(dc); >>>>> #else >>>>> TCGv_i32 ti = tcg_const_i32(I16); >>>>> gen_helper_mfspr(cpu_R[rd], cpu_env, cpu_R[rd], cpu_R[ra], ti); >>>>> tcg_temp_free_i32(ti); >>>>> #endif >>>>> } >>>>> break; >>>>> >>>>> case 0x30: /* l.mtspr */ >>>>> LOG_DIS("l.mtspr %d, r%d, r%d, %d\n", I5, ra, rb, I11); >>>>> { >>>>> #if defined(CONFIG_USER_ONLY) >>>>> gen_illegal_exception(dc); >>>>> #else >>>>> TCGv_i32 im = tcg_const_i32(tmp); >>>>> gen_helper_mtspr(cpu_env, cpu_R[ra], cpu_R[rb], im); >>>>> tcg_temp_free_i32(im); >>>>> #endif >>>>> } >>>>> break; >>>>> >>>> >>>> -- >>>> Thanks. >>>> -- Max >>> >>> Regards, >>> Jia. > > Regards, > Jia