[PATCH 08/13] conf: Parse and format HMAT

[Daniel Henrique Barboza]

On 6/24/20 10:49 AM, Michal Privoznik wrote:
> To cite ACPI specification:
> 
>    Heterogeneous Memory Attribute Table describes the memory
>    attributes, such as memory side cache attributes and bandwidth
>    and latency details, related to the System Physical Address
>    (SPA) Memory Ranges. The software is expected to use this
>    information as hint for optimization.
> 
> According to our upstream discussion [1] this is exposed under
> <numa/> as <cache/> under NUMA <cell/> and <latency> or
> <bandwidth/> under numa/latencies.
> 
> 1: https://www.redhat.com/archives/libvir-list/2020-January/msg00422.html
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>   docs/formatdomain.html.in                  |  88 ++++++
>   docs/schemas/cputypes.rng                  | 110 ++++++-
>   src/conf/numa_conf.c                       | 350 ++++++++++++++++++++-
>   src/conf/numa_conf.h                       |  33 ++
>   src/libvirt_private.syms                   |   6 +
>   tests/qemuxml2argvdata/numatune-hmat.xml   |  52 +++
>   tests/qemuxml2xmloutdata/numatune-hmat.xml |   1 +
>   tests/qemuxml2xmltest.c                    |   1 +
>   8 files changed, 625 insertions(+), 16 deletions(-)
>   create mode 100644 tests/qemuxml2argvdata/numatune-hmat.xml
>   create mode 120000 tests/qemuxml2xmloutdata/numatune-hmat.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 07dcca57f5..78b2d2828d 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1931,6 +1931,94 @@
>         using 10 for local and 20 for remote distances.
>       </p>
>   
> +    <h4><a id="hmat">Heterogeneous Memory Attribute Table</a></h4>
> +
> +<pre>
> +...
> +<cpu>
> +  ...
> +  <numa>
> +    <cell id='0' cpus='0-3' memory='512000' unit='KiB' discard='yes'/>
> +    <cell id='1' cpus='4-7' memory='512000' unit='KiB' memAccess='shared'/>
> +    <cell id='3' cpus='0-3' memory='2097152' unit='KiB'>
> +      <cache level='1' associativity='direct' policy='writeback'>
> +        <size value='10' unit='KiB'/>
> +        <line value='8' unit='B'/>
> +      </cache>
> +    </cell>
> +    <latencies>
> +      <latency initiator='0' target='0' type='access' value='5'/>
> +      <latency initiator='0' target='0' cache='1' type='access' value='10'/>
> +      <bandwidth initiator='0' target='0' type='access' value='204800' unit='KiB'/>
> +    </latencies>
> +  </numa>
> +  ...
> +</cpu>
> +...</pre>
> +
> +    <p>
> +      <span class='since'>Since 6.5.0</span> the <code>cell</code> element can
> +      have <code>cache</code> child element which describes memory side cache

"have a <code>cache</code> child ...."

> +      for memory proximity domains. The <code>cache</code> element has

"<code>cache</code> element has a"

> +      <code>level</code> attribute describing the cache level and thus the
> +      element can be repeated multiple times to describe different levels of
> +      the cache.
> +    </p>
> +
> +    <p>
> +      The <code>cache</code> element then has <code>associativity</code>
> +      attribute (accepted values are <code>none</code>, <code>direct</code> and
> +      <code>full</code>) describing the cache associativity, and
> +      <code>policy</code> attribute (accepted values are <code>none</code>,
> +      <code>writeback</code> and <code>writethrough</code>) describing cache
> +      write associativity. The element has two mandatory child elements then:
> +      <code>size</code> and <code>line</code> which describe cache size and
> +      cache line size. Both elements accept two attributes: <code>value</code>
> +      and <code>unit</code> which set the value of corresponding cache
> +      attribute.
> +    </p>


This second paragraph is about the same <cache> element that you started to describe
in the previous paragraph. Might as well make a big paragraph talking just about the
cache element.

And there's a lot of different elements and attributes at different levels to unpack at
once here. I think you can use a description list (<dl>) here, like you did below with
the <latencies> element, or any other hierarchical HTML struct for that matter, to make
it easier to display all the info you're providing here.


> +
> +    <p>
> +      The NUMA description has optional <code>latencies</code> element that

"The NUMA description has an optional ..."


Everything else LGTM and these HTML changes are somewhat cosmetic, so:


Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>