[PATCH 0/4] docs: improve -smp documentation

[PATCH 0/4] docs: improve -smp documentation

[Daniel P. Berrangé]

This is a spin off from this thread:

  https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg06135.html

to improve the documentation of the current behaviour of -smp
arg. In that thread it is suggested that we change the behaviour
to prefer use of cores over sockets, when topology is omitted. This
documentation update explicitly notes that default topology is liable
to change, to allow for this enhancement.

Daniel P. Berrangé (4):
  docs: fix typo s/Intel/AMD/ in CPU model notes
  qemu-options: re-arrange CPU topology options
  qemu-options: tweak to show that CPU count is optional
  qemu-options: rewrite help for -smp options

 docs/system/cpu-models-x86.rst.inc |  2 +-
 qemu-options.hx                    | 37 ++++++++++++++++++++----------
 2 files changed, 26 insertions(+), 13 deletions(-)

-- 
2.31.1

[PATCH 1/4] docs: fix typo s/Intel/AMD/ in CPU model notes

[Daniel P. Berrangé]

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/system/cpu-models-x86.rst.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/system/cpu-models-x86.rst.inc b/docs/system/cpu-models-x86.rst.inc
index f40ee03ecc..9119f5dff5 100644
--- a/docs/system/cpu-models-x86.rst.inc
+++ b/docs/system/cpu-models-x86.rst.inc
@@ -227,7 +227,7 @@ features are included if using "Host passthrough" or "Host model".
 Preferred CPU models for AMD x86 hosts
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-The following CPU models are preferred for use on Intel hosts.
+The following CPU models are preferred for use on AMD hosts.
 Administrators / applications are recommended to use the CPU model that
 matches the generation of the host CPUs in use. In a deployment with a
 mixture of host CPU models between machines, if live migration
-- 
2.31.1

Re: [PATCH 1/4] docs: fix typo s/Intel/AMD/ in CPU model notes

[wangyanan (Y)]

Hi Daniel,
On 2021/6/28 19:30, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   docs/system/cpu-models-x86.rst.inc | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/docs/system/cpu-models-x86.rst.inc b/docs/system/cpu-models-x86.rst.inc
> index f40ee03ecc..9119f5dff5 100644
> --- a/docs/system/cpu-models-x86.rst.inc
> +++ b/docs/system/cpu-models-x86.rst.inc
> @@ -227,7 +227,7 @@ features are included if using "Host passthrough" or "Host model".
>   Preferred CPU models for AMD x86 hosts
>   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   
> -The following CPU models are preferred for use on Intel hosts.
> +The following CPU models are preferred for use on AMD hosts.
>   Administrators / applications are recommended to use the CPU model that
>   matches the generation of the host CPUs in use. In a deployment with a
>   mixture of host CPU models between machines, if live migration
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,
Yanan
.

[PATCH 2/4] qemu-options: re-arrange CPU topology options

[Daniel P. Berrangé]

The list of CPU topology options are presented in a fairly arbitrary
order currently. Re-arrange them so that they're ordered from largest to
smallest unit

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qemu-options.hx | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index ba3ca9da1d..aa33dfdcfd 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -196,17 +196,17 @@ SRST
 ERST
 
 DEF("smp", HAS_ARG, QEMU_OPTION_smp,
-    "-smp [cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads][,dies=dies][,sockets=sockets]\n"
+    "-smp [cpus=]n[,maxcpus=cpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
     "                set the number of CPUs to 'n' [default=1]\n"
     "                maxcpus= maximum number of total cpus, including\n"
     "                offline CPUs for hotplug, etc\n"
+    "                sockets= number of discrete sockets in the system\n",
+    "                dies= number of CPU dies on one socket (for PC only)\n"
     "                cores= number of CPU cores on one socket (for PC, it's on one die)\n"
     "                threads= number of threads on one CPU core\n"
-    "                dies= number of CPU dies on one socket (for PC only)\n"
-    "                sockets= number of discrete sockets in the system\n",
         QEMU_ARCH_ALL)
 SRST
