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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.