On 12/25/2013 01:42 AM, Alexey Kardashevskiy wrote:
> On 12/17/2013 05:37 PM, Alexey Kardashevskiy wrote:
>> On 12/04/2013 04:34 PM, Alexey Kardashevskiy wrote:
>>> On 11/16/2013 03:58 AM, Alexey Kardashevskiy wrote:
>>>> 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.
>>>
>>>
>>> Anyone, ping? Thanks.
>>
>>
>> Everyone ignores me :(
>
>
> Ping?
Does it make any sense pinging the list about this patch? Is it too ugly?
>
>
>>
>>
>>>
>>>
>>>>> 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();
>>>>>> }
>>>>
>>>>
>>>
>>>
>>
>>
>
>
--
Alexey
