Re: Plugin Memory Callback Debugging

2022-11-29 Thread Aaron Lindsay via
On Nov 22 10:57, Aaron Lindsay wrote: > On Nov 21 18:22, Richard Henderson wrote: > > On 11/21/22 13:51, Alex Bennée wrote: > > > > > > Aaron Lindsay writes: > > > > > > > On Nov 15 22:36, Alex Bennée wrote: > > > > > Aaron Lindsay writes: > > > > > > I believe the code *should* always reset

Re: Plugin Memory Callback Debugging

2022-11-22 Thread Aaron Lindsay via
On Nov 21 22:02, Alex Bennée wrote: > > Aaron Lindsay writes: > > > Sorry, left off the very end of my timeline: > > > > On Nov 18 16:58, Aaron Lindsay wrote: > >> I have, so far, discovered the following timeline: > >> 1. My plugin receives a instruction execution callback for a load > >>

Re: Plugin Memory Callback Debugging

2022-11-22 Thread Aaron Lindsay via
On Nov 21 18:22, Richard Henderson wrote: > On 11/21/22 13:51, Alex Bennée wrote: > > > > Aaron Lindsay writes: > > > > > On Nov 15 22:36, Alex Bennée wrote: > > > > Aaron Lindsay writes: > > > > > I believe the code *should* always reset `cpu->plugin_mem_cbs` to > > > > > NULL at the > > > >

Re: Plugin Memory Callback Debugging

2022-11-21 Thread Aaron Lindsay via
On Nov 15 22:36, Alex Bennée wrote: > Aaron Lindsay writes: > > I believe the code *should* always reset `cpu->plugin_mem_cbs` to NULL at > > the > > end of an instruction/TB's execution, so its not exactly clear to me how > > this > > is occurring. However, I suspect it may be relevant that we

Re: Plugin Memory Callback Debugging

2022-11-18 Thread Aaron Lindsay via
On Nov 15 22:36, Alex Bennée wrote: > > Aaron Lindsay writes: > > > Hello, > > > > I have been wrestling with what might be a bug in the plugin memory > > callbacks. The immediate error is that I hit the > > `g_assert_not_reached()` in the 'default:' case in > > qemu_plugin_vcpu_mem_cb,

Re: Plugins Not Reporting AArch64 SVE Memory Operations

2022-03-29 Thread Aaron Lindsay via
On Mar 28 16:30, Alex Bennée wrote: > > Aaron Lindsay writes: > > > Hi folks, > > > > I see there has been some previous discussion [1] about 1.5 years ago > > around the fact that AArch64 SVE instructions do not emit any memory > > operations via the plugin interface, as one might expect them

Re: [PATCH v1 12/22] plugins: stxp test case from Aaron (!upstream)

2022-02-02 Thread Aaron Lindsay via
On Feb 01 15:29, Alex Bennée wrote: > > Aaron Lindsay writes: > > > On Jan 24 20:15, Alex Bennée wrote: > >> Signed-off-by: Alex Bennée > >> Cc: Aaron Lindsay > >> Message-ID: > >> > >> --- > >> [AJB] this was for testing, I think you can show the same stuff with > >> the much more complete

Re: [PATCH v1 12/22] plugins: stxp test case from Aaron (!upstream)

2022-02-01 Thread Aaron Lindsay via
On Jan 25 09:17, Thomas Huth wrote: > On 24/01/2022 21.15, Alex Bennée wrote: > > Signed-off-by: Alex Bennée > > Cc: Aaron Lindsay > > Message-ID: > > > > --- > > [AJB] this was for testing, I think you can show the same stuff with > > the much more complete execlog now. > > --- > >

Re: [PATCH v1 12/22] plugins: stxp test case from Aaron (!upstream)

2022-02-01 Thread Aaron Lindsay via
On Jan 24 20:15, Alex Bennée wrote: > Signed-off-by: Alex Bennée > Cc: Aaron Lindsay > Message-ID: > > --- > [AJB] this was for testing, I think you can show the same stuff with > the much more complete execlog now. Is it true that execlog can also reproduce the duplicate loads which are

Re: plugins: Missing Store Exclusive Memory Accesses

2021-10-21 Thread Aaron Lindsay via
On Oct 21 13:28, Alex Bennée wrote: > It's a bit clearer if you use the contrib/execlog plugin: > > ./qemu-aarch64 -plugin contrib/plugins/libexeclog.so -d plugin > ./tests/tcg/aarch64-linux-user/stxp > > 0, 0x400910, 0xf9800011, "prfm pstl1strm, [x0] > 0, 0x400914, 0xc87f4410, "ldxp

