| Message ID | 20260902152044.31291-6-philmd@oss.qualcomm.com |
|---|---|
| State | New |
| Headers | show |
| Series | accel/tcg: Push BQL down into per-target do_interrupt handlers | expand |
On 9/2/2026 8:20 AM, Philippe Mathieu-Daudé wrote: > Expand docstring to clarify the handler is called with BQL held. > Add assertion that this callback is mandatory and never NULL, since > all targets must implement interrupt handling. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> > --- > include/accel/tcg/cpu-ops.h | 11 ++++++++++- > accel/tcg/cpu-exec.c | 1 + > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/include/accel/tcg/cpu-ops.h b/include/accel/tcg/cpu-ops.h > index 87850402203..13df70eaa9f 100644 > --- a/include/accel/tcg/cpu-ops.h > +++ b/include/accel/tcg/cpu-ops.h > @@ -169,7 +169,16 @@ struct TCGCPUOps { > */ > vaddr (*untagged_addr)(CPUState *cs, vaddr addr); > #else > - /** @do_interrupt: Callback for interrupt handling. */ > + /** > + * @do_interrupt: Deliver a pending exception/interrupt to the CPU > + * @cpu: cpu context > + * > + * Called when cs->exception_index contains an exception code to deliver. > + * Updates CPU architectural state (usually before executing a guest > + * exception handler). > + * > + * Called from cpu_handle_exception() with BQL held. Would that be clearer to ensure this with an assert instead or relying on comment? > + */ > void (*do_interrupt)(CPUState *cpu); > > /** > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c > index 609931a0d6e..2fae2c024b8 100644 > --- a/accel/tcg/cpu-exec.c > +++ b/accel/tcg/cpu-exec.c > @@ -1059,6 +1059,7 @@ bool tcg_exec_realizefn(CPUState *cpu, Error **errp) > assert(tcg_ops->cpu_exec_halt); > assert(tcg_ops->cpu_exec_interrupt); > assert(tcg_ops->cpu_exec_reset); > + assert(tcg_ops->do_interrupt); > assert(tcg_ops->pointer_wrap); > #endif /* !CONFIG_USER_ONLY */ > assert(tcg_ops->translate_code);
On 2/9/26 21:54, Pierrick Bouvier wrote: > On 9/2/2026 8:20 AM, Philippe Mathieu-Daudé wrote: >> Expand docstring to clarify the handler is called with BQL held. >> Add assertion that this callback is mandatory and never NULL, since >> all targets must implement interrupt handling. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> >> --- >> include/accel/tcg/cpu-ops.h | 11 ++++++++++- >> accel/tcg/cpu-exec.c | 1 + >> 2 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/include/accel/tcg/cpu-ops.h b/include/accel/tcg/cpu-ops.h >> index 87850402203..13df70eaa9f 100644 >> --- a/include/accel/tcg/cpu-ops.h >> +++ b/include/accel/tcg/cpu-ops.h >> @@ -169,7 +169,16 @@ struct TCGCPUOps { >> */ >> vaddr (*untagged_addr)(CPUState *cs, vaddr addr); >> #else >> - /** @do_interrupt: Callback for interrupt handling. */ >> + /** >> + * @do_interrupt: Deliver a pending exception/interrupt to the CPU >> + * @cpu: cpu context >> + * >> + * Called when cs->exception_index contains an exception code to deliver. >> + * Updates CPU architectural state (usually before executing a guest >> + * exception handler). >> + * >> + * Called from cpu_handle_exception() with BQL held. > > Would that be clearer to ensure this with an assert instead or relying > on comment? This docstring contract is a help to implement callees. > >> + */ >> void (*do_interrupt)(CPUState *cpu); >> >> /** >> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c >> index 609931a0d6e..2fae2c024b8 100644 >> --- a/accel/tcg/cpu-exec.c >> +++ b/accel/tcg/cpu-exec.c >> @@ -1059,6 +1059,7 @@ bool tcg_exec_realizefn(CPUState *cpu, Error **errp) >> assert(tcg_ops->cpu_exec_halt); >> assert(tcg_ops->cpu_exec_interrupt); >> assert(tcg_ops->cpu_exec_reset); >> + assert(tcg_ops->do_interrupt); >> assert(tcg_ops->pointer_wrap); >> #endif /* !CONFIG_USER_ONLY */ >> assert(tcg_ops->translate_code); > >
diff --git a/include/accel/tcg/cpu-ops.h b/include/accel/tcg/cpu-ops.h index 87850402203..13df70eaa9f 100644 --- a/include/accel/tcg/cpu-ops.h +++ b/include/accel/tcg/cpu-ops.h @@ -169,7 +169,16 @@ struct TCGCPUOps { */ vaddr (*untagged_addr)(CPUState *cs, vaddr addr); #else - /** @do_interrupt: Callback for interrupt handling. */ + /** + * @do_interrupt: Deliver a pending exception/interrupt to the CPU + * @cpu: cpu context + * + * Called when cs->exception_index contains an exception code to deliver. + * Updates CPU architectural state (usually before executing a guest + * exception handler). + * + * Called from cpu_handle_exception() with BQL held. + */ void (*do_interrupt)(CPUState *cpu); /** diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index 609931a0d6e..2fae2c024b8 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -1059,6 +1059,7 @@ bool tcg_exec_realizefn(CPUState *cpu, Error **errp) assert(tcg_ops->cpu_exec_halt); assert(tcg_ops->cpu_exec_interrupt); assert(tcg_ops->cpu_exec_reset); + assert(tcg_ops->do_interrupt); assert(tcg_ops->pointer_wrap); #endif /* !CONFIG_USER_ONLY */ assert(tcg_ops->translate_code);
Expand docstring to clarify the handler is called with BQL held. Add assertion that this callback is mandatory and never NULL, since all targets must implement interrupt handling. Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> --- include/accel/tcg/cpu-ops.h | 11 ++++++++++- accel/tcg/cpu-exec.c | 1 + 2 files changed, 11 insertions(+), 1 deletion(-)