On Feb 12 16:00, Alex Bennée wrote:
>
> Alex Bennée <alex.ben...@linaro.org> writes:
>
> > Aaron Lindsay <aa...@os.amperecomputing.com> 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 instrumentation for the next TB.
> >>> As this is part of the hashed compile flags we will only execute the
> >>> generated TB while coming out of a cpu_io_recompile.
> >>
> >> Unfortunately this patch works a little too well!
> >>
> >> With this change, the memory access callbacks registered via
> >> `qemu_plugin_register_vcpu_mem_cb()` are never called for the
> >> re-translated instruction making the IO access, since we've disabled all
> >> instrumentation.
> >>
> >> Is it possible to selectively disable only instruction callbacks using
> >> this mechanism, while still allowing others that would not yet have been
> >> called for the re-translated instruction?
> >
> > Can you try the following fugly patch on top of this series:
> >
> <snip>
> > @@ -120,8 +128,13 @@ void qemu_plugin_register_vcpu_mem_cb(struct
> > qemu_plugin_insn *insn,
> > enum qemu_plugin_mem_rw rw,
> > void *udata)
> > {
> > -
> > plugin_register_vcpu_mem_cb(&insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR],
> > - cb, flags, rw, udata);
> > + if (insn->store_only && (rw & QEMU_PLUGIN_MEM_W)) {
> > +
> > plugin_register_vcpu_mem_cb(&insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR],
> > + cb, flags, QEMU_PLUGIN_MEM_W, udata);
> > + } else {
> > +
> > plugin_register_vcpu_mem_cb(&insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR],
> > + cb, flags, rw, udata);
> > + }
> > }
> <snip>
>
> Actually I'm wondering if I've got my sense the wrong way around. Should
> it be loads only:
>
> void qemu_plugin_register_vcpu_mem_cb(struct qemu_plugin_insn *insn,
> qemu_plugin_vcpu_mem_cb_t cb,
> enum qemu_plugin_cb_flags flags,
> enum qemu_plugin_mem_rw rw,
> void *udata)
> {
> if (insn->store_only && (rw & QEMU_PLUGIN_MEM_R)) {
>
> plugin_register_vcpu_mem_cb(&insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR],
> cb, flags, QEMU_PLUGIN_MEM_R, udata);
> } else {
>
> plugin_register_vcpu_mem_cb(&insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR],
> cb, flags, rw, udata);
> }
> }
>
> obviously I'd have to rename the variables :-/
This gets me only loads and no stores. I've modified it to be just:
void qemu_plugin_register_vcpu_mem_cb(struct qemu_plugin_insn *insn,
qemu_plugin_vcpu_mem_cb_t cb,
enum qemu_plugin_cb_flags flags,
enum qemu_plugin_mem_rw rw,
void *udata)
{
plugin_register_vcpu_mem_cb(&insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULAR],
cb, flags, rw, udata);
}
And that appears to get me one memory callback both for loads and stores.
-Aaron