Re: plugins: Missing Store Exclusive Memory Accesses

2021-10-20 Thread Aaron Lindsay via
On Oct 20 18:54, Alex Bennée wrote: > Have you got a test case you are using so I can try and replicate the > failure you are seeing? So far by inspection everything looks OK to me. I took some time today to put together a minimal(ish) reproducer using usermode. The source files used are below, I

Re: plugins: Missing Store Exclusive Memory Accesses

2021-10-20 Thread Aaron Lindsay via
On Sep 22 16:22, Aaron Lindsay wrote: > On Sep 21 16:28, Aaron Lindsay wrote: > > On Sep 17 12:05, Alex Bennée wrote: > > > Aaron Lindsay writes: > > > > I recently noticed that the plugin interface does not appear to be > > > > emitting callbacks to functions registered via > > > >

Re: plugins: Missing Store Exclusive Memory Accesses

2021-09-22 Thread Aaron Lindsay via
On Sep 21 16:28, Aaron Lindsay wrote: > On Sep 17 12:05, Alex Bennée wrote: > > Aaron Lindsay writes: > > > I recently noticed that the plugin interface does not appear to be > > > emitting callbacks to functions registered via > > > `qemu_plugin_register_vcpu_mem_cb` for AArch64 store

Re: plugins: Missing Store Exclusive Memory Accesses

