Re: [PATCH v16 17/18] plugins: Allow to read registers

[Akihiko Odaki]

On 2023/11/02 20:26, Alex Bennée wrote:
> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
>
> > It is based on GDB protocol to ensure interface stability.
> >
> > The timing of the vcpu init hook is also changed so that the hook will
> > get called after GDB features are initialized.
>
> I guess this was moved into an earlier patch.
> >   }
> > +
> > +int qemu_plugin_find_register_file(unsigned int vcpu_index, const char *name)
> > +{
> > +    QEMU_IOTHREAD_LOCK_GUARD();
>
> What is this lock protecting against?
>
> gdb_register_feature doesn't protect cpu->gdb_regs() when registering
> features as it is implicitly per-CPU.
>
> If GDB should be locking I would push that down into gdbstub itself (and
> not use the global IOTHREAD).

Without lock, gdb_find_feature() may be called while the vCPU is exiting 
on another thread. The vCPU exit only happens in iothread so taking 
iothread ensures such race will not happen. Many functions in QEMU 
expect the iothread lock is taken so I make the same assumption for 
gdb_find_feature().