Re: [Qemu-devel] [RFC 18/48] tcg: add memory callbacks for plugins (WIP)

[Alex Bennée]

Emilio G. Cota  writes:

> XXX: store hostaddr from non-i386 TCG backends
> XXX: what hostaddr to return for I/O accesses?
> XXX: what hostaddr to return for cross-page accesses?
>
> Here the trickiest feature is passing the host address to
> memory callbacks that request it. Perhaps it would be more
> appropriate to pass a "physical" address to plugins, but since
> in QEMU host addr ~= guest physical, I'm going with that for
> simplicity.
>
> To keep the implementation simple we piggy-back on the TLB fast path,
> and thus can only provide the host address _after_ memory accesses
> have occurred. For the slow path, it's a bit tedious because there
> are many places to update, but it's fairly simple.
>
> However, note that cross-page accesses are tricky, since the
> access might be to non-contiguous host addresses. So I'm punting
> on that and just passing NULL.
>
> Signed-off-by: Emilio G. Cota 
> ---
>  accel/tcg/atomic_template.h   |  8 -
>  accel/tcg/softmmu_template.h  | 39 
>  include/exec/cpu-defs.h   |  2 ++
>  include/exec/cpu_ldst_template.h  | 43 +++
>  include/exec/cpu_ldst_useronly_template.h | 42 +++---
>  tcg/tcg-op.h  |  5 +++
>  tcg/tcg.h |  4 +++
>  tcg/i386/tcg-target.inc.c |  5 +++
>  tcg/tcg-op.c  | 37 ++-
>  tcg/tcg.c |  3 ++
>  10 files changed, 152 insertions(+), 36 deletions(-)
>
> diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
> index b13318c1ce..3de34dc462 100644
> --- a/accel/tcg/atomic_template.h
> +++ b/accel/tcg/atomic_template.h
> @@ -18,6 +18,7 @@
>   * License along with this library; if not, see 
> .
>   */
>
> +#include "qemu/plugin.h"
>  #include "trace/mem.h"
>
>  #if DATA_SIZE == 16
> @@ -66,17 +67,22 @@
>  trace_guest_mem_before_exec(ENV_GET_CPU(env), addr, info | 
> TRACE_MEM_ST); \
>  } while (0)
>
> -# define ATOMIC_TRACE_RMW_POST  \
> +# define ATOMIC_TRACE_RMW_POST do {  
>   \
> +  qemu_plugin_vcpu_mem_cb(ENV_GET_CPU(env), addr, haddr, info);  
>   \
> +  qemu_plugin_vcpu_mem_cb(ENV_GET_CPU(env), addr, haddr, info | 
> TRACE_MEM_ST); \
> +} while (0)
>
>  # define ATOMIC_TRACE_LD_PRE\
>  trace_guest_mem_before_exec(ENV_GET_CPU(env), addr, info)
>
>  # define ATOMIC_TRACE_LD_POST   \
> +qemu_plugin_vcpu_mem_cb(ENV_GET_CPU(env), addr, haddr, info)
>
>  # define ATOMIC_TRACE_ST_PRE\
>  trace_guest_mem_before_exec(ENV_GET_CPU(env), addr, info)
>
>  # define ATOMIC_TRACE_ST_POST   \
> +qemu_plugin_vcpu_mem_cb(ENV_GET_CPU(env), addr, haddr, info)
>
>  #endif /* ATOMIC_TRACE_RMW_PRE */
>
> diff --git a/accel/tcg/softmmu_template.h b/accel/tcg/softmmu_template.h
> index b0adea045e..f6d2f60b81 100644
> --- a/accel/tcg/softmmu_template.h
> +++ b/accel/tcg/softmmu_template.h
> @@ -103,6 +103,11 @@ static inline DATA_TYPE glue(io_read, 
> SUFFIX)(CPUArchState *env,
>MMUAccessType access_type)
>  {
>  CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index];
> +
> +/* XXX Any sensible choice other than NULL? */
> +if (tcg_ctx->plugin_mem_cb) {
> +env->hostaddr = NULL;
> +}

This is more argument for getting the softmmu de-macrofiction in first.


--
Alex Bennée

[Qemu-devel] [RFC 18/48] tcg: add memory callbacks for plugins (WIP)

[Emilio G. Cota]

XXX: store hostaddr from non-i386 TCG backends
XXX: what hostaddr to return for I/O accesses?
XXX: what hostaddr to return for cross-page accesses?

Here the trickiest feature is passing the host address to
memory callbacks that request it. Perhaps it would be more
appropriate to pass a "physical" address to plugins, but since
in QEMU host addr ~= guest physical, I'm going with that for
simplicity.

To keep the implementation simple we piggy-back on the TLB fast path,
and thus can only provide the host address _after_ memory accesses
have occurred. For the slow path, it's a bit tedious because there
are many places to update, but it's fairly simple.

