| Message ID | 161130982491.1038646.15688151175539344664.stgit@pasha-ThinkPad-X280 |
|---|---|
| State | New |
| Headers | show |
| Series | util/log: flush TB cache when log level changes | expand |
Hi Pavel, On 1/22/21 11:03 AM, Pavel Dovgalyuk wrote: > 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.Dovgalyuk@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 ... > /* > * 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); Why not declare in "exec/exec-all.h"?
On 22.01.2021 13:32, Philippe Mathieu-Daudé wrote: > Hi Pavel, > > On 1/22/21 11:03 AM, Pavel Dovgalyuk wrote: >> 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.Dovgalyuk@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 > ... > >> /* >> * 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); > > Why not declare in "exec/exec-all.h"? > It includes cpu.h, which is not available for all tools, that use logs. Pavel Dovgalyuk
Pavel Dovgalyuk <pavel.dovgalyuk@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.Dovgalyuk@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: if (log_flags & LOG_TRANSLATION) { tb_flush(); } > /* > * In all cases we only log if qemu_loglevel is set. > * Also:
On 1/22/21 12:42 PM, Alex Bennée wrote: > > Pavel Dovgalyuk <pavel.dovgalyuk@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.Dovgalyuk@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 ... > 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 Indeed, with QTest accelerator it is NULL. I also remember some odd cases when running commands from the monitor. Regards, Phil.
On 22.01.2021 14:42, Alex Bennée wrote: > > Pavel Dovgalyuk <pavel.dovgalyuk@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.Dovgalyuk@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? > if (log_flags & LOG_TRANSLATION) { > tb_flush(); > } > >> /* >> * In all cases we only log if qemu_loglevel is set. >> * Also: > >
Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes: > On 22.01.2021 14:42, Alex Bennée wrote: >> >> Pavel Dovgalyuk <pavel.dovgalyuk@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.Dovgalyuk@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: >> >>
On 25.01.2021 14:09, Alex Bennée wrote: > > Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes: > >> On 22.01.2021 14:42, Alex Bennée wrote: >>> >>> Pavel Dovgalyuk <pavel.dovgalyuk@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.Dovgalyuk@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 It the following true? We can't get rid of CPU in tb_flush, because do_tb_flush must be executed in vCPU thread. Can one CPU break others execution in case of SMP? Can we move flush to BH somehow? > something like: > > CPUStatus *cpu = current_cpu || first_cpu; > if (cpu) { > tb_flush(cpu); > } > Then we'll have to expose all this CPU stuff to utils and add stubs for them. > 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: >>> >>> > >
Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes: > On 25.01.2021 14:09, Alex Bennée wrote: >> >> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes: >> >>> On 22.01.2021 14:42, Alex Bennée wrote: >>>> >>>> Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> writes: <snip> >>>>> #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 > > It the following true? We can't get rid of CPU in tb_flush, because > do_tb_flush must be executed in vCPU thread. > Can one CPU break others execution in case of SMP? Can we move flush to > BH somehow? The flush has to execute in the CPU context because it uses the exclusive mechanism to ensure all other CPUs are currently quiescent before doing the flush. Currently the exclusive mechanism is only aware of CPU threads and extending it would be essentially re-introducing a BQL. > >> something like: >> >> CPUStatus *cpu = current_cpu || first_cpu; >> if (cpu) { >> tb_flush(cpu); >> } >> > > Then we'll have to expose all this CPU stuff to utils and add stubs for > them. > >> 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: >>>> >>>> >> >>
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); + } +} + /* * 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(); + /* * In all cases we only log if qemu_loglevel is set. * Also:
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.Dovgalyuk@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