On 30/11/23 14:51, Richard Henderson wrote:
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@xxxxxxxxxx>
---
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.
It was added by mistake in commit 99df7dce8a ("cpu: Move can_do_io
field from CPU_COMMON to CPUState", 2013) to cpu_common_reset() and
commit 626cf8f4c6 ("icount: set can_do_io outside TB execution", 2014),
then moved in commits 57038a92bb ("cpus: extract out kvm-specific code
to accel/kvm"), b86f59c715 ("accel: replace struct CpusAccel with
AccelOpsClass") and the one you mentioned, 464dacf609 ("accel/tcg: Move
can_do_io to CPUNegativeOffsetState").
The culprit is 626cf8f4c6 I guess, so maybe:
Fixes: 626cf8f4c6 ("icount: set can_do_io outside TB execution")
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@xxxxxxxxxx>
Thanks!
