[v15,10/12] hmat acpi: Build System Locality Latency and Bandwidth Information Structure(s)

From: Liu Jingqi <jingqi.liu@intel.com>

This structure describes the memory access latency and bandwidth
information from various memory access initiator proximity domains.
The latency and bandwidth numbers represented in this structure
correspond to rated latency and bandwidth for the platform.
The software could use this information as hint for optimization.

Signed-off-by: Liu Jingqi <jingqi.liu@intel.com>
Signed-off-by: Tao Xu <tao3.xu@intel.com>
---

No changes in v15.

Changes in v13:
    - Calculate the entries in a new patch.
---
 hw/acpi/hmat.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 95 insertions(+), 1 deletion(-)

On Thu,  7 Nov 2019 15:45:09 +0800
Tao Xu <tao3.xu@intel.com> wrote:

> From: Liu Jingqi <jingqi.liu@intel.com>
> 
> This structure describes the memory access latency and bandwidth
> information from various memory access initiator proximity domains.
> The latency and bandwidth numbers represented in this structure
> correspond to rated latency and bandwidth for the platform.
> The software could use this information as hint for optimization.
> 
> Signed-off-by: Liu Jingqi <jingqi.liu@intel.com>
> Signed-off-by: Tao Xu <tao3.xu@intel.com>
> ---
> 
> No changes in v15.
> 
> Changes in v13:
>     - Calculate the entries in a new patch.
> ---
>  hw/acpi/hmat.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 95 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/hmat.c b/hw/acpi/hmat.c
> index c595098ba7..6ec1310e62 100644
> --- a/hw/acpi/hmat.c
> +++ b/hw/acpi/hmat.c
> @@ -27,6 +27,7 @@
>  #include "qemu/osdep.h"
>  #include "sysemu/numa.h"
>  #include "hw/acpi/hmat.h"
> +#include "qemu/error-report.h"
>  
>  /*
>   * ACPI 6.3:
> @@ -67,11 +68,81 @@ static void build_hmat_mpda(GArray *table_data, uint16_t flags,
>      build_append_int_noprefix(table_data, 0, 8);
>  }
>  
> +/*
> + * ACPI 6.3: 5.2.27.4 System Locality Latency and Bandwidth Information
> + * Structure: Table 5-146
> + */
> +static void build_hmat_lb(GArray *table_data, HMAT_LB_Info *hmat_lb,
> +                          uint32_t num_initiator, uint32_t num_target,
> +                          uint32_t *initiator_list)
> +{
> +    int i;
> +    uint16_t *lb_data;
> +    uint32_t base;
> +    /*
> +     * Length in bytes for entire structure, including 32 bytes of
> +     * fixed length, length of initiator proximity domain list,
> +     * length of target proximity domain list and length of entries
> +     * provides latency/bandwidth values.
> +     */
> +    uint32_t lb_length = 32 + 4 * num_initiator + 4 * num_target +
> +                              2 * num_initiator * num_target;
might be described better using field names from spec, as:

  uint32_t len = 32 /* table length upto and including Entry Base Unit */
                 + 4 * num_initiator /* Initiator Proximity Domain List */
                 + 4 * num_target /* Target Proximity Domain */
                 ....

> +
> +    /* Type */
> +    build_append_int_noprefix(table_data, 1, 2);
> +    /* Reserved */
> +    build_append_int_noprefix(table_data, 0, 2);
> +    /* Length */
> +    build_append_int_noprefix(table_data, lb_length, 4);
> +    /* Flags: Bits [3:0] Memory Hierarchy, Bits[7:4] Reserved */
> +    assert(!(hmat_lb->hierarchy >> 4));
> +    build_append_int_noprefix(table_data, hmat_lb->hierarchy, 1);
> +    /* Data Type */
> +    build_append_int_noprefix(table_data, hmat_lb->data_type, 1);
> +    /* Reserved */
> +    build_append_int_noprefix(table_data, 0, 2);
> +    /* Number of Initiator Proximity Domains (s) */
> +    build_append_int_noprefix(table_data, num_initiator, 4);
> +    /* Number of Target Proximity Domains (t) */
> +    build_append_int_noprefix(table_data, num_target, 4);
> +    /* Reserved */
> +    build_append_int_noprefix(table_data, 0, 4);
> +

> +    if (hmat_lb->data_type <= HMAT_LB_DATA_WRITE_LATENCY) {
> +        base = hmat_lb->base_latency;
> +        lb_data = hmat_lb->entry_latency;
> +    } else {
> +        base = hmat_lb->base_bandwidth;
> +        lb_data = hmat_lb->entry_bandwidth;
> +    }
I'd move this out into caller hmat_build_table_structs() and instead of
hmat_lb pass as arguments data_type, base and data, to keep subtable
API independent of internal NUMA structures.

On this note, considering that a single hmat_lb can represent either
latency or bandwidth only, I'd suggest to unify
   hmat_lb->base_latency/hmat_lb->base_bandwidth
     into hmat_lb->base
   
and do the same for data entries and use hmat_lb->data_type as discriminator.

That should simplify code here and in CLI parsing part as well,
I don't see any reason to keep them separate. 

> +
> +    /* Entry Base Unit */
> +    build_append_int_noprefix(table_data, base, 8);
> +
> +    /* Initiator Proximity Domain List */
> +    for (i = 0; i < num_initiator; i++) {
> +        build_append_int_noprefix(table_data, initiator_list[i], 4);
> +    }
> +
> +    /* Target Proximity Domain List */
> +    for (i = 0; i < num_target; i++) {
> +        build_append_int_noprefix(table_data, i, 4);
> +    }
> +
> +    /* Latency or Bandwidth Entries */
> +    for (i = 0; i < num_initiator * num_target; i++) {
> +        build_append_int_noprefix(table_data, lb_data[i], 2);
> +    }
> +}
> +
>  /* Build HMAT sub table structures */
>  static void hmat_build_table_structs(GArray *table_data, NumaState *numa_state)
>  {
>      uint16_t flags;
> -    int i;
> +    uint32_t num_initiator = 0;
> +    uint32_t initiator_list[MAX_NODES];
> +    int i, hierarchy, type;
> +    HMAT_LB_Info *hmat_lb;
>  
>      for (i = 0; i < numa_state->num_nodes; i++) {
>          flags = 0;
> @@ -82,6 +153,29 @@ static void hmat_build_table_structs(GArray *table_data, NumaState *numa_state)
>  
>          build_hmat_mpda(table_data, flags, numa_state->nodes[i].initiator, i);
>      }
> +
> +    for (i = 0; i < numa_state->num_nodes; i++) {
> +        if (numa_state->nodes[i].has_cpu) {
> +            initiator_list[num_initiator++] = i;
> +        }
> +    }
> +
> +    /*
> +     * ACPI 6.3: 5.2.27.4 System Locality Latency and Bandwidth Information
> +     * Structure: Table 5-146
> +     */
> +    for (hierarchy = HMAT_LB_MEM_MEMORY;
> +         hierarchy <= HMAT_LB_MEM_CACHE_3RD_LEVEL; hierarchy++) {
> +        for (type = HMAT_LB_DATA_ACCESS_LATENCY;
> +             type <= HMAT_LB_DATA_WRITE_BANDWIDTH; type++) {
> +            hmat_lb = numa_state->hmat_lb[hierarchy][type];
> +
> +            if (hmat_lb) {
> +                build_hmat_lb(table_data, hmat_lb, num_initiator,
> +                              numa_state->num_nodes, initiator_list);
> +            }
> +        }
> +    }
>  }
>  
>  void build_hmat(GArray *table_data, BIOSLinker *linker, NumaState *numa_state)