Re: [Qemu-devel] [PATCH v1 5/7] numa: Extend the command-line to provide memory latency and bandwidth information

[Eric Blake <eblake@redhat.com>]

On 05/09/2018 03:36 AM, Liu Jingqi wrote:
> Add -numa hmat-lb option to provide System Locality Latency and
> Bandwidth Information. These memory attributes help to build
> System Locality Latency and Bandwidth Information Structure(s)
> in ACPI Heterogeneous Memory Attribute Table (HMAT).
> 
> Signed-off-by: Liu Jingqi <jingqi.liu@intel.com>
> ---

> +++ b/qapi/misc.json
> @@ -2705,7 +2705,7 @@
>   # Since: 2.1
>   ##
>   { 'enum': 'NumaOptionsType',
> -  'data': [ 'node', 'dist', 'cpu' ] }
> +  'data': [ 'node', 'dist', 'cpu', 'hmat-lb' ] }

Missing documentation for the new addition, particularly that it is 
'(since 2.13)'.


>   ##
> +# @NumaHmatLBOptions:
> +#
> +# Set the system locality latency and bandwidth information between Initiator and Target proximity Domains.

Long line here and elsewhere. Please keep this file wrapped below 80 
columns.

> +#
> +# @initiator: the Initiator Proximity Domain.
> +#
> +# @target: the Target Proximity Domain.
> +#
> +# @hierarchy: the Memory Hierarchy. Indicates the performance of memory or side cache.
> +#
> +# @level: indicates the level of side cache, if it's the performance of side cache.
> +#
> +# @data-type: presents the type of data, access/read/write latency or hit latency.
> +#
> +# @base-lat: the base unit for latency in nanosecondsbytes.

Is that supposed to be nanoseconds/bytes?

> +#
> +# @base-bw: the base unit for bandwidth in megabytes per second(MB/s).
> +#
> +# @latency: the value of latency based on Base Unit from @initiator to @target proximity domain.
> +#
> +# @bandwidth: the value of bandwidth based on Base Unit between @initiator to @target proximity domain.
> +#
> +# Since: 2.10

Not even close.  The soonest this could make it in is 2.13.

> +##
> +{ 'struct': 'NumaHmatLBOptions',
> +  'data': {
> +   'initiator': 'uint16',
> +   'target': 'uint16',
> +   'hierarchy': 'str',

Is this really a free-form string, or is it better as an enum type?

> +   '*level': 'uint8',
> +   'data-type': 'str',

Ditto.

> +   '*base-lat': 'uint64',
> +   '*base-bw': 'uint64',
> +   '*latency': 'uint16',
> +   '*bandwidth': 'uint16' }}
> +


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org