On Feb 12 11:22, Alex Bennée wrote: > 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. > > Hmm well we correctly don't instrument stores (as we have already > executed the plugin for them) - but of course the load instrumentation > is after the fact so we are now missing them.
I do not believe I am seeing memory callbacks for stores, either. Are you saying I definitely should be? My original observation was that the callbacks for store instructions to IO followed the same pattern as loads: 1) Initial instruction callback (presumably as part of larger block) 2) Second instruction callback (presumably as part of single-instruction block) 3) Memory callback (presumably as part of single-instruction block) After applying v2 of your patchset I now see only 1), even for stores. > > 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? > > Hmmm let me see if I can finesse the CF_NOINSTR logic to allow > plugin_gen_insn_end() without the rest? It probably needs a better name > for the flag as well. Funny, the first time reading through this patch I was unsure for a second whether "CF_NOINSTR" stood for "NO INSTRuction callbacks" or "NO INSTRumentation"! -Aaron