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

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

[Alexey Kardashevskiy]

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 <aik@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.


---
 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

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 <aik@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 <aik@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.


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
> 
> 

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

[Alexey Kardashevskiy]

On 16.11.2013 0:15, Alexander Graf wrote:
> 
> 
> Am 15.11.2013 um 00:12 schrieb Alexey Kardashevskiy <aik@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 <aik@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

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

[Alexey Kardashevskiy]

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 <aik@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 <aik@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.


>> 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

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

[Alexey Kardashevskiy]

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 <aik@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 <aik@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 :(


> 
> 
>>> 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

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

[Alexey Kardashevskiy]

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 <aik@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 <aik@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?


> 
> 
>>
>>
>>>> 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

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

[Alexey Kardashevskiy]

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 <aik@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 <aik@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