On 6/27/24 12:10, Richard Henderson wrote:
On 6/26/24 16:37, Pierrick Bouvier wrote:
Different code paths handle memory accesses:
- tcg generated code
- load/store helpers
- atomic helpers

This value is saved in cpu->plugin_state.

Atomic operations are doing read/write at the same time, so we generate
two memory callbacks instead of one, to allow plugins to access distinct
values.

Signed-off-by: Pierrick Bouvier <pierrick.bouv...@linaro.org>
---
   accel/tcg/atomic_template.h   | 66 ++++++++++++++++++++++++++++----
   include/qemu/plugin.h         |  8 ++++
   plugins/core.c                |  7 ++++
   tcg/tcg-op-ldst.c             | 72 +++++++++++++++++++++++++++++++----
   accel/tcg/atomic_common.c.inc | 13 ++++++-
   accel/tcg/ldst_common.c.inc   | 38 +++++++++++-------
   6 files changed, 173 insertions(+), 31 deletions(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index 1dc2151dafd..830e4f16069 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -53,6 +53,14 @@
   # error unsupported data size
   #endif
+#if DATA_SIZE == 16
+# define UPPER_MEMORY_VALUE(val) int128_gethi(val)
+# define LOWER_MEMORY_VALUE(val) int128_getlo(val)
+#else
+# define UPPER_MEMORY_VALUE(val) 0
+# define LOWER_MEMORY_VALUE(val) val
+#endif
+
   #if DATA_SIZE >= 4
   # define ABI_TYPE  DATA_TYPE
   #else
@@ -83,7 +91,12 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, abi_ptr 
addr,
       ret = qatomic_cmpxchg__nocheck(haddr, cmpv, newv);
   #endif
       ATOMIC_MMU_CLEANUP;
-    atomic_trace_rmw_post(env, addr, oi);
+    atomic_trace_rmw_post(env, addr,
+                          UPPER_MEMORY_VALUE(ret),
+                          LOWER_MEMORY_VALUE(ret),
+                          UPPER_MEMORY_VALUE(newv),
+                          LOWER_MEMORY_VALUE(newv),
+                          oi);

Just a nit, but tcg is consistent in using little-endian argument ordering for 
values
passed by parts.  I would prefer we continue with that.


Didn't notice that, but I'll stick to this. Any preference on the naming as well while I'm at it? (low/hi vs upper/lower)?


@@ -142,9 +142,13 @@ struct qemu_plugin_tb {
   /**
    * struct CPUPluginState - per-CPU state for plugins
    * @event_mask: plugin event bitmap. Modified only via async work.
+ * @mem_value_upper_bits: 64 upper bits of latest accessed mem value.
+ * @mem_value_lower_bits: 64 lower bits of latest accessed mem value.
    */
   struct CPUPluginState {
       DECLARE_BITMAP(event_mask, QEMU_PLUGIN_EV_MAX);
+    uint64_t mem_value_upper_bits;
+    uint64_t mem_value_lower_bits;
   };

At some point we may well support 32 byte acceses, for better guest vector 
support.  Do we
have a plan for this beyond "add more fields here"?


For now, I sticked to native tcg ops (up to 128 bits), with this simple solution. Do you think tcg core will be extended to support more, or will helper simply load/store four 128bits words, emitting four callbacks as well?

If you have a better idea, I'm open to implement an alternative, but didn't want to think too much ahead.


r~

Reply via email to