From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56821) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aRIrb-0007DK-UG for qemu-devel@nongnu.org; Thu, 04 Feb 2016 07:18:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aRIrW-0007Oi-Qe for qemu-devel@nongnu.org; Thu, 04 Feb 2016 07:18:19 -0500 Received: from mail-wm0-x241.google.com ([2a00:1450:400c:c09::241]:34948) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aRIrW-0007OG-Gh for qemu-devel@nongnu.org; Thu, 04 Feb 2016 07:18:14 -0500 Received: by mail-wm0-x241.google.com with SMTP id g62so175959wme.2 for ; Thu, 04 Feb 2016 04:18:14 -0800 (PST) References: <1454586455-10202-1-git-send-email-imammedo@redhat.com> <1454586455-10202-2-git-send-email-imammedo@redhat.com> From: Marcel Apfelbaum Message-ID: <56B34182.5040905@gmail.com> Date: Thu, 4 Feb 2016 14:18:10 +0200 MIME-Version: 1.0 In-Reply-To: <1454586455-10202-2-git-send-email-imammedo@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/9] machine: introduce MachineClass.possible_cpu_arch_ids() hook Reply-To: marcel@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov , qemu-devel@nongnu.org Cc: pbonzini@redhat.com, ehabkost@redhat.com, mst@redhat.com On 02/04/2016 01:47 PM, Igor Mammedov 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 figure out > what IDs are correct on its own. > > Signed-off-by: Igor Mammedov > --- > hw/i386/pc.c | 16 ++++++++++++++++ > include/hw/boards.h | 18 ++++++++++++++++++ > 2 files changed, 34 insertions(+) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index d72246d..2fd8fc8 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1946,6 +1946,21 @@ static unsigned pc_cpu_index_to_socket_id(unsigned cpu_index) > return topo.pkg_id; > } > > +static GArray *pc_possible_cpu_arch_ids(void) > +{ > + int i; > + GArray *list = g_array_new (FALSE, FALSE, sizeof (CPUArchId)); > + > + for (i = 0; i < max_cpus; i++) { > + CPUArchId val; > + > + val.arch_id = x86_cpu_apic_id_from_index(i); > + val.cpu = qemu_get_cpu_by_arch_id(val.arch_id); > + g_array_append_val(list, val); > + } > + return list; > +} > + > static void pc_machine_class_init(ObjectClass *oc, void *data) > { > MachineClass *mc = MACHINE_CLASS(oc); > @@ -1968,6 +1983,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 0f30959..bd85f46 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,16 @@ 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 ii it's present on NULL otherwise > + */ > +typedef struct { > + uint64_t arch_id; > + struct CPUState *cpu; > +} CPUArchId; > + > +/** > * MachineClass: > * @get_hotplug_handler: this function is called during bus-less > * device hotplug. If defined it returns pointer to an instance > @@ -57,6 +68,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 >*/ > @@ -99,8 +114,11 @@ struct MachineClass { > HotplugHandler *(*get_hotplug_handler)(MachineState *machine, > DeviceState *dev); > unsigned (*cpu_index_to_socket_id)(unsigned cpu_index); > + GArray *(*possible_cpu_arch_ids)(void); Hi Igor, Can't this be a GArray filled in at machine init time instead of a method? Just wondering. Thanks, Marcel > }; > > +#define FETCH_CPU_ARCH_ID(array, idx) g_array_index(array, CPUArchId, idx) > + > /** > * MachineState: > */ >