Richard Henderson <richard.hender...@linaro.org> writes:
> On 7/21/21 12:38 AM, Alex Bennée wrote:
>> Richard Henderson <richard.hender...@linaro.org> writes:
>>
>>> Set CF_SINGLE_STEP when single-stepping is enabled.
>>> This avoids the need to flush all tb's when turning
>>> single-stepping on or off.
>>>
>>> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
>>> ---
>>> include/exec/exec-all.h | 1 +
>>> accel/tcg/cpu-exec.c | 7 ++++++-
>>> accel/tcg/translate-all.c | 4 ----
>>> accel/tcg/translator.c | 7 +------
>>> cpu.c | 4 ----
>>> 5 files changed, 8 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
>>> index 6873cce8df..5d1b6d80fb 100644
>>> --- a/include/exec/exec-all.h
>>> +++ b/include/exec/exec-all.h
>>> @@ -497,6 +497,7 @@ struct TranslationBlock {
>>> #define CF_COUNT_MASK 0x000001ff
>>> #define CF_NO_GOTO_TB 0x00000200 /* Do not chain with goto_tb */
>>> #define CF_NO_GOTO_PTR 0x00000400 /* Do not chain with goto_ptr */
>>> +#define CF_SINGLE_STEP 0x00000800 /* gdbstub single-step in effect */
>>> #define CF_LAST_IO 0x00008000 /* Last insn may be an IO access. */
>>> #define CF_MEMI_ONLY 0x00010000 /* Only instrument memory ops */
>>> #define CF_USE_ICOUNT 0x00020000
>>> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>>> index 5cc6363f4c..fc895cf51e 100644
>>> --- a/accel/tcg/cpu-exec.c
>>> +++ b/accel/tcg/cpu-exec.c
>>> @@ -150,10 +150,15 @@ uint32_t curr_cflags(CPUState *cpu)
>>> uint32_t cflags = cpu->tcg_cflags;
>>> /*
>>> + * Record gdb single-step. We should be exiting the TB by raising
>>> + * EXCP_DEBUG, but to simplify other tests, disable chaining too.
>>> + *
>>> * For singlestep and -d nochain, suppress goto_tb so that
>>> * we can log -d cpu,exec after every TB.
>>> */
>>> - if (singlestep) {
>>> + if (unlikely(cpu->singlestep_enabled)) {
>>> + cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR | CF_SINGLE_STEP |
>>> 1;
>> What does CF_SINGLE_STEP achieve that isn't already handled by
>> having:
>> cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR | 1;
>
> It sets DisasContextBase.singlestep_enabled.
Ahh fair enough... I was only thinking of the effect on stored and
looked up translations. I guess we still have bits we can rob if we need
to until the day we expand cflags and flags to full 64 bit values.
> With only this patch set, we still check that and emit EXCP_DEBUG at
> the end of every TB. After the 6.2 singlestep cleanup, we still have
> one reference to DisasContextBase.singlestep_enabled in target/mips
> for the branch delay slot thing that we discussed on IRC yesterday.
>
>> (btw did we mask CF_COUNT_MASK somewhere else?). Because surely the
>> CF_COUNT is part of cflags so limits the TB's that could be returned
>> anyway?
>
> Here in curr_cflags(), CF_COUNT_MASK begins at zero.
OK:
Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
--
Alex Bennée
