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

[Richard Henderson <richard.henderson@linaro.org>]

On 11/29/23 14:50, Philippe Mathieu-Daudé wrote:
> 'can_do_io' is specific to TCG. Having it set in non-TCG
> code is confusing, so remove it from QTest / HVF / KVM.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   accel/dummy-cpus.c        | 1 -
>   accel/hvf/hvf-accel-ops.c | 1 -
>   accel/kvm/kvm-accel-ops.c | 1 -
>   3 files changed, 3 deletions(-)
> 
> diff --git a/accel/dummy-cpus.c b/accel/dummy-cpus.c
> index b75c919ac3..1005ec6f56 100644
> --- a/accel/dummy-cpus.c
> +++ b/accel/dummy-cpus.c
> @@ -27,7 +27,6 @@ static void *dummy_cpu_thread_fn(void *arg)
>       qemu_mutex_lock_iothread();
>       qemu_thread_get_self(cpu->thread);
>       cpu->thread_id = qemu_get_thread_id();
> -    cpu->neg.can_do_io = true;
>       current_cpu = cpu;

I expect this is ok...

When I was moving this variable around just recently, 464dacf6, I wondered about these 
other settings, and I wondered if they used to be required for implementing some i/o on 
behalf of hw accel.  Something that has now been factored out to e.g. kvm_handle_io, which 
now uses address_space_rw directly.

I see 3 reads of can_do_io: accel/tcg/{cputlb.c, icount-common.c} and system/watchpoint.c. 
  The final one is nested within replay_running_debug(), which implies icount and tcg.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

>   
>   #ifndef _WIN32
> diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
> index abe7adf7ee..2bba54cf70 100644
> --- a/accel/hvf/hvf-accel-ops.c
> +++ b/accel/hvf/hvf-accel-ops.c
> @@ -428,7 +428,6 @@ static void *hvf_cpu_thread_fn(void *arg)
>       qemu_thread_get_self(cpu->thread);
>   
>       cpu->thread_id = qemu_get_thread_id();
> -    cpu->neg.can_do_io = true;
>       current_cpu = cpu;
>   
>       hvf_init_vcpu(cpu);
> diff --git a/accel/kvm/kvm-accel-ops.c b/accel/kvm/kvm-accel-ops.c
> index 6195150a0b..f273f415db 100644
> --- a/accel/kvm/kvm-accel-ops.c
> +++ b/accel/kvm/kvm-accel-ops.c
> @@ -36,7 +36,6 @@ static void *kvm_vcpu_thread_fn(void *arg)
>       qemu_mutex_lock_iothread();
>       qemu_thread_get_self(cpu->thread);
>       cpu->thread_id = qemu_get_thread_id();
> -    cpu->neg.can_do_io = true;
>       current_cpu = cpu;
>   
>       r = kvm_init_vcpu(cpu, &error_fatal);