Re: [Qemu-devel] [PATCH v1 1/7] hmat acpi: Build Memory Subsystem Address Range Structre(s) in ACPI HMAT

[Eduardo Habkost <ehabkost@redhat.com>]

On Wed, May 09, 2018 at 04:34:29PM +0800, Liu Jingqi wrote:
> HMAT is defined in ACPI 6.2: 5.2.27 Heterogeneous Memory Attribute Table (HMAT).
> The specification references below link:
> http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
> 
> It 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.
> 
> This structure describes the System Physical Address(SPA) range
> occupied by memory subsystem and its associativity with processor
> proximity domain as well as hint for memory usage.
> 
[...]
> +/*
> + * The Proximity Domain of System Physical Address ranges defined
> + * in the HMAT, NFIT and SRAT tables shall match each other.
> + */
> +static void hmat_build_spa(GArray *table_data, PCMachineState *pcms)
> +{
> +    GSList *device_list = NULL;
> +    AcpiHmatSpaRange *hmat_spa;
> +    uint64_t mem_base, next_base, mem_len;
> +    int node;
> +
> +    next_base = 0;
> +    for (node = 0; node < nb_numa_nodes; node++) {
> +        mem_len = numa_info[node].node_mem;
> +        if (!mem_len) {
> +            continue;
> +        }
> +
> +        mem_base = next_base;
> +        next_base = mem_base + mem_len;
> +
> +        /* Cut out the 640K hole */
> +        if (mem_base <= HOLE_640K_START &&
> +            next_base > HOLE_640K_START) {
> +            mem_len -= next_base - HOLE_640K_START;
> +            if (mem_len > 0) {
> +                hmat_spa = acpi_data_push(table_data, sizeof(*hmat_spa));
> +                hmat_build_spa_info(hmat_spa, mem_base, mem_len, node);
> +            }
> +
> +            /* Check for the rare case: 640K < RAM < 1M */
> +            if (next_base <= HOLE_640K_END) {
> +                next_base = HOLE_640K_END;
> +                continue;
> +            }
> +            mem_base = HOLE_640K_END;
> +            mem_len = next_base - HOLE_640K_END;
> +        }
> +
> +        /* Cut out the ACPI_PCI hole */
> +        if (mem_base <= pcms->below_4g_mem_size &&
> +            next_base > pcms->below_4g_mem_size) {
> +            mem_len -= next_base - pcms->below_4g_mem_size;
> +            if (mem_len > 0) {
> +                hmat_spa = acpi_data_push(table_data, sizeof(*hmat_spa));
> +                hmat_build_spa_info(hmat_spa, mem_base, mem_len, node);
> +            }
> +            mem_base = 1ULL << 32;
> +            mem_len = next_base - pcms->below_4g_mem_size;
> +            next_base = mem_base + mem_len;
> +        }

This duplicates very complex logic that already exists in
build_srat().  We need to make the existing logic reusable.


> +        hmat_spa = acpi_data_push(table_data, sizeof(*hmat_spa));
> +        hmat_build_spa_info(hmat_spa, mem_base, mem_len, node);
> +    }
> +
> +    /* Build HMAT SPA structures for PC-DIMM devices. */
> +    object_child_foreach(qdev_get_machine(), pc_dimm_device_list, &device_list);
> +
> +    for (; device_list; device_list = device_list->next) {
> +        PCDIMMDevice *dimm = device_list->data;
> +        mem_base = object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PROP,
> +                                            NULL);
> +        mem_len = object_property_get_uint(OBJECT(dimm), PC_DIMM_SIZE_PROP,
> +                                           NULL);
> +        node = object_property_get_uint(OBJECT(dimm), PC_DIMM_NODE_PROP, NULL);
> +
> +        hmat_spa = acpi_data_push(table_data, sizeof(*hmat_spa));
> +        hmat_build_spa_info(hmat_spa, mem_base, mem_len, node);
> +    }
> +}

-- 
Eduardo