| Message ID | 20240606032926.83599-9-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | plugins: Use unwind info for special gdb registers | expand |
Richard Henderson <richard.henderson@linaro.org> writes: > For the moment, this is an exact copy of arm_tcg_ops. > Export arm_cpu_exec_interrupt for the cross-file reference. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/internals.h | 1 + > target/arm/cpu.c | 2 +- > target/arm/cpu64.c | 30 ++++++++++++++++++++++++++++++ > 3 files changed, 32 insertions(+), 1 deletion(-) > > diff --git a/target/arm/internals.h b/target/arm/internals.h > index 11b5da2562..dc53d86249 100644 > --- a/target/arm/internals.h > +++ b/target/arm/internals.h > @@ -364,6 +364,7 @@ void arm_restore_state_to_opc(CPUState *cs, > > #ifdef CONFIG_TCG > void arm_cpu_synchronize_from_tb(CPUState *cs, const TranslationBlock *tb); > +bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request); > #endif /* CONFIG_TCG */ > > typedef enum ARMFPRounding { > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 35fa281f1b..3cd4711064 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -824,7 +824,7 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx, > return unmasked || pstate_unmasked; > } > > -static bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request) > +bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request) > { > CPUClass *cc = CPU_GET_CLASS(cs); > CPUARMState *env = cpu_env(cs); > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index 262a1d6c0b..7ba80099af 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -31,6 +31,9 @@ > #include "hvf_arm.h" > #include "qapi/visitor.h" > #include "hw/qdev-properties.h" > +#ifdef CONFIG_TCG > +#include "hw/core/tcg-cpu-ops.h" > +#endif > #include "internals.h" > #include "cpu-features.h" > #include "cpregs.h" > @@ -793,6 +796,29 @@ static const gchar *aarch64_gdb_arch_name(CPUState *cs) > return "aarch64"; > } > > +#ifdef CONFIG_TCG > +static const TCGCPUOps aarch64_tcg_ops = { > + .initialize = arm_translate_init, > + .synchronize_from_tb = arm_cpu_synchronize_from_tb, > + .debug_excp_handler = arm_debug_excp_handler, > + .restore_state_to_opc = arm_restore_state_to_opc, > + > +#ifdef CONFIG_USER_ONLY > + .record_sigsegv = arm_cpu_record_sigsegv, > + .record_sigbus = arm_cpu_record_sigbus, > +#else > + .tlb_fill = arm_cpu_tlb_fill, > + .cpu_exec_interrupt = arm_cpu_exec_interrupt, > + .do_interrupt = arm_cpu_do_interrupt, > + .do_transaction_failed = arm_cpu_do_transaction_failed, > + .do_unaligned_access = arm_cpu_do_unaligned_access, > + .adjust_watchpoint_address = arm_adjust_watchpoint_address, > + .debug_check_watchpoint = arm_debug_check_watchpoint, > + .debug_check_breakpoint = arm_debug_check_breakpoint, > +#endif /* !CONFIG_USER_ONLY */ > +}; > +#endif /* CONFIG_TCG */ > + My principle concern is duplicating an otherwise identical structure just gives us more opportunity to miss a change. > static void aarch64_cpu_class_init(ObjectClass *oc, void *data) > { > CPUClass *cc = CPU_CLASS(oc); > @@ -802,6 +828,10 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data) > cc->gdb_core_xml_file = "aarch64-core.xml"; > cc->gdb_arch_name = aarch64_gdb_arch_name; > > +#ifdef CONFIG_TCG > + cc->tcg_ops = &aarch64_tcg_ops; > +#endif > + What happens when the CPU is running mixed mode code and jumping between 64 and 32 bit? Wouldn't it be easier to have a helper that routes to the correct unwinder, c.f. gen_intermediate_code #ifdef TARGET_AARCH64 if (EX_TBFLAG_ANY(tb_flags, AARCH64_STATE)) { ops = &aarch64_translator_ops; } #endif which I guess for a runtime helper would be: if (is_a64(cpu_env(cs))) { aarch64_plugin_need_unwind_for_reg(...); } else { arm_plugin_need_unwind_for_reg(...); } etc... > object_class_property_add_bool(oc, "aarch64", aarch64_cpu_get_aarch64, > aarch64_cpu_set_aarch64); > object_class_property_set_description(oc, "aarch64",
On 6/12/24 07:36, Alex Bennée wrote: > What happens when the CPU is running mixed mode code and jumping between > 64 and 32 bit? Wouldn't it be easier to have a helper that routes to the > correct unwinder, c.f. gen_intermediate_code GDB can't switch modes, so there is *never* any mode switching. r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 6/12/24 07:36, Alex Bennée wrote: >> What happens when the CPU is running mixed mode code and jumping between >> 64 and 32 bit? Wouldn't it be easier to have a helper that routes to the >> correct unwinder, c.f. gen_intermediate_code > > GDB can't switch modes, so there is *never* any mode switching. Hmm but code can. We do want to solve the gdb mode switching case as well although that requires some work on the gdbstub.
diff --git a/target/arm/internals.h b/target/arm/internals.h index 11b5da2562..dc53d86249 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -364,6 +364,7 @@ void arm_restore_state_to_opc(CPUState *cs, #ifdef CONFIG_TCG void arm_cpu_synchronize_from_tb(CPUState *cs, const TranslationBlock *tb); +bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request); #endif /* CONFIG_TCG */ typedef enum ARMFPRounding { diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 35fa281f1b..3cd4711064 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -824,7 +824,7 @@ static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx, return unmasked || pstate_unmasked; } -static bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request) +bool arm_cpu_exec_interrupt(CPUState *cs, int interrupt_request) { CPUClass *cc = CPU_GET_CLASS(cs); CPUARMState *env = cpu_env(cs); diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 262a1d6c0b..7ba80099af 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -31,6 +31,9 @@ #include "hvf_arm.h" #include "qapi/visitor.h" #include "hw/qdev-properties.h" +#ifdef CONFIG_TCG +#include "hw/core/tcg-cpu-ops.h" +#endif #include "internals.h" #include "cpu-features.h" #include "cpregs.h" @@ -793,6 +796,29 @@ static const gchar *aarch64_gdb_arch_name(CPUState *cs) return "aarch64"; } +#ifdef CONFIG_TCG +static const TCGCPUOps aarch64_tcg_ops = { + .initialize = arm_translate_init, + .synchronize_from_tb = arm_cpu_synchronize_from_tb, + .debug_excp_handler = arm_debug_excp_handler, + .restore_state_to_opc = arm_restore_state_to_opc, + +#ifdef CONFIG_USER_ONLY + .record_sigsegv = arm_cpu_record_sigsegv, + .record_sigbus = arm_cpu_record_sigbus, +#else + .tlb_fill = arm_cpu_tlb_fill, + .cpu_exec_interrupt = arm_cpu_exec_interrupt, + .do_interrupt = arm_cpu_do_interrupt, + .do_transaction_failed = arm_cpu_do_transaction_failed, + .do_unaligned_access = arm_cpu_do_unaligned_access, + .adjust_watchpoint_address = arm_adjust_watchpoint_address, + .debug_check_watchpoint = arm_debug_check_watchpoint, + .debug_check_breakpoint = arm_debug_check_breakpoint, +#endif /* !CONFIG_USER_ONLY */ +}; +#endif /* CONFIG_TCG */ + static void aarch64_cpu_class_init(ObjectClass *oc, void *data) { CPUClass *cc = CPU_CLASS(oc); @@ -802,6 +828,10 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data) cc->gdb_core_xml_file = "aarch64-core.xml"; cc->gdb_arch_name = aarch64_gdb_arch_name; +#ifdef CONFIG_TCG + cc->tcg_ops = &aarch64_tcg_ops; +#endif + object_class_property_add_bool(oc, "aarch64", aarch64_cpu_get_aarch64, aarch64_cpu_set_aarch64); object_class_property_set_description(oc, "aarch64",
For the moment, this is an exact copy of arm_tcg_ops. Export arm_cpu_exec_interrupt for the cross-file reference. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/internals.h | 1 + target/arm/cpu.c | 2 +- target/arm/cpu64.c | 30 ++++++++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-)