A pair of HMAT docs fixes

[PATCH v2 1/2] qemu-options.hx: Mark all hmat-cache attributes required

Posted by Michal Privoznik 5 years, 8 months ago

The documentation to `-numa hmat-cache` says that @node-id, @size
and @level are the only required attributes. The rest
(@associativity, @policy and @line) is optional. Well, not quite
- if I try to start QEMU with only the three required attributes
defined the QAPI code is complaining about associativity missing.

According to QAPI all attributes are required. Make the docs
reflect that.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 qemu-options.hx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 93bde2bbc8..b1a399079a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -188,7 +188,7 @@ DEF("numa", HAS_ARG, QEMU_OPTION_numa,
     "-numa dist,src=source,dst=destination,val=distance\n"
     "-numa cpu,node-id=node[,socket-id=x][,core-id=y][,thread-id=z]\n"
     "-numa hmat-lb,initiator=node,target=node,hierarchy=memory|first-level|second-level|third-level,data-type=access-latency|read-latency|write-latency[,latency=lat][,bandwidth=bw]\n"
-    "-numa hmat-cache,node-id=node,size=size,level=level[,associativity=none|direct|complex][,policy=none|write-back|write-through][,line=size]\n",
+    "-numa hmat-cache,node-id=node,size=size,level=level,associativity=none|direct|complex,policy=none|write-back|write-through,line=size\n",
     QEMU_ARCH_ALL)
 SRST
 ``-numa node[,mem=size][,cpus=firstcpu[-lastcpu]][,nodeid=node][,initiator=initiator]``
@@ -201,7 +201,7 @@ SRST
   \ 
 ``-numa hmat-lb,initiator=node,target=node,hierarchy=hierarchy,data-type=tpye[,latency=lat][,bandwidth=bw]``
   \ 
-``-numa hmat-cache,node-id=node,size=size,level=level[,associativity=str][,policy=str][,line=size]``
+``-numa hmat-cache,node-id=node,size=size,level=level,associativity=str,policy=str,line=size``
     Define a NUMA node and assign RAM and VCPUs to it. Set the NUMA
     distance from a source node to a destination node. Set the ACPI
     Heterogeneous Memory Attributes for the given nodes.
-- 
2.26.2

Re: [PATCH v2 1/2] qemu-options.hx: Mark all hmat-cache attributes required

Posted by Markus Armbruster 5 years, 7 months ago

Cc: the people involved in commit c412a48d4d "numa: Extend CLI to
provide memory side cache information".

Michal Privoznik <mprivozn@redhat.com> writes:

> The documentation to `-numa hmat-cache` says that @node-id, @size
> and @level are the only required attributes. The rest
> (@associativity, @policy and @line) is optional. Well, not quite
> - if I try to start QEMU with only the three required attributes
> defined the QAPI code is complaining about associativity missing.

Only because @associativity visited first.

> According to QAPI all attributes are required. Make the docs
> reflect that.

Correct.

> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  qemu-options.hx | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 93bde2bbc8..b1a399079a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -188,7 +188,7 @@ DEF("numa", HAS_ARG, QEMU_OPTION_numa,
>      "-numa dist,src=source,dst=destination,val=distance\n"
>      "-numa cpu,node-id=node[,socket-id=x][,core-id=y][,thread-id=z]\n"
>      "-numa hmat-lb,initiator=node,target=node,hierarchy=memory|first-level|second-level|third-level,data-type=access-latency|read-latency|write-latency[,latency=lat][,bandwidth=bw]\n"
> -    "-numa hmat-cache,node-id=node,size=size,level=level[,associativity=none|direct|complex][,policy=none|write-back|write-through][,line=size]\n",
> +    "-numa hmat-cache,node-id=node,size=size,level=level,associativity=none|direct|complex,policy=none|write-back|write-through,line=size\n",
>      QEMU_ARCH_ALL)
>  SRST
>  ``-numa node[,mem=size][,cpus=firstcpu[-lastcpu]][,nodeid=node][,initiator=initiator]``
> @@ -201,7 +201,7 @@ SRST
>    \ 
>  ``-numa hmat-lb,initiator=node,target=node,hierarchy=hierarchy,data-type=tpye[,latency=lat][,bandwidth=bw]``
>    \ 
> -``-numa hmat-cache,node-id=node,size=size,level=level[,associativity=str][,policy=str][,line=size]``
> +``-numa hmat-cache,node-id=node,size=size,level=level,associativity=str,policy=str,line=size``
>      Define a NUMA node and assign RAM and VCPUs to it. Set the NUMA
>      distance from a source node to a destination node. Set the ACPI
>      Heterogeneous Memory Attributes for the given nodes.