2021-09-21 Thread Aaron Lindsay via
On Sep 17 12:05, Alex Bennée wrote: > Aaron Lindsay writes: > > I recently noticed that the plugin interface does not appear to be > > emitting callbacks to functions registered via > > `qemu_plugin_register_vcpu_mem_cb` for AArch64 store exclusives. This > > would include instructions like `stxp

Re: plugins: Missing Store Exclusive Memory Accesses

2021-09-17 Thread Aaron Lindsay via
On Sep 17 12:05, Alex Bennée wrote: > Aaron Lindsay writes: > > In looking at QEMU's source, I *think* this is because the > > `gen_store_exclusive` function in translate-a64.c is not making the same > > calls to `plugin_gen_mem_callbacks` & company that are being made by > > "normal" stores

Re: Plugin virtual-to-physical translation incorrect for some IO accesses

2021-07-07 Thread Aaron Lindsay via
On Jul 07 07:35, Aaron Lindsay wrote: > On Jul 07 09:53, Philippe Mathieu-Daudé wrote: > > On 7/6/21 11:56 PM, Aaron Lindsay wrote: > > > On Jul 06 23:10, Philippe Mathieu-Daudé wrote: > > >> +Peter/Paolo > > >> > > >> On 7/6/21 10:47 PM, Aaron Lindsay wrote: > > >>> Hello, > > >>> > > >>> I

Re: Plugin virtual-to-physical translation incorrect for some IO accesses

2021-07-07 Thread Aaron Lindsay via
On Jul 07 09:53, Philippe Mathieu-Daudé wrote: > On 7/6/21 11:56 PM, Aaron Lindsay wrote: > > On Jul 06 23:10, Philippe Mathieu-Daudé wrote: > >> +Peter/Paolo > >> > >> On 7/6/21 10:47 PM, Aaron Lindsay wrote: > >>> Hello, > >>> > >>> I previously supplied a patch which modified the plugin

Re: Plugin virtual-to-physical translation incorrect for some IO accesses

2021-07-06 Thread Aaron Lindsay via
On Jul 06 23:10, Philippe Mathieu-Daudé wrote: > +Peter/Paolo > > On 7/6/21 10:47 PM, Aaron Lindsay wrote: > > Hello, > > > > I previously supplied a patch which modified the plugin interface such > > that it will return physical addresses for IO regions [0]. However, I > > have now found a case

Re: [PATCH v1 11/14] plugins: expand kernel-doc for instruction query and instrumentation

2021-03-16 Thread Aaron Lindsay via
On Mar 16 13:48, Alex Bennée wrote: > Aaron Lindsay writes: > > On Mar 12 17:28, Alex Bennée wrote: > >> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn() > >> + * > >> + * Returns: hardware (physical) address of instruction > >> + */ > >> void *qemu_plugin_insn_haddr(const

Re: [PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc

2021-03-16 Thread Aaron Lindsay via
On Mar 16 13:40, Alex Bennée wrote: > > Aaron Lindsay writes: > > > On Mar 12 17:28, Alex Bennée wrote: > >> Also add a note to explain currently they are unused. > >> > >> Signed-off-by: Alex Bennée > > > > I'm personally interested in one clarification below, but don't think > > that

Re: [PATCH v1 12/14] plugins: expand kernel-doc for memory query and instrumentation

2021-03-12 Thread Aaron Lindsay via
On Mar 12 17:28, Alex Bennée wrote: > Signed-off-by: Alex Bennée Small comment below, but otherwise: Reviewed-by: Aaron Lindsay > --- > include/qemu/qemu-plugin.h | 35 --- > 1 file changed, 28 insertions(+), 7 deletions(-) > > diff --git

Re: [PATCH v1 09/14] plugins: add qemu_plugin_id_t to kernel-doc

2021-03-12 Thread Aaron Lindsay via
On Mar 12 17:28, Alex Bennée wrote: > Signed-off-by: Alex Bennée Reviewed-by: Aaron Lindsay > --- > include/qemu/qemu-plugin.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h > index c98866a637..5ac6fe5f02 100644 > ---

Re: [PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc

2021-03-12 Thread Aaron Lindsay via
On Mar 12 17:28, Alex Bennée wrote: > Also add a note to explain currently they are unused. > > Signed-off-by: Alex Bennée I'm personally interested in one clarification below, but don't think that affects my: Reviewed-by: Aaron Lindsay > --- > include/qemu/qemu-plugin.h | 16

Re: [PATCH v1 06/14] plugins: expand the callback typedef kernel-docs

2021-03-12 Thread Aaron Lindsay via
On Mar 12 17:28, Alex Bennée wrote: > Signed-off-by: Alex Bennée One nit below, but otherwise: Reviewed-by: Aaron Lindsay > --- > include/qemu/qemu-plugin.h | 25 ++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/include/qemu/qemu-plugin.h

Re: [PATCH v1 11/14] plugins: expand kernel-doc for instruction query and instrumentation

2021-03-12 Thread Aaron Lindsay via
A few clarifications inline: On Mar 12 17:28, Alex Bennée wrote: > Signed-off-by: Alex Bennée > --- > include/qemu/qemu-plugin.h | 52 -- > 1 file changed, 50 insertions(+), 2 deletions(-) > > diff --git a/include/qemu/qemu-plugin.h

Re: [PATCH v1 10/14] plugins: expand inline exec kernel-doc documentation.

2021-03-12 Thread Aaron Lindsay via
On Mar 12 17:28, Alex Bennée wrote: > Remove the extraneous @cb parameter and document the non-atomic nature > of the INLINE_ADD_U64 operation. > > Signed-off-by: Alex Bennée Reviewed-by: Aaron Lindsay > --- > include/qemu/qemu-plugin.h | 12 +++- > 1 file changed, 11 insertions(+),

Re: [PATCH v1 07/14] plugins: expand the typedef kernel-docs for translation

2021-03-12 Thread Aaron Lindsay via
On Mar 12 17:28, Alex Bennée wrote: > Signed-off-by: Alex Bennée Reviewed-by: Aaron Lindsay > --- > include/qemu/qemu-plugin.h | 17 ++--- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h > index

Re: [PATCH v1 05/14] plugins: cleanup kernel-doc for qemu_plugin_install

2021-03-12 Thread Aaron Lindsay via
On Mar 12 17:28, Alex Bennée wrote: > kernel-doc doesn't like multiple Note sections. Also add an explicit > Return. > > Signed-off-by: Alex Bennée Reviewed-by: Aaron Lindsay > --- > include/qemu/qemu-plugin.h | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff

Re: [PATCH v1 04/14] plugins: expand kernel-doc for qemu_info_t

2021-03-12 Thread Aaron Lindsay via
On Mar 12 17:28, Alex Bennée wrote: > It seems kernel-doc struggles a bit with typedef structs but with > enough encouragement we can get something out of it. > > Signed-off-by: Alex Bennée Reviewed-by: Aaron Lindsay > --- > include/qemu/qemu-plugin.h | 22 +++--- > 1 file

Re: [PATCH v1 03/14] docs/devel: include the plugin API information from the headers

2021-03-12 Thread Aaron Lindsay via
On Mar 12 17:28, Alex Bennée wrote: > We have kerneldoc tags for the headers so we might as well extract > them into our developer documentation whilst we are at it. > > Signed-off-by: Alex Bennée Reviewed-by: Aaron Lindsay > --- > docs/devel/tcg-plugins.rst | 5 + > 1 file changed, 5

Re: [PATCH] plugins: Expose physical addresses instead of device offsets

2021-03-09 Thread Aaron Lindsay via
On Mar 09 17:45, Alex Bennée wrote: > Aaron Lindsay writes: > > On Mar 09 10:08, Peter Maydell wrote: > >> On Mon, 8 Mar 2021 at 20:14, Aaron Lindsay > >> wrote: > >> > > >> > This allows plugins to query for full virtual-to-physical address > >> > translation for a given `qemu_plugin_hwaddr`

Re: Plugin Address Translations Inconsistent/Incorrect?

2021-03-02 Thread Aaron Lindsay via
On Mar 02 16:06, Alex Bennée wrote: > > Aaron Lindsay writes: > > > On Feb 23 15:53, Aaron Lindsay wrote: > >> On Feb 22 15:48, Aaron Lindsay wrote: > >> > On Feb 22 19:30, Alex Bennée wrote: > >> > > Aaron Lindsay writes: > >> > > That said I think we could add an additional helper to

Re: Plugin Address Translations Inconsistent/Incorrect?

2021-03-02 Thread Aaron Lindsay via
On Feb 23 15:53, Aaron Lindsay wrote: > On Feb 22 15:48, Aaron Lindsay wrote: > > On Feb 22 19:30, Alex Bennée wrote: > > > Aaron Lindsay writes: > > > That said I think we could add an additional helper to translate a > > > hwaddr to a global address space address. I'm open to suggestions of the

Re: Plugin Address Translations Inconsistent/Incorrect?

2021-02-23 Thread Aaron Lindsay via
On Feb 22 15:48, Aaron Lindsay wrote: > On Feb 22 19:30, Alex Bennée wrote: > > Aaron Lindsay writes: > > That said I think we could add an additional helper to translate a > > hwaddr to a global address space address. I'm open to suggestions of the > > best way to structure this. > > Haven't

Re: Plugin Address Translations Inconsistent/Incorrect?

2021-02-22 Thread Aaron Lindsay via
On Feb 22 19:30, Alex Bennée wrote: > Aaron Lindsay writes: > > If I call (inside a memory callback): > > > > `uint64_t pa = qemu_plugin_hwaddr_device_offset(hwaddr);` > > > > I see that `pa` takes the value 0xe0e58760. If, however, I plumb > > `cpu_get_phys_page_debug` through to the plugin

Re: [PATCH v3 20/23] accel/tcg: allow plugin instrumentation to be disable via cflags

2021-02-17 Thread Aaron Lindsay via
On Feb 13 13:03, Alex Bennée wrote: > When icount is enabled and we recompile an MMIO access we end up > double counting the instruction execution. To avoid this we introduce > the CF_MEMI cflag which only allows memory instrumentation for the > next TB (which won't yet have been counted). As this

Re: [PATCH v2 20/21] accel/tcg: allow plugin instrumentation to be disable via cflags

2021-02-17 Thread Aaron Lindsay via
On Feb 16 10:34, Alex Bennée wrote: > > Aaron Lindsay writes: > > > On Feb 12 16:04, Alex Bennée wrote: > >> Do you see two stores or one store? I think I got the sense the wrong > >> way around because the store is instrumented before the mmu code, > >> hence should be skipped on a

Re: [PATCH v2 20/21] accel/tcg: allow plugin instrumentation to be disable via cflags

2021-02-12 Thread Aaron Lindsay via
On Feb 12 16:00, Alex Bennée wrote: > > Alex Bennée writes: > > > Aaron Lindsay writes: > > > >> On Feb 10 22:10, Alex Bennée wrote: > >>> When icount is enabled and we recompile an MMIO access we end up > >>> double counting the instruction execution. To avoid this we introduce > >>> the

Re: [PATCH v2 20/21] accel/tcg: allow plugin instrumentation to be disable via cflags

2021-02-12 Thread Aaron Lindsay via
On Feb 12 16:04, Alex Bennée wrote: > Do you see two stores or one store? I think I got the sense the wrong > way around because the store is instrumented before the mmu code, > hence should be skipped on a re-instrumented block. I only see one store between the instruction callback for the store

Re: [PATCH v2 20/21] accel/tcg: allow plugin instrumentation to be disable via cflags

2021-02-12 Thread Aaron Lindsay via
On Feb 12 14:43, Alex Bennée wrote: > Aaron Lindsay writes: > > On Feb 10 22:10, Alex Bennée wrote: > >> When icount is enabled and we recompile an MMIO access we end up > >> double counting the instruction execution. To avoid this we introduce > >> the CF_NOINSTR cflag which disables

Re: [PATCH v2 20/21] accel/tcg: allow plugin instrumentation to be disable via cflags

2021-02-12 Thread Aaron Lindsay via
On Feb 12 11:22, Alex Bennée wrote: > Aaron Lindsay writes: > > On Feb 10 22:10, Alex Bennée wrote: > >> When icount is enabled and we recompile an MMIO access we end up > >> double counting the instruction execution. To avoid this we introduce > >> the CF_NOINSTR cflag which disables

Re: [PATCH v2 20/21] accel/tcg: allow plugin instrumentation to be disable via cflags

2021-02-11 Thread Aaron Lindsay via
On Feb 10 22:10, Alex Bennée wrote: > When icount is enabled and we recompile an MMIO access we end up > double counting the instruction execution. To avoid this we introduce > the CF_NOINSTR cflag which disables instrumentation for the next TB. > As this is part of the hashed compile flags we

Re: Detecting Faulting Instructions From Plugins

2021-02-11 Thread Aaron Lindsay via
On Feb 11 17:27, Alex Bennée wrote: > Aaron Lindsay writes: > > On Feb 05 15:03, Alex Bennée wrote: > >> Aaron Lindsay writes: > >> > Assuming you're right that TCG is detecting "a io_readx/io_writex when > >> > ->can_do_io is not true", could we detect this case when it occurs and > >> > omit

Re: Detecting Faulting Instructions From Plugins

2021-02-05 Thread Aaron Lindsay via
On Feb 05 15:03, Alex Bennée wrote: > Aaron Lindsay writes: > > Assuming you're right that TCG is detecting "a io_readx/io_writex when > > ->can_do_io is not true", could we detect this case when it occurs and > > omit the instruction callbacks for the re-translation of the single > > instruction

Re: Detecting Faulting Instructions From Plugins

2021-02-05 Thread Aaron Lindsay via
On Feb 05 15:03, Alex Bennée wrote: > I'll see what Richard thinks. I must admit I thought can_do_io was only > an issue for -icount modes but I think the real picture is slightly more > confused than that. I am using -icount. I apologize for not including that originally - I didn't realize it

Re: Detecting Faulting Instructions From Plugins

2021-02-05 Thread Aaron Lindsay via
On Feb 05 11:19, Alex Bennée wrote: > Aaron Lindsay writes: > > > For the below output, I've got a plugin which registers a callback via > > `qemu_plugin_register_vcpu_insn_exec_cb` for each instruction executed. > > I've enabled `-d in_asm` and added prints in my instruction execution > >

Re: Plugin Register Accesses

2021-01-07 Thread Aaron Lindsay via
On Jan 07 16:49, Alex Bennée wrote: > > Aaron Lindsay writes: > > > On Dec 08 14:44, Aaron Lindsay wrote: > >> On Dec 08 17:56, Alex Bennée wrote: > >> > Aaron Lindsay writes: > >> > > On Dec 08 12:17, Alex Bennée wrote: > >> > >> Aaron Lindsay writes: > >> > >> Memory is a little trickier

Re: Plugin Register Accesses

2020-12-30 Thread Aaron Lindsay via
On Dec 08 14:44, Aaron Lindsay wrote: > On Dec 08 17:56, Alex Bennée wrote: > > Aaron Lindsay writes: > > > On Dec 08 12:17, Alex Bennée wrote: > > >> Aaron Lindsay writes: > > >> Memory is a little trickier because you can't know at any point if a > > >> given virtual address is actually

Re: Plugin Register Accesses

2020-12-08 Thread Aaron Lindsay via
On Dec 08 17:56, Alex Bennée wrote: > Aaron Lindsay writes: > > On Dec 08 12:17, Alex Bennée wrote: > >> For registers I think there needs to be some re-factoring of QEMU's > >> internals to do it cleanly. Currently we have each front-end providing > >> hooks to the gdbstub as well as

Re: Plugin Register Accesses

2020-12-08 Thread Aaron Lindsay via
On Dec 08 17:56, Alex Bennée wrote: > Aaron Lindsay writes: > > On Dec 08 12:17, Alex Bennée wrote: > >> Aaron Lindsay writes: > >> Memory is a little trickier because you can't know at any point if a > >> given virtual address is actually mapped to real memory. The safest way > >> would

Re: Plugin Register Accesses

2020-12-08 Thread Aaron Lindsay via
On Dec 08 12:17, Alex Bennée wrote: > Aaron Lindsay writes: > > > I'm trying to migrate to using the new plugin interface. I see the > > following in include/qemu/qemu-plugin.h: > > > >> enum qemu_plugin_cb_flags { > >> QEMU_PLUGIN_CB_NO_REGS, /* callback does not access the CPU's regs */ >