On 27.01.2014, at 20:53, Tom Musta <tommu...@gmail.com> wrote:
> On 1/27/2014 12:55 PM, Alexander Graf wrote:
>> You're mixing two semantically separate things here. legal_in_user_mode
>> doesn't really indicate that le_mode isn't usable. I'm sure if you just make
>> this two if()'s with two separate bools that get assigned the same value gcc
>> will be smart enough to optimize it just as well as this combined branch.
>>
>
> Hmmm ... I'm not sure that I see the problem. Perhaps the comment should be
> clearer.
> And I guess there is really no need to compute the legal_in_user_mode flag
> since it
> is only used once.
>
> Prior to ISA 2.07, lq was not legal in user mode; attempting to execute lq
> when MSR[PR]=1
> resulted in a privileged instruction exception. Also, when MSR[PR]=0 and
> MSR[LE]=1, an
> alignment exception was generated irrespective of the computed address.
>
> Starting with ISA 2.07, both of these restrictions are lifted. So the
> proposed code is
> as follows:
>
> static void gen_lq(DisasContext *ctx)
> {
> /* lq is a legal user mode instruction starting in ISA 2.07 */
> int legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0;
>
> if (!legal_in_user_mode) {
> #if defined(CONFIG_USER_ONLY)
> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> return;
> #else
> if (unlikely(ctx->mem_idx == 0)) {
> gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
> return;
> }
>
> if (unlikely(ctx->le_mode)) {
Right, but the fact that "we're legal in user mode" has nothing to do with "we
can handle LE mode". I was thinking of something along the lines of
{
bool legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207);
bool can_handle_le = (ctx->insns_flags2 & PPC2_LSQ_ISA207);
if (!legal_in_user_mode && is_in_user_mode(ctx)) {
gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
return;
}
if (!can_handle_le && ctx->le_mode) {
gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
return;
}
[...]
}
> /* Little-endian mode is not handled */
> gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE);
> return;
> }
> #endif
> }
>
> int ra, rd;
> TCGv EA;
> ... // rest of implementation
>
>
> P.S. I think there should be an alignment check after the EA is computed.
I'm fairly sure this isn't the only place missing alignment checks :). But then
again alignment checks are tricky because your host kernel may fix them up for
you in linux only mode and in general they're not particularly useful.
Alex
