On Fri, 22 Apr 2016 15:26:05 +0200
Andrew Jones <drjo...@redhat.com> wrote:
> On Thu, Apr 21, 2016 at 02:23:54PM +0800, Shannon Zhao wrote:
> > From: Shannon Zhao <shannon.z...@linaro.org>
> >
> > To support NUMA, it needs to generate SRAT ACPI table.
> >
> > Signed-off-by: Shannon Zhao <shannon.z...@linaro.org>
> > ---
> > hw/arm/virt-acpi-build.c | 58
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 58 insertions(+)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index f51fe39..a618210 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -43,6 +43,7 @@
> > #include "hw/acpi/aml-build.h"
> > #include "hw/pci/pcie_host.h"
> > #include "hw/pci/pci.h"
> > +#include "sysemu/numa.h"
> >
> > #define ARM_SPI_BASE 32
> > #define ACPI_POWER_BUTTON_DEVICE "PWRB"
> > @@ -414,6 +415,58 @@ build_spcr(GArray *table_data, GArray *linker,
> > VirtGuestInfo *guest_info)
> > }
> >
> > static void
> > +build_srat(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> > +{
> > + AcpiSystemResourceAffinityTable *srat;
> > + AcpiSratProcessorGiccAffinity *core;
> > + AcpiSratMemoryAffinity *numamem;
> > + int i, j, srat_start;
> > + uint64_t mem_len, mem_base;
> > + uint32_t *cpu_node = g_malloc0(guest_info->smp_cpus * sizeof
> > *cpu_node);
>
> nit1: the majority of qemu code uses () with sizeof
> nit2: sizeof(uint32_t) might be nicer than the cyclic reference
>
> > +
> > + for (i = 0; i < guest_info->smp_cpus; i++) {
> > + for (j = 0; j < nb_numa_nodes; j++) {
> > + if (test_bit(i, numa_info[j].node_cpu)) {
> > + cpu_node[i] = j;
> > + break;
> > + }
> > + }
> > + }
> > +
> > + srat_start = table_data->len;
> > + srat = acpi_data_push(table_data, sizeof *srat);
> > + srat->reserved1 = cpu_to_le32(1);
> > +
> > + for (i = 0; i < guest_info->smp_cpus; ++i) {
> > + core = acpi_data_push(table_data, sizeof *core);
> > + core->type = ACPI_SRAT_PROCESSOR_GICC;
> > + core->length = sizeof(*core);
> > + core->proximity = cpu_node[i];
> > + core->acpi_processor_uid = i;
>
> We'll want to use get_arch_id() here (after I implement it as part of
> the cpu topology work I keep promising).
>
> > + core->flags = cpu_to_le32(1);
> > + }
> > + g_free(cpu_node);
> > +
> > + mem_base = guest_info->memmap[VIRT_MEM].base;
> > + for (i = 0; i < nb_numa_nodes; ++i) {
> > + mem_len = numa_info[i].node_mem;
> > + numamem = acpi_data_push(table_data, sizeof *numamem);
> > + numamem->type = ACPI_SRAT_MEMORY;
> > + numamem->length = sizeof(*numamem);
> > + memset(numamem->proximity, 0, 4);
> > + numamem->proximity[0] = i;
>
> This is weird (but I see x86 does it too). The spec says proximity is
> "Integer that represents the proximity domain to which the processor
> belongs", and its 4 bytes. So why doesn't the structure define it as
> a uint32_t and then we'd just do
>
> numamem->proximity = cpu_to_le32(i);
It's probably just some legacy code and should be fixed as you're suggesting.
>
> (adding Igor)
>
> > + numamem->flags = cpu_to_le32(1);
> > + numamem->base_addr = cpu_to_le64(mem_base);
> > + numamem->range_length = cpu_to_le64(mem_len);
>
> How about moving acpi_build_srat_memory from hw/i386/acpi-build.c to
> somewhere in hw/acpi and reusing it?
>
> > + mem_base += mem_len;
> > + }
> > +
> > + build_header(linker, table_data,
> > + (void *)(table_data->data + srat_start), "SRAT",
> > + table_data->len - srat_start, 3, NULL, NULL);
> > +}
> > +
> > +static void
> > build_mcfg(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
> > {
> > AcpiTableMcfg *mcfg;
> > @@ -638,6 +691,11 @@ void virt_acpi_build(VirtGuestInfo *guest_info,
> > AcpiBuildTables *tables)
> > acpi_add_table(table_offsets, tables_blob);
> > build_spcr(tables_blob, tables->linker, guest_info);
> >
> > + if (nb_numa_nodes > 0) {
> > + acpi_add_table(table_offsets, tables_blob);
> > + build_srat(tables_blob, tables->linker, guest_info);
> > + }
> > +
> > /* RSDT is pointed to by RSDP */
> > rsdt = tables_blob->len;
> > build_rsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
> > --
> > 2.0.4
> >
> >
>
> Thanks,
> drew
>
