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

[Gustavo Romero]

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