| Message ID | 20211123205729.2205806-2-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | more tcg, plugin, test and build fixes | expand |
On 11/23/21 9:57 PM, Alex Bennée wrote: > From: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> > > Watchpoint may be processed in two phases. First one is detecting > the instruction with target memory access. And the second one is > executing only one instruction and setting the debug interrupt flag. > Hardware interrupts can break this sequence when they happen after > the first watchpoint phase. > This patch postpones the interrupt request until watchpoint is > processed. > > Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > Reviewed-by: David Hildenbrand <david@redhat.com> > Message-Id: <163662451431.125458.14945698834107669531.stgit@pasha-ThinkPad-X280> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > accel/tcg/cpu-exec.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c > index 2d14d02f6c..9cb892e326 100644 > --- a/accel/tcg/cpu-exec.c > +++ b/accel/tcg/cpu-exec.c > @@ -742,6 +742,11 @@ static inline bool cpu_handle_interrupt(CPUState *cpu, > qemu_mutex_unlock_iothread(); > return true; > } > + /* Process watchpoints first, or interrupts will ruin everything */ > + if (cpu->watchpoint_hit) { > + qemu_mutex_unlock_iothread(); > + return false; > + } I think this is redundant with the next patch. r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 11/23/21 9:57 PM, Alex Bennée wrote: >> From: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> >> Watchpoint may be processed in two phases. First one is detecting >> the instruction with target memory access. And the second one is >> executing only one instruction and setting the debug interrupt flag. >> Hardware interrupts can break this sequence when they happen after >> the first watchpoint phase. >> This patch postpones the interrupt request until watchpoint is >> processed. >> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru> >> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> >> Reviewed-by: David Hildenbrand <david@redhat.com> >> Message-Id: <163662451431.125458.14945698834107669531.stgit@pasha-ThinkPad-X280> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> accel/tcg/cpu-exec.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c >> index 2d14d02f6c..9cb892e326 100644 >> --- a/accel/tcg/cpu-exec.c >> +++ b/accel/tcg/cpu-exec.c >> @@ -742,6 +742,11 @@ static inline bool cpu_handle_interrupt(CPUState *cpu, >> qemu_mutex_unlock_iothread(); >> return true; >> } >> + /* Process watchpoints first, or interrupts will ruin everything */ >> + if (cpu->watchpoint_hit) { >> + qemu_mutex_unlock_iothread(); >> + return false; >> + } > > I think this is redundant with the next patch. OK I'll drop it. The function is getting messy anyway.
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index 2d14d02f6c..9cb892e326 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -742,6 +742,11 @@ static inline bool cpu_handle_interrupt(CPUState *cpu, qemu_mutex_unlock_iothread(); return true; } + /* Process watchpoints first, or interrupts will ruin everything */ + if (cpu->watchpoint_hit) { + qemu_mutex_unlock_iothread(); + return false; + } #if !defined(CONFIG_USER_ONLY) if (replay_mode == REPLAY_MODE_PLAY && !replay_has_interrupt()) { /* Do nothing */