[libvirt] [PATCH 03/10] conf: Add CMT capability to host

[John Ferlan]

On 08/27/2018 07:23 AM, Wang Huaqiang wrote:
> CMT capability for each cache bank, includes
> -. Maximum CMT monitoring groups(sharing with MBM) could be created,
>    which reflects the maximum hardware RMID count.
> -. 'cache threshold'.
> -. Statistical information of last level cache, the actual cache
>    occupancy.
> 
> cache is splitted into 'bank's, each bank MAY have different cache
> configuration, report cache monitoring capability in unit of cache bank.
> 
> cache monitor capability is shown as below:
> 
>     <cache>
>       <bank id='0' level='3' type='both' size='15' unit='MiB' cpus='0-5'>
>         <control granularity='768' unit='KiB' type='code' maxAllocs='8'/>
>         <control granularity='768' unit='KiB' type='data' maxAllocs='8'/>
> +       <monitor threshold='540672' unit='B' maxAllocs='176'/>
> +         <feature name=llc_occupancy/>
> +       </monitor>
>       </bank>
>       <bank id='1' level='3' type='both' size='15' unit='MiB' cpus='6-11'>
>         <control granularity='768' unit='KiB' type='code' maxAllocs='8'/>
>         <control granularity='768' unit='KiB' type='data' maxAllocs='8'/>
> +       <monitor threshold='540672' unit='B' maxAllocs='176'/>
> +         <feature name=llc_occupancy/>
> +       </monitor>
> 
>       </bank>
>     </cache>
> 
> Signed-off-by: Wang Huaqiang <huaqiang.wang at intel.com>
> ---
>  docs/schemas/capability.rng | 28 ++++++++++++++++++++++++++++
>  src/conf/capabilities.c     | 17 +++++++++++++++++
>  2 files changed, 45 insertions(+)
> 

This output would be combined with part of existing patch2.

> diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
> index d61515c..67498f1 100644
> --- a/docs/schemas/capability.rng
> +++ b/docs/schemas/capability.rng
> @@ -314,6 +314,24 @@
>                </attribute>
>              </element>
>            </zeroOrMore>
> +          <zeroOrMore>
> +            <element name='monitor'>
> +              <attribute name='threshold'>
> +                <ref name='unsignedInt'/>
> +              </attribute>
> +              <attribute name='unit'>
> +                <ref name='unit'/>
> +              </attribute>
> +              <attribute name='maxAllocs'>
> +                <ref name='unsignedInt'/>
> +              </attribute>
> +              <zeroOrMore>
> +                <element name='feature'>
> +                  <ref name='monitorFeature'/>
> +                </element>
> +              </zeroOrMore>
> +            </element>
> +          </zeroOrMore>
>          </element>
>        </oneOrMore>
>      </element>
> @@ -329,6 +347,16 @@
>      </attribute>
>    </define>
>  
> +  <define name='monitorFeature'>
> +    <attribute name='name'>
> +      <choice>
> +        <value>llc_occupancy</value>
> +        <value>mbm_total_bytes</value>
> +        <value>mbm_local_bytes</value>

So these are the only 3 values you'll ever expect?  Probably not a good
idea to list them like this or the current algorithm is overkill looking
for prefixed "llc_" and "mbm_" values.

If "llc_somethingnew" shows up some day, then the schema is invalidated.

If all you're supporting or care about is the 3 values, then each should
be fetched separately.  Just the names make me wonder if they come with
some associated value that would be in some file.  Perhaps answered in
later patches.
> +      </choice>
> +    </attribute>
> +  </define>
> +
>    <define name='memory_bandwidth'>
>      <element name='memory_bandwidth'>
>        <oneOrMore>
> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 5280348..7932088 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -942,6 +942,23 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
>                                controls->max_allocation);
>          }
>  
> +        if (bank->monitor &&
> +            bank->monitor->nfeatures) {
> +            virBufferAsprintf(&childrenBuf,
> +                              "<monitor threshold='%u' unit='B' "

Why is "unit='B' " - does it really matter and is it technically right?
If it's only ever going to be 'B', then easy enough to document that way.

> +                              "maxAllocs='%u'>\n",
> +                              bank->monitor->cache_threshold,
> +                              bank->monitor->max_allocation);
> +            for (j = 0; j < bank->monitor->nfeatures; j++) {
> +                virBufferAdjustIndent(&childrenBuf, 2);
> +                virBufferAsprintf(&childrenBuf,
> +                                  "<feature name='%s'/>\n",
> +                                  bank->monitor->features[j]);
> +                virBufferAdjustIndent(&childrenBuf, -2);
> +            }
> +            virBufferAddLit(&childrenBuf, "</monitor>\n");
> +        }
> +

Not clear this data is at the right level, still.

John

>          if (virBufferCheckError(&childrenBuf) < 0)
>              return -1;
>  
>