-``-smp [cpus=]n[,cores=cores][,threads=threads][,dies=dies][,sockets=sockets][,maxcpus=maxcpus]``
+``-smp [cpus=]n[,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]``
     Simulate an SMP system with n CPUs. On the PC target, up to 255 CPUs
     are supported. On Sparc32 target, Linux limits the number of usable
     CPUs to 4. For the PC target, the number of cores per die, the
-- 
2.31.1

Re: [PATCH 2/4] qemu-options: re-arrange CPU topology options

[wangyanan (Y)]

On 2021/6/28 19:30, Daniel P. Berrangé wrote:
> The list of CPU topology options are presented in a fairly arbitrary
> order currently. Re-arrange them so that they're ordered from largest to
> smallest unit
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   qemu-options.hx | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index ba3ca9da1d..aa33dfdcfd 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -196,17 +196,17 @@ SRST
>   ERST
>   
>   DEF("smp", HAS_ARG, QEMU_OPTION_smp,
> -    "-smp [cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads][,dies=dies][,sockets=sockets]\n"
> +    "-smp [cpus=]n[,maxcpus=cpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
>       "                set the number of CPUs to 'n' [default=1]\n"
>       "                maxcpus= maximum number of total cpus, including\n"
nit: There are always "CPUs" elsewhere, it's better to also use "total 
CPUs" here.
>       "                offline CPUs for hotplug, etc\n"
> +    "                sockets= number of discrete sockets in the system\n",
> +    "                dies= number of CPU dies on one socket (for PC only)\n"
>       "                cores= number of CPU cores on one socket (for PC, it's on one die)\n"
>       "                threads= number of threads on one CPU core\n"
> -    "                dies= number of CPU dies on one socket (for PC only)\n"
> -    "                sockets= number of discrete sockets in the system\n",
>           QEMU_ARCH_ALL)
>   SRST
> -``-smp [cpus=]n[,cores=cores][,threads=threads][,dies=dies][,sockets=sockets][,maxcpus=maxcpus]``
> +``-smp [cpus=]n[,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]``
>       Simulate an SMP system with n CPUs. On the PC target, up to 255 CPUs
>       are supported. On Sparc32 target, Linux limits the number of usable
>       CPUs to 4. For the PC target, the number of cores per die, the
This improves the readability a lot,

Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,
Yanan
.

Re: [PATCH 2/4] qemu-options: re-arrange CPU topology options

[Daniel P. Berrangé]

On Mon, Jun 28, 2021 at 12:30:45PM +0100, Daniel P. Berrangé wrote:
> The list of CPU topology options are presented in a fairly arbitrary
> order currently. Re-arrange them so that they're ordered from largest to
> smallest unit
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  qemu-options.hx | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index ba3ca9da1d..aa33dfdcfd 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -196,17 +196,17 @@ SRST
>  ERST
>  
>  DEF("smp", HAS_ARG, QEMU_OPTION_smp,
> -    "-smp [cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads][,dies=dies][,sockets=sockets]\n"
> +    "-smp [cpus=]n[,maxcpus=cpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
>      "                set the number of CPUs to 'n' [default=1]\n"
>      "                maxcpus= maximum number of total cpus, including\n"
>      "                offline CPUs for hotplug, etc\n"
> +    "                sockets= number of discrete sockets in the system\n",
> +    "                dies= number of CPU dies on one socket (for PC only)\n"
>      "                cores= number of CPU cores on one socket (for PC, it's on one die)\n"
>      "                threads= number of threads on one CPU core\n"
> -    "                dies= number of CPU dies on one socket (for PC only)\n"
> -    "                sockets= number of discrete sockets in the system\n",
>          QEMU_ARCH_ALL)

Stupid typo in this posting - didn't adjust the trailing ',' when moving
the lines.