Assuming non-optional is what we want:
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v2 1/2] qemu-options.hx: Mark all hmat-cache attributes required

Posted by Michal Privoznik 5 years, 7 months ago

On 6/15/20 10:00 AM, Markus Armbruster wrote:
> Cc: the people involved in commit c412a48d4d "numa: Extend CLI to
> provide memory side cache information".
> 
> Michal Privoznik <mprivozn@redhat.com> writes:
> 
>> The documentation to `-numa hmat-cache` says that @node-id, @size
>> and @level are the only required attributes. The rest
>> (@associativity, @policy and @line) is optional. Well, not quite
>> - if I try to start QEMU with only the three required attributes
>> defined the QAPI code is complaining about associativity missing.
> 
> Only because @associativity visited first.
> 
>> According to QAPI all attributes are required. Make the docs
>> reflect that.
> 
> Correct.
> 
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>   qemu-options.hx | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>

> 
> Assuming non-optional is what we want:
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

Indeed, it is:

https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg08411.html

Thanks,
Michal

Re: [PATCH v2 1/2] qemu-options.hx: Mark all hmat-cache attributes required

Posted by Igor Mammedov 5 years, 7 months ago

On Wed, 10 Jun 2020 15:17:34 +0200
Michal Privoznik <mprivozn@redhat.com> wrote:

> The documentation to `-numa hmat-cache` says that @node-id, @size
> and @level are the only required attributes. The rest
> (@associativity, @policy and @line) is optional. Well, not quite
> - if I try to start QEMU with only the three required attributes
> defined the QAPI code is complaining about associativity missing.
> 
> According to QAPI all attributes are required. Make the docs
> reflect that.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  qemu-options.hx | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 93bde2bbc8..b1a399079a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -188,7 +188,7 @@ DEF("numa", HAS_ARG, QEMU_OPTION_numa,
>      "-numa dist,src=source,dst=destination,val=distance\n"
>      "-numa cpu,node-id=node[,socket-id=x][,core-id=y][,thread-id=z]\n"
>      "-numa hmat-lb,initiator=node,target=node,hierarchy=memory|first-level|second-level|third-level,data-type=access-latency|read-latency|write-latency[,latency=lat][,bandwidth=bw]\n"
> -    "-numa hmat-cache,node-id=node,size=size,level=level[,associativity=none|direct|complex][,policy=none|write-back|write-through][,line=size]\n",
> +    "-numa hmat-cache,node-id=node,size=size,level=level,associativity=none|direct|complex,policy=none|write-back|write-through,line=size\n",
>      QEMU_ARCH_ALL)
>  SRST
>  ``-numa node[,mem=size][,cpus=firstcpu[-lastcpu]][,nodeid=node][,initiator=initiator]``
> @@ -201,7 +201,7 @@ SRST
>    \ 
>  ``-numa hmat-lb,initiator=node,target=node,hierarchy=hierarchy,data-type=tpye[,latency=lat][,bandwidth=bw]``
>    \ 
> -``-numa hmat-cache,node-id=node,size=size,level=level[,associativity=str][,policy=str][,line=size]``
> +``-numa hmat-cache,node-id=node,size=size,level=level,associativity=str,policy=str,line=size``
>      Define a NUMA node and assign RAM and VCPUs to it. Set the NUMA
>      distance from a source node to a destination node. Set the ACPI
>      Heterogeneous Memory Attributes for the given nodes.