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

["Alex Bennée" <alex.bennee@linaro.org>]

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