Re: [PATCH 13/35] target/avr: call plugin trap callbacks

["Philippe Mathieu-Daudé" <philmd@oss.qualcomm.com>]

Hi Julian,

On 2025-10-27 12:03, Alex Bennée wrote:
> From: Julian Ganz <neither@nut.email>
> 
> 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 the hook for AVR targets. That architecture appears
> to only know interrupts.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Julian Ganz <neither@nut.email>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   target/avr/helper.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/target/avr/helper.c b/target/avr/helper.c
> index 4b29ab35263..365c8c60e19 100644
> --- a/target/avr/helper.c
> +++ b/target/avr/helper.c
> @@ -28,6 +28,7 @@
>   #include "exec/target_page.h"
>   #include "accel/tcg/cpu-ldst.h"
>   #include "exec/helper-proto.h"
> +#include "qemu/plugin.h"
>   
>   bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>   {
> @@ -102,6 +103,8 @@ void avr_cpu_do_interrupt(CPUState *cs)
>       env->sregI = 0; /* clear Global Interrupt Flag */
>   
>       cs->exception_index = -1;
> +
> +    qemu_plugin_vcpu_interrupt_cb(cs, ret);
>   }
Expanding a bit ...

  77 void avr_cpu_do_interrupt(CPUState *cs)
  78 {
  79     CPUAVRState *env = cpu_env(cs);
  80
  81     uint32_t ret = env->pc_w;
  82     int vector = 0;
  83     int size = avr_feature(env, AVR_FEATURE_JMP_CALL) ? 2 : 1;
  84     int base = 0;
  85
  86     if (cs->exception_index == EXCP_RESET) {
  87         vector = 0;
  88     } else if (env->intsrc != 0) {
  89         vector = ctz64(env->intsrc) + 1;
  90     }

Unconditionally recording exception vector as interrupt seems wrong,
shouldn't we call qemu_plugin_vcpu_exception_cb() in that case?

  91
  92     if (avr_feature(env, AVR_FEATURE_3_BYTE_PC)) {
  93         do_stb(env, env->sp--, ret, 0);
  94         do_stb(env, env->sp--, ret >> 8, 0);
  95         do_stb(env, env->sp--, ret >> 16, 0);
  96     } else if (avr_feature(env, AVR_FEATURE_2_BYTE_PC)) {
  97         do_stb(env, env->sp--, ret, 0);
  98         do_stb(env, env->sp--, ret >> 8, 0);
  99     } else {
100         do_stb(env, env->sp--, ret, 0);
101     }
102
103     env->pc_w = base + vector * size;
104     env->sregI = 0; /* clear Global Interrupt Flag */
105
106     cs->exception_index = -1;
107
108     qemu_plugin_vcpu_interrupt_cb(cs, ret);
109 }