[PULL 00/18] tcg plugins (deprecations, mem apis, contrib plugins)

Alex Bennée posted 18 patches 2 weeks, 3 days ago
There is a newer version of this series
docs/about/deprecated.rst                          |  19 +
docs/about/emulation.rst                           |  44 ++-
configure                                          |  32 +-
accel/tcg/atomic_template.h                        |  66 +++-
include/hw/core/cpu.h                              |   4 +
include/qemu/plugin.h                              |   4 +
include/qemu/qemu-plugin.h                         |  64 +++-
contrib/plugins/bbv.c                              | 158 +++++++++
contrib/plugins/cflow.c                            | 388 +++++++++++++++++++++
contrib/plugins/ips.c                              |   6 +
plugins/api.c                                      |  53 +++
plugins/core.c                                     |   6 +
tcg/tcg-op-ldst.c                                  |  66 +++-
tests/tcg/multiarch/system/memory.c                | 123 ++++---
tests/tcg/multiarch/test-plugin-mem-access.c       | 177 ++++++++++
tests/tcg/plugins/mem.c                            | 250 ++++++++++++-
tests/tcg/plugins/syscall.c                        | 117 +++++++
util/qemu-timer.c                                  |  14 +-
accel/tcg/atomic_common.c.inc                      |  13 +-
accel/tcg/ldst_common.c.inc                        |  38 +-
contrib/plugins/Makefile                           |   2 +
plugins/qemu-plugins.symbols                       |   2 +
tests/tcg/Makefile.target                          |  12 +-
tests/tcg/alpha/Makefile.softmmu-target            |   2 +-
tests/tcg/alpha/Makefile.target                    |   3 +
tests/tcg/multiarch/Makefile.target                |  11 +
tests/tcg/multiarch/check-plugin-output.sh         |  36 ++
tests/tcg/multiarch/system/Makefile.softmmu-target |   6 +
.../tcg/multiarch/system/validate-memory-counts.py | 130 +++++++
tests/tcg/ppc64/Makefile.target                    |   5 +
tests/tcg/s390x/Makefile.softmmu-target            |   8 +-
31 files changed, 1776 insertions(+), 83 deletions(-)
create mode 100644 contrib/plugins/bbv.c
create mode 100644 contrib/plugins/cflow.c
create mode 100644 tests/tcg/multiarch/test-plugin-mem-access.c
create mode 100755 tests/tcg/multiarch/check-plugin-output.sh
create mode 100755 tests/tcg/multiarch/system/validate-memory-counts.py
[PULL 00/18] tcg plugins (deprecations, mem apis, contrib plugins)
Posted by Alex Bennée 2 weeks, 3 days ago
The following changes since commit 2b81c046252fbfb375ad30632362fc16e6e22bd5:

  Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2024-09-17 14:02:18 +0100)

are available in the Git repository at:

  https://gitlab.com/stsquad/qemu.git tags/pull-tcg-plugin-memory-180924-2

for you to fetch changes up to a33f4871e0a0f4bf1cb037ab29fae7df7f2fc658:

  contrib/plugins: avoid hanging program (2024-09-18 21:02:36 +0100)

----------------------------------------------------------------
TCG plugin memory instrumentation updates

  - deprecate plugins on 32 bit hosts
  - deprecate plugins with TCI
  - extend memory API to save value
  - add check-tcg tests to exercise new memory API
  - fix timer deadlock with non-changing timer
  - add basic block vector plugin to contrib
  - add cflow plugin to contrib
  - extend syscall plugin to dump write memory
  - validate ips plugin arguments meet minimum slice value

----------------------------------------------------------------
Akihiko Odaki (1):
      contrib/plugins: Add a plugin to generate basic block vectors

Alex Bennée (9):
      deprecation: don't enable TCG plugins by default on 32 bit hosts
      deprecation: don't enable TCG plugins by default with TCI
      contrib/plugins: control flow plugin
      tests/tcg: clean up output of memory system test
      tests/tcg: only read/write 64 bit words on 64 bit systems
      tests/tcg: ensure s390x-softmmu output redirected
      tests/tcg: add a system test to check memory instrumentation
      util/timer: avoid deadlock when shutting down
      contrib/plugins: avoid hanging program

Pierrick Bouvier (6):
      plugins: save value during memory accesses
      plugins: extend API to get latest memory value accessed
      tests/tcg: add mechanism to run specific tests with plugins
      tests/tcg: allow to check output of plugins
      tests/tcg/plugins/mem: add option to print memory accesses
      tests/tcg/multiarch: add test for plugin memory access

