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

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

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