All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hw/i386/acpi-build: Get NUMA information from struct NumaState
@ 2021-08-03  6:30 Jingqi Liu
  2021-08-05  9:26 ` Igor Mammedov
  0 siblings, 1 reply; 3+ messages in thread
From: Jingqi Liu @ 2021-08-03  6:30 UTC (permalink / raw)
  To: imammedo, mst, marcel.apfelbaum, pbonzini, ehabkost, richard.henderson
  Cc: Jingqi Liu, qemu-devel

The NUMA information in PCMachineState is copied from MachineState.
We get this information uniformly from struct NumaState in MachineState.

Signed-off-by: Jingqi Liu <jingqi.liu@intel.com>
---
 hw/i386/acpi-build.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 17836149fe..e3c9ad011e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1902,6 +1902,8 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
     X86MachineState *x86ms = X86_MACHINE(machine);
     const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
     PCMachineState *pcms = PC_MACHINE(machine);
+    int nb_numa_nodes = machine->numa_state->num_nodes;
+    NodeInfo *numa_info = machine->numa_state->nodes;
     ram_addr_t hotplugabble_address_space_size =
         object_property_get_int(OBJECT(pcms), PC_MACHINE_DEVMEM_REGION_SIZE,
                                 NULL);
@@ -1945,9 +1947,9 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
     next_base = 0;
     numa_start = table_data->len;
 
-    for (i = 1; i < pcms->numa_nodes + 1; ++i) {
+    for (i = 1; i < nb_numa_nodes + 1; ++i) {
         mem_base = next_base;
-        mem_len = pcms->node_mem[i - 1];
+        mem_len = numa_info[i - 1].node_mem;
         next_base = mem_base + mem_len;
 
         /* Cut out the 640K hole */
@@ -1995,7 +1997,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
     }
 
     slots = (table_data->len - numa_start) / sizeof *numamem;
-    for (; slots < pcms->numa_nodes + 2; slots++) {
+    for (; slots < nb_numa_nodes + 2; slots++) {
         numamem = acpi_data_push(table_data, sizeof *numamem);
         build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
     }
@@ -2011,7 +2013,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
     if (hotplugabble_address_space_size) {
         numamem = acpi_data_push(table_data, sizeof *numamem);
         build_srat_memory(numamem, machine->device_memory->base,
-                          hotplugabble_address_space_size, pcms->numa_nodes - 1,
+                          hotplugabble_address_space_size, nb_numa_nodes - 1,
                           MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
     }
 
@@ -2513,7 +2515,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
         }
     }
 #endif
-    if (pcms->numa_nodes) {
+    if (machine->numa_state->num_nodes) {
         acpi_add_table(table_offsets, tables_blob);
         build_srat(tables_blob, tables->linker, machine);
         if (machine->numa_state->have_numa_distance) {
-- 
2.21.3



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] hw/i386/acpi-build: Get NUMA information from struct NumaState
  2021-08-03  6:30 [PATCH] hw/i386/acpi-build: Get NUMA information from struct NumaState Jingqi Liu
@ 2021-08-05  9:26 ` Igor Mammedov
  2021-08-06  3:00   ` Liu, Jingqi
  0 siblings, 1 reply; 3+ messages in thread
From: Igor Mammedov @ 2021-08-05  9:26 UTC (permalink / raw)
  To: Jingqi Liu; +Cc: ehabkost, mst, richard.henderson, qemu-devel, pbonzini

On Tue,  3 Aug 2021 14:30:05 +0800
Jingqi Liu <jingqi.liu@intel.com> wrote:

> The NUMA information in PCMachineState is copied from MachineState.
> We get this information uniformly from struct NumaState in MachineState.
Is there a another reason behind this patch?

As cleanup it's not complete, why do you keep
PCMachineState::numa_nodes & co around?
I'd suggest to remove it completely and use data from
MachineState everywhere.

> Signed-off-by: Jingqi Liu <jingqi.liu@intel.com>
> ---
>  hw/i386/acpi-build.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 17836149fe..e3c9ad011e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1902,6 +1902,8 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>      X86MachineState *x86ms = X86_MACHINE(machine);
>      const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
>      PCMachineState *pcms = PC_MACHINE(machine);
> +    int nb_numa_nodes = machine->numa_state->num_nodes;
> +    NodeInfo *numa_info = machine->numa_state->nodes;
>      ram_addr_t hotplugabble_address_space_size =
>          object_property_get_int(OBJECT(pcms), PC_MACHINE_DEVMEM_REGION_SIZE,
>                                  NULL);
> @@ -1945,9 +1947,9 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>      next_base = 0;
>      numa_start = table_data->len;
>  
> -    for (i = 1; i < pcms->numa_nodes + 1; ++i) {
> +    for (i = 1; i < nb_numa_nodes + 1; ++i) {
>          mem_base = next_base;
> -        mem_len = pcms->node_mem[i - 1];
> +        mem_len = numa_info[i - 1].node_mem;
>          next_base = mem_base + mem_len;
>  
>          /* Cut out the 640K hole */
> @@ -1995,7 +1997,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>      }
>  
>      slots = (table_data->len - numa_start) / sizeof *numamem;
> -    for (; slots < pcms->numa_nodes + 2; slots++) {
> +    for (; slots < nb_numa_nodes + 2; slots++) {
>          numamem = acpi_data_push(table_data, sizeof *numamem);
>          build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
>      }
> @@ -2011,7 +2013,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>      if (hotplugabble_address_space_size) {
>          numamem = acpi_data_push(table_data, sizeof *numamem);
>          build_srat_memory(numamem, machine->device_memory->base,
> -                          hotplugabble_address_space_size, pcms->numa_nodes - 1,
> +                          hotplugabble_address_space_size, nb_numa_nodes - 1,
>                            MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
>      }
>  
> @@ -2513,7 +2515,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>          }
>      }
>  #endif
> -    if (pcms->numa_nodes) {
> +    if (machine->numa_state->num_nodes) {
>          acpi_add_table(table_offsets, tables_blob);
>          build_srat(tables_blob, tables->linker, machine);
>          if (machine->numa_state->have_numa_distance) {



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] hw/i386/acpi-build: Get NUMA information from struct NumaState
  2021-08-05  9:26 ` Igor Mammedov
