From: Roman Bolshakov <r.bolshakov@yadro.com>
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>,
"Alex Bennée" <alex.bennee@linaro.org>,
haxm-team@intel.com, "Marcelo Tosatti" <mtosatti@redhat.com>,
qemu-devel@nongnu.org, "Markus Armbruster" <armbru@redhat.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 12/16] cpus: add handle_interrupt to the CpusAccel interface
Date: Tue, 1 Sep 2020 12:38:26 +0300 [thread overview]
Message-ID: <20200901093826.GD22344@SPB-NB-133.local> (raw)
In-Reply-To: <20200901072201.7133-13-cfontana@suse.de>
On Tue, Sep 01, 2020 at 09:21:57AM +0200, Claudio Fontana wrote:
> kvm: uses the generic handler
> qtest: uses the generic handler
> whpx: changed to use the generic handler (identical implementation)
> hax: changed to use the generic handler (identical implementation)
> hvf: changed to use the generic handler (identical implementation)
> tcg: adapt tcg-cpus to point to the tcg-specific handler
>
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
> accel/tcg/tcg-all.c | 26 --------------------------
> accel/tcg/tcg-cpus.c | 28 ++++++++++++++++++++++++++++
> hw/core/cpu.c | 13 -------------
> include/hw/core/cpu.h | 14 --------------
> include/sysemu/cpus.h | 2 ++
> softmmu/cpus.c | 18 ++++++++++++++++++
> target/i386/hax-all.c | 10 ----------
> target/i386/hvf/hvf.c | 9 ---------
> target/i386/whpx-all.c | 10 ----------
> 9 files changed, 48 insertions(+), 82 deletions(-)
>
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index 01957b130d..af9bf5c5bb 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -47,31 +47,6 @@ typedef struct TCGState {
> #define TCG_STATE(obj) \
> OBJECT_CHECK(TCGState, (obj), TYPE_TCG_ACCEL)
>
> -/* mask must never be zero, except for A20 change call */
> -static void tcg_handle_interrupt(CPUState *cpu, int mask)
> -{
> - int old_mask;
> - g_assert(qemu_mutex_iothread_locked());
> -
> - old_mask = cpu->interrupt_request;
> - cpu->interrupt_request |= mask;
> -
> - /*
> - * If called from iothread context, wake the target cpu in
> - * case its halted.
> - */
> - if (!qemu_cpu_is_self(cpu)) {
> - qemu_cpu_kick(cpu);
> - } else {
> - atomic_set(&cpu_neg(cpu)->icount_decr.u16.high, -1);
> - if (icount_enabled() &&
> - !cpu->can_do_io
> - && (mask & ~old_mask) != 0) {
> - cpu_abort(cpu, "Raised interrupt while not in I/O function");
> - }
> - }
> -}
> -
> /*
> * We default to false if we know other options have been enabled
> * which are currently incompatible with MTTCG. Otherwise when each
> @@ -128,7 +103,6 @@ static int tcg_init(MachineState *ms)
> TCGState *s = TCG_STATE(current_accel());
>
> tcg_exec_init(s->tb_size * 1024 * 1024);
> - cpu_interrupt_handler = tcg_handle_interrupt;
> mttcg_enabled = s->mttcg_enabled;
> cpus_register_accel(&tcg_cpus);
>
> diff --git a/accel/tcg/tcg-cpus.c b/accel/tcg/tcg-cpus.c
> index 72696f6d86..2bb209e2c6 100644
> --- a/accel/tcg/tcg-cpus.c
> +++ b/accel/tcg/tcg-cpus.c
> @@ -533,9 +533,37 @@ static int64_t tcg_get_elapsed_ticks(void)
> return cpu_get_ticks();
> }
>
> +/* mask must never be zero, except for A20 change call */
> +static void tcg_handle_interrupt(CPUState *cpu, int mask)
> +{
> + int old_mask;
> + g_assert(qemu_mutex_iothread_locked());
> +
> + old_mask = cpu->interrupt_request;
> + cpu->interrupt_request |= mask;
> +
> + /*
> + * If called from iothread context, wake the target cpu in
> + * case its halted.
> + */
> + if (!qemu_cpu_is_self(cpu)) {
> + qemu_cpu_kick(cpu);
> + } else {
> + atomic_set(&cpu_neg(cpu)->icount_decr.u16.high, -1);
> + if (icount_enabled() &&
> + !cpu->can_do_io
> + && (mask & ~old_mask) != 0) {
> + cpu_abort(cpu, "Raised interrupt while not in I/O function");
> + }
> + }
> +}
> +
> const CpusAccel tcg_cpus = {
> .create_vcpu_thread = tcg_start_vcpu_thread,
> .kick_vcpu_thread = tcg_kick_vcpu_thread,
> +
> + .handle_interrupt = tcg_handle_interrupt,
> +
> .get_virtual_clock = tcg_get_virtual_clock,
> .get_elapsed_ticks = tcg_get_elapsed_ticks,
> };
> diff --git a/hw/core/cpu.c b/hw/core/cpu.c
> index fa8602493b..451b3d5ee7 100644
> --- a/hw/core/cpu.c
> +++ b/hw/core/cpu.c
> @@ -35,8 +35,6 @@
> #include "qemu/plugin.h"
> #include "sysemu/hw_accel.h"
>
> -CPUInterruptHandler cpu_interrupt_handler;
> -
> CPUState *cpu_by_arch_id(int64_t id)
> {
> CPUState *cpu;
> @@ -394,17 +392,6 @@ static vaddr cpu_adjust_watchpoint_address(CPUState *cpu, vaddr addr, int len)
> return addr;
> }
>
> -static void generic_handle_interrupt(CPUState *cpu, int mask)
> -{
> - cpu->interrupt_request |= mask;
> -
> - if (!qemu_cpu_is_self(cpu)) {
> - qemu_cpu_kick(cpu);
> - }
> -}
> -
> -CPUInterruptHandler cpu_interrupt_handler = generic_handle_interrupt;
> -
> static void cpu_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 8f145733ce..efd33d87fd 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -838,12 +838,6 @@ bool cpu_exists(int64_t id);
> */
> CPUState *cpu_by_arch_id(int64_t id);
>
> -#ifndef CONFIG_USER_ONLY
> -
> -typedef void (*CPUInterruptHandler)(CPUState *, int);
> -
> -extern CPUInterruptHandler cpu_interrupt_handler;
> -
> /**
> * cpu_interrupt:
> * @cpu: The CPU to set an interrupt on.
> @@ -851,17 +845,9 @@ extern CPUInterruptHandler cpu_interrupt_handler;
> *
> * Invokes the interrupt handler.
> */
> -static inline void cpu_interrupt(CPUState *cpu, int mask)
> -{
> - cpu_interrupt_handler(cpu, mask);
> -}
> -
> -#else /* USER_ONLY */
>
> void cpu_interrupt(CPUState *cpu, int mask);
>
> -#endif /* USER_ONLY */
> -
> #ifdef NEED_CPU_H
>
> #ifdef CONFIG_SOFTMMU
> diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
> index 26171697f5..231685955d 100644
> --- a/include/sysemu/cpus.h
> +++ b/include/sysemu/cpus.h
> @@ -16,6 +16,8 @@ typedef struct CpusAccel {
> void (*synchronize_state)(CPUState *cpu);
> void (*synchronize_pre_loadvm)(CPUState *cpu);
>
> + void (*handle_interrupt)(CPUState *cpu, int mask);
> +
> int64_t (*get_virtual_clock)(void);
> int64_t (*get_elapsed_ticks)(void);
> } CpusAccel;
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index f32ecb4bb9..7068666579 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -225,6 +225,24 @@ int64_t cpus_get_elapsed_ticks(void)
> return cpu_get_ticks();
> }
>
> +static void generic_handle_interrupt(CPUState *cpu, int mask)
> +{
> + cpu->interrupt_request |= mask;
> +
> + if (!qemu_cpu_is_self(cpu)) {
> + qemu_cpu_kick(cpu);
> + }
> +}
> +
> +void cpu_interrupt(CPUState *cpu, int mask)
> +{
> + if (cpus_accel->handle_interrupt) {
> + cpus_accel->handle_interrupt(cpu, mask);
> + } else {
> + generic_handle_interrupt(cpu, mask);
> + }
> +}
> +
The handlers is not something that dynamically changes, the functions
can be assigned once during accel registration. Accel registraton is
also the place where the checks can take place.
Regards,
Roman
> static int do_vm_stop(RunState state, bool send_stop)
> {
> int ret = 0;
> diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
> index b66ddeb8bf..fd1ab673d7 100644
> --- a/target/i386/hax-all.c
> +++ b/target/i386/hax-all.c
> @@ -297,15 +297,6 @@ int hax_vm_destroy(struct hax_vm *vm)
> return 0;
> }
>
> -static void hax_handle_interrupt(CPUState *cpu, int mask)
> -{
> - cpu->interrupt_request |= mask;
> -
> - if (!qemu_cpu_is_self(cpu)) {
> - qemu_cpu_kick(cpu);
> - }
> -}
> -
> static int hax_init(ram_addr_t ram_size, int max_cpus)
> {
> struct hax_state *hax = NULL;
> @@ -350,7 +341,6 @@ static int hax_init(ram_addr_t ram_size, int max_cpus)
> qversion.cur_version = hax_cur_version;
> qversion.min_version = hax_min_version;
> hax_notify_qemu_version(hax->vm->fd, &qversion);
> - cpu_interrupt_handler = hax_handle_interrupt;
>
> return ret;
> error:
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index 7ac6987c1b..ed9356565c 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -262,14 +262,6 @@ static void update_apic_tpr(CPUState *cpu)
>
> #define VECTORING_INFO_VECTOR_MASK 0xff
>
> -static void hvf_handle_interrupt(CPUState * cpu, int mask)
> -{
> - cpu->interrupt_request |= mask;
> - if (!qemu_cpu_is_self(cpu)) {
> - qemu_cpu_kick(cpu);
> - }
> -}
> -
> void hvf_handle_io(CPUArchState *env, uint16_t port, void *buffer,
> int direction, int size, int count)
> {
> @@ -894,7 +886,6 @@ static int hvf_accel_init(MachineState *ms)
> }
>
> hvf_state = s;
> - cpu_interrupt_handler = hvf_handle_interrupt;
> memory_listener_register(&hvf_memory_listener, &address_space_memory);
> cpus_register_accel(&hvf_cpus);
> return 0;
> diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
> index 8b6986c864..b3d17fbe04 100644
> --- a/target/i386/whpx-all.c
> +++ b/target/i386/whpx-all.c
> @@ -1413,15 +1413,6 @@ static void whpx_memory_init(void)
> memory_listener_register(&whpx_memory_listener, &address_space_memory);
> }
>
> -static void whpx_handle_interrupt(CPUState *cpu, int mask)
> -{
> - cpu->interrupt_request |= mask;
> -
> - if (!qemu_cpu_is_self(cpu)) {
> - qemu_cpu_kick(cpu);
> - }
> -}
> -
> /*
> * Load the functions from the given library, using the given handle. If a
> * handle is provided, it is used, otherwise the library is opened. The
> @@ -1576,7 +1567,6 @@ static int whpx_accel_init(MachineState *ms)
>
> whpx_memory_init();
>
> - cpu_interrupt_handler = whpx_handle_interrupt;
> cpus_register_accel(&whpx_cpus);
>
> printf("Windows Hypervisor Platform accelerator is operational\n");
> --
> 2.26.2
>
next prev parent reply other threads:[~2020-09-01 9: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
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 [this message]
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=20200901093826.GD22344@SPB-NB-133.local \
--to=r.bolshakov@yadro.com \
--cc=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=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).