Rowan Hart (2):
      plugins: add plugin API to read guest memory
      plugins: add option to dump write argument to syscall plugin

 docs/about/deprecated.rst                          |  19 +
 docs/about/emulation.rst                           |  44 ++-
 configure                                          |  32 +-
 accel/tcg/atomic_template.h                        |  66 +++-
 include/hw/core/cpu.h                              |   4 +
 include/qemu/plugin.h                              |   4 +
 include/qemu/qemu-plugin.h                         |  64 +++-
 contrib/plugins/bbv.c                              | 158 +++++++++
 contrib/plugins/cflow.c                            | 388 +++++++++++++++++++++
 contrib/plugins/ips.c                              |   6 +
 plugins/api.c                                      |  53 +++
 plugins/core.c                                     |   6 +
 tcg/tcg-op-ldst.c                                  |  66 +++-
 tests/tcg/multiarch/system/memory.c                | 123 ++++---
 tests/tcg/multiarch/test-plugin-mem-access.c       | 177 ++++++++++
 tests/tcg/plugins/mem.c                            | 250 ++++++++++++-
 tests/tcg/plugins/syscall.c                        | 117 +++++++
 util/qemu-timer.c                                  |  14 +-
 accel/tcg/atomic_common.c.inc                      |  13 +-
 accel/tcg/ldst_common.c.inc                        |  38 +-
 contrib/plugins/Makefile                           |   2 +
 plugins/qemu-plugins.symbols                       |   2 +
 tests/tcg/Makefile.target                          |  12 +-
 tests/tcg/alpha/Makefile.softmmu-target            |   2 +-
 tests/tcg/alpha/Makefile.target                    |   3 +
 tests/tcg/multiarch/Makefile.target                |  11 +
 tests/tcg/multiarch/check-plugin-output.sh         |  36 ++
 tests/tcg/multiarch/system/Makefile.softmmu-target |   6 +
 .../tcg/multiarch/system/validate-memory-counts.py | 130 +++++++
 tests/tcg/ppc64/Makefile.target                    |   5 +
 tests/tcg/s390x/Makefile.softmmu-target            |   8 +-
 31 files changed, 1776 insertions(+), 83 deletions(-)
 create mode 100644 contrib/plugins/bbv.c
 create mode 100644 contrib/plugins/cflow.c
 create mode 100644 tests/tcg/multiarch/test-plugin-mem-access.c
 create mode 100755 tests/tcg/multiarch/check-plugin-output.sh
 create mode 100755 tests/tcg/multiarch/system/validate-memory-counts.py

-- 
2.39.5


Re: [PULL 00/18] tcg plugins (deprecations, mem apis, contrib plugins)
Posted by Peter Maydell 2 weeks, 2 days ago
On Wed, 18 Sept 2024 at 22:08, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> The following changes since commit 2b81c046252fbfb375ad30632362fc16e6e22bd5:
>
>   Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2024-09-17 14:02:18 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/stsquad/qemu.git tags/pull-tcg-plugin-memory-180924-2
>
> for you to fetch changes up to a33f4871e0a0f4bf1cb037ab29fae7df7f2fc658:
>
>   contrib/plugins: avoid hanging program (2024-09-18 21:02:36 +0100)
>
> ----------------------------------------------------------------
> TCG plugin memory instrumentation updates
>
>   - deprecate plugins on 32 bit hosts
>   - deprecate plugins with TCI
>   - extend memory API to save value
>   - add check-tcg tests to exercise new memory API
>   - fix timer deadlock with non-changing timer
>   - add basic block vector plugin to contrib
>   - add cflow plugin to contrib
>   - extend syscall plugin to dump write memory
>   - validate ips plugin arguments meet minimum slice value
>
> ----------------------------------------------------------------

Fails to build on macos:
https://gitlab.com/qemu-project/qemu/-/jobs/7865151156

../tests/tcg/plugins/mem.c:12:10: fatal error: 'endian.h' file not found

endian.h is a Linuxism.

While I'm looking at the code, this caught my eye:

    case QEMU_PLUGIN_MEM_VALUE_U64:
    {
        uint64_t *p = (uint64_t *) &ri->data[offset];
        uint64_t val = be ? htobe64(value.data.u64) : htole64(value.data.u64);
        if (is_store) {
            *p = val;
        } else if (*p != val) {
            unseen_data = true;
        }
        break;
    }

