Re: [PATCH] target/ppc: Integrate icount to purr, vtb, and tbu40

[Gustavo Romero <gromero@linux.vnet.ibm.com>]

On 8/11/20 11:24 AM, Peter Maydell wrote:
> On Tue, 11 Aug 2020 at 14:33, Gustavo Romero <gromero@linux.vnet.ibm.com> wrote:
>> On 8/11/20 6:31 AM, Peter Maydell wrote:
>>> You don't want to call gen_io_end; you just need to ensure that
>>> you end the TB immediately after this insn. See
>>> docs/devel/tcg-icount.rst.
>>
>> I understand that to ensure that TB ends immediately after these
>> instructions (I understood you meant all the cases, not just the
>> spr_read_purr case, right?), the instructions should be a branch
>> or change CPU state in a way that cannot be deduced at translation
>> time, and I don't know how to ensure that in these cases, they
>> are neither, specially for the read access, which doesn't change
>> any CPU state specifically afaics.
> 
> No, you have that the wrong way around. *If* an instruction
> is a branch or a state-changing one, *then* it must end
> the TB. That doesn't mean that *only* those kinds of insn
> can end the TB -- other things also can end a TB. (It also
> doesn't mean that a branch etc will automatically end the
> TB -- it means that if you're writing the bit of target
> code that generates code for a branch/etc then you must
> specifically ensure that you end the TB.)

ah, ok. I got it now :) Thanks for the explanation.


>> If I remove the gen_io_end() from all these cases the VM gets
>> stuck at apparently random points of execution (I'm digging
>> into details right now trying to understand why).
> 
> Probably because you're not ending the TB after the insn.
> 
> PowerPC seems to be doing something slightly weird in this area --
> it classifies "stop translation" as a kind of exception
> (POWERPC_EXCP_STOP) rather than just using the common-code
> is_jmp machinery and setting it to DISAS_EXIT. So you'll
> need a ppc expert to say what the right thing is, but my
> guess is you want to call gen_stop_exception() -- compare
> gen_darn().

Right, if I change the code to call gen_stop_exception() like
in gen_darn() everything goes fine.

So, I'll send a v2 based on your review then PPC64 folks can
probably take a look at it. Could I add:

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

to v2?


Cheers,
Gustavo