diff mbox series

[v6,04/26] plugins: Drop tcg_flags from struct qemu_plugin_dyn_cb

Message ID 20210502235727.1979457-5-richard.henderson@linaro.org
State Superseded
Headers show
Series TCI fixes and cleanups | expand

Commit Message

Richard Henderson May 2, 2021, 11:57 p.m. UTC 
As noted by qemu-plugins.h, enum qemu_plugin_cb_flags is
currently unused -- plugins can neither read nor write
guest registers.

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

---
 accel/tcg/plugin-helpers.h |  1 -
 include/qemu/plugin.h      |  1 -
 accel/tcg/plugin-gen.c     |  8 ++++----
 plugins/core.c             | 30 ++++++------------------------
 4 files changed, 10 insertions(+), 30 deletions(-)

-- 
2.25.1

Comments

Philippe Mathieu-Daudé May 16, 2021, 12:53 p.m. UTC | #1 
Hi Richard,

On 5/3/21 1:57 AM, Richard Henderson wrote:
> As noted by qemu-plugins.h, enum qemu_plugin_cb_flags is

> currently unused -- plugins can neither read nor write

> guest registers.

> 

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

> ---

>  accel/tcg/plugin-helpers.h |  1 -

>  include/qemu/plugin.h      |  1 -

>  accel/tcg/plugin-gen.c     |  8 ++++----

>  plugins/core.c             | 30 ++++++------------------------

>  4 files changed, 10 insertions(+), 30 deletions(-)

> 

> diff --git a/accel/tcg/plugin-helpers.h b/accel/tcg/plugin-helpers.h

> index 1916ee7920..853bd21677 100644

> --- a/accel/tcg/plugin-helpers.h

> +++ b/accel/tcg/plugin-helpers.h

> @@ -1,5 +1,4 @@

>  #ifdef CONFIG_PLUGIN

> -/* Note: no TCG flags because those are overwritten later */

>  DEF_HELPER_2(plugin_vcpu_udata_cb, void, i32, ptr)

>  DEF_HELPER_4(plugin_vcpu_mem_cb, void, i32, i32, i64, ptr)

>  #endif

> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h

> index c5a79a89f0..0fefbc6084 100644

> --- a/include/qemu/plugin.h

> +++ b/include/qemu/plugin.h

> @@ -79,7 +79,6 @@ enum plugin_dyn_cb_subtype {

>  struct qemu_plugin_dyn_cb {

>      union qemu_plugin_cb_sig f;

>      void *userp;

> -    unsigned tcg_flags;

>      enum plugin_dyn_cb_subtype type;

>      /* @rw applies to mem callbacks only (both regular and inline) */

>      enum qemu_plugin_mem_rw rw;

> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c

> index eb99be52d0..1e7f201cd2 100644

> --- a/accel/tcg/plugin-gen.c

> +++ b/accel/tcg/plugin-gen.c

> @@ -385,7 +385,7 @@ static TCGOp *copy_st_ptr(TCGOp **begin_op, TCGOp *op)

>  }

>  

>  static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *empty_func,

> -                        void *func, unsigned tcg_flags, int *cb_idx)

> +                        void *func, int *cb_idx)

>  {

>      /* copy all ops until the call */

>      do {

> @@ -412,7 +412,7 @@ static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *empty_func,

>          tcg_debug_assert(i < MAX_OPC_PARAM_ARGS);

>      }

>      op->args[*cb_idx] = (uintptr_t)func;

> -    op->args[*cb_idx + 1] = tcg_flags;

> +    op->args[*cb_idx + 1] = (*begin_op)->args[*cb_idx + 1];


I don't understand this change, can you explain?

>  

>      return op;

>  }

> @@ -439,7 +439,7 @@ static TCGOp *append_udata_cb(const struct qemu_plugin_dyn_cb *cb,

>  

>      /* call */

>      op = copy_call(&begin_op, op, HELPER(plugin_vcpu_udata_cb),

> -                   cb->f.vcpu_udata, cb->tcg_flags, cb_idx);

> +                   cb->f.vcpu_udata, cb_idx);

>  

>      return op;

>  }

> @@ -490,7 +490,7 @@ static TCGOp *append_mem_cb(const struct qemu_plugin_dyn_cb *cb,

>      if (type == PLUGIN_GEN_CB_MEM) {

>          /* call */

>          op = copy_call(&begin_op, op, HELPER(plugin_vcpu_mem_cb),

> -                       cb->f.vcpu_udata, cb->tcg_flags, cb_idx);

> +                       cb->f.vcpu_udata, cb_idx);

>      }