However, note that cross-page accesses are tricky, since the
access might be to non-contiguous host addresses. So I'm punting
on that and just passing NULL.

Signed-off-by: Emilio G. Cota 
---
 accel/tcg/atomic_template.h   |  8 -
 accel/tcg/softmmu_template.h  | 39 
 include/exec/cpu-defs.h   |  2 ++
 include/exec/cpu_ldst_template.h  | 43 +++
 include/exec/cpu_ldst_useronly_template.h | 42 +++---
 tcg/tcg-op.h  |  5 +++
 tcg/tcg.h |  4 +++
 tcg/i386/tcg-target.inc.c |  5 +++
 tcg/tcg-op.c  | 37 ++-
 tcg/tcg.c |  3 ++
 10 files changed, 152 insertions(+), 36 deletions(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index b13318c1ce..3de34dc462 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -18,6 +18,7 @@
  * License along with this library; if not, see .
  */
 
+#include "qemu/plugin.h"
 #include "trace/mem.h"
 
 #if DATA_SIZE == 16
@@ -66,17 +67,22 @@
 trace_guest_mem_before_exec(ENV_GET_CPU(env), addr, info | TRACE_MEM_ST); \
 } while (0)
 
-# define ATOMIC_TRACE_RMW_POST  \
+# define ATOMIC_TRACE_RMW_POST do {
\
+  qemu_plugin_vcpu_mem_cb(ENV_GET_CPU(env), addr, haddr, info);
\
+  qemu_plugin_vcpu_mem_cb(ENV_GET_CPU(env), addr, haddr, info | TRACE_MEM_ST); 
\
+} while (0)
 
 # define ATOMIC_TRACE_LD_PRE\
 trace_guest_mem_before_exec(ENV_GET_CPU(env), addr, info)
 
 # define ATOMIC_TRACE_LD_POST   \
+qemu_plugin_vcpu_mem_cb(ENV_GET_CPU(env), addr, haddr, info)
 
 # define ATOMIC_TRACE_ST_PRE\
 trace_guest_mem_before_exec(ENV_GET_CPU(env), addr, info)
 
 # define ATOMIC_TRACE_ST_POST   \
+qemu_plugin_vcpu_mem_cb(ENV_GET_CPU(env), addr, haddr, info)
 
 #endif /* ATOMIC_TRACE_RMW_PRE */
 
diff --git a/accel/tcg/softmmu_template.h b/accel/tcg/softmmu_template.h
index b0adea045e..f6d2f60b81 100644
--- a/accel/tcg/softmmu_template.h
+++ b/accel/tcg/softmmu_template.h
@@ -103,6 +103,11 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState 
*env,
   MMUAccessType access_type)
 {
 CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index];
+
+/* XXX Any sensible choice other than NULL? */
+if (tcg_ctx->plugin_mem_cb) {
+env->hostaddr = NULL;
+}
 return io_readx(env, iotlbentry, mmu_idx, addr, retaddr, recheck,
 access_type, DATA_SIZE);
 }
@@ -162,12 +167,23 @@ WORD_TYPE helper_le_ld_name(CPUArchState *env, 
target_ulong addr,
 res2 = helper_le_ld_name(env, addr2, oi, retaddr);
 shift = (addr & (DATA_SIZE - 1)) * 8;
 
+/*
+ * XXX cross-page accesses would have to be split into separate 
accesses
+ * for the host address to make sense. For now, just return NULL.
+ */
+if (tcg_ctx->plugin_mem_cb) {
+env->hostaddr = NULL;
+}
+
 /* Little-endian combine.  */
 res = (res1 >> shift) | (res2 << ((DATA_SIZE * 8) - shift));
 return res;
 }
 
 haddr = addr + entry->addend;
+if (tcg_ctx->plugin_mem_cb) {
+env->hostaddr = (void *)haddr;
+}
 #if DATA_SIZE == 1
 res = glue(glue(ld, LSUFFIX), _p)((uint8_t *)haddr);
 #else
@@ -231,12 +247,19 @@ WORD_TYPE helper_be_ld_name(CPUArchState *env, 
target_ulong addr,
 res2 = helper_be_ld_name(env, addr2, oi, retaddr);
 shift = (addr & (DATA_SIZE - 1)) * 8;
 
+if (tcg_ctx->plugin_mem_cb) {
+env->hostaddr = NULL;
+}
+
 /* Big-endian combine.  */
 res = (res1 << shift) | (res2 >> ((DATA_SIZE * 8) - shift));
 return res;
 }
 
 haddr = addr + entry->addend;
+if (tcg_ctx->plugin_mem_cb) {
+env->hostaddr = (void *)haddr;
+}
 res = glue(glue(ld, LSUFFIX), _be_p)((uint8_t *)haddr);
 return res;
 }
@@ -270,6 +293,10 @@ sta