Re: [PATCH] accel: Do not set CPUState::can_do_io in non-TCG accels

[Claudio Fontana]

On 11/30/23 14:31, Philippe Mathieu-Daudé wrote:
> Hi Claudio,
> 
> On 30/11/23 13:48, Claudio Fontana wrote:
>> Hi Philippe,
>>
>> took a quick look with
>>
>> grep -R can_do_io
>>
>> and this seems to be in include/hw/core/cpu.h as well as cpu-common.c,
>>
>> maybe there is more meat to address to fully solve this?
>>
>> Before we had stuff for reset in cpu-common.c under a
>> if (tcg_enabled()) {
>> }
>>
>> but now we have cpu_exec_reset_hold(),
>> should the implementation for tcg of cpu_exec_reset_hold() do that (and 
>> potentially other tcg-specific non-arch-specific cpu variables we might 
>> need)?
> 
> Later we eventually get there:
> 
> diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
> index 9b038b1af5..e2c5cf97dc 100644
> --- a/accel/tcg/tcg-accel-ops.c
> +++ b/accel/tcg/tcg-accel-ops.c
> @@ -89,6 +89,9 @@ static void tcg_cpu_reset_hold(CPUState *cpu)
> 
>       cpu->accel->icount_extra = 0;
>       cpu->accel->mem_io_pc = 0;
> +
> +    qatomic_set(&cpu->neg.icount_decr.u32, 0);
> +    cpu->neg.can_do_io = true;
>   }
> 
> My branch is huge, I'm trying to split it, maybe I shouldn't have
> sent this single non-TCG patch out of it. I'll Cc you.

Thanks and congrats for the rework effort there!

Ciao,

Claudio

> 
>> If can_do_io is TCG-specific, maybe the whole field existence / visibility 
>> can be conditioned on TCG actually being at least compiled-in?
>> This might help find problems of the field being used in the wrong context, 
>> by virtue of getting an error when compiling with --disable-tcg for example.
>>
>> Ciao,
>>
>> Claudio
>