| Message ID | 1461219834-10416-6-git-send-email-zhaoshenglong@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Apr 21, 2016 at 02:23:54PM +0800, Shannon Zhao wrote: > From: Shannon Zhao <shannon.zhao@linaro.org> > > To support NUMA, it needs to generate SRAT ACPI table. > > Signed-off-by: Shannon Zhao <shannon.zhao@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); (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
On 2016/4/22 21:26, Andrew Jones wrote: >> + 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); > > (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? > Good point! Will do that. Thanks,
On Fri, 22 Apr 2016 15:26:05 +0200 Andrew Jones <drjones@redhat.com> wrote: > On Thu, Apr 21, 2016 at 02:23:54PM +0800, Shannon Zhao wrote: > > From: Shannon Zhao <shannon.zhao@linaro.org> > > > > To support NUMA, it needs to generate SRAT ACPI table. > > > > Signed-off-by: Shannon Zhao <shannon.zhao@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 >
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); + + 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; + 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; + numamem->flags = cpu_to_le32(1); + numamem->base_addr = cpu_to_le64(mem_base); + numamem->range_length = cpu_to_le64(mem_len); + 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);