Re: [Qemu-devel] [PATCH 010/126] target-s390: Reorg exception handling

[Alexander Graf]

On 19.09.2012, at 13:34, Alexander Graf wrote:

> 
> On 19.09.2012, at 13:29, Peter Maydell wrote:
> 
>> On 19 September 2012 01:14, Richard Henderson <address@hidden> wrote:
>>> On 09/18/2012 01:18 PM, Alexander Graf wrote:
>>>>> -    /* remember what pgm exeption this was */
>>>>> +    /* Remember what pgm exeption this was.  */
>> 
>> ...if you're changing this comment then at least fix the typo
>> ("exception")...
>> 
>>>>>     tmp = tcg_const_i32(code);
>>>>>     tcg_gen_st_i32(tmp, cpu_env, offsetof(CPUS390XState, int_pgm_code));
>>>>>     tcg_temp_free_i32(tmp);
>>>>> -    tmp = tcg_const_i32(ilc);
>>>>> +    tmp = tcg_const_i32(s->next_pc - s->pc);
>>>> 
>>>> Mind to explain this one?
>>> 
>>> ILC = the size of the insn.  Rather than passing ILC around into
>>> gen_program_exception, get it back from the s->next_pc value that
>>> we stored into DisasContext.
>> 
>> ...but isn't (s-next_pc - s->pc) ilc*2, not ilc? Compare this hunk
>> from elsewhere in the patch:
>> 
>> -        tmp32_2 = tcg_const_i32(ilc * 2);
>> -        tmp32_3 = tcg_const_i32(EXCP_SVC);
>> +        tmp32_2 = tcg_const_i32(s->next_pc - s->pc);
> 
> Yes, it is, and hence it's correct. The exception field encodes the real 
> instruction length, not the ILC bits in the opcode. Maybe the naming is 
> unfortunate...

Or maybe not. For SVC we seem to pass in the full instruction length, while PGM 
only gets ILC. So the above hunk is wrong.

Richard, please double-check all the ilc passing again with the spec.


Alex