qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
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 11/16] cpus: remove checks for non-NULL cpus_accel
Date: Tue, 1 Sep 2020 12:34:18 +0300	[thread overview]
Message-ID: <20200901093418.GC22344@SPB-NB-133.local> (raw)
In-Reply-To: <20200901072201.7133-12-cfontana@suse.de>

On Tue, Sep 01, 2020 at 09:21:56AM +0200, Claudio Fontana wrote:
> now that all accelerators support the CpusAccel interface,
> we can remove most checks for non-NULL cpus_accel,
> we just add a sanity check/assert at vcpu creation.
> 
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
>  softmmu/cpus.c | 33 +++++++++++++++++++++------------
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index 3d8350fba9..f32ecb4bb9 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -166,34 +166,46 @@ void cpu_synchronize_all_pre_loadvm(void)
>  
>  void cpu_synchronize_state(CPUState *cpu)
>  {
> -    if (cpus_accel && cpus_accel->synchronize_state) {
> +    if (cpus_accel->synchronize_state) {
>          cpus_accel->synchronize_state(cpu);
>      }
>  }
>  
>  void cpu_synchronize_post_reset(CPUState *cpu)
>  {
> -    if (cpus_accel && cpus_accel->synchronize_post_reset) {
> +    if (cpus_accel->synchronize_post_reset) {
>          cpus_accel->synchronize_post_reset(cpu);
>      }
>  }
>  
>  void cpu_synchronize_post_init(CPUState *cpu)
>  {
> -    if (cpus_accel && cpus_accel->synchronize_post_init) {
> +    if (cpus_accel->synchronize_post_init) {
>          cpus_accel->synchronize_post_init(cpu);
>      }
>  }
>  
>  void cpu_synchronize_pre_loadvm(CPUState *cpu)
>  {
> -    if (cpus_accel && cpus_accel->synchronize_pre_loadvm) {
> +    if (cpus_accel->synchronize_pre_loadvm) {
>          cpus_accel->synchronize_pre_loadvm(cpu);
>      }
>  }
>  
>  int64_t cpus_get_virtual_clock(void)
>  {
> +    /*
> +     * XXX
> +     *
> +     * need to check that cpus_accel is not NULL, because qcow2 calls
> +     * qemu_get_clock_ns(CLOCK_VIRTUAL) without any accel initialized and
> +     * with ticks disabled in some io-tests:
> +     * 030 040 041 060 099 120 127 140 156 161 172 181 191 192 195 203 229 249 256 267
> +     *
> +     * is this expected?
> +     *
> +     * XXX
> +     */
>      if (cpus_accel && cpus_accel->get_virtual_clock) {
>          return cpus_accel->get_virtual_clock();
>      }
> @@ -207,7 +219,7 @@ int64_t cpus_get_virtual_clock(void)
>   */
>  int64_t cpus_get_elapsed_ticks(void)
>  {
> -    if (cpus_accel && cpus_accel->get_elapsed_ticks) {
> +    if (cpus_accel->get_elapsed_ticks) {
>          return cpus_accel->get_elapsed_ticks();
>      }
>      return cpu_get_ticks();
> @@ -399,7 +411,7 @@ void cpus_kick_thread(CPUState *cpu)
>  void qemu_cpu_kick(CPUState *cpu)
>  {
>      qemu_cond_broadcast(cpu->halt_cond);
> -    if (cpus_accel && cpus_accel->kick_vcpu_thread) {
> +    if (cpus_accel->kick_vcpu_thread) {
>          cpus_accel->kick_vcpu_thread(cpu);
>      } else { /* default */
>          cpus_kick_thread(cpu);
> @@ -573,12 +585,9 @@ void qemu_init_vcpu(CPUState *cpu)
>          cpu_address_space_init(cpu, 0, "cpu-memory", cpu->memory);
>      }
>  
> -    if (cpus_accel) {
> -        /* accelerator already implements the CpusAccel interface */
> -        cpus_accel->create_vcpu_thread(cpu);
> -    } else {
> -        g_assert_not_reached();
> -    }
> +    /* accelerators all implement the CpusAccel interface */
> +    g_assert(cpus_accel != NULL && cpus_accel->create_vcpu_thread != NULL);
> +    cpus_accel->create_vcpu_thread(cpu);
>  
>      while (!cpu->created) {
>          qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
> -- 
> 2.26.2
> 

Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>

but I still find the condition (if cpus_accel->func) redundant, is it
feasible to drop it?

Regards,
Roman


  reply	other threads:[~2020-09-01  9:35 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 [this message]
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=20200901093418.GC22344@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).