On 16.11.2013 0:15, Alexander Graf wrote:
>
>
> Am 15.11.2013 um 00:12 schrieb Alexey Kardashevskiy <a...@ozlabs.ru>:
>
>> 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 <a...@ozlabs.ru>
>> ---
>>
>> (!!!)
>>
>> 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.
Defaulting to 1 thread on P7 is a bad thing (other threads stay unused -
what is good about this?) and the only reason which I know why it is
still threads=1 is that it is hard to make a patch for upstream to
change this default.
Paul, please, help.
>
>
> 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();
>> }
--
With best regards
Alexey Kardashevskiy -- icq: 52150396