Casting a random byte pointer to uint64_t* like that
and dereferencing it isn't valid -- it can fault if
it's not aligned correctly.

I suspect the plugin needs to define versions of at least some
of the functionality in qemu's include/qemu/bswap.h.

thanks
-- PMM
Re: [PULL 00/18] tcg plugins (deprecations, mem apis, contrib plugins)
Posted by Alex Bennée 2 weeks, 2 days ago
Peter Maydell <peter.maydell@linaro.org> writes:

> On Wed, 18 Sept 2024 at 22:08, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> The following changes since commit 2b81c046252fbfb375ad30632362fc16e6e22bd5:
>>
>>   Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2024-09-17 14:02:18 +0100)
>>
>> are available in the Git repository at:
>>
>>   https://gitlab.com/stsquad/qemu.git tags/pull-tcg-plugin-memory-180924-2
>>
>> for you to fetch changes up to a33f4871e0a0f4bf1cb037ab29fae7df7f2fc658:
>>
>>   contrib/plugins: avoid hanging program (2024-09-18 21:02:36 +0100)
>>
>> ----------------------------------------------------------------
>> TCG plugin memory instrumentation updates
>>
>>   - deprecate plugins on 32 bit hosts
>>   - deprecate plugins with TCI
>>   - extend memory API to save value
>>   - add check-tcg tests to exercise new memory API
>>   - fix timer deadlock with non-changing timer
>>   - add basic block vector plugin to contrib
>>   - add cflow plugin to contrib
>>   - extend syscall plugin to dump write memory
>>   - validate ips plugin arguments meet minimum slice value
>>
>> ----------------------------------------------------------------
>
> Fails to build on macos:
> https://gitlab.com/qemu-project/qemu/-/jobs/7865151156
>
> ../tests/tcg/plugins/mem.c:12:10: fatal error: 'endian.h' file not found
>
> endian.h is a Linuxism.

Doh - I'd written it off the failure as waiting for the MacOS bump and
didn't see the actual error. I'll see what we can do.

>
> While I'm looking at the code, this caught my eye:
>
>     case QEMU_PLUGIN_MEM_VALUE_U64:
>     {
>         uint64_t *p = (uint64_t *) &ri->data[offset];
>         uint64_t val = be ? htobe64(value.data.u64) : htole64(value.data.u64);
>         if (is_store) {
>             *p = val;
>         } else if (*p != val) {
>             unseen_data = true;
>         }
>         break;
>     }
>
> Casting a random byte pointer to uint64_t* like that
> and dereferencing it isn't valid -- it can fault if
> it's not aligned correctly.

Hmm in the normal case of x86 hosts we will never hit this. I guess we
could do a memcpy step and then the byteswap?

> I suspect the plugin needs to define versions of at least some
> of the functionality in qemu's include/qemu/bswap.h.

Could it be included directly without bringing in the rest of QEMU's
include deps?

>
> thanks
> -- PMM

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PULL 00/18] tcg plugins (deprecations, mem apis, contrib plugins)
Posted by Peter Maydell 2 weeks, 2 days ago
On Thu, 19 Sept 2024 at 14:11, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
> > While I'm looking at the code, this caught my eye:
> >
> >     case QEMU_PLUGIN_MEM_VALUE_U64:
> >     {
> >         uint64_t *p = (uint64_t *) &ri->data[offset];
> >         uint64_t val = be ? htobe64(value.data.u64) : htole64(value.data.u64);
> >         if (is_store) {
> >             *p = val;
> >         } else if (*p != val) {
> >             unseen_data = true;
> >         }
> >         break;
> >     }
> >
> > Casting a random byte pointer to uint64_t* like that
> > and dereferencing it isn't valid -- it can fault if
> > it's not aligned correctly.
>
> Hmm in the normal case of x86 hosts we will never hit this.

Not necessarily -- some x86 SIMD insns enforce alignment.

> I guess we
> could do a memcpy step and then the byteswap?

That's what bswap.h does, yes.

> Could it be included directly without bringing in the rest of QEMU's
> include deps?

It's technically quite close to standalone I think,
but I think it would be a bad idea to directly include
it because once you put QEMU's include/ on the plugin
compile include path then that's a slippery slope to
the plugins not actually being standalone code any more.

-- PMM
Re: [PULL 00/18] tcg plugins (deprecations, mem apis, contrib plugins)
Posted by Alex Bennée 2 weeks, 2 days ago
Peter Maydell <peter.maydell@linaro.org> writes:

