| Message ID | 20180521122100.22602-3-michael@walle.cc (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 21 May 2018 at 13:21, Michael Walle <michael@walle.cc> wrote: > Changing the IP/IM registers may cause interrupts, so hold the BQL. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Michael Walle <michael@walle.cc> > --- > 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. thanks -- PMM
On 21 May 2018 at 13:25, Peter Maydell <peter.maydell@linaro.org> wrote: > On 21 May 2018 at 13:21, Michael Walle <michael@walle.cc> wrote: >> Changing the IP/IM registers may cause interrupts, so hold the BQL. >> >> Cc: qemu-stable@nongnu.org >> Signed-off-by: Michael Walle <michael@walle.cc> >> --- >> 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
Am 2018-05-21 14:25, schrieb Peter Maydell: > On 21 May 2018 at 13:21, Michael Walle <michael@walle.cc> wrote: >> Changing the IP/IM registers may cause interrupts, so hold the BQL. >> >> Cc: qemu-stable@nongnu.org >> Signed-off-by: Michael Walle <michael@walle.cc> >> --- >> 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. You're right. The gdbstub is already holding the lock. So i'll drop this and send the pull request right now. -michael
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; } }
Changing the IP/IM registers may cause interrupts, so hold the BQL. Cc: qemu-stable@nongnu.org Signed-off-by: Michael Walle <michael@walle.cc> --- target/lm32/gdbstub.c | 5 +++++ 1 file changed, 5 insertions(+)