| Message ID | 20241115152053.66442-7-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | accel/tcg: API prototype cleanups | expand |
On Fri, 15 Nov 2024 at 15:23, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/exec/translate-all.h | 3 +-- > accel/tcg/translate-all.c | 2 +- > target/i386/helper.c | 3 ++- > target/openrisc/sys_helper.c | 7 +++---- > 4 files changed, 7 insertions(+), 8 deletions(-) > > diff --git a/include/exec/translate-all.h b/include/exec/translate-all.h > index f06cfedd52..9303318953 100644 > --- a/include/exec/translate-all.h > +++ b/include/exec/translate-all.h > @@ -23,7 +23,6 @@ > > /** > * cpu_unwind_state_data: > - * @cpu: the cpu context > * @host_pc: the host pc within the translation > * @data: output data > * > @@ -32,7 +31,7 @@ > * function returns false; otherwise @data is loaded. > * This is the same unwind info as given to restore_state_to_opc. > */ > -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data); > +bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data); > > /* translate-all.c */ > void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr); > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c > index fdf6d8ac19..8d5530e341 100644 > --- a/accel/tcg/translate-all.c > +++ b/accel/tcg/translate-all.c > @@ -243,7 +243,7 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc) > return false; > } > > -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data) > +bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data) > { > if (in_code_gen_buffer((const void *)(host_pc - tcg_splitwx_diff))) { > TranslationBlock *tb = tcg_tb_lookup(host_pc); > diff --git a/target/i386/helper.c b/target/i386/helper.c > index 01a268a30b..b848936441 100644 > --- a/target/i386/helper.c > +++ b/target/i386/helper.c > @@ -29,6 +29,7 @@ > #endif > #include "qemu/log.h" > #ifdef CONFIG_TCG > +#include "exec/translate-all.h" Ah, here are those includes. These should be in the previous patch I think. -- PMM
On 11/15/24 07:20, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/exec/translate-all.h | 3 +-- > accel/tcg/translate-all.c | 2 +- > target/i386/helper.c | 3 ++- > target/openrisc/sys_helper.c | 7 +++---- > 4 files changed, 7 insertions(+), 8 deletions(-) > > diff --git a/include/exec/translate-all.h b/include/exec/translate-all.h > index f06cfedd52..9303318953 100644 > --- a/include/exec/translate-all.h > +++ b/include/exec/translate-all.h > @@ -23,7 +23,6 @@ > > /** > * cpu_unwind_state_data: > - * @cpu: the cpu context > * @host_pc: the host pc within the translation > * @data: output data > * > @@ -32,7 +31,7 @@ > * function returns false; otherwise @data is loaded. > * This is the same unwind info as given to restore_state_to_opc. > */ > -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data); > +bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data); Hmm. I wonder if it should be called "cpu_*" at all then? Worth renaming to "tcg_*" or something? Anyway, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On Fri, 15 Nov 2024 at 17:24, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 11/15/24 07:20, Philippe Mathieu-Daudé wrote: > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > --- > > include/exec/translate-all.h | 3 +-- > > accel/tcg/translate-all.c | 2 +- > > target/i386/helper.c | 3 ++- > > target/openrisc/sys_helper.c | 7 +++---- > > 4 files changed, 7 insertions(+), 8 deletions(-) > > > > diff --git a/include/exec/translate-all.h b/include/exec/translate-all.h > > index f06cfedd52..9303318953 100644 > > --- a/include/exec/translate-all.h > > +++ b/include/exec/translate-all.h > > @@ -23,7 +23,6 @@ > > > > /** > > * cpu_unwind_state_data: > > - * @cpu: the cpu context > > * @host_pc: the host pc within the translation > > * @data: output data > > * > > @@ -32,7 +31,7 @@ > > * function returns false; otherwise @data is loaded. > > * This is the same unwind info as given to restore_state_to_opc. > > */ > > -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data); > > +bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data); > > Hmm. I wonder if it should be called "cpu_*" at all then? > Worth renaming to "tcg_*" or something? Yes, it's odd, isn't it? What's the plan for this function in a multi-target emulation world? At the moment it (or functions it calls) uses TARGET_INSN_START_WORDS which is a target-CPU-type-specific value. If in the future we're going to want it instead to look that up as e.g. some property of the CPU class then maybe we should keep passing it the CPU pointer? Or would we instead say that we'll define TARGET_INSN_START_WORDS as the worst case for any target, since it's always between 1 and 3, so it doesn't waste that much space if we have a couple of extra sleb128 zero values for targets that don't need all 3 words? thanks -- PMM
diff --git a/include/exec/translate-all.h b/include/exec/translate-all.h index f06cfedd52..9303318953 100644 --- a/include/exec/translate-all.h +++ b/include/exec/translate-all.h @@ -23,7 +23,6 @@ /** * cpu_unwind_state_data: - * @cpu: the cpu context * @host_pc: the host pc within the translation * @data: output data * @@ -32,7 +31,7 @@ * function returns false; otherwise @data is loaded. * This is the same unwind info as given to restore_state_to_opc. */ -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data); +bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data); /* translate-all.c */ void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr); diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index fdf6d8ac19..8d5530e341 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -243,7 +243,7 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc) return false; } -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t *data) +bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data) { if (in_code_gen_buffer((const void *)(host_pc - tcg_splitwx_diff))) { TranslationBlock *tb = tcg_tb_lookup(host_pc); diff --git a/target/i386/helper.c b/target/i386/helper.c index 01a268a30b..b848936441 100644 --- a/target/i386/helper.c +++ b/target/i386/helper.c @@ -29,6 +29,7 @@ #endif #include "qemu/log.h" #ifdef CONFIG_TCG +#include "exec/translate-all.h" #include "tcg/insn-start-words.h" #endif @@ -526,7 +527,7 @@ static inline target_ulong get_memio_eip(CPUX86State *env) uint64_t data[TARGET_INSN_START_WORDS]; CPUState *cs = env_cpu(env); - if (!cpu_unwind_state_data(cs, cs->mem_io_pc, data)) { + if (!cpu_unwind_state_data(cs->mem_io_pc, data)) { return env->eip; } diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c index 77567afba4..67e1f53fca 100644 --- a/target/openrisc/sys_helper.c +++ b/target/openrisc/sys_helper.c @@ -20,7 +20,7 @@ #include "qemu/osdep.h" #include "cpu.h" -#include "exec/exec-all.h" +#include "exec/translate-all.h" #include "exec/helper-proto.h" #include "exception.h" #ifndef CONFIG_USER_ONLY @@ -219,7 +219,6 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd, #ifndef CONFIG_USER_ONLY uint64_t data[TARGET_INSN_START_WORDS]; MachineState *ms = MACHINE(qdev_get_machine()); - CPUState *cs = env_cpu(env); int idx; #endif @@ -260,7 +259,7 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd, return env->evbar; case TO_SPR(0, 16): /* NPC (equals PC) */ - if (cpu_unwind_state_data(cs, GETPC(), data)) { + if (cpu_unwind_state_data(GETPC(), data)) { return data[0]; } return env->pc; @@ -269,7 +268,7 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd, return cpu_get_sr(env); case TO_SPR(0, 18): /* PPC */ - if (cpu_unwind_state_data(cs, GETPC(), data)) { + if (cpu_unwind_state_data(GETPC(), data)) { if (data[1] & 2) { return data[0] - 4; }
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/exec/translate-all.h | 3 +-- accel/tcg/translate-all.c | 2 +- target/i386/helper.c | 3 ++- target/openrisc/sys_helper.c | 7 +++---- 4 files changed, 7 insertions(+), 8 deletions(-)