Re: [Qemu-devel] [PATCH 1/5] target-tricore: Add trap handling

[Bastian Koppelmann <kbastian@mail.uni-paderborn.de>]

On 02/12/2016 07:55 PM, Richard Henderson wrote:
> On 02/12/2016 10:12 PM, Bastian Koppelmann wrote:
>> On 02/12/2016 03:39 AM, Richard Henderson wrote:
>>
>> What do you mean by "pre-interrupt state"? The register context is saved
>> by generate_trap() calls.
> 
> Because do_interrupt is normally for handling asynchronous interrupts. 
> Stuff that isn't related at all to the instruction stream.  Which
> therefore could not have saved anything at all.
> 

Ah, I see. This makes a lot of sense now. I didn't know that
do_interrupt() is only for asynchronous interrupts.

> It does get (ab)used on some targets for synchronous events, because the
> method by which kernel entry is made is the same and the author wanted
> to share code.
> 
> But if you have no external hardware devices, no timer interrupt or
> anything, let's not start by passing synchronous events through this
> function.  It sounds like the code here in do_interrupt more properly
> belongs at the end of generate_trap.
> 
> Err.. the generate_trap in op_helper I mean.  It's confusing that you
> have two (static) functions of the same name in different files.

Fair enough. I guess it makes sense to always use the helper for
generate_trap, since a helper call is necessary anyways.

> 
> I do wonder if these two functions ought to share more code.  For
> instance, op_helper.c places TIN into a_15, whereas translate.c places
> TIN into d_15.

Woops... good catch :).

Thanks for the review.

Cheers,
Bastian