We recently introduced API for registering callbacks for trap related
events as well as the corresponding hook functions. Due to differences
between architectures, the latter need to be called from target specific
code.

This change places hooks for Alpha targets.

Signed-off-by: Julian Ganz <neither@nut.email>
---
 target/alpha/helper.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/target/alpha/helper.c b/target/alpha/helper.c
index 096eac3445..a9af52a928 100644
--- a/target/alpha/helper.c
+++ b/target/alpha/helper.c
@@ -27,6 +27,7 @@
 #include "exec/helper-proto.h"
 #include "qemu/qemu-print.h"
 #include "system/memory.h"
+#include "qemu/plugin.h"
 
 
 #define CONVERT_BIT(X, SRC, DST) \
@@ -328,6 +329,7 @@ void alpha_cpu_do_interrupt(CPUState *cs)
 {
     CPUAlphaState *env = cpu_env(cs);
     int i = cs->exception_index;
+    uint64_t last_pc = env->pc;
 
     if (qemu_loglevel_mask(CPU_LOG_INT)) {
         static int count;
@@ -431,6 +433,17 @@ void alpha_cpu_do_interrupt(CPUState *cs)
 
     /* Switch to PALmode.  */
     env->flags |= ENV_FLAG_PAL_MODE;
+
+    switch (i) {
+    case EXCP_SMP_INTERRUPT:
+    case EXCP_CLK_INTERRUPT:
+    case EXCP_DEV_INTERRUPT:
+        qemu_plugin_vcpu_interrupt_cb(cs, last_pc);
+        break;
+    default:
+        qemu_plugin_vcpu_exception_cb(cs, last_pc);
+        break;
+    }
 }
 
 bool alpha_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
-- 
2.49.0

On 5/19/25 16:19, Julian Ganz wrote:
> We recently introduced API for registering callbacks for trap related
> events as well as the corresponding hook functions. Due to differences
> between architectures, the latter need to be called from target specific
> code.
> 
> This change places hooks for Alpha targets.
> 
> Signed-off-by: Julian Ganz <neither@nut.email>
> ---
>   target/alpha/helper.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/target/alpha/helper.c b/target/alpha/helper.c
> index 096eac3445..a9af52a928 100644
> --- a/target/alpha/helper.c
> +++ b/target/alpha/helper.c
> @@ -27,6 +27,7 @@
>   #include "exec/helper-proto.h"
>   #include "qemu/qemu-print.h"
>   #include "system/memory.h"
> +#include "qemu/plugin.h"
>   
>   
>   #define CONVERT_BIT(X, SRC, DST) \
> @@ -328,6 +329,7 @@ void alpha_cpu_do_interrupt(CPUState *cs)
>   {
>       CPUAlphaState *env = cpu_env(cs);
>       int i = cs->exception_index;
> +    uint64_t last_pc = env->pc;
>   
>       if (qemu_loglevel_mask(CPU_LOG_INT)) {
>           static int count;
> @@ -431,6 +433,17 @@ void alpha_cpu_do_interrupt(CPUState *cs)
>   
>       /* Switch to PALmode.  */
>       env->flags |= ENV_FLAG_PAL_MODE;
> +
> +    switch (i) {
> +    case EXCP_SMP_INTERRUPT:
> +    case EXCP_CLK_INTERRUPT:
> +    case EXCP_DEV_INTERRUPT:
> +        qemu_plugin_vcpu_interrupt_cb(cs, last_pc);
> +        break;
> +    default:
> +        qemu_plugin_vcpu_exception_cb(cs, last_pc);
> +        break;
> +    }

Having read the whole series now, I think it would be better to change the 
TCGCPUOps.do_interrupt interface.

Instead of having each target call qemu_plugin_*, instead have each do_interrupt return 
the discontinuity type, or 0 if the interrupt is blocked so no state change.

Change to cpu_handle_exception would be of the form:

     if (qemu_plugin_discon_enabled(cpu)) {
         vaddr from = tcg_ops->get_pc(cpu);
         unsigned ev = tcg_ops->do_interrupt(cpu);
         if (ev) {
             qemu_plugin_vcpu_discon_cb(cpu, ev, from);
         }
     } else {
         tcg_ops->do_interrupt(cpu);
     }


r~

Hi Richard,

May 25, 2025 at 2:14 PM, Richard Henderson wrote:
> Having read the whole series now, I think it would be better to change the TCGCPUOps.do_interrupt interface.
> 
> Instead of having each target call qemu_plugin_*, instead have each do_interrupt return the discontinuity type, or 0 if the interrupt is blocked so no state change.
> 
> Change to cpu_handle_exception would be of the form:
> 
>  if (qemu_plugin_discon_enabled(cpu)) {
>  vaddr from = tcg_ops->get_pc(cpu);
>  unsigned ev = tcg_ops->do_interrupt(cpu);
>  if (ev) {
>  qemu_plugin_vcpu_discon_cb(cpu, ev, from);
>  }
>  } else {
>  tcg_ops->do_interrupt(cpu);
>  }

Personally, I'd be in favour of that. However, I do see some obstacles
to that.

Quite a few targets to call their do_interrupt function internally,
usually from their exec_interrupt. We would then handle that function's
return value at the call site?

Also, some targets such as tricore only have a dummy/stub do_interrupt
and handle exceptions differently inside non-returning functions. For
those, we would call the hooks directly from there as we do now?

And then we have some targets that deviate in some other way. For
example, s390x_cpu_do_interrupt effectively contains a loop, and we
potentially need to call one of the hooks for each iteration.

Regards,
Julian

On 5/25/25 21:16, Julian Ganz wrote:
> Hi Richard,
> 
> May 25, 2025 at 2:14 PM, Richard Henderson wrote:
>> Having read the whole series now, I think it would be better to change the TCGCPUOps.do_interrupt interface.
>>
>> Instead of having each target call qemu_plugin_*, instead have each do_interrupt return the discontinuity type, or 0 if the interrupt is blocked so no state change.
>>
>> Change to cpu_handle_exception would be of the form:
>>
>>   if (qemu_plugin_discon_enabled(cpu)) {
>>   vaddr from = tcg_ops->get_pc(cpu);
>>   unsigned ev = tcg_ops->do_interrupt(cpu);
>>   if (ev) {
>>   qemu_plugin_vcpu_discon_cb(cpu, ev, from);
>>   }
>>   } else {
>>   tcg_ops->do_interrupt(cpu);
>>   }
> 
> Personally, I'd be in favour of that. However, I do see some obstacles
> to that.
> 
> Quite a few targets to call their do_interrupt function internally,
> usually from their exec_interrupt. We would then handle that function's
> return value at the call site?

Yes, I think we'd alter the return value of exec_interrupt to match do_interrupt.
There's a comment about exec_interrupt may longjmp away, but that *appears* to be 
historical.  I couldn't identify an existing target that does that.

> Also, some targets such as tricore only have a dummy/stub do_interrupt
> and handle exceptions differently inside non-returning functions. For
> those, we would call the hooks directly from there as we do now?

It may be only tricore.  And you're right, it would be a non-trivial reorg to make tricore 
fall in line with other implementations.  So retaining the separate 
qemu_plugin_vcpu_exception_cb will be required in the short term.


> And then we have some targets that deviate in some other way. For
> example, s390x_cpu_do_interrupt effectively contains a loop, and we
> potentially need to call one of the hooks for each iteration.

That is distinctly odd.  I don't understand what's going on there.

David, can you elucidate re ce204cba74b?  Is this intending to stack all outstanding 
interrupts all at once, leaving the highest priority, then return-from-interrupt processes 
the lower priorities?  It's definitely unusual, but most things about s390x are...  :-)


r~

Hi Richard,

May 26, 2025 at 11:01 AM, Richard Henderson wrote:
> On 5/25/25 21:16, Julian Ganz wrote:
> > Also, some targets such as tricore only have a dummy/stub do_interrupt
> >  and handle exceptions differently inside non-returning functions. For
> >  those, we would call the hooks directly from there as we do now?
> > 
> It may be only tricore. And you're right, it would be a non-trivial reorg to make tricore fall in line with other implementations. So retaining the separate qemu_plugin_vcpu_exception_cb will be required in the short term.

Yes, tricore (and hexagon, which I decided to not support) are the only
ones that don't have a do_interrupt at all.

Note that x86, and maybe other targets as well, may not return from its
do_interrupt but call cpu_loop_exit_restore or something similar,
arbitrarily deep in their call stack. But as far as I can tell that's
also exclusively for exceptions.

Regards,
Julian

On 5/26/25 10:54, Julian Ganz wrote:
> Hi Richard,
> 
> May 26, 2025 at 11:01 AM, Richard Henderson wrote:
>> On 5/25/25 21:16, Julian Ganz wrote:
>>> Also, some targets such as tricore only have a dummy/stub do_interrupt
>>>   and handle exceptions differently inside non-returning functions. For
>>>   those, we would call the hooks directly from there as we do now?
>>>
>> It may be only tricore. And you're right, it would be a non-trivial reorg to make tricore fall in line with other implementations. So retaining the separate qemu_plugin_vcpu_exception_cb will be required in the short term.
> 
> Yes, tricore (and hexagon, which I decided to not support) are the only
> ones that don't have a do_interrupt at all.

Hexagon does not yet support system mode, and so is out-of-scope.


r~

On 5/19/25 16:19, Julian Ganz wrote:
> We recently introduced API for registering callbacks for trap related
> events as well as the corresponding hook functions. Due to differences
> between architectures, the latter need to be called from target specific
> code.
> 
> This change places hooks for Alpha targets.
> 
> Signed-off-by: Julian Ganz<neither@nut.email>
> ---
>   target/alpha/helper.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~