Re: [Qemu-stable] [Qemu-devel] [PATCH v2 2/2] target/lm32: hold BQL in gdbstub

[Peter Maydell]

On 21 May 2018 at 13:25, Peter Maydell <address@hidden> wrote:
> On 21 May 2018 at 13:21, Michael Walle <address@hidden> wrote:
>> Changing the IP/IM registers may cause interrupts, so hold the BQL.
>>
>> Cc: address@hidden
>> Signed-off-by: Michael Walle <address@hidden>
>> ---
>>  target/lm32/gdbstub.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/target/lm32/gdbstub.c b/target/lm32/gdbstub.c
>> index cf929dd392..dac9418a2b 100644
>> --- a/target/lm32/gdbstub.c
>> +++ b/target/lm32/gdbstub.c
>> @@ -18,6 +18,7 @@
>>   * License along with this library; if not, see 
>> <http://www.gnu.org/licenses/>.
>>   */
>>  #include "qemu/osdep.h"
>> +#include "qemu/main-loop.h"
>>  #include "qemu-common.h"
>>  #include "cpu.h"
>>  #include "exec/gdbstub.h"
>> @@ -82,10 +83,14 @@ int lm32_cpu_gdb_write_register(CPUState *cs, uint8_t 
>> *mem_buf, int n)
>>              env->ie = tmp;
>>              break;
>>          case 37:
>> +            qemu_mutex_lock_iothread();
>>              lm32_pic_set_im(env->pic_state, tmp);
>> +            qemu_mutex_unlock_iothread();
>>              break;
>>          case 38:
>> +            qemu_mutex_lock_iothread();
>>              lm32_pic_set_ip(env->pic_state, tmp);
>> +            qemu_mutex_unlock_iothread();
>>              break;
>>          }
>>      }
>
> Are you sure this is necessary? I would have expected the gdbstub to
> be operating under the qemu lock anyway.

...experimentation suggests that the gdbstub is called via
chardev write events which are triggered by glib_pollfds_poll(),
which is called by os_host_main_loop_wait() only when it holds
the iothread lock.

thanks
-- PMM