>  SRST
> -``-smp [cpus=]n[,cores=cores][,threads=threads][,dies=dies][,sockets=sockets][,maxcpus=maxcpus]``
> +``-smp [cpus=]n[,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]``
>      Simulate an SMP system with n CPUs. On the PC target, up to 255 CPUs
>      are supported. On Sparc32 target, Linux limits the number of usable
>      CPUs to 4. For the PC target, the number of cores per die, the
> -- 
> 2.31.1
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 2/4] qemu-options: re-arrange CPU topology options

[wangyanan (Y)]

On 2021/7/12 18:10, Daniel P. Berrangé wrote:
> On Mon, Jun 28, 2021 at 12:30:45PM +0100, Daniel P. Berrangé wrote:
>> The list of CPU topology options are presented in a fairly arbitrary
>> order currently. Re-arrange them so that they're ordered from largest to
>> smallest unit
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>   qemu-options.hx | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index ba3ca9da1d..aa33dfdcfd 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -196,17 +196,17 @@ SRST
>>   ERST
>>   
>>   DEF("smp", HAS_ARG, QEMU_OPTION_smp,
>> -    "-smp [cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads][,dies=dies][,sockets=sockets]\n"
>> +    "-smp [cpus=]n[,maxcpus=cpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
>>       "                set the number of CPUs to 'n' [default=1]\n"
>>       "                maxcpus= maximum number of total cpus, including\n"
>>       "                offline CPUs for hotplug, etc\n"
>> +    "                sockets= number of discrete sockets in the system\n",
>> +    "                dies= number of CPU dies on one socket (for PC only)\n"
>>       "                cores= number of CPU cores on one socket (for PC, it's on one die)\n"
>>       "                threads= number of threads on one CPU core\n"
>> -    "                dies= number of CPU dies on one socket (for PC only)\n"
>> -    "                sockets= number of discrete sockets in the system\n",
>>           QEMU_ARCH_ALL)
> Stupid typo in this posting - didn't adjust the trailing ',' when moving
> the lines.
I can fix it incidentally in [1] if you wish. :)

[1] https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg00259.html

Thanks,
Yanan
.
>>   SRST
>> -``-smp [cpus=]n[,cores=cores][,threads=threads][,dies=dies][,sockets=sockets][,maxcpus=maxcpus]``
>> +``-smp [cpus=]n[,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]``
>>       Simulate an SMP system with n CPUs. On the PC target, up to 255 CPUs
>>       are supported. On Sparc32 target, Linux limits the number of usable
>>       CPUs to 4. For the PC target, the number of cores per die, the
>> -- 
>> 2.31.1
>>
> Regards,
> Daniel

Re: [PATCH 2/4] qemu-options: re-arrange CPU topology options

[Pankaj Gupta]

> The list of CPU topology options are presented in a fairly arbitrary
> order currently. Re-arrange them so that they're ordered from largest to
> smallest unit
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  qemu-options.hx | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index ba3ca9da1d..aa33dfdcfd 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -196,17 +196,17 @@ SRST
>  ERST
>
>  DEF("smp", HAS_ARG, QEMU_OPTION_smp,
> -    "-smp [cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads][,dies=dies][,sockets=sockets]\n"
> +    "-smp [cpus=]n[,maxcpus=cpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
>      "                set the number of CPUs to 'n' [default=1]\n"
>      "                maxcpus= maximum number of total cpus, including\n"
>      "                offline CPUs for hotplug, etc\n"
> +    "                sockets= number of discrete sockets in the system\n",
> +    "                dies= number of CPU dies on one socket (for PC only)\n"
>      "                cores= number of CPU cores on one socket (for PC, it's on one die)\n"
>      "                threads= number of threads on one CPU core\n"
> -    "                dies= number of CPU dies on one socket (for PC only)\n"
> -    "                sockets= number of discrete sockets in the system\n",
>          QEMU_ARCH_ALL)
>  SRST
> -``-smp [cpus=]n[,cores=cores][,threads=threads][,dies=dies][,sockets=sockets][,maxcpus=maxcpus]``
> +``-smp [cpus=]n[,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]``
>      Simulate an SMP system with n CPUs. On the PC target, up to 255 CPUs
>      are supported. On Sparc32 target, Linux limits the number of usable
>      CPUs to 4. For the PC target, the number of cores per die, the

