| Message ID | 1457015336-288204-1-git-send-email-imammedo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/08/2016 04:43 PM, Igor Mammedov wrote: > On Tue, 8 Mar 2016 16:03:43 +0200 > Marcel Apfelbaum <marcel@redhat.com> wrote: > >> On 03/08/2016 02:28 PM, Igor Mammedov wrote: >>> On Thu, 3 Mar 2016 15:28:56 +0100 >>> Igor Mammedov <imammedo@redhat.com> wrote: >>> >>>> on x86 currently range 0..max_cpus is used to generate >>>> architecture-dependent CPU ID (APIC Id) for each present >>>> and possible CPUs. However architecture-dependent CPU IDs >>>> list could be sparse and code that needs to enumerate >>>> all IDs (ACPI) ended up doing guess work enumerating all >>>> possible and impossible IDs up to >>>> apic_id_limit = x86_cpu_apic_id_from_index(max_cpus). >>>> >>>> That leads to creation of MADT entries and Processor >>>> objects in ACPI tables for not possible CPUs. >>>> Fix it by allowing board specify a concrete list of >>>> CPU IDs accourding its own rules (which for x86 depends >>>> on topology). So that code that needs this list could >>>> request it from board instead of trying to guess >>>> what IDs are correct on its own. >>>> >>>> This interface will also allow to help making AML >>>> part of CPU hotplug target independent so it could >>>> be reused for ARM target. >>>> >>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com> >>>> --- >>>> v5: >>>> s/CPUArchId cpus[1]/CPUArchId cpus[0]/ to make length >>>> calculation simpler. Marcel Apfelbaum <marcel@redhat.com> >>> >>> Marcel, does v5 looks good to you? >> >> Definitely, thanks. >> >> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com> > Ops, I'm sorry but I've forgot to CC qemu-devel > could you reply to list list pls? > Sure, cc-ed qemu-devel. Thanks, Marcel >> >>> >>>> --- >>>> hw/i386/pc.c | 44 ++++++++++++++++++++++++++++++++++++++++---- >>>> include/hw/boards.h | 26 ++++++++++++++++++++++++++ >>>> include/hw/i386/pc.h | 1 + >>>> 3 files changed, 67 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >>>> index 151a64c..1af95a1 100644 >>>> --- a/hw/i386/pc.c >>>> +++ b/hw/i386/pc.c >>>> @@ -1132,10 +1132,17 @@ void pc_cpus_init(PCMachineState *pcms) >>>> exit(1); >>>> } >>>> >>>> - for (i = 0; i < smp_cpus; i++) { >>>> - cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i), >>>> - &error_fatal); >>>> - object_unref(OBJECT(cpu)); >>>> + pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) + >>>> + sizeof(CPUArchId) * max_cpus); >>>> + for (i = 0; i < max_cpus; i++) { >>>> + pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i); >>>> + pcms->possible_cpus->len++; >>>> + if (i < smp_cpus) { >>>> + cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i), >>>> + &error_fatal); >>>> + pcms->possible_cpus->cpus[i].cpu = CPU(cpu); >>>> + object_unref(OBJECT(cpu)); >>>> + } >>>> } >>>> >>>> /* tell smbios about cpuid version and features */ >>>> @@ -1658,9 +1665,19 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev, >>>> error_propagate(errp, local_err); >>>> } >>>> >>>> +static int pc_apic_cmp(const void *a, const void *b) >>>> +{ >>>> + CPUArchId *apic_a = (CPUArchId *)a; >>>> + CPUArchId *apic_b = (CPUArchId *)b; >>>> + >>>> + return apic_a->arch_id - apic_b->arch_id; >>>> +} >>>> + >>>> static void pc_cpu_plug(HotplugHandler *hotplug_dev, >>>> DeviceState *dev, Error **errp) >>>> { >>>> + CPUClass *cc = CPU_GET_CLASS(dev); >>>> + CPUArchId apic_id, *found_cpu; >>>> HotplugHandlerClass *hhc; >>>> Error *local_err = NULL; >>>> PCMachineState *pcms = PC_MACHINE(hotplug_dev); >>>> @@ -1683,6 +1700,13 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev, >>>> >>>> /* increment the number of CPUs */ >>>> rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1); >>>> + >>>> + apic_id.arch_id = cc->get_arch_id(CPU(dev)); >>>> + found_cpu = bsearch(&apic_id, pcms->possible_cpus->cpus, >>>> + pcms->possible_cpus->len, sizeof(*pcms->possible_cpus->cpus), >>>> + pc_apic_cmp); >>>> + assert(found_cpu); >>>> + found_cpu->cpu = CPU(dev); >>>> out: >>>> error_propagate(errp, local_err); >>>> } >>>> @@ -1925,6 +1949,17 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index) >>>> return topo.pkg_id; >>>> } >>>> >>>> +static CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine) >>>> +{ >>>> + PCMachineState *pcms = PC_MACHINE(machine); >>>> + int len = sizeof(CPUArchIdList) + >>>> + sizeof(CPUArchId) * (pcms->possible_cpus->len); >>>> + CPUArchIdList *list = g_malloc(len); >>>> + >>>> + memcpy(list, pcms->possible_cpus, len); >>>> + return list; >>>> +} >>>> + >>>> static void pc_machine_class_init(ObjectClass *oc, void *data) >>>> { >>>> MachineClass *mc = MACHINE_CLASS(oc); >>>> @@ -1947,6 +1982,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) >>>> pcmc->save_tsc_khz = true; >>>> mc->get_hotplug_handler = pc_get_hotpug_handler; >>>> mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id; >>>> + mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids; >>>> mc->default_boot_order = "cad"; >>>> mc->hot_add_cpu = pc_hot_add_cpu; >>>> mc->max_cpus = 255; >>>> diff --git a/include/hw/boards.h b/include/hw/boards.h >>>> index b5d7eae..4b3fdbe 100644 >>>> --- a/include/hw/boards.h >>>> +++ b/include/hw/boards.h >>>> @@ -8,6 +8,7 @@ >>>> #include "sysemu/accel.h" >>>> #include "hw/qdev.h" >>>> #include "qom/object.h" >>>> +#include "qom/cpu.h" >>>> >>>> void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, >>>> const char *name, >>>> @@ -42,6 +43,26 @@ bool machine_dump_guest_core(MachineState *machine); >>>> bool machine_mem_merge(MachineState *machine); >>>> >>>> /** >>>> + * CPUArchId: >>>> + * @arch_id - architecture-dependent CPU ID of present or possible CPU >>>> + * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise >>>> + */ >>>> +typedef struct { >>>> + uint64_t arch_id; >>>> + struct CPUState *cpu; >>>> +} CPUArchId; >>>> + >>>> +/** >>>> + * CPUArchIdList: >>>> + * @len - number of @CPUArchId items in @cpus array >>>> + * @cpus - array of present or possible CPUs for current machine configuration >>>> + */ >>>> +typedef struct { >>>> + int len; >>>> + CPUArchId cpus[0]; >>>> +} CPUArchIdList; >>>> + >>>> +/** >>>> * MachineClass: >>>> * @get_hotplug_handler: this function is called during bus-less >>>> * device hotplug. If defined it returns pointer to an instance >>>> @@ -57,6 +78,10 @@ bool machine_mem_merge(MachineState *machine); >>>> * Set only by old machines because they need to keep >>>> * compatibility on code that exposed QEMU_VERSION to guests in >>>> * the past (and now use qemu_hw_version()). >>>> + * @possible_cpu_arch_ids: >>>> + * Returns an array of @CPUArchId architecture-dependent CPU IDs >>>> + * which includes CPU IDs for present and possible to hotplug CPUs. >>>> + * Caller is responsible for freeing returned list. >>>> */ >>>> struct MachineClass { >>>> /*< private >*/ >>>> @@ -98,6 +123,7 @@ struct MachineClass { >>>> HotplugHandler *(*get_hotplug_handler)(MachineState *machine, >>>> DeviceState *dev); >>>> unsigned (*cpu_index_to_socket_id)(unsigned cpu_index); >>>> + CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine); >>>> }; >>>> >>>> /** >>>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h >>>> index 8b3546e..3e09232 100644 >>>> --- a/include/hw/i386/pc.h >>>> +++ b/include/hw/i386/pc.h >>>> @@ -65,6 +65,7 @@ struct PCMachineState { >>>> /* CPU and apic information: */ >>>> bool apic_xrupt_override; >>>> unsigned apic_id_limit; >>>> + CPUArchIdList *possible_cpus; >>>> >>>> /* NUMA information: */ >>>> uint64_t numa_nodes; >>> >> >> >
On Thu, 3 Mar 2016 15:28:56 +0100 Igor Mammedov <imammedo@redhat.com> wrote: > on x86 currently range 0..max_cpus is used to generate > architecture-dependent CPU ID (APIC Id) for each present > and possible CPUs. However architecture-dependent CPU IDs > list could be sparse and code that needs to enumerate > all IDs (ACPI) ended up doing guess work enumerating all > possible and impossible IDs up to > apic_id_limit = x86_cpu_apic_id_from_index(max_cpus). > > That leads to creation of MADT entries and Processor > objects in ACPI tables for not possible CPUs. > Fix it by allowing board specify a concrete list of > CPU IDs accourding its own rules (which for x86 depends > on topology). So that code that needs this list could > request it from board instead of trying to guess > what IDs are correct on its own. > > This interface will also allow to help making AML > part of CPU hotplug target independent so it could > be reused for ARM target. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v5: > s/CPUArchId cpus[1]/CPUArchId cpus[0]/ to make length > calculation simpler. Marcel Apfelbaum <marcel@redhat.com> Marcel, does v5 looks good to you? /me, sorry for replying/asking out of thread the first time./ > --- > hw/i386/pc.c | 44 ++++++++++++++++++++++++++++++++++++++++---- > include/hw/boards.h | 26 ++++++++++++++++++++++++++ > include/hw/i386/pc.h | 1 + > 3 files changed, 67 insertions(+), 4 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 151a64c..1af95a1 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1132,10 +1132,17 @@ void pc_cpus_init(PCMachineState *pcms) > exit(1); > } > > - for (i = 0; i < smp_cpus; i++) { > - cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i), > - &error_fatal); > - object_unref(OBJECT(cpu)); > + pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) + > + sizeof(CPUArchId) * max_cpus); > + for (i = 0; i < max_cpus; i++) { > + pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i); > + pcms->possible_cpus->len++; > + if (i < smp_cpus) { > + cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i), > + &error_fatal); > + pcms->possible_cpus->cpus[i].cpu = CPU(cpu); > + object_unref(OBJECT(cpu)); > + } > } > > /* tell smbios about cpuid version and features */ > @@ -1658,9 +1665,19 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev, > error_propagate(errp, local_err); > } > > +static int pc_apic_cmp(const void *a, const void *b) > +{ > + CPUArchId *apic_a = (CPUArchId *)a; > + CPUArchId *apic_b = (CPUArchId *)b; > + > + return apic_a->arch_id - apic_b->arch_id; > +} > + > static void pc_cpu_plug(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp) > { > + CPUClass *cc = CPU_GET_CLASS(dev); > + CPUArchId apic_id, *found_cpu; > HotplugHandlerClass *hhc; > Error *local_err = NULL; > PCMachineState *pcms = PC_MACHINE(hotplug_dev); > @@ -1683,6 +1700,13 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev, > > /* increment the number of CPUs */ > rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1); > + > + apic_id.arch_id = cc->get_arch_id(CPU(dev)); > + found_cpu = bsearch(&apic_id, pcms->possible_cpus->cpus, > + pcms->possible_cpus->len, sizeof(*pcms->possible_cpus->cpus), > + pc_apic_cmp); > + assert(found_cpu); > + found_cpu->cpu = CPU(dev); > out: > error_propagate(errp, local_err); > } > @@ -1925,6 +1949,17 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index) > return topo.pkg_id; > } > > +static CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine) > +{ > + PCMachineState *pcms = PC_MACHINE(machine); > + int len = sizeof(CPUArchIdList) + > + sizeof(CPUArchId) * (pcms->possible_cpus->len); > + CPUArchIdList *list = g_malloc(len); > + > + memcpy(list, pcms->possible_cpus, len); > + return list; > +} > + > static void pc_machine_class_init(ObjectClass *oc, void *data) > { > MachineClass *mc = MACHINE_CLASS(oc); > @@ -1947,6 +1982,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) > pcmc->save_tsc_khz = true; > mc->get_hotplug_handler = pc_get_hotpug_handler; > mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id; > + mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids; > mc->default_boot_order = "cad"; > mc->hot_add_cpu = pc_hot_add_cpu; > mc->max_cpus = 255; > diff --git a/include/hw/boards.h b/include/hw/boards.h > index b5d7eae..4b3fdbe 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -8,6 +8,7 @@ > #include "sysemu/accel.h" > #include "hw/qdev.h" > #include "qom/object.h" > +#include "qom/cpu.h" > > void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, > const char *name, > @@ -42,6 +43,26 @@ bool machine_dump_guest_core(MachineState *machine); > bool machine_mem_merge(MachineState *machine); > > /** > + * CPUArchId: > + * @arch_id - architecture-dependent CPU ID of present or possible CPU > + * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise > + */ > +typedef struct { > + uint64_t arch_id; > + struct CPUState *cpu; > +} CPUArchId; > + > +/** > + * CPUArchIdList: > + * @len - number of @CPUArchId items in @cpus array > + * @cpus - array of present or possible CPUs for current machine configuration > + */ > +typedef struct { > + int len; > + CPUArchId cpus[0]; > +} CPUArchIdList; > + > +/** > * MachineClass: > * @get_hotplug_handler: this function is called during bus-less > * device hotplug. If defined it returns pointer to an instance > @@ -57,6 +78,10 @@ bool machine_mem_merge(MachineState *machine); > * Set only by old machines because they need to keep > * compatibility on code that exposed QEMU_VERSION to guests in > * the past (and now use qemu_hw_version()). > + * @possible_cpu_arch_ids: > + * Returns an array of @CPUArchId architecture-dependent CPU IDs > + * which includes CPU IDs for present and possible to hotplug CPUs. > + * Caller is responsible for freeing returned list. > */ > struct MachineClass { > /*< private >*/ > @@ -98,6 +123,7 @@ struct MachineClass { > HotplugHandler *(*get_hotplug_handler)(MachineState *machine, > DeviceState *dev); > unsigned (*cpu_index_to_socket_id)(unsigned cpu_index); > + CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine); > }; > > /** > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 8b3546e..3e09232 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -65,6 +65,7 @@ struct PCMachineState { > /* CPU and apic information: */ > bool apic_xrupt_override; > unsigned apic_id_limit; > + CPUArchIdList *possible_cpus; > > /* NUMA information: */ > uint64_t numa_nodes;
On 03/08/2016 06:29 PM, Igor Mammedov wrote: > On Thu, 3 Mar 2016 15:28:56 +0100 > Igor Mammedov <imammedo@redhat.com> wrote: > >> on x86 currently range 0..max_cpus is used to generate >> architecture-dependent CPU ID (APIC Id) for each present >> and possible CPUs. However architecture-dependent CPU IDs >> list could be sparse and code that needs to enumerate >> all IDs (ACPI) ended up doing guess work enumerating all >> possible and impossible IDs up to >> apic_id_limit = x86_cpu_apic_id_from_index(max_cpus). >> >> That leads to creation of MADT entries and Processor >> objects in ACPI tables for not possible CPUs. >> Fix it by allowing board specify a concrete list of >> CPU IDs accourding its own rules (which for x86 depends >> on topology). So that code that needs this list could >> request it from board instead of trying to guess >> what IDs are correct on its own. >> >> This interface will also allow to help making AML >> part of CPU hotplug target independent so it could >> be reused for ARM target. >> >> Signed-off-by: Igor Mammedov <imammedo@redhat.com> >> --- >> v5: >> s/CPUArchId cpus[1]/CPUArchId cpus[0]/ to make length >> calculation simpler. Marcel Apfelbaum <marcel@redhat.com> > Marcel, does v5 looks good to you? > > /me, sorry for replying/asking out of thread the first time./ > No problem, I think I already sent this, but to be sure Reviewed-by: Marcel Apfelbaum <marcel@redhat.com> Thanks, Marcel >> --- >> hw/i386/pc.c | 44 ++++++++++++++++++++++++++++++++++++++++---- >> include/hw/boards.h | 26 ++++++++++++++++++++++++++ >> include/hw/i386/pc.h | 1 + >> 3 files changed, 67 insertions(+), 4 deletions(-) >> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index 151a64c..1af95a1 100644 >> --- a/hw/i386/pc.c >> +++ b/hw/i386/pc.c >> @@ -1132,10 +1132,17 @@ void pc_cpus_init(PCMachineState *pcms) >> exit(1); >> } >> >> - for (i = 0; i < smp_cpus; i++) { >> - cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i), >> - &error_fatal); >> - object_unref(OBJECT(cpu)); >> + pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) + >> + sizeof(CPUArchId) * max_cpus); >> + for (i = 0; i < max_cpus; i++) { >> + pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i); >> + pcms->possible_cpus->len++; >> + if (i < smp_cpus) { >> + cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i), >> + &error_fatal); >> + pcms->possible_cpus->cpus[i].cpu = CPU(cpu); >> + object_unref(OBJECT(cpu)); >> + } >> } >> >> /* tell smbios about cpuid version and features */ >> @@ -1658,9 +1665,19 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev, >> error_propagate(errp, local_err); >> } >> >> +static int pc_apic_cmp(const void *a, const void *b) >> +{ >> + CPUArchId *apic_a = (CPUArchId *)a; >> + CPUArchId *apic_b = (CPUArchId *)b; >> + >> + return apic_a->arch_id - apic_b->arch_id; >> +} >> + >> static void pc_cpu_plug(HotplugHandler *hotplug_dev, >> DeviceState *dev, Error **errp) >> { >> + CPUClass *cc = CPU_GET_CLASS(dev); >> + CPUArchId apic_id, *found_cpu; >> HotplugHandlerClass *hhc; >> Error *local_err = NULL; >> PCMachineState *pcms = PC_MACHINE(hotplug_dev); >> @@ -1683,6 +1700,13 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev, >> >> /* increment the number of CPUs */ >> rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1); >> + >> + apic_id.arch_id = cc->get_arch_id(CPU(dev)); >> + found_cpu = bsearch(&apic_id, pcms->possible_cpus->cpus, >> + pcms->possible_cpus->len, sizeof(*pcms->possible_cpus->cpus), >> + pc_apic_cmp); >> + assert(found_cpu); >> + found_cpu->cpu = CPU(dev); >> out: >> error_propagate(errp, local_err); >> } >> @@ -1925,6 +1949,17 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index) >> return topo.pkg_id; >> } >> >> +static CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine) >> +{ >> + PCMachineState *pcms = PC_MACHINE(machine); >> + int len = sizeof(CPUArchIdList) + >> + sizeof(CPUArchId) * (pcms->possible_cpus->len); >> + CPUArchIdList *list = g_malloc(len); >> + >> + memcpy(list, pcms->possible_cpus, len); >> + return list; >> +} >> + >> static void pc_machine_class_init(ObjectClass *oc, void *data) >> { >> MachineClass *mc = MACHINE_CLASS(oc); >> @@ -1947,6 +1982,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) >> pcmc->save_tsc_khz = true; >> mc->get_hotplug_handler = pc_get_hotpug_handler; >> mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id; >> + mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids; >> mc->default_boot_order = "cad"; >> mc->hot_add_cpu = pc_hot_add_cpu; >> mc->max_cpus = 255; >> diff --git a/include/hw/boards.h b/include/hw/boards.h >> index b5d7eae..4b3fdbe 100644 >> --- a/include/hw/boards.h >> +++ b/include/hw/boards.h >> @@ -8,6 +8,7 @@ >> #include "sysemu/accel.h" >> #include "hw/qdev.h" >> #include "qom/object.h" >> +#include "qom/cpu.h" >> >> void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, >> const char *name, >> @@ -42,6 +43,26 @@ bool machine_dump_guest_core(MachineState *machine); >> bool machine_mem_merge(MachineState *machine); >> >> /** >> + * CPUArchId: >> + * @arch_id - architecture-dependent CPU ID of present or possible CPU >> + * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise >> + */ >> +typedef struct { >> + uint64_t arch_id; >> + struct CPUState *cpu; >> +} CPUArchId; >> + >> +/** >> + * CPUArchIdList: >> + * @len - number of @CPUArchId items in @cpus array >> + * @cpus - array of present or possible CPUs for current machine configuration >> + */ >> +typedef struct { >> + int len; >> + CPUArchId cpus[0]; >> +} CPUArchIdList; >> + >> +/** >> * MachineClass: >> * @get_hotplug_handler: this function is called during bus-less >> * device hotplug. If defined it returns pointer to an instance >> @@ -57,6 +78,10 @@ bool machine_mem_merge(MachineState *machine); >> * Set only by old machines because they need to keep >> * compatibility on code that exposed QEMU_VERSION to guests in >> * the past (and now use qemu_hw_version()). >> + * @possible_cpu_arch_ids: >> + * Returns an array of @CPUArchId architecture-dependent CPU IDs >> + * which includes CPU IDs for present and possible to hotplug CPUs. >> + * Caller is responsible for freeing returned list. >> */ >> struct MachineClass { >> /*< private >*/ >> @@ -98,6 +123,7 @@ struct MachineClass { >> HotplugHandler *(*get_hotplug_handler)(MachineState *machine, >> DeviceState *dev); >> unsigned (*cpu_index_to_socket_id)(unsigned cpu_index); >> + CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine); >> }; >> >> /** >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h >> index 8b3546e..3e09232 100644 >> --- a/include/hw/i386/pc.h >> +++ b/include/hw/i386/pc.h >> @@ -65,6 +65,7 @@ struct PCMachineState { >> /* CPU and apic information: */ >> bool apic_xrupt_override; >> unsigned apic_id_limit; >> + CPUArchIdList *possible_cpus; >> >> /* NUMA information: */ >> uint64_t numa_nodes; > >
diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 151a64c..1af95a1 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1132,10 +1132,17 @@ void pc_cpus_init(PCMachineState *pcms) exit(1); } - for (i = 0; i < smp_cpus; i++) { - cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i), - &error_fatal); - object_unref(OBJECT(cpu)); + pcms->possible_cpus = g_malloc0(sizeof(CPUArchIdList) + + sizeof(CPUArchId) * max_cpus); + for (i = 0; i < max_cpus; i++) { + pcms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(i); + pcms->possible_cpus->len++; + if (i < smp_cpus) { + cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i), + &error_fatal); + pcms->possible_cpus->cpus[i].cpu = CPU(cpu); + object_unref(OBJECT(cpu)); + } } /* tell smbios about cpuid version and features */ @@ -1658,9 +1665,19 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev, error_propagate(errp, local_err); } +static int pc_apic_cmp(const void *a, const void *b) +{ + CPUArchId *apic_a = (CPUArchId *)a; + CPUArchId *apic_b = (CPUArchId *)b; + + return apic_a->arch_id - apic_b->arch_id; +} + static void pc_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { + CPUClass *cc = CPU_GET_CLASS(dev); + CPUArchId apic_id, *found_cpu; HotplugHandlerClass *hhc; Error *local_err = NULL; PCMachineState *pcms = PC_MACHINE(hotplug_dev); @@ -1683,6 +1700,13 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev, /* increment the number of CPUs */ rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1); + + apic_id.arch_id = cc->get_arch_id(CPU(dev)); + found_cpu = bsearch(&apic_id, pcms->possible_cpus->cpus, + pcms->possible_cpus->len, sizeof(*pcms->possible_cpus->cpus), + pc_apic_cmp); + assert(found_cpu); + found_cpu->cpu = CPU(dev); out: error_propagate(errp, local_err); } @@ -1925,6 +1949,17 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index) return topo.pkg_id; } +static CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *machine) +{ + PCMachineState *pcms = PC_MACHINE(machine); + int len = sizeof(CPUArchIdList) + + sizeof(CPUArchId) * (pcms->possible_cpus->len); + CPUArchIdList *list = g_malloc(len); + + memcpy(list, pcms->possible_cpus, len); + return list; +} + static void pc_machine_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); @@ -1947,6 +1982,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) pcmc->save_tsc_khz = true; mc->get_hotplug_handler = pc_get_hotpug_handler; mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id; + mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids; mc->default_boot_order = "cad"; mc->hot_add_cpu = pc_hot_add_cpu; mc->max_cpus = 255; diff --git a/include/hw/boards.h b/include/hw/boards.h index b5d7eae..4b3fdbe 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -8,6 +8,7 @@ #include "sysemu/accel.h" #include "hw/qdev.h" #include "qom/object.h" +#include "qom/cpu.h" void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, const char *name, @@ -42,6 +43,26 @@ bool machine_dump_guest_core(MachineState *machine); bool machine_mem_merge(MachineState *machine); /** + * CPUArchId: + * @arch_id - architecture-dependent CPU ID of present or possible CPU + * @cpu - pointer to corresponding CPU object if it's present on NULL otherwise + */ +typedef struct { + uint64_t arch_id; + struct CPUState *cpu; +} CPUArchId; + +/** + * CPUArchIdList: + * @len - number of @CPUArchId items in @cpus array + * @cpus - array of present or possible CPUs for current machine configuration + */ +typedef struct { + int len; + CPUArchId cpus[0]; +} CPUArchIdList; + +/** * MachineClass: * @get_hotplug_handler: this function is called during bus-less * device hotplug. If defined it returns pointer to an instance @@ -57,6 +78,10 @@ bool machine_mem_merge(MachineState *machine); * Set only by old machines because they need to keep * compatibility on code that exposed QEMU_VERSION to guests in * the past (and now use qemu_hw_version()). + * @possible_cpu_arch_ids: + * Returns an array of @CPUArchId architecture-dependent CPU IDs + * which includes CPU IDs for present and possible to hotplug CPUs. + * Caller is responsible for freeing returned list. */ struct MachineClass { /*< private >*/ @@ -98,6 +123,7 @@ struct MachineClass { HotplugHandler *(*get_hotplug_handler)(MachineState *machine, DeviceState *dev); unsigned (*cpu_index_to_socket_id)(unsigned cpu_index); + CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine); }; /** diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 8b3546e..3e09232 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -65,6 +65,7 @@ struct PCMachineState { /* CPU and apic information: */ bool apic_xrupt_override; unsigned apic_id_limit; + CPUArchIdList *possible_cpus; /* NUMA information: */ uint64_t numa_nodes;
on x86 currently range 0..max_cpus is used to generate architecture-dependent CPU ID (APIC Id) for each present and possible CPUs. However architecture-dependent CPU IDs list could be sparse and code that needs to enumerate all IDs (ACPI) ended up doing guess work enumerating all possible and impossible IDs up to apic_id_limit = x86_cpu_apic_id_from_index(max_cpus). That leads to creation of MADT entries and Processor objects in ACPI tables for not possible CPUs. Fix it by allowing board specify a concrete list of CPU IDs accourding its own rules (which for x86 depends on topology). So that code that needs this list could request it from board instead of trying to guess what IDs are correct on its own. This interface will also allow to help making AML part of CPU hotplug target independent so it could be reused for ARM target. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- v5: s/CPUArchId cpus[1]/CPUArchId cpus[0]/ to make length calculation simpler. Marcel Apfelbaum <marcel@redhat.com> --- hw/i386/pc.c | 44 ++++++++++++++++++++++++++++++++++++++++---- include/hw/boards.h | 26 ++++++++++++++++++++++++++ include/hw/i386/pc.h | 1 + 3 files changed, 67 insertions(+), 4 deletions(-)