@ 2021-08-06  3:00   ` Liu, Jingqi
  0 siblings, 0 replies; 3+ messages in thread
From: Liu, Jingqi @ 2021-08-06  3:00 UTC (permalink / raw)
  To: Igor Mammedov; +Cc: ehabkost, mst, richard.henderson, qemu-devel, pbonzini



On 8/5/2021 5:26 PM, Igor Mammedov wrote:
> On Tue,  3 Aug 2021 14:30:05 +0800
> Jingqi Liu <jingqi.liu@intel.com> wrote:
> 
>> The NUMA information in PCMachineState is copied from MachineState.
>> We get this information uniformly from struct NumaState in MachineState.
> Is there a another reason behind this patch?
> 
> As cleanup it's not complete, why do you keep
> PCMachineState::numa_nodes & co around?
> I'd suggest to remove it completely and use data from
> MachineState everywhere.
> 
Thanks for your confirmation.
I think so too.
We can get NUMA information completely from MachineState::numa_state.

Actually, MachineState::numa_state is used everywhere except for 
pc_guest_info_init(), which just copies NUMA information to 
PCMachineState::numa_nodes and PCMachineState::node_mem.

I'll remove them completely.

Thanks,
Jingqi

>> Signed-off-by: Jingqi Liu <jingqi.liu@intel.com>
>> ---
>>   hw/i386/acpi-build.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 17836149fe..e3c9ad011e 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -1902,6 +1902,8 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>>       X86MachineState *x86ms = X86_MACHINE(machine);
>>       const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
>>       PCMachineState *pcms = PC_MACHINE(machine);
>> +    int nb_numa_nodes = machine->numa_state->num_nodes;
>> +    NodeInfo *numa_info = machine->numa_state->nodes;
>>       ram_addr_t hotplugabble_address_space_size =
>>           object_property_get_int(OBJECT(pcms), PC_MACHINE_DEVMEM_REGION_SIZE,
>>                                   NULL);
>> @@ -1945,9 +1947,9 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>>       next_base = 0;
>>       numa_start = table_data->len;
>>   
>> -    for (i = 1; i < pcms->numa_nodes + 1; ++i) {
>> +    for (i = 1; i < nb_numa_nodes + 1; ++i) {
>>           mem_base = next_base;
>> -        mem_len = pcms->node_mem[i - 1];
>> +        mem_len = numa_info[i - 1].node_mem;
>>           next_base = mem_base + mem_len;
>>   
>>           /* Cut out the 640K hole */
>> @@ -1995,7 +1997,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>>       }
>>   
>>       slots = (table_data->len - numa_start) / sizeof *numamem;
>> -    for (; slots < pcms->numa_nodes + 2; slots++) {
>> +    for (; slots < nb_numa_nodes + 2; slots++) {
>>           numamem = acpi_data_push(table_data, sizeof *numamem);
>>           build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
>>       }
>> @@ -2011,7 +2013,7 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>>       if (hotplugabble_address_space_size) {
>>           numamem = acpi_data_push(table_data, sizeof *numamem);
>>           build_srat_memory(numamem, machine->device_memory->base,
>> -                          hotplugabble_address_space_size, pcms->numa_nodes - 1,
>> +                          hotplugabble_address_space_size, nb_numa_nodes - 1,
>>                             MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
>>       }
>>   
>> @@ -2513,7 +2515,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>>           }
>>       }
>>   #endif
>> -    if (pcms->numa_nodes) {
>> +    if (machine->numa_state->num_nodes) {
>>           acpi_add_table(table_offsets, tables_blob);
>>           build_srat(tables_blob, tables->linker, machine);
>>           if (machine->numa_state->have_numa_distance) {
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-08-06  3:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-03  6:30 [PATCH] hw/i386/acpi-build: Get NUMA information from struct NumaState Jingqi Liu
2021-08-05  9:26 ` Igor Mammedov
2021-08-06  3:00   ` Liu, Jingqi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.