diff mbox series

[v3,9/9] s390x/tcg: fix CPU hotplug with single-threaded TCG

Message ID 20180625115352.6889-10-david@redhat.com
State New
Headers show
Series s390x: TOD refactoring + TCG CPU hotplug support | expand

Commit Message

David Hildenbrand June 25, 2018, 11:53 a.m. UTC
run_on_cpu() doesn't seem to work reliably until the CPU has been fully
created if the single-threaded TCG main loop is already running.

Therefore, hotplugging a CPU under single-threaded TCG does currently
not work. We should use the direct call instead of going via
run_on_cpu().

So let's use run_on_cpu() for KVM only - KVM requires it due to the initial
CPU reset ioctl. As a nice side effect, we get rif of the ifdef.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Comments

Cornelia Huck June 26, 2018, 12:33 p.m. UTC | #1
On Mon, 25 Jun 2018 13:53:52 +0200
David Hildenbrand <david@redhat.com> wrote:

> run_on_cpu() doesn't seem to work reliably until the CPU has been fully
> created if the single-threaded TCG main loop is already running.
> 
> Therefore, hotplugging a CPU under single-threaded TCG does currently
> not work. We should use the direct call instead of going via
> run_on_cpu().
> 
> So let's use run_on_cpu() for KVM only - KVM requires it due to the initial
> CPU reset ioctl. As a nice side effect, we get rif of the ifdef.

s/rif/rid/

{Although "we get rif the thing" would be a nice contraction of "rid of" :}

> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 40d6980229..271c5ce652 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -218,11 +218,18 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
>  #endif
>      s390_cpu_gdb_init(cs);
>      qemu_init_vcpu(cs);
> -#if !defined(CONFIG_USER_ONLY)
> -    run_on_cpu(cs, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
> -#else
> -    cpu_reset(cs);
> -#endif
> +
> +    /*
> +     * KVM requires the initial CPU reset ioctl to be executed on the target
> +     * CPU thread. CPU hotplug under single-threaded TCG will not work with
> +     * run_on_cpu(), as run_on_cpu() will not work properly if called while
> +     * the main thread is already running but the CPU hasn't been realized.
> +     */
> +    if (kvm_enabled()) {
> +        run_on_cpu(cs, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
> +    } else {
> +        cpu_reset(cs);
> +    }
>  
>      scc->parent_realize(dev, &err);
>  out:
Thomas Huth June 27, 2018, 11:19 a.m. UTC | #2
On 25.06.2018 13:53, David Hildenbrand wrote:
> run_on_cpu() doesn't seem to work reliably until the CPU has been fully
> created if the single-threaded TCG main loop is already running.
> 
> Therefore, hotplugging a CPU under single-threaded TCG does currently
> not work. We should use the direct call instead of going via
> run_on_cpu().
> 
> So let's use run_on_cpu() for KVM only - KVM requires it due to the initial
> CPU reset ioctl. As a nice side effect, we get rif of the ifdef.

With the typo fixed:

Reviewed-by: Thomas Huth <thuth@redhat.com>
diff mbox series

Patch

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 40d6980229..271c5ce652 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -218,11 +218,18 @@  static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
 #endif
     s390_cpu_gdb_init(cs);
     qemu_init_vcpu(cs);
-#if !defined(CONFIG_USER_ONLY)
-    run_on_cpu(cs, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
-#else
-    cpu_reset(cs);
-#endif
+
+    /*
+     * KVM requires the initial CPU reset ioctl to be executed on the target
+     * CPU thread. CPU hotplug under single-threaded TCG will not work with
+     * run_on_cpu(), as run_on_cpu() will not work properly if called while
+     * the main thread is already running but the CPU hasn't been realized.
+     */
+    if (kvm_enabled()) {
+        run_on_cpu(cs, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
+    } else {
+        cpu_reset(cs);
+    }
 
     scc->parent_realize(dev, &err);
 out: