[PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc

Alex Bennée posted 14 patches 4 years, 11 months ago
[PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc
Posted by Alex Bennée 4 years, 11 months ago
Also add a note to explain currently they are unused.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/qemu/qemu-plugin.h | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 9ae3940d89..c98866a637 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -207,10 +207,20 @@ struct qemu_plugin_tb;
 /** struct qemu_plugin_insn - Opaque handle for a translated instruction */
 struct qemu_plugin_insn;
 
+/**
+ * enum qemu_plugin_cb_flags - type of callback
+ *
+ * @QEMU_PLUGIN_CB_NO_REGS: callback does not access the CPU's regs
+ * @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs
+ * @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs
+ *
+ * Note: currently unused, plugins cannot read or change system
+ * register state.
+ */
 enum qemu_plugin_cb_flags {
-    QEMU_PLUGIN_CB_NO_REGS, /* callback does not access the CPU's regs */
-    QEMU_PLUGIN_CB_R_REGS,  /* callback reads the CPU's regs */
-    QEMU_PLUGIN_CB_RW_REGS, /* callback reads and writes the CPU's regs */
+    QEMU_PLUGIN_CB_NO_REGS,
+    QEMU_PLUGIN_CB_R_REGS,
+    QEMU_PLUGIN_CB_RW_REGS,
 };
 
 enum qemu_plugin_mem_rw {
-- 
2.20.1


Re: [PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc
Posted by Aaron Lindsay via 4 years, 11 months ago
On Mar 12 17:28, Alex Bennée wrote:
> Also add a note to explain currently they are unused.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

I'm personally interested in one clarification below, but don't think
that affects my:

Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>

> ---
>  include/qemu/qemu-plugin.h | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 9ae3940d89..c98866a637 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -207,10 +207,20 @@ struct qemu_plugin_tb;
>  /** struct qemu_plugin_insn - Opaque handle for a translated instruction */
>  struct qemu_plugin_insn;
>  
> +/**
> + * enum qemu_plugin_cb_flags - type of callback
> + *
> + * @QEMU_PLUGIN_CB_NO_REGS: callback does not access the CPU's regs
> + * @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs
> + * @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs
> + *
> + * Note: currently unused, plugins cannot read or change system
> + * register state.

They are unused in the sense that the current plugin interface does not
provide a way to make use of them. But are they completely free from
side effects?

-Aaron

Re: [PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc
Posted by Alex Bennée 4 years, 11 months ago
Aaron Lindsay <aaron@os.amperecomputing.com> writes:

> On Mar 12 17:28, Alex Bennée wrote:
>> Also add a note to explain currently they are unused.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> I'm personally interested in one clarification below, but don't think
> that affects my:
>
> Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
>
>> ---
>>  include/qemu/qemu-plugin.h | 16 +++++++++++++---
>>  1 file changed, 13 insertions(+), 3 deletions(-)
>> 
>> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
>> index 9ae3940d89..c98866a637 100644
>> --- a/include/qemu/qemu-plugin.h
>> +++ b/include/qemu/qemu-plugin.h
>> @@ -207,10 +207,20 @@ struct qemu_plugin_tb;
>>  /** struct qemu_plugin_insn - Opaque handle for a translated instruction */
>>  struct qemu_plugin_insn;
>>  
>> +/**
>> + * enum qemu_plugin_cb_flags - type of callback
>> + *
>> + * @QEMU_PLUGIN_CB_NO_REGS: callback does not access the CPU's regs
>> + * @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs
>> + * @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs
>> + *
>> + * Note: currently unused, plugins cannot read or change system
>> + * register state.
>
> They are unused in the sense that the current plugin interface does not
> provide a way to make use of them. But are they completely free from
> side effects?

They are free of side effects visible to the plugin. Under the covers it
uses the existing TCG_CALL_NO_* mechanics to ensure that register state
is synced to/from TCG temporaries before the callback.

>
> -Aaron


-- 
Alex Bennée

Re: [PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc
Posted by Aaron Lindsay via 4 years, 11 months ago
On Mar 16 13:40, Alex Bennée wrote:
> 
> Aaron Lindsay <aaron@os.amperecomputing.com> writes:
> 
> > On Mar 12 17:28, Alex Bennée wrote:
> >> Also add a note to explain currently they are unused.
> >> 
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >
> > I'm personally interested in one clarification below, but don't think
> > that affects my:
> >
> > Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
> >
> >> ---
> >>  include/qemu/qemu-plugin.h | 16 +++++++++++++---
> >>  1 file changed, 13 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> >> index 9ae3940d89..c98866a637 100644
> >> --- a/include/qemu/qemu-plugin.h
> >> +++ b/include/qemu/qemu-plugin.h
> >> @@ -207,10 +207,20 @@ struct qemu_plugin_tb;
> >>  /** struct qemu_plugin_insn - Opaque handle for a translated instruction */
> >>  struct qemu_plugin_insn;
> >>  
> >> +/**
> >> + * enum qemu_plugin_cb_flags - type of callback
> >> + *
> >> + * @QEMU_PLUGIN_CB_NO_REGS: callback does not access the CPU's regs
> >> + * @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs
> >> + * @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs
> >> + *
> >> + * Note: currently unused, plugins cannot read or change system
> >> + * register state.
> >
> > They are unused in the sense that the current plugin interface does not
> > provide a way to make use of them. But are they completely free from
> > side effects?
> 
> They are free of side effects visible to the plugin. Under the covers it
> uses the existing TCG_CALL_NO_* mechanics to ensure that register state
> is synced to/from TCG temporaries before the callback.

I would currently find it useful to have that information included in
the documentation since there is no register state exposed and I am
basically hacking something together for my own use in the meantime...
but I understand that is in tension with the general philosophy of the
plugins to not expose implementation details.

-Aaron