Re: [Qemu-devel] [PATCH 03/19] target/hppa: Convert move to/from system registers

[Richard Henderson]

On 04/06/2018 11:14 PM, Bastian Koppelmann wrote:
> On 02/17/2018 09:31 PM, Richard Henderson wrote:
>> Signed-off-by: Richard Henderson <address@hidden>
>> ---
>>  target/hppa/translate.c  | 57 
>> +++++++++++++++++++++---------------------------
>>  target/hppa/insns.decode | 15 +++++++++++++
>>  2 files changed, 40 insertions(+), 32 deletions(-)
>>
> [...]
>> @@ -2267,24 +2265,26 @@ static void trans_ssm(DisasContext *ctx, uint32_t 
>> insn, const DisasInsn *di)
>>      ctx->base.is_jmp = DISAS_IAQ_N_STALE_EXIT;
>>      nullify_end(ctx);
>>  }
>> +#endif /* !CONFIG_USER_ONLY */
> 
> This seems to not belong to this patch.

It does though.

>> -static void trans_mtsm(DisasContext *ctx, uint32_t insn, const DisasInsn 
>> *di)
>> +static void trans_mtsm(DisasContext *ctx, arg_mtsm *a, uint32_t insn)
>>  {
>> -    unsigned rr = extract32(insn, 16, 5);
>> -    TCGv_reg tmp, reg;
>> -
>>      CHECK_MOST_PRIVILEGED(EXCP_PRIV_OPR);
>> +#ifndef CONFIG_USER_ONLY
> 
> Why do you need to make this softmmu only in a simple convert patch?
> This makes it at least confusing for the reviewer.

Actually, it moves the function *out* of the softmmu only block.
That's the #endif being added above.

(1) The patterns in insns.decode are not (and cannot be) conditional
    on softmmu, so this function is now always called.

(2) This also enables a fix to a (trivial) emulation error between
    SIGILL (ILL_ILLOPC) and SIGILL (ILL_PRVOPC).  Although I think
    I would need an additional change in linux-user/ to effect this.


r~