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

[Gustavo Romero]

Hi Peter,

On 8/11/20 6:31 AM, Peter Maydell wrote:
> On Tue, 11 Aug 2020 at 02:29, Gustavo Romero <gromero@linux.ibm.com> wrote:
> > Currently if option '-icount auto' is passed to the QEMU TCG to enable
> > counting instructions the VM crashes with the following error report when
> > Linux runs on it:
> >
> > qemu-system-ppc64: Bad icount read
> >
> > This happens because read/write access to the SPRs PURR, VTB, and TBU40
> > is not integrated to the icount framework.
> >
> > This commit fixes that issue by making the read/write access of these
> > SPRs aware of icount framework, adding the proper gen_io_start/end() calls
> > before/after calling the helpers to load/store these SPRs in TCG.
> >
> > Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
> > @@ -284,12 +284,26 @@ static void spr_write_atbu(DisasContext *ctx, int sprn, 
> > int gprn)
> >   ATTRIBUTE_UNUSED
> >   static void spr_read_purr(DisasContext *ctx, int gprn, int sprn)
> >   {
> > +    if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> > +        gen_io_start();
> > +    }
> >       gen_helper_load_purr(cpu_gpr[gprn], cpu_env);
> > +    if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
> > +        gen_io_end();
> > +        gen_stop_exception(ctx);
> > +    }
>
> 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.

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).

Thanks a lot.


Cheers,
Gustavo