| Message ID | 20231207154550.65087-5-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | sysemu/replay: Restrict icount to TCG system emulation | expand |
On 12/7/23 07:45, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > stubs/icount.c | 6 ------ > system/vl.c | 6 +++++- > 2 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/stubs/icount.c b/stubs/icount.c > index a5202e2dd9..b060b03a73 100644 > --- a/stubs/icount.c > +++ b/stubs/icount.c > @@ -1,5 +1,4 @@ > #include "qemu/osdep.h" > -#include "qapi/error.h" > #include "sysemu/cpu-timers.h" > > /* icount - Instruction Counter API */ > @@ -10,11 +9,6 @@ void icount_update(CPUState *cpu) > { > abort(); > } > -void icount_configure(QemuOpts *opts, Error **errp) > -{ > - /* signal error */ > - error_setg(errp, "cannot configure icount, TCG support not available"); > -} > int64_t icount_get_raw(void) > { > abort(); > diff --git a/system/vl.c b/system/vl.c > index 2bcd9efb9a..8c99c5f681 100644 > --- a/system/vl.c > +++ b/system/vl.c > @@ -2270,7 +2270,11 @@ static void user_register_global_props(void) > > static int do_configure_icount(void *opaque, QemuOpts *opts, Error **errp) > { > - icount_configure(opts, errp); > + if (tcg_enabled()) { > + icount_configure(opts, errp); > + } else { > + error_setg(errp, "cannot configure icount, TCG support not available"); > + } > return 0; > } This is called before the accelerator is chosen -- even before the set of available accelerators is even found. Indeed, that's the very next thing that configure_accelerators does. OTOH, I don't see why icount_configure is being called so early. r~ >
On 7/12/23 23:38, Richard Henderson wrote: > On 12/7/23 07:45, Philippe Mathieu-Daudé wrote: >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> stubs/icount.c | 6 ------ >> system/vl.c | 6 +++++- >> 2 files changed, 5 insertions(+), 7 deletions(-) >> >> diff --git a/stubs/icount.c b/stubs/icount.c >> index a5202e2dd9..b060b03a73 100644 >> --- a/stubs/icount.c >> +++ b/stubs/icount.c >> @@ -1,5 +1,4 @@ >> #include "qemu/osdep.h" >> -#include "qapi/error.h" >> #include "sysemu/cpu-timers.h" >> /* icount - Instruction Counter API */ >> @@ -10,11 +9,6 @@ void icount_update(CPUState *cpu) >> { >> abort(); >> } >> -void icount_configure(QemuOpts *opts, Error **errp) >> -{ >> - /* signal error */ >> - error_setg(errp, "cannot configure icount, TCG support not >> available"); >> -} >> int64_t icount_get_raw(void) >> { >> abort(); >> diff --git a/system/vl.c b/system/vl.c >> index 2bcd9efb9a..8c99c5f681 100644 >> --- a/system/vl.c >> +++ b/system/vl.c >> @@ -2270,7 +2270,11 @@ static void user_register_global_props(void) >> static int do_configure_icount(void *opaque, QemuOpts *opts, Error >> **errp) >> { >> - icount_configure(opts, errp); >> + if (tcg_enabled()) { >> + icount_configure(opts, errp); >> + } else { >> + error_setg(errp, "cannot configure icount, TCG support not >> available"); >> + } >> return 0; >> } > > This is called before the accelerator is chosen -- even before the set > of available accelerators is even found. Indeed, that's the very next > thing that configure_accelerators does. > > OTOH, I don't see why icount_configure is being called so early. See commit 7f8b6126e7: vl: move icount configuration earlier Once qemu_tcg_configure is turned into a QOM property setter, it will not be able to set a default value for mttcg_enabled. Setting the default will move to the TCG instance_init function, which currently runs before "-icount" is processed. However, it is harmless to do configure_icount for all accelerators; we will just fail later if a non-TCG accelerator is selected. So do that. But few commits after we have 28a0961757 ("vl: merge -accel processing into configure_accelerators"), so it shouldn't be an issue now. I'll consolidate.
diff --git a/stubs/icount.c b/stubs/icount.c index a5202e2dd9..b060b03a73 100644 --- a/stubs/icount.c +++ b/stubs/icount.c @@ -1,5 +1,4 @@ #include "qemu/osdep.h" -#include "qapi/error.h" #include "sysemu/cpu-timers.h" /* icount - Instruction Counter API */ @@ -10,11 +9,6 @@ void icount_update(CPUState *cpu) { abort(); } -void icount_configure(QemuOpts *opts, Error **errp) -{ - /* signal error */ - error_setg(errp, "cannot configure icount, TCG support not available"); -} int64_t icount_get_raw(void) { abort(); diff --git a/system/vl.c b/system/vl.c index 2bcd9efb9a..8c99c5f681 100644 --- a/system/vl.c +++ b/system/vl.c @@ -2270,7 +2270,11 @@ static void user_register_global_props(void) static int do_configure_icount(void *opaque, QemuOpts *opts, Error **errp) { - icount_configure(opts, errp); + if (tcg_enabled()) { + icount_configure(opts, errp); + } else { + error_setg(errp, "cannot configure icount, TCG support not available"); + } return 0; }
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- stubs/icount.c | 6 ------ system/vl.c | 6 +++++- 2 files changed, 5 insertions(+), 7 deletions(-)