Re: [RFC PATCH v3 01/11] plugins: add types for callbacks related to certain discontinuities

[Julian Ganz]

Hi Alex,

January 9, 2025 at 2:52 PM, "Alex Bennée" wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> >  On 12/2/24 11:26, Julian Ganz wrote:
> > >  diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> > >  index 0fba36ae02..9c67374b7e 100644
> > >  --- a/include/qemu/qemu-plugin.h
> > >  +++ b/include/qemu/qemu-plugin.h
> > >  @@ -154,6 +154,49 @@ typedef void 
> > > (*qemu_plugin_vcpu_simple_cb_t)(qemu_plugin_id_t id,
> > >  typedef void (*qemu_plugin_vcpu_udata_cb_t)(unsigned int vcpu_index,
> > >  void *userdata);
> > >  +
> > >  +/**
> > >  + * enum qemu_plugin_discon_type - type of a (potential) PC discontinuity
> > >  + *
> > >  + * @QEMU_PLUGIN_DISCON_INTERRUPT: an interrupt, defined across all 
> > > architectures
> > >  + * as an asynchronous event, usually originating
> > >  + * from outside the CPU
> > >  + * @QEMU_PLUGIN_DISCON_EXCEPTION: an exception, defined across all 
> > > architectures
> > >  + * as a synchronous event in response to a
> > >  + * specific instruction being executed
> > >  + * @QEMU_PLUGIN_DISCON_HOSTCALL: a host call, functionally a special 
> > > kind of
> > >  + * exception that is not handled by code run by
> > >  + * the vCPU but machinery outside the vCPU
> > >  + * @QEMU_PLUGIN_DISCON_ALL: all types of disconinuity events currently 
> > > covered
> > >  + */
> > >  +enum qemu_plugin_discon_type {
> > >  + QEMU_PLUGIN_DISCON_INTERRUPT = 1,
> > >  + QEMU_PLUGIN_DISCON_EXCEPTION = 2,
> > >  + QEMU_PLUGIN_DISCON_HOSTCALL = 4,
> > >  + QEMU_PLUGIN_DISCON_ALL = 7
> > >  +};
> > > 
> >  Matter of style, but would be better to use:
> > 
> >  enum qemu_plugin_discon_type {
> >  QEMU_PLUGIN_DISCON_INTERRUPT = 1 << 0,
> >  QEMU_PLUGIN_DISCON_EXCEPTION = 1 << 1,
> >  QEMU_PLUGIN_DISCON_HOSTCALL = 1 << 2,
> >  QEMU_PLUGIN_DISCON_ALL = -1
> >  };
> > 
> <snip>
> 
> Is this really a bit field though? If you will only report type of
> discontinuity at a time a simple 0 based enum with
> QEMU_PLUGIN_DISCON_MAX would be simpler.

We don't only use this type to communicate the kind of discontinuity but
also when registering callbacks. I'll make this more clear in the commit
message and/or documentation in the next series.

Regards,
Julian