Re: [Qemu-devel] [PATCH v9 11/15] target-or32: Add system instructions

[Jia Liu <proljc@gmail.com>]

Hi Blue

On Sat, Jul 14, 2012 at 10:42 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Sat, Jul 14, 2012 at 2:08 PM, Jia Liu <proljc@gmail.com> wrote:
>> Hi Max
>>
>> On Sat, Jul 14, 2012 at 9:49 PM, Max Filippov <jcmvbkbc@gmail.com> 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);
#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;


>>
>>>>
>>>>     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