Pavel Dovgalyuk <pavel.dovgal...@ispras.ru> writes:
> On 22.01.2021 14:42, Alex Bennée wrote:
>>
>> Pavel Dovgalyuk <pavel.dovgal...@ispras.ru> writes:
>>
>>> Sometimes we need to collect the translation logs starting
>>> from some point of the execution. Some TB listings may
>>> be missed in this case, when blocks were translated before.
>>> This patch clears TB cache to allow re-translation of such
>>> code blocks.
>>>
>>> Signed-off-by: Pavel Dovgalyuk <pavel.dovgal...@ispras.ru>
>>> ---
>>> accel/tcg/translate-all.c | 8 ++++++++
>>> include/sysemu/tcg.h | 1 +
>>> stubs/meson.build | 1 +
>>> stubs/tcg.c | 12 ++++++++++++
>>> util/log.c | 3 +++
>>> 5 files changed, 25 insertions(+)
>>> create mode 100644 stubs/tcg.c
>>>
>>> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
>>> index e9de6ff9dd..3acb227c57 100644
>>> --- a/accel/tcg/translate-all.c
>>> +++ b/accel/tcg/translate-all.c
>>> @@ -1461,6 +1461,14 @@ void tb_flush(CPUState *cpu)
>>> }
>>> }
>>>
>>> +void tb_flush_all(void)
>>> +{
>>> + CPUState *cpu;
>>> + CPU_FOREACH(cpu) {
>>> + tb_flush(cpu);
>>> + }
>>> +}
>>> +
>>
>> This isn't needed - tb_flush flushes all translations although it does
>> need to be executed in a CPU context to do so.
>>
>>> /*
>>> * Formerly ifdef DEBUG_TB_CHECK. These debug functions are
>>> user-mode-only,
>>> * so in order to prevent bit rot we compile them unconditionally in
>>> user-mode,
>>> diff --git a/include/sysemu/tcg.h b/include/sysemu/tcg.h
>>> index 00349fb18a..7415f11022 100644
>>> --- a/include/sysemu/tcg.h
>>> +++ b/include/sysemu/tcg.h
>>> @@ -9,6 +9,7 @@
>>> #define SYSEMU_TCG_H
>>>
>>> void tcg_exec_init(unsigned long tb_size, int splitwx);
>>> +void tb_flush_all(void);
>>>
>>> #ifdef CONFIG_TCG
>>> extern bool tcg_allowed;
>>> diff --git a/stubs/meson.build b/stubs/meson.build
>>> index 80b1d81a31..95e70f8542 100644
>>> --- a/stubs/meson.build
>>> +++ b/stubs/meson.build
>>> @@ -38,6 +38,7 @@ stub_ss.add(files('set-fd-handler.c'))
>>> stub_ss.add(files('sysbus.c'))
>>> stub_ss.add(files('target-get-monitor-def.c'))
>>> stub_ss.add(files('target-monitor-defs.c'))
>>> +stub_ss.add(files('tcg.c'))
>>> stub_ss.add(files('tpm.c'))
>>> stub_ss.add(files('trace-control.c'))
>>> stub_ss.add(files('uuid.c'))
>>> diff --git a/stubs/tcg.c b/stubs/tcg.c
>>> new file mode 100644
>>> index 0000000000..775a748c77
>>> --- /dev/null
>>> +++ b/stubs/tcg.c
>>> @@ -0,0 +1,12 @@
>>> +/*
>>> + * TCG stubs
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>>> later.
>>> + * See the COPYING file in the top-level directory.
>>> + */
>>> +
>>> +#include "sysemu/tcg.h"
>>> +
>>> +void tb_flush_all(void)
>>> +{
>>> +}
>>> diff --git a/util/log.c b/util/log.c
>>> index 2ee1500bee..2ff342a91b 100644
>>> --- a/util/log.c
>>> +++ b/util/log.c
>>> @@ -26,6 +26,7 @@
>>> #include "trace/control.h"
>>> #include "qemu/thread.h"
>>> #include "qemu/lockable.h"
>>> +#include "sysemu/tcg.h"
>>>
>>> static char *logfilename;
>>> static QemuMutex qemu_logfile_mutex;
>>> @@ -84,6 +85,8 @@ void qemu_set_log(int log_flags)
>>> #ifdef CONFIG_TRACE_LOG
>>> qemu_loglevel |= LOG_TRACE;
>>> #endif
>>> + tb_flush_all();
>>> +
>>
>> I would call tb_flush(current_cpu) or first_cpu here. But two things:
>>
>> - I'm not sure you have a CPU at all times qemu_set_log is called
>> - It seems overly aggressive to throw away all translations every time
>> the log level is changed. I would define a mask in log.h and have
>> something like:
>
> Do you propose removing the parameter from tb_flush or omitting the loop
> from tb_flush_all?
No tb_flush should keep the CPU interface. In normal usage from the
emulation we always have a CPU to call. However for qemu_set_log you
will need to find a CPU to call or bail out if you can't. Maybe
something like:
CPUStatus *cpu = current_cpu || first_cpu;
if (cpu) {
tb_flush(cpu);
}
my only worry is if qemu_set_log is called from outside a CPU context
(current_cpu will always be NULL) while first_cpu is in a exclusive
region. We could extend cpu_in_exclusive_context to be:
cpu == current_cpu && cpu->in_exclusive_context
but that seems a little icky to me. Paolo, any thoughts?
>
>> if (log_flags & LOG_TRANSLATION) {
>> tb_flush();
>> }
>>
>>> /*
>>> * In all cases we only log if qemu_loglevel is set.
>>> * Also:
>>
>>
--
Alex Bennée