Looks cleaner. With the nit mentioned by Dan:

Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>

[PATCH 3/4] qemu-options: tweak to show that CPU count is optional

[Daniel P. Berrangé]

The initial CPU count number is not required, if any of the topology
options are given, since it can be computed.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qemu-options.hx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index aa33dfdcfd..5871df7291 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -196,7 +196,7 @@ SRST
 ERST
 
 DEF("smp", HAS_ARG, QEMU_OPTION_smp,
-    "-smp [cpus=]n[,maxcpus=cpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
+    "-smp [[cpus=]n][,maxcpus=cpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
     "                set the number of CPUs to 'n' [default=1]\n"
     "                maxcpus= maximum number of total cpus, including\n"
     "                offline CPUs for hotplug, etc\n"
@@ -206,7 +206,7 @@ DEF("smp", HAS_ARG, QEMU_OPTION_smp,
     "                threads= number of threads on one CPU core\n"
         QEMU_ARCH_ALL)
 SRST
-``-smp [cpus=]n[,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]``
+``-smp [[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]``
     Simulate an SMP system with n CPUs. On the PC target, up to 255 CPUs
     are supported. On Sparc32 target, Linux limits the number of usable
     CPUs to 4. For the PC target, the number of cores per die, the
-- 
2.31.1

Re: [PATCH 3/4] qemu-options: tweak to show that CPU count is optional

[wangyanan (Y)]

On 2021/6/28 19:30, Daniel P. Berrangé wrote:
> The initial CPU count number is not required, if any of the topology
> options are given, since it can be computed.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   qemu-options.hx | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index aa33dfdcfd..5871df7291 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -196,7 +196,7 @@ SRST
>   ERST
>   
>   DEF("smp", HAS_ARG, QEMU_OPTION_smp,
> -    "-smp [cpus=]n[,maxcpus=cpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
> +    "-smp [[cpus=]n][,maxcpus=cpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]\n"
>       "                set the number of CPUs to 'n' [default=1]\n"
>       "                maxcpus= maximum number of total cpus, including\n"
>       "                offline CPUs for hotplug, etc\n"
> @@ -206,7 +206,7 @@ DEF("smp", HAS_ARG, QEMU_OPTION_smp,
>       "                threads= number of threads on one CPU core\n"
>           QEMU_ARCH_ALL)
>   SRST
> -``-smp [cpus=]n[,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]``
> +``-smp [[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]``
>       Simulate an SMP system with n CPUs. On the PC target, up to 255 CPUs
>       are supported. On Sparc32 target, Linux limits the number of usable
>       CPUs to 4. For the PC target, the number of cores per die, the
This looks correct to me:

Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,
Yanan
.

[PATCH 4/4] qemu-options: rewrite help for -smp options

[Daniel P. Berrangé]

The -smp option help is peculiarly specific about mentioning the CPU
upper limits, but these are wrong. The "PC" target has varying max
CPU counts depending on the machine type picked. Notes about guest
OS limits are inappropriate for QEMU docs. There are way too many
machine types for it to be practical to mention actual limits, and
some limits are even modified by downstream distribtions. Thus it
is better to remove the specific limits entirely.

The CPU topology reporting is also not neccessarily specific to the
PC platform and descriptions around the rules of usage are somewhat
terse. Expand this information with some examples to show effects
of defaulting.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qemu-options.hx | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 5871df7291..0021e9ec7b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -207,14 +207,27 @@ DEF("smp", HAS_ARG, QEMU_OPTION_smp,
         QEMU_ARCH_ALL)
 SRST
 ``-smp [[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]``
-    Simulate an SMP system with n CPUs. On the PC target, up to 255 CPUs
-    are supported. On Sparc32 target, Linux limits the number of usable
-    CPUs to 4. For the PC target, the number of cores per die, the
-    number of threads per cores, the number of dies per packages and the
-    total number of sockets can be specified. Missing values will be
-    computed. If any on the three values is given, the total number of
-    CPUs n can be omitted. maxcpus specifies the maximum number of
-    hotpluggable CPUs.
+    Simulate an SMP system with '\ ``n``\ ' CPUs initially present on
+    the machine type board. On boards supporting CPU hotplug, the optional
+    '\ ``maxcpus``\ ' parameter can be set to enable further CPUs to be
+    added at runtime. If omitted the maximum number of CPUs will be
+    set to match the initial CPU count. Both parameters are subject to
+    an upper limit that is determined by the specific machine type chosen.
+
+    To control reporting of CPU topology information, the number of sockets,
+    dies per socket, cores per die, and threads per core can be specified.
+    The sum `` sockets * cores * dies * threads `` must be equal to the
+    maximum CPU count. CPU targets may only support a subset of the topology
+    parameters. Where a CPU target does not support use of a particular
+    topology parameter, its value should be assumed to be 1 for the purpose
+    of computing the CPU maximum count.
+
+    Either the initial CPU count, or at least one of the topology parameters
+    must be specified. Values for any omitted parameters will be computed
+    from those which are given. Historically preference was given to the
+    coarsest topology parameters when computing missing values (ie sockets
+    preferred over cores, which were preferred over threads), however, this
+    behaviour is considered liable to change.
 ERST
 
 DEF("numa", HAS_ARG, QEMU_OPTION_numa,
-- 
2.31.1

Re: [PATCH 4/4] qemu-options: rewrite help for -smp options

[wangyanan (Y)]

Hi Daniel,

On 2021/6/28 19:30, Daniel P. Berrangé wrote:
> The -smp option help is peculiarly specific about mentioning the CPU
> upper limits, but these are wrong. The "PC" target has varying max
> CPU counts depending on the machine type picked. Notes about guest
> OS limits are inappropriate for QEMU docs. There are way too many
> machine types for it to be practical to mention actual limits, and
> some limits are even modified by downstream distribtions. Thus it
> is better to remove the specific limits entirely.
>
> The CPU topology reporting is also not neccessarily specific to the
> PC platform and descriptions around the rules of usage are somewhat
> terse. Expand this information with some examples to show effects
> of defaulting.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   qemu-options.hx | 29 +++++++++++++++++++++--------
>   1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 5871df7291..0021e9ec7b 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -207,14 +207,27 @@ DEF("smp", HAS_ARG, QEMU_OPTION_smp,
>           QEMU_ARCH_ALL)
>   SRST
>   ``-smp [[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]``
> -    Simulate an SMP system with n CPUs. On the PC target, up to 255 CPUs
> -    are supported. On Sparc32 target, Linux limits the number of usable
> -    CPUs to 4. For the PC target, the number of cores per die, the
> -    number of threads per cores, the number of dies per packages and the
> -    total number of sockets can be specified. Missing values will be
> -    computed. If any on the three values is given, the total number of
> -    CPUs n can be omitted. maxcpus specifies the maximum number of
> -    hotpluggable CPUs.
> +    Simulate an SMP system with '\ ``n``\ ' CPUs initially present on
Should be "a SMP system".
> +    the machine type board. On boards supporting CPU hotplug, the optional
> +    '\ ``maxcpus``\ ' parameter can be set to enable further CPUs to be
> +    added at runtime. If omitted the maximum number of CPUs will be
> +    set to match the initial CPU count. Both parameters are subject to
> +    an upper limit that is determined by the specific machine type chosen.
> +
> +    To control reporting of CPU topology information, the number of sockets,
> +    dies per socket, cores per die, and threads per core can be specified.
> +    The sum `` sockets * cores * dies * threads `` must be equal to the
> +    maximum CPU count. CPU targets may only support a subset of the topology
> +    parameters. Where a CPU target does not support use of a particular
> +    topology parameter, its value should be assumed to be 1 for the purpose
> +    of computing the CPU maximum count.
> +
Explicitly saying "sockets * dies * cores * threads" seems not 
arch-neutral at
first glance, although we have the explanation behind. How about the
following statement for this paragraph?

"
To control reporting of CPU topology information, at most the number of 
sockets,
dies per socket, cores per die, and threads per core can be specified. 
CPU targets
may only support a subset of the topology parameters. If a CPU target 
does not
support use of a particular topology parameter, it must not be 
specified. The sum
of the supported subset of parameters must be equal to the maximum CPU 
count.
"

I think this also make it easier to expand if we are going to add one 
more topology
parameter, e.g, cluster, in the future.
> +    Either the initial CPU count, or at least one of the topology parameters
> +    must be specified. Values for any omitted parameters will be computed
> +    from those which are given. Historically preference was given to the
> +    coarsest topology parameters when computing missing values (ie sockets
> +    preferred over cores, which were preferred over threads), however, this
> +    behaviour is considered liable to change.
>   ERST
>   
>   DEF("numa", HAS_ARG, QEMU_OPTION_numa,
Thanks,
Yanan
.

Re: [PATCH 4/4] qemu-options: rewrite help for -smp options

[Daniel P. Berrangé]

On Mon, Jun 28, 2021 at 09:46:16PM +0800, wangyanan (Y) wrote:
> Hi Daniel,
> 
> On 2021/6/28 19:30, Daniel P. Berrangé wrote:
> > The -smp option help is peculiarly specific about mentioning the CPU
> > upper limits, but these are wrong. The "PC" target has varying max
> > CPU counts depending on the machine type picked. Notes about guest
> > OS limits are inappropriate for QEMU docs. There are way too many
> > machine types for it to be practical to mention actual limits, and
> > some limits are even modified by downstream distribtions. Thus it
> > is better to remove the specific limits entirely.
> > 
> > The CPU topology reporting is also not neccessarily specific to the
> > PC platform and descriptions around the rules of usage are somewhat
> > terse. Expand this information with some examples to show effects
> > of defaulting.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   qemu-options.hx | 29 +++++++++++++++++++++--------
> >   1 file changed, 21 insertions(+), 8 deletions(-)
> > 
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 5871df7291..0021e9ec7b 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -207,14 +207,27 @@ DEF("smp", HAS_ARG, QEMU_OPTION_smp,
> >           QEMU_ARCH_ALL)
> >   SRST
> >   ``-smp [[cpus=]n][,maxcpus=maxcpus][,sockets=sockets][,dies=dies][,cores=cores][,threads=threads]``
> > -    Simulate an SMP system with n CPUs. On the PC target, up to 255 CPUs
> > -    are supported. On Sparc32 target, Linux limits the number of usable
> > -    CPUs to 4. For the PC target, the number of cores per die, the
> > -    number of threads per cores, the number of dies per packages and the
> > -    total number of sockets can be specified. Missing values will be
> > -    computed. If any on the three values is given, the total number of
> > -    CPUs n can be omitted. maxcpus specifies the maximum number of
> > -    hotpluggable CPUs.
> > +    Simulate an SMP system with '\ ``n``\ ' CPUs initially present on
> Should be "a SMP system".

Pre-existing bug, but I'll fix it anyway


> > +    the machine type board. On boards supporting CPU hotplug, the optional
> > +    '\ ``maxcpus``\ ' parameter can be set to enable further CPUs to be
> > +    added at runtime. If omitted the maximum number of CPUs will be
> > +    set to match the initial CPU count. Both parameters are subject to
> > +    an upper limit that is determined by the specific machine type chosen.
> > +
> > +    To control reporting of CPU topology information, the number of sockets,
> > +    dies per socket, cores per die, and threads per core can be specified.
> > +    The sum `` sockets * cores * dies * threads `` must be equal to the
> > +    maximum CPU count. CPU targets may only support a subset of the topology
> > +    parameters. Where a CPU target does not support use of a particular
> > +    topology parameter, its value should be assumed to be 1 for the purpose
> > +    of computing the CPU maximum count.
> > +
> Explicitly saying "sockets * dies * cores * threads" seems not arch-neutral
> at
> first glance, although we have the explanation behind. How about the
> following statement for this paragraph?
> 
> "
> To control reporting of CPU topology information, at most the number of
> sockets,
> dies per socket, cores per die, and threads per core can be specified. CPU
> targets
> may only support a subset of the topology parameters. If a CPU target does
> not
> support use of a particular topology parameter, it must not be specified.
> The sum
> of the supported subset of parameters must be equal to the maximum CPU
> count.
> "
> 
> I think this also make it easier to expand if we are going to add one more
> topology
> parameter, e.g, cluster, in the future.

I won't make this suggested change, since we discussed against another
patch that mgmt apps like libvirt will already be setting 'dies=1' for
any target. We merely need QEMU to reject values > 1 if not supported.

> > +    Either the initial CPU count, or at least one of the topology parameters
> > +    must be specified. Values for any omitted parameters will be computed
> > +    from those which are given. Historically preference was given to the
> > +    coarsest topology parameters when computing missing values (ie sockets
> > +    preferred over cores, which were preferred over threads), however, this
> > +    behaviour is considered liable to change.
> >   ERST
> >   DEF("numa", HAS_ARG, QEMU_OPTION_numa,
> Thanks,
> Yanan
> .
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 0/4] docs: improve -smp documentation

[Andrew Jones]

On Mon, Jun 28, 2021 at 12:30:43PM +0100, Daniel P. Berrangé wrote:
> This is a spin off from this thread:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg06135.html
> 
> to improve the documentation of the current behaviour of -smp
> arg. In that thread it is suggested that we change the behaviour
> to prefer use of cores over sockets, when topology is omitted. This
> documentation update explicitly notes that default topology is liable
> to change, to allow for this enhancement.
> 
> Daniel P. Berrangé (4):
>   docs: fix typo s/Intel/AMD/ in CPU model notes
>   qemu-options: re-arrange CPU topology options
>   qemu-options: tweak to show that CPU count is optional
>   qemu-options: rewrite help for -smp options
> 
>  docs/system/cpu-models-x86.rst.inc |  2 +-
>  qemu-options.hx                    | 37 ++++++++++++++++++++----------
>  2 files changed, 26 insertions(+), 13 deletions(-)
> 
> -- 
> 2.31.1
> 
>

For the series

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [PATCH 0/4] docs: improve -smp documentation

[Eduardo Habkost]

On Mon, Jun 28, 2021 at 12:30:43PM +0100, Daniel P. Berrangé wrote:
> This is a spin off from this thread:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg06135.html
> 
> to improve the documentation of the current behaviour of -smp
> arg. In that thread it is suggested that we change the behaviour
> to prefer use of cores over sockets, when topology is omitted. This
> documentation update explicitly notes that default topology is liable
> to change, to allow for this enhancement.

Queued, thanks!

-- 
Eduardo

Re: [PATCH 0/4] docs: improve -smp documentation

[Eduardo Habkost]

On Fri, Jul 09, 2021 at 04:48:57PM -0400, Eduardo Habkost wrote:
> On Mon, Jun 28, 2021 at 12:30:43PM +0100, Daniel P. Berrangé wrote:
> > This is a spin off from this thread:
> > 
> >   https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg06135.html
> > 
> > to improve the documentation of the current behaviour of -smp
> > arg. In that thread it is suggested that we change the behaviour
> > to prefer use of cores over sockets, when topology is omitted. This
> > documentation update explicitly notes that default topology is liable
> > to change, to allow for this enhancement.
> 
> Queued, thanks!

I had to remove it due to build failures:
https://gitlab.com/ehabkost/qemu/-/jobs/1413791527#L1139


FAILED: libqemu-microblazeel-softmmu.fa.p/softmmu_vl.c.o
cc -Ilibqemu-microblazeel-softmmu.fa.p -I. -I.. -Itarget/microblaze -I../target/microblaze -I../capstone/include/capstone -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -fdiagnostics-color=auto -pipe -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /builds/ehabkost/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/ehabkost/qemu -iquote /builds/ehabkost/qemu/include -iquote /builds/ehabkost/qemu/disas/libvixl -iquote /builds/ehabkost/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -m32 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -m32 -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIC -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="microblazeel-softmmu-config-target.h"' '-DCONFIG_DEVICES="microblazeel-softmmu-config-devices.h"' -MD -MQ libqemu-microblazeel-softmmu.fa.p/softmmu_vl.c.o -MF libqemu-microblazeel-softmmu.fa.p/softmmu_vl.c.o.d -o libqemu-microblazeel-softmmu.fa.p/softmmu_vl.c.o -c ../softmmu/vl.c
./qemu-options.def: In function 'help':
./qemu-options.def:52:1: error: expected ')' before 'QEMU_ARCH_ALL'
   52 | QEMU_ARCH_ALL)
      | ^~~~~~~~~~~~~
../softmmu/vl.c:809:10: note: in definition of macro 'DEF'
  809 |     if ((arch_mask) & arch_type)                               \
      |          ^~~~~~~~~
../softmmu/vl.c:809:9: note: to match this '('
  809 |     if ((arch_mask) & arch_type)                               \
      |         ^
./qemu-options.def:43:1: note: in expansion of macro 'DEF'
   43 | DEF("smp", HAS_ARG, QEMU_OPTION_smp,
      | ^~~
../softmmu/vl.c:809:21: error: invalid operands to binary & (have 'const char *' and 'uint32_t' {aka 'const unsigned int'})
  809 |     if ((arch_mask) & arch_type)                               \
      |         ~           ^
      |         |
      |         const char *
./qemu-options.def:43:1: note: in expansion of macro 'DEF'
   43 | DEF("smp", HAS_ARG, QEMU_OPTION_smp,
      | ^~~
./qemu-options.def: At top level:
./qemu-options.def:49:1: error: initialization of 'unsigned int' from 'const char *' makes integer from pointer without a cast [-Werror=int-conversion]
   49 | "                dies= number of CPU dies on one socket (for PC only)\n"
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../softmmu/vl.c:845:34: note: in definition of macro 'DEF'
  845 |     { option, opt_arg, opt_enum, arch_mask },
      |                                  ^~~~~~~~~
./qemu-options.def:49:1: note: (near initialization for 'qemu_options[7].arch_mask')
   49 | "                dies= number of CPU dies on one socket (for PC only)\n"
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../softmmu/vl.c:845:34: note: in definition of macro 'DEF'
  845 |     { option, opt_arg, opt_enum, arch_mask },
      |                                  ^~~~~~~~~
./qemu-options.def:52:1: error: expected '}' before 'QEMU_ARCH_ALL'
   52 | QEMU_ARCH_ALL)
      | ^~~~~~~~~~~~~
../softmmu/vl.c:845:34: note: in definition of macro 'DEF'
  845 |     { option, opt_arg, opt_enum, arch_mask },
      |                                  ^~~~~~~~~
../softmmu/vl.c:845:5: note: to match this '{'
  845 |     { option, opt_arg, opt_enum, arch_mask },
      |     ^
./qemu-options.def:43:1: note: in expansion of macro 'DEF'
   43 | DEF("smp", HAS_ARG, QEMU_OPTION_smp,
      | ^~~
cc1: all warnings being treated as errors
[663/5259] Compiling C object libqemu-microblazeel-softmmu.fa.p/accel_tcg_tcg-accel-ops-icount.c.o
ninja: build stopped: subcommand failed.
make: *** [Makefile:154: run-ninja] Error 1

-- 
Eduardo