> On Thu, 19 Sept 2024 at 14:11, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> > While I'm looking at the code, this caught my eye:
>> >
>> >     case QEMU_PLUGIN_MEM_VALUE_U64:
>> >     {
>> >         uint64_t *p = (uint64_t *) &ri->data[offset];
>> >         uint64_t val = be ? htobe64(value.data.u64) : htole64(value.data.u64);
>> >         if (is_store) {
>> >             *p = val;
>> >         } else if (*p != val) {
>> >             unseen_data = true;
>> >         }
>> >         break;
>> >     }
>> >
>> > Casting a random byte pointer to uint64_t* like that
>> > and dereferencing it isn't valid -- it can fault if
>> > it's not aligned correctly.
>>
>> Hmm in the normal case of x86 hosts we will never hit this.
>
> Not necessarily -- some x86 SIMD insns enforce alignment.
>
>> I guess we
>> could do a memcpy step and then the byteswap?
>
> That's what bswap.h does, yes.
>
>> Could it be included directly without bringing in the rest of QEMU's
>> include deps?
>
> It's technically quite close to standalone I think,
> but I think it would be a bad idea to directly include
> it because once you put QEMU's include/ on the plugin
> compile include path then that's a slippery slope to
> the plugins not actually being standalone code any more.

In this case tests/tcg/plugins are built for testing the implementation.
We could let it slide to save on just copy and pasting the whole file:

--8<---------------cut here---------------start------------->8---
modified   tests/tcg/plugins/mem.c
@@ -9,10 +9,23 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
-#include <endian.h>
 #include <stdio.h>
 #include <glib.h>
 
+/*
+ * plugins should not include anything from QEMU aside from the
+ * API header. However the bswap.h header is sufficiently self
+ * contained that we include it here to avoid code duplication.
+ */
+#define HOST_BIG_ENDIAN (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)
+#define HOST_LONG_BITS (__SIZEOF_POINTER__ * 8)
+#ifndef glue
+#define xglue(x, y) x ## y
+#define glue(x, y) xglue(x, y)
+#define stringify(s) tostring(s)
+#define tostring(s) #s
+#endif
+#include <bswap.h>
 #include <qemu-plugin.h>
 
 QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
@@ -154,33 +167,45 @@ static void update_region_info(uint64_t region, uint64_t offset,
     case QEMU_PLUGIN_MEM_VALUE_U16:
     {
         uint16_t *p = (uint16_t *) &ri->data[offset];
-        uint16_t val = be ? htobe16(value.data.u16) : htole16(value.data.u16);
         if (is_store) {
-            *p = val;
-        } else if (*p != val) {
-            unseen_data = true;
+            if (be) {
+                stw_be_p(p, value.data.u16);
+            } else {
+                stw_le_p(p, value.data.u16);
+            }
+        } else {
+            uint16_t val = be ? lduw_be_p(p) : lduw_le_p(p);
+            unseen_data = val != value.data.u16;
         }
         break;
     }
     case QEMU_PLUGIN_MEM_VALUE_U32:
     {
         uint32_t *p = (uint32_t *) &ri->data[offset];
-        uint32_t val = be ? htobe32(value.data.u32) : htole32(value.data.u32);
         if (is_store) {
-            *p = val;
-        } else if (*p != val) {
-            unseen_data = true;
+            if (be) {
+                stl_be_p(p, value.data.u32);
+            } else {
+                stl_le_p(p, value.data.u32);
+            }
+        } else {
+            uint32_t val = be ? ldl_be_p(p) : ldl_le_p(p);
+            unseen_data = val != value.data.u32;
         }
         break;
     }
     case QEMU_PLUGIN_MEM_VALUE_U64:
     {
         uint64_t *p = (uint64_t *) &ri->data[offset];
-        uint64_t val = be ? htobe64(value.data.u64) : htole64(value.data.u64);
         if (is_store) {
-            *p = val;
-        } else if (*p != val) {
-            unseen_data = true;
+            if (be) {
+                stq_be_p(p, value.data.u64);
+            } else {
+                stq_le_p(p, value.data.u64);
+            }
+        } else {
+            uint64_t val = be ? ldq_be_p(p) : ldq_le_p(p);
+            unseen_data = val != value.data.u64;
         }
         break;
--8<---------------cut here---------------end--------------->8---

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro