| Message ID | 20210902151715.383678-5-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | accel/tcg: Restrict TCGCPUOps::cpu_exec_interrupt() to sysemu | expand |
On Thu, Sep 2, 2021 at 9:17 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > do_interrupt() is sysemu specific. However due to some X86 > specific hack, it is also used in user-mode emulation, which > is why it couldn't be restricted to CONFIG_SOFTMMU (see the > comment around added in commit 78271684719: "cpu: tcg_ops: > move to tcg-cpu-ops.h, keep a pointer in CPUClass"). > Keep the hack but rename the handler as fake_user_exception() > and restrict do_interrupt() to sysemu. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > RFC: Any better name / idea here? > Maybe user_mode_exception()? but I'm not sure that's better... > --- > include/hw/core/tcg-cpu-ops.h | 22 ++++++++++++++-------- > accel/tcg/cpu-exec.c | 4 ++-- > target/i386/tcg/tcg-cpu.c | 6 ++++-- > 3 files changed, 20 insertions(+), 12 deletions(-) > Reviewed-by: Warner Losh <imp@bsdimp.com> > diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h > index eab27d0c030..600f0349659 100644 > --- a/include/hw/core/tcg-cpu-ops.h > +++ b/include/hw/core/tcg-cpu-ops.h > @@ -37,14 +37,6 @@ struct TCGCPUOps { > void (*cpu_exec_exit)(CPUState *cpu); > /** @cpu_exec_interrupt: Callback for processing interrupts in > cpu_exec */ > bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request); > - /** > - * @do_interrupt: Callback for interrupt handling. > - * > - * note that this is in general SOFTMMU only, but it actually isn't > - * because of an x86 hack (accel/tcg/cpu-exec.c), so we cannot put it > - * in the SOFTMMU section in general. > - */ > - void (*do_interrupt)(CPUState *cpu); > /** > * @tlb_fill: Handle a softmmu tlb miss or user-only address fault > * > @@ -61,6 +53,20 @@ struct TCGCPUOps { > void (*debug_excp_handler)(CPUState *cpu); > > #ifdef NEED_CPU_H > +#if defined(CONFIG_USER_ONLY) && defined(TARGET_I386) > + /** > + * @fake_user_exception: Callback for 'fake exception' handling. > + * > + * Simulate 'fake exception' which will be handled outside the > + * cpu execution loop (hack for x86 user mode). > + */ > + void (*fake_user_exception)(CPUState *cpu); > +#else > + /** > + * @do_interrupt: Callback for interrupt handling. > + */ > + void (*do_interrupt)(CPUState *cpu); > +#endif /* !CONFIG_USER_ONLY || !TARGET_I386 */ > #ifdef CONFIG_SOFTMMU > /** > * @do_transaction_failed: Callback for handling failed memory > transactions > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c > index e5c0ccd1a2a..3e387c944c5 100644 > --- a/accel/tcg/cpu-exec.c > +++ b/accel/tcg/cpu-exec.c > @@ -651,8 +651,8 @@ static inline bool cpu_handle_exception(CPUState *cpu, > int *ret) > loop */ > #if defined(TARGET_I386) > CPUClass *cc = CPU_GET_CLASS(cpu); > - cc->tcg_ops->do_interrupt(cpu); > -#endif > + cc->tcg_ops->fake_user_exception(cpu); > +#endif /* TARGET_I386 */ > *ret = cpu->exception_index; > cpu->exception_index = -1; > return true; > diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c > index 93a79a57415..dce800a8953 100644 > --- a/target/i386/tcg/tcg-cpu.c > +++ b/target/i386/tcg/tcg-cpu.c > @@ -73,9 +73,11 @@ static const struct TCGCPUOps x86_tcg_ops = { > .cpu_exec_enter = x86_cpu_exec_enter, > .cpu_exec_exit = x86_cpu_exec_exit, > .cpu_exec_interrupt = x86_cpu_exec_interrupt, > - .do_interrupt = x86_cpu_do_interrupt, > .tlb_fill = x86_cpu_tlb_fill, > -#ifndef CONFIG_USER_ONLY > +#ifdef CONFIG_USER_ONLY > + .fake_user_exception = x86_cpu_do_interrupt, > +#else > + .do_interrupt = x86_cpu_do_interrupt, > .debug_excp_handler = breakpoint_handler, > .debug_check_breakpoint = x86_debug_check_breakpoint, > #endif /* !CONFIG_USER_ONLY */ > -- > 2.31.1 > >
On 9/2/21 5:16 PM, Philippe Mathieu-Daudé wrote: > do_interrupt() is sysemu specific. However due to some X86 > specific hack, it is also used in user-mode emulation, which > is why it couldn't be restricted to CONFIG_SOFTMMU (see the > comment around added in commit 78271684719: "cpu: tcg_ops: > move to tcg-cpu-ops.h, keep a pointer in CPUClass"). > Keep the hack but rename the handler as fake_user_exception() > and restrict do_interrupt() to sysemu. > > Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org> > --- > RFC: Any better name / idea here? I guess I'm ok with fake_user_interrupt. But I believe that this could all be moved into cpu_loop.c. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 9/3/21 9:07 PM, Richard Henderson wrote: > On 9/2/21 5:16 PM, Philippe Mathieu-Daudé wrote: >> do_interrupt() is sysemu specific. However due to some X86 >> specific hack, it is also used in user-mode emulation, which >> is why it couldn't be restricted to CONFIG_SOFTMMU (see the >> comment around added in commit 78271684719: "cpu: tcg_ops: >> move to tcg-cpu-ops.h, keep a pointer in CPUClass"). >> Keep the hack but rename the handler as fake_user_exception() >> and restrict do_interrupt() to sysemu. >> >> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org> >> --- >> RFC: Any better name / idea here? > > I guess I'm ok with fake_user_interrupt. I can use do_fake_user_interrupt (closer match to do_interrupt equivalent). Alternative name is "do_interrupt_user", same as the handler: /* * fake user mode interrupt. is_int is TRUE if coming from the int * instruction. next_eip is the env->eip value AFTER the interrupt * instruction. It is only relevant if is_int is TRUE or if intno * is EXCP_SYSCALL. */ static void do_interrupt_user(CPUX86State *env, int intno, int is_int, int error_code, target_ulong next_eip) > But I believe that this could all be moved into cpu_loop.c. I tried to give it a try, but seems out of my comfort zone. I'll create an issue to do it as a future cleanup on top of this series. > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Thanks, Phil.
diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h index eab27d0c030..600f0349659 100644 --- a/include/hw/core/tcg-cpu-ops.h +++ b/include/hw/core/tcg-cpu-ops.h @@ -37,14 +37,6 @@ struct TCGCPUOps { void (*cpu_exec_exit)(CPUState *cpu); /** @cpu_exec_interrupt: Callback for processing interrupts in cpu_exec */ bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request); - /** - * @do_interrupt: Callback for interrupt handling. - * - * note that this is in general SOFTMMU only, but it actually isn't - * because of an x86 hack (accel/tcg/cpu-exec.c), so we cannot put it - * in the SOFTMMU section in general. - */ - void (*do_interrupt)(CPUState *cpu); /** * @tlb_fill: Handle a softmmu tlb miss or user-only address fault * @@ -61,6 +53,20 @@ struct TCGCPUOps { void (*debug_excp_handler)(CPUState *cpu); #ifdef NEED_CPU_H +#if defined(CONFIG_USER_ONLY) && defined(TARGET_I386) + /** + * @fake_user_exception: Callback for 'fake exception' handling. + * + * Simulate 'fake exception' which will be handled outside the + * cpu execution loop (hack for x86 user mode). + */ + void (*fake_user_exception)(CPUState *cpu); +#else + /** + * @do_interrupt: Callback for interrupt handling. + */ + void (*do_interrupt)(CPUState *cpu); +#endif /* !CONFIG_USER_ONLY || !TARGET_I386 */ #ifdef CONFIG_SOFTMMU /** * @do_transaction_failed: Callback for handling failed memory transactions diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index e5c0ccd1a2a..3e387c944c5 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -651,8 +651,8 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret) loop */ #if defined(TARGET_I386) CPUClass *cc = CPU_GET_CLASS(cpu); - cc->tcg_ops->do_interrupt(cpu); -#endif + cc->tcg_ops->fake_user_exception(cpu); +#endif /* TARGET_I386 */ *ret = cpu->exception_index; cpu->exception_index = -1; return true; diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c index 93a79a57415..dce800a8953 100644 --- a/target/i386/tcg/tcg-cpu.c +++ b/target/i386/tcg/tcg-cpu.c @@ -73,9 +73,11 @@ static const struct TCGCPUOps x86_tcg_ops = { .cpu_exec_enter = x86_cpu_exec_enter, .cpu_exec_exit = x86_cpu_exec_exit, .cpu_exec_interrupt = x86_cpu_exec_interrupt, - .do_interrupt = x86_cpu_do_interrupt, .tlb_fill = x86_cpu_tlb_fill, -#ifndef CONFIG_USER_ONLY +#ifdef CONFIG_USER_ONLY + .fake_user_exception = x86_cpu_do_interrupt, +#else + .do_interrupt = x86_cpu_do_interrupt, .debug_excp_handler = breakpoint_handler, .debug_check_breakpoint = x86_debug_check_breakpoint, #endif /* !CONFIG_USER_ONLY */
do_interrupt() is sysemu specific. However due to some X86 specific hack, it is also used in user-mode emulation, which is why it couldn't be restricted to CONFIG_SOFTMMU (see the comment around added in commit 78271684719: "cpu: tcg_ops: move to tcg-cpu-ops.h, keep a pointer in CPUClass"). Keep the hack but rename the handler as fake_user_exception() and restrict do_interrupt() to sysemu. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- RFC: Any better name / idea here? --- include/hw/core/tcg-cpu-ops.h | 22 ++++++++++++++-------- accel/tcg/cpu-exec.c | 4 ++-- target/i386/tcg/tcg-cpu.c | 6 ++++-- 3 files changed, 20 insertions(+), 12 deletions(-)