On 11/15/24 08:21, Paolo Bonzini wrote:
On 11/15/24 16:54, Peter Maydell wrote:
On Fri, 15 Nov 2024 at 15:22, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
CPUClass set_pc() and get_pc() handlers are target specific.
Rather than passing a generic CPUState and forcing QOM casts,
we can directly pass the target CPUArchState, simplifying.
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index db8a6fbc6e..70f5f8c3bf 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -160,8 +160,8 @@ struct CPUClass {
int64_t (*get_arch_id)(CPUState *cpu);
bool (*cpu_persistent_status)(CPUState *cpu);
bool (*cpu_enabled_status)(CPUState *cpu);
- void (*set_pc)(CPUState *cpu, vaddr value);
- vaddr (*get_pc)(CPUState *cpu);
+ void (*set_pc)(CPUArchState *env, vaddr value);
+ vaddr (*get_pc)(CPUArchState *env);
int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
vaddr (*gdb_adjust_breakpoint)(CPUState *cpu, vaddr addr);
This is effectively the table of methods for the CPUClass
class. I think that methods on class A should take a pointer to the
object of that type, not to something else.
Yeah, the diffstat is enticing but I agree. It's particularly confusing because there
isn't a single type called CPUArchState*, it's different depending on which target/
subdirectory you're compiling. In a multi-target binary it'd have to be void*, and then
you get back the casts.
Likewise. Any cleanup should be to swap the QOM casts with cpu_env() within the
individual methods.
r~
