On Mon, 3 Apr 2023 at 19:33, Richard Henderson <richard.henderson@xxxxxxxxxx> wrote: > > On 4/3/23 07:46, Peter Maydell wrote: > > uint32_t curr_cflags(CPUState *cpu) > > { > > uint32_t cflags = cpu->tcg_cflags; > > + TCGState *tcgstate = TCG_STATE(current_accel()); > > As mentioned against the cover, this is a very hot path. > > We should try for something less expensive. Perhaps as simple as > > return cpu->tcg_cflags | tcg_cflags_global; > > where cpu->tcg_cflags is updated with cpu->singlestep_enabled. I feel like that introduces atomicity issues. If I'm reading the code right, curr_cflags() is called without any kind of lock held. At the moment we get away with this because 'singlestep' is an int and is always going to be atomically updated. If we make tcg_cflags_global a value which might have multiple bits set or not set I'm not entirely sure what the right way is to handle the reads and writes of it. I think we can assume we have the iothread lock at any point where we want to change either 'singlestep' or the 'nochain' option, at least. Any suggestions? I'm not very familiar with the qemu atomic primitives... thanks -- PMM