Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH] smp: autodetect numbers of threads per core

[Alexander Graf]

Am 15.11.2013 um 00:12 schrieb Alexey Kardashevskiy <address@hidden>:

> At the moment only a whole CPU core can be assigned to a KVM. Since
> POWER7/8 support several threads per core, we want all threads of a core
> to go to the same KVM so every time we run QEMU with -enable-kvm on
> POWER, we have to add -smp X,threads=(4|8)" (4 for POWER7 and
> 8 for POWER8).
> 
> This patch tries to read smp_threads number from an accelerator and
> falls back to the default value (1) if the accelerator did not care
> to change it.
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> 
> (!!!)
> 
> The usual question - what would be the normal way of doing this?
> What does this patch break? I cannot think of anything right now.

Is this really what the user wants? On p7 you can run in no-smt, smt2 and smt4 
mode. Today we simply default to no-smt. Changing defaults is usually a bad 
thing.


Alex

> 
> 
> ---
> target-ppc/kvm.c |  6 ++++++
> vl.c             | 29 ++++++++++++++++-------------
> 2 files changed, 22 insertions(+), 13 deletions(-)
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 10d0cd9..80c0386 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -109,6 +109,12 @@ int kvm_arch_init(KVMState *s)
>                         "VM to stall at times!\n");
>     }
> 
> +    if (!smp_threads) {
> +        smp_threads = cap_ppc_smt;
> +    } else {
> +        smp_threads = MIN(smp_threads, cap_ppc_smt);
> +    }
> +
>     kvm_ppc_register_host_cpu_type();
> 
>     return 0;
> diff --git a/vl.c b/vl.c
> index 4ad15b8..97fa203 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -210,7 +210,7 @@ int singlestep = 0;
> int smp_cpus = 1;
> int max_cpus = 0;
> int smp_cores = 1;
> -int smp_threads = 1;
> +int smp_threads = 0;
> #ifdef CONFIG_VNC
> const char *vnc_display;
> #endif
> @@ -1395,7 +1395,9 @@ static void smp_parse(QemuOpts *opts)
>         if (cpus == 0 || sockets == 0) {
>             sockets = sockets > 0 ? sockets : 1;
>             cores = cores > 0 ? cores : 1;
> -            threads = threads > 0 ? threads : 1;
> +            if (!threads) {
> +                threads = smp_threads > 0 ? smp_threads : 1;
> +            }
>             if (cpus == 0) {
>                 cpus = cores * threads * sockets;
>             }
> @@ -1413,7 +1415,8 @@ static void smp_parse(QemuOpts *opts)
>         smp_cpus = cpus;
>         smp_cores = cores > 0 ? cores : 1;
>         smp_threads = threads > 0 ? threads : 1;
> -
> +    } else if (!smp_threads) {
> +        smp_threads = 1;
>     }
> 
>     if (max_cpus == 0) {
> @@ -3880,16 +3883,6 @@ int main(int argc, char **argv, char **envp)
>         data_dir[data_dir_idx++] = CONFIG_QEMU_DATADIR;
>     }
> 
> -    smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
> -
> -    machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */
> -    if (smp_cpus > machine->max_cpus) {
> -        fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus 
> "
> -                "supported by machine `%s' (%d)\n", smp_cpus,  machine->name,
> -                machine->max_cpus);
> -        exit(1);
> -    }
> -
>     /*
>      * Get the default machine options from the machine if it is not already
>      * specified either by the configuration file or by the command line.
> @@ -4039,6 +4032,16 @@ int main(int argc, char **argv, char **envp)
> 
>     configure_accelerator();
> 
> +    smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
> +
> +    machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */
> +    if (smp_cpus > machine->max_cpus) {
> +        fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus 
> "
> +                "supported by machine `%s' (%d)\n", smp_cpus,  machine->name,
> +                machine->max_cpus);
> +        exit(1);
> +    }
> +
>     if (!qtest_enabled() && qtest_chrdev) {
>         qtest_init();
>     }
> -- 
> 1.8.4.rc4
> 
>