Re: [Qemu-devel] [PATCH 4/9] pc: acpi: SRAT: create only valid processor lapic entries

[Eduardo Habkost]

On Thu, Feb 04, 2016 at 12:47:30PM +0100, Igor Mammedov wrote:
> When APIC IDs are sparse*, in addition to valid LAPIC
> antries the SRAT is also filled invalid ones for non
> posiible APIC IDs.
> Fix it by asking machine for all possible APIC IDs
> instead of wrongly assuming that all APIC IDs in
> range 0..apic_id_limit are possible.
> 
> * sparse lapic topology CLI:
>      -smp x,sockets=2,cores=3,maxcpus=6
> Signed-off-by: Igor Mammedov <address@hidden>

The only reason we did that is because we were trying to match
Seabios tables byte-by-byte, and Seabios didn't know which APIC
IDs were really valid or not.

Now QEMU has more information than Seabios has, and can be more
efficient.

Reviewed-by: Eduardo Habkost <address@hidden>

(However, see my suggestion to make possible_cpu_arch_ids() just
return an uint64_t array instead of struct CPUArchId).

> ---
>  hw/i386/acpi-build.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index faf541c..3077061 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2376,6 +2376,8 @@ build_srat(GArray *table_data, GArray *linker, 
> PcGuestInfo *guest_info,
>      uint64_t curnode;
>      int srat_start, numa_start, slots;
>      uint64_t mem_len, mem_base, next_base;
> +    MachineClass *mc = MACHINE_GET_CLASS(machine);
> +    GArray *apic_id_list = mc->possible_cpu_arch_ids();
>      PCMachineState *pcms = PC_MACHINE(machine);
>      ram_addr_t hotplugabble_address_space_size =
>          object_property_get_int(OBJECT(pcms), PC_MACHINE_MEMHP_REGION_SIZE,
> @@ -2387,12 +2389,15 @@ build_srat(GArray *table_data, GArray *linker, 
> PcGuestInfo *guest_info,
>      srat->reserved1 = cpu_to_le32(1);
>      core = (void *)(srat + 1);
>  
> -    for (i = 0; i < guest_info->apic_id_limit; ++i) {
> +    for (i = 0; i < apic_id_list->len; i++) {
> +        CPUArchId id = FETCH_CPU_ARCH_ID(apic_id_list, i);
> +        int apic_id = id.arch_id;
> +
>          core = acpi_data_push(table_data, sizeof *core);
>          core->type = ACPI_SRAT_PROCESSOR;
>          core->length = sizeof(*core);
> -        core->local_apic_id = i;
> -        curnode = guest_info->node_cpu[i];
> +        core->local_apic_id = apic_id;
> +        curnode = guest_info->node_cpu[apic_id];
>          core->proximity_lo = curnode;
>          memset(core->proximity_hi, 0, 3);
>          core->local_sapic_eid = 0;
> @@ -2457,6 +2462,7 @@ build_srat(GArray *table_data, GArray *linker, 
> PcGuestInfo *guest_info,
>                   (void *)(table_data->data + srat_start),
>                   "SRAT",
>                   table_data->len - srat_start, 1, NULL);
> +    g_array_free(apic_id_list, true);
>  }
>  
>  static void
> -- 
> 1.8.3.1
> 
> 

-- 
Eduardo