>  

>      return op;

> diff --git a/plugins/core.c b/plugins/core.c

> index 87b823bbc4..03e0a4c806 100644

> --- a/plugins/core.c

> +++ b/plugins/core.c

> @@ -297,33 +297,15 @@ void plugin_register_inline_op(GArray **arr,

>      dyn_cb->inline_insn.imm = imm;

>  }

>  

> -static inline uint32_t cb_to_tcg_flags(enum qemu_plugin_cb_flags flags)

> -{

> -    uint32_t ret;

> -

> -    switch (flags) {

> -    case QEMU_PLUGIN_CB_RW_REGS:

> -        ret = 0;

> -        break;

> -    case QEMU_PLUGIN_CB_R_REGS:

> -        ret = TCG_CALL_NO_WG;

> -        break;

> -    case QEMU_PLUGIN_CB_NO_REGS:

> -    default:

> -        ret = TCG_CALL_NO_RWG;

> -    }

> -    return ret;

> -}

> -

> -inline void

> -plugin_register_dyn_cb__udata(GArray **arr,

> -                              qemu_plugin_vcpu_udata_cb_t cb,

> -                              enum qemu_plugin_cb_flags flags, void *udata)

> +void plugin_register_dyn_cb__udata(GArray **arr,

> +                                   qemu_plugin_vcpu_udata_cb_t cb,

> +                                   enum qemu_plugin_cb_flags flags,

> +                                   void *udata)

>  {

>      struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);

>  

>      dyn_cb->userp = udata;

> -    dyn_cb->tcg_flags = cb_to_tcg_flags(flags);

> +    /* Note flags are discarded as unused. */

>      dyn_cb->f.vcpu_udata = cb;

>      dyn_cb->type = PLUGIN_CB_REGULAR;

>  }

> @@ -338,7 +320,7 @@ void plugin_register_vcpu_mem_cb(GArray **arr,

>  

>      dyn_cb = plugin_get_dyn_cb(arr);

>      dyn_cb->userp = udata;

> -    dyn_cb->tcg_flags = cb_to_tcg_flags(flags);

> +    /* Note flags are discarded as unused. */

>      dyn_cb->type = PLUGIN_CB_REGULAR;

>      dyn_cb->rw = rw;

>      dyn_cb->f.generic = cb;

>
Richard Henderson May 17, 2021, 4:13 p.m. UTC | #2 
On 5/16/21 7:53 AM, Philippe Mathieu-Daudé wrote:
>> -    op->args[*cb_idx + 1] = tcg_flags;

>> +    op->args[*cb_idx + 1] = (*begin_op)->args[*cb_idx + 1];

> 

> I don't understand this change, can you explain?


This patch drops a mostly-unimplemented feature from plugins, where in theory 
the registration of the plugin would specify the TCG_CALL_* flags.

Instead, take the flags from the plugin template function -- i.e. copy them 
across from the original begin_op.

>> -static inline uint32_t cb_to_tcg_flags(enum qemu_plugin_cb_flags flags)

>> -{

>> -    uint32_t ret;

>> -

>> -    switch (flags) {

>> -    case QEMU_PLUGIN_CB_RW_REGS:

>> -        ret = 0;

>> -        break;

>> -    case QEMU_PLUGIN_CB_R_REGS:

>> -        ret = TCG_CALL_NO_WG;

>> -        break;

>> -    case QEMU_PLUGIN_CB_NO_REGS:

>> -    default:

>> -        ret = TCG_CALL_NO_RWG;

>> -    }

>> -    return ret;

>> -}


This is where the plugin interface was supposed to convert flags from one form 
to another.  This got stored in a structure and then passed along as an 
argument to the function containing that first hunk above.


r~
diff mbox series

Patch

diff --git a/accel/tcg/plugin-helpers.h b/accel/tcg/plugin-helpers.h
index 1916ee7920..853bd21677 100644
--- a/accel/tcg/plugin-helpers.h
+++ b/accel/tcg/plugin-helpers.h
@@ -1,5 +1,4 @@ 
 #ifdef CONFIG_PLUGIN
-/* Note: no TCG flags because those are overwritten later */
 DEF_HELPER_2(plugin_vcpu_udata_cb, void, i32, ptr)
 DEF_HELPER_4(plugin_vcpu_mem_cb, void, i32, i32, i64, ptr)
 #endif
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index c5a79a89f0..0fefbc6084 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -79,7 +79,6 @@  enum plugin_dyn_cb_subtype {
 struct qemu_plugin_dyn_cb {
     union qemu_plugin_cb_sig f;
     void *userp;
-    unsigned tcg_flags;
     enum plugin_dyn_cb_subtype type;
     /* @rw applies to mem callbacks only (both regular and inline) */
     enum qemu_plugin_mem_rw rw;
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index eb99be52d0..1e7f201cd2 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -385,7 +385,7 @@  static TCGOp *copy_st_ptr(TCGOp **begin_op, TCGOp *op)
 }
 
 static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *empty_func,
-                        void *func, unsigned tcg_flags, int *cb_idx)
+                        void *func, int *cb_idx)
 {
     /* copy all ops until the call */
     do {
@@ -412,7 +412,7 @@  static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *empty_func,
         tcg_debug_assert(i < MAX_OPC_PARAM_ARGS);
     }
     op->args[*cb_idx] = (uintptr_t)func;
-    op->args[*cb_idx + 1] = tcg_flags;
+    op->args[*cb_idx + 1] = (*begin_op)->args[*cb_idx + 1];
 
     return op;
 }
@@ -439,7 +439,7 @@  static TCGOp *append_udata_cb(const struct qemu_plugin_dyn_cb *cb,
 
     /* call */
     op = copy_call(&begin_op, op, HELPER(plugin_vcpu_udata_cb),
-                   cb->f.vcpu_udata, cb->tcg_flags, cb_idx);
+                   cb->f.vcpu_udata, cb_idx);
 
     return op;
 }
@@ -490,7 +490,7 @@  static TCGOp *append_mem_cb(const struct qemu_plugin_dyn_cb *cb,
     if (type == PLUGIN_GEN_CB_MEM) {
         /* call */
         op = copy_call(&begin_op, op, HELPER(plugin_vcpu_mem_cb),
-                       cb->f.vcpu_udata, cb->tcg_flags, cb_idx);
+                       cb->f.vcpu_udata, cb_idx);
     }
 
     return op;
diff --git a/plugins/core.c b/plugins/core.c
index 87b823bbc4..03e0a4c806 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -297,33 +297,15 @@  void plugin_register_inline_op(GArray **arr,
     dyn_cb->inline_insn.imm = imm;
 }
 
-static inline uint32_t cb_to_tcg_flags(enum qemu_plugin_cb_flags flags)
-{
-    uint32_t ret;
-
-    switch (flags) {
-    case QEMU_PLUGIN_CB_RW_REGS:
-        ret = 0;
-        break;
-    case QEMU_PLUGIN_CB_R_REGS:
-        ret = TCG_CALL_NO_WG;
-        break;
-    case QEMU_PLUGIN_CB_NO_REGS:
-    default:
-        ret = TCG_CALL_NO_RWG;
-    }
-    return ret;
-}
-
-inline void
-plugin_register_dyn_cb__udata(GArray **arr,
-                              qemu_plugin_vcpu_udata_cb_t cb,
-                              enum qemu_plugin_cb_flags flags, void *udata)
+void plugin_register_dyn_cb__udata(GArray **arr,
+                                   qemu_plugin_vcpu_udata_cb_t cb,
+                                   enum qemu_plugin_cb_flags flags,
+                                   void *udata)
 {
     struct qemu_plugin_dyn_cb *dyn_cb = plugin_get_dyn_cb(arr);
 
     dyn_cb->userp = udata;
-    dyn_cb->tcg_flags = cb_to_tcg_flags(flags);
+    /* Note flags are discarded as unused. */
     dyn_cb->f.vcpu_udata = cb;
     dyn_cb->type = PLUGIN_CB_REGULAR;
 }
@@ -338,7 +320,7 @@  void plugin_register_vcpu_mem_cb(GArray **arr,
 
     dyn_cb = plugin_get_dyn_cb(arr);
     dyn_cb->userp = udata;
-    dyn_cb->tcg_flags = cb_to_tcg_flags(flags);
+    /* Note flags are discarded as unused. */
     dyn_cb->type = PLUGIN_CB_REGULAR;
     dyn_cb->rw = rw;
     dyn_cb->f.generic = cb;