From: "Alex Bennée" <alex.bennee@linaro.org>
To: Claudio Fontana <cfontana@suse.de>
Cc: "Laurent Vivier" <lvivier@redhat.com>,
"Peter Maydell" <peter.maydell@linaro.org>,
"Thomas Huth" <thuth@redhat.com>,
"Alberto Garcia" <berto@igalia.com>,
"Eduardo Habkost" <ehabkost@redhat.com>,
"Pavel Dovgalyuk" <dovgaluk@ispras.ru>,
haxm-team@intel.com, "Marcelo Tosatti" <mtosatti@redhat.com>,
qemu-devel@nongnu.org, "Markus Armbruster" <armbru@redhat.com>,
"Roman Bolshakov" <r.bolshakov@yadro.com>,
"Colin Xu" <colin.xu@intel.com>,
"Wenchao Wang" <wenchao.wang@intel.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Sunil Muthuswamy" <sunilmut@microsoft.com>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>,
"Richard Henderson" <rth@twiddle.net>
Subject: Re: [PATCH v6 03/16] cpus: prepare new CpusAccel cpu accelerator interface
Date: Tue, 01 Sep 2020 11:38:36 +0100 [thread overview]
Message-ID: <87k0xd6atv.fsf@linaro.org> (raw)
In-Reply-To: <20200901072201.7133-4-cfontana@suse.de>
Claudio Fontana <cfontana@suse.de> writes:
> The new interface starts unused, will start being used by the
> next patches.
>
> It provides methods for each accelerator to start a vcpu, kick a vcpu,
> synchronize state, get cpu virtual clock and elapsed ticks.
>
> In qemu_wait_io_event, make it clear that APC is used only for HAX
> on Windows.
>
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
<snip>
>
> /* signal CPU creation */
> - cpu->created = true;
> - qemu_cond_signal(&qemu_cpu_cond);
> + cpu_thread_signal_created(cpu);
> qemu_guest_random_seed_thread_part2(cpu->random_seed);
>
> do {
> @@ -482,8 +582,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
> } while (!cpu->unplug || cpu_can_run(cpu));
>
> qemu_kvm_destroy_vcpu(cpu);
> - cpu->created = false;
> - qemu_cond_signal(&qemu_cpu_cond);
> + cpu_thread_signal_destroyed(cpu);
> qemu_mutex_unlock_iothread();
> rcu_unregister_thread();
> return NULL;
> @@ -511,8 +610,7 @@ static void *qemu_dummy_cpu_thread_fn(void *arg)
> sigaddset(&waitset, SIG_IPI);
>
> /* signal CPU creation */
> - cpu->created = true;
> - qemu_cond_signal(&qemu_cpu_cond);
> + cpu_thread_signal_created(cpu);
> qemu_guest_random_seed_thread_part2(cpu->random_seed);
>
> do {
> @@ -660,8 +758,7 @@ static void deal_with_unplugged_cpus(void)
> CPU_FOREACH(cpu) {
> if (cpu->unplug && !cpu_can_run(cpu)) {
> qemu_tcg_destroy_vcpu(cpu);
> - cpu->created = false;
> - qemu_cond_signal(&qemu_cpu_cond);
> + cpu_thread_signal_destroyed(cpu);
> break;
> }
> }
> @@ -688,9 +785,8 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
> qemu_thread_get_self(cpu->thread);
>
> cpu->thread_id = qemu_get_thread_id();
> - cpu->created = true;
> cpu->can_do_io = 1;
> - qemu_cond_signal(&qemu_cpu_cond);
> + cpu_thread_signal_created(cpu);
> qemu_guest_random_seed_thread_part2(cpu->random_seed);
>
> /* wait for initial kick-off after machine start */
> @@ -800,11 +896,9 @@ static void *qemu_hax_cpu_thread_fn(void *arg)
> qemu_thread_get_self(cpu->thread);
>
> cpu->thread_id = qemu_get_thread_id();
> - cpu->created = true;
> current_cpu = cpu;
> -
> hax_init_vcpu(cpu);
> - qemu_cond_signal(&qemu_cpu_cond);
> + cpu_thread_signal_created(cpu);
> qemu_guest_random_seed_thread_part2(cpu->random_seed);
>
> do {
> @@ -843,8 +937,7 @@ static void *qemu_hvf_cpu_thread_fn(void *arg)
> hvf_init_vcpu(cpu);
>
> /* signal CPU creation */
> - cpu->created = true;
> - qemu_cond_signal(&qemu_cpu_cond);
> + cpu_thread_signal_created(cpu);
> qemu_guest_random_seed_thread_part2(cpu->random_seed);
>
> do {
> @@ -858,8 +951,7 @@ static void *qemu_hvf_cpu_thread_fn(void *arg)
> } while (!cpu->unplug || cpu_can_run(cpu));
>
> hvf_vcpu_destroy(cpu);
> - cpu->created = false;
> - qemu_cond_signal(&qemu_cpu_cond);
> + cpu_thread_signal_destroyed(cpu);
> qemu_mutex_unlock_iothread();
> rcu_unregister_thread();
> return NULL;
> @@ -884,8 +976,7 @@ static void *qemu_whpx_cpu_thread_fn(void *arg)
> }
>
> /* signal CPU creation */
> - cpu->created = true;
> - qemu_cond_signal(&qemu_cpu_cond);
> + cpu_thread_signal_created(cpu);
> qemu_guest_random_seed_thread_part2(cpu->random_seed);
>
> do {
> @@ -902,8 +993,7 @@ static void *qemu_whpx_cpu_thread_fn(void *arg)
> } while (!cpu->unplug || cpu_can_run(cpu));
>
> whpx_destroy_vcpu(cpu);
> - cpu->created = false;
> - qemu_cond_signal(&qemu_cpu_cond);
> + cpu_thread_signal_destroyed(cpu);
> qemu_mutex_unlock_iothread();
> rcu_unregister_thread();
> return NULL;
> @@ -936,10 +1026,9 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
> qemu_thread_get_self(cpu->thread);
>
> cpu->thread_id = qemu_get_thread_id();
> - cpu->created = true;
> cpu->can_do_io = 1;
> current_cpu = cpu;
> - qemu_cond_signal(&qemu_cpu_cond);
> + cpu_thread_signal_created(cpu);
> qemu_guest_random_seed_thread_part2(cpu->random_seed);
>
> /* process any pending work */
> @@ -980,14 +1069,13 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
> } while (!cpu->unplug || cpu_can_run(cpu));
>
> qemu_tcg_destroy_vcpu(cpu);
> - cpu->created = false;
> - qemu_cond_signal(&qemu_cpu_cond);
> + cpu_thread_signal_destroyed(cpu);
> qemu_mutex_unlock_iothread();
> rcu_unregister_thread();
> return NULL;
I suspect this clean-up could be a separate patch.
<snip>
>
> +/* signal CPU creation */
> +void cpu_thread_signal_created(CPUState *cpu)
> +{
> + cpu->created = true;
> + qemu_cond_signal(&qemu_cpu_cond);
> +}
> +
> +/* signal CPU destruction */
> +void cpu_thread_signal_destroyed(CPUState *cpu)
> +{
> + cpu->created = false;
> + qemu_cond_signal(&qemu_cpu_cond);
> +}
> +
> +
> static bool all_vcpus_paused(void)
> {
> CPUState *cpu;
> @@ -1163,9 +1269,6 @@ void cpu_remove_sync(CPUState *cpu)
> qemu_mutex_lock_iothread();
> }
>
> -/* For temporary buffers for forming a name */
> -#define VCPU_THREAD_NAME_SIZE 16
> -
Lets kill this rather than move it around. The thread functions could
more cleanly do:
g_autoptr(GString) thread_name = g_string_new(NULL);
...
g_string_printf(thread_name, "CPU %d/DUMMY", cpu->cpu_index);
qemu_thread_create(..., thread_name->str, ...);
> static void qemu_tcg_init_vcpu(CPUState *cpu)
> {
> char thread_name[VCPU_THREAD_NAME_SIZE];
> @@ -1286,6 +1389,13 @@ static void qemu_whpx_start_vcpu(CPUState *cpu)
> #endif
> }
>
> +void cpus_register_accel(const CpusAccel *ca)
> +{
> + assert(ca != NULL);
> + assert(ca->create_vcpu_thread != NULL); /* mandatory */
> + cpus_accel = ca;
> +}
> +
> static void qemu_dummy_start_vcpu(CPUState *cpu)
> {
> char thread_name[VCPU_THREAD_NAME_SIZE];
> @@ -1316,7 +1426,10 @@ void qemu_init_vcpu(CPUState *cpu)
> cpu_address_space_init(cpu, 0, "cpu-memory", cpu->memory);
> }
>
> - if (kvm_enabled()) {
> + if (cpus_accel) {
> + /* accelerator already implements the CpusAccel interface */
> + cpus_accel->create_vcpu_thread(cpu);
> + } else if (kvm_enabled()) {
> qemu_kvm_start_vcpu(cpu);
> } else if (hax_enabled()) {
> qemu_hax_start_vcpu(cpu);
> diff --git a/stubs/cpu-synchronize-state.c b/stubs/cpu-synchronize-state.c
> new file mode 100644
> index 0000000000..d9211da66c
> --- /dev/null
> +++ b/stubs/cpu-synchronize-state.c
> @@ -0,0 +1,9 @@
> +#include "qemu/osdep.h"
> +#include "sysemu/hw_accel.h"
> +
> +void cpu_synchronize_state(CPUState *cpu)
> +{
> +}
> +void cpu_synchronize_post_init(CPUState *cpu)
> +{
> +}
> diff --git a/stubs/cpus-get-virtual-clock.c b/stubs/cpus-get-virtual-clock.c
> new file mode 100644
> index 0000000000..fd447d53f3
> --- /dev/null
> +++ b/stubs/cpus-get-virtual-clock.c
> @@ -0,0 +1,8 @@
> +#include "qemu/osdep.h"
> +#include "sysemu/cpu-timers.h"
> +#include "qemu/main-loop.h"
> +
> +int64_t cpus_get_virtual_clock(void)
> +{
> + return cpu_get_clock();
> +}
> diff --git a/stubs/meson.build b/stubs/meson.build
> index 57fec4f8c8..54d4a3f6d4 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -5,6 +5,7 @@ stub_ss.add(files('blockdev-close-all-bdrv-states.c'))
> stub_ss.add(files('change-state-handler.c'))
> stub_ss.add(files('cmos.c'))
> stub_ss.add(files('cpu-get-clock.c'))
> +stub_ss.add(files('cpus-get-virtual-clock.c'))
> stub_ss.add(files('qemu-timer-notify-cb.c'))
> stub_ss.add(files('icount.c'))
> stub_ss.add(files('dump.c'))
> @@ -45,6 +46,7 @@ stub_ss.add(files('vmgenid.c'))
> stub_ss.add(files('vmstate.c'))
> stub_ss.add(files('vm-stop.c'))
> stub_ss.add(files('win32-kbd-hook.c'))
> +stub_ss.add(files('cpu-synchronize-state.c'))
> if have_system
> stub_ss.add(files('semihost.c'))
> endif
> diff --git a/util/qemu-timer.c b/util/qemu-timer.c
> index db51e68f25..50b325c65b 100644
> --- a/util/qemu-timer.c
> +++ b/util/qemu-timer.c
> @@ -635,13 +635,7 @@ int64_t qemu_clock_get_ns(QEMUClockType type)
> return get_clock();
> default:
> case QEMU_CLOCK_VIRTUAL:
> - if (icount_enabled()) {
> - return icount_get();
> - } else if (qtest_enabled()) { /* for qtest_clock_warp */
> - return qtest_get_virtual_clock();
> - } else {
> - return cpu_get_clock();
> - }
> + return cpus_get_virtual_clock();
> case QEMU_CLOCK_HOST:
> return REPLAY_CLOCK(REPLAY_CLOCK_HOST, get_clock_realtime());
> case QEMU_CLOCK_VIRTUAL_RT:
Otherwise:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
next prev parent reply other threads:[~2020-09-01 10:39 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-01 7:21 [PATCH v6 00/16] QEMU cpus.c refactoring part2 Claudio Fontana
2020-09-01 7:21 ` [PATCH v6 01/16] cpu-timers, icount: new modules Claudio Fontana
2020-09-01 10:20 ` Alex Bennée
2020-09-01 12:45 ` Claudio Fontana
2020-09-01 17:28 ` Richard Henderson
2020-09-01 7:21 ` [PATCH v6 02/16] icount: rename functions to be consistent with the module name Claudio Fontana
2020-09-01 10:23 ` Alex Bennée
2020-09-01 17:18 ` Richard Henderson
2020-09-01 7:21 ` [PATCH v6 03/16] cpus: prepare new CpusAccel cpu accelerator interface Claudio Fontana
2020-09-01 10:38 ` Alex Bennée [this message]
2020-09-01 17:31 ` Richard Henderson
2020-09-01 7:21 ` [PATCH v6 04/16] cpus: extract out TCG-specific code to accel/tcg Claudio Fontana
2020-09-01 11:08 ` Alex Bennée
2020-09-01 17:34 ` Richard Henderson
2020-09-01 7:21 ` [PATCH v6 05/16] cpus: extract out qtest-specific code to accel/qtest Claudio Fontana
2020-09-01 17:35 ` Richard Henderson
2020-09-01 7:21 ` [PATCH v6 06/16] cpus: extract out kvm-specific code to accel/kvm Claudio Fontana
2020-09-01 7:21 ` [PATCH v6 07/16] cpus: extract out hax-specific code to target/i386/ Claudio Fontana
2020-09-01 7:21 ` [PATCH v6 08/16] cpus: extract out whpx-specific " Claudio Fontana
2020-09-01 7:21 ` [PATCH v6 09/16] cpus: extract out hvf-specific code to target/i386/hvf/ Claudio Fontana
2020-09-01 7:21 ` [PATCH v6 10/16] cpus: cleanup now unneeded includes Claudio Fontana
2020-09-01 11:08 ` Alex Bennée
2020-09-01 7:21 ` [PATCH v6 11/16] cpus: remove checks for non-NULL cpus_accel Claudio Fontana
2020-09-01 9:34 ` Roman Bolshakov
2020-09-01 9:44 ` Claudio Fontana
2020-09-01 17:40 ` Richard Henderson
2020-09-01 7:21 ` [PATCH v6 12/16] cpus: add handle_interrupt to the CpusAccel interface Claudio Fontana
2020-09-01 9:38 ` Roman Bolshakov
2020-09-01 9:46 ` Claudio Fontana
2020-09-01 17:41 ` Richard Henderson
2020-09-01 7:21 ` [PATCH v6 13/16] hvf: remove hvf specific functions from global includes Claudio Fontana
2020-09-01 9:30 ` Roman Bolshakov
2020-09-01 7:21 ` [PATCH v6 14/16] whpx: remove whpx " Claudio Fontana
2020-09-01 7:22 ` [PATCH v6 15/16] hax: remove hax " Claudio Fontana
2020-09-01 7:22 ` [PATCH v6 16/16] kvm: remove kvm " Claudio Fontana
2020-09-01 15:17 ` [PATCH v6 00/16] QEMU cpus.c refactoring part2 Alberto Garcia
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87k0xd6atv.fsf@linaro.org \
--to=alex.bennee@linaro.org \
--cc=armbru@redhat.com \
--cc=berto@igalia.com \
--cc=cfontana@suse.de \
--cc=colin.xu@intel.com \
--cc=dovgaluk@ispras.ru \
--cc=ehabkost@redhat.com \
--cc=haxm-team@intel.com \
--cc=lvivier@redhat.com \
--cc=mtosatti@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=philmd@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=r.bolshakov@yadro.com \
--cc=rth@twiddle.net \
--cc=sunilmut@microsoft.com \
--cc=thuth@redhat.com \
--cc=wenchao.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).