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 :-/
--
Alex Bennée
