From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56152) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aRK5t-00079w-RX for qemu-devel@nongnu.org; Thu, 04 Feb 2016 08:37:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aRK5o-0004fU-Mi for qemu-devel@nongnu.org; Thu, 04 Feb 2016 08:37:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42774) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aRK5o-0004fJ-F8 for qemu-devel@nongnu.org; Thu, 04 Feb 2016 08:37:04 -0500 Date: Thu, 4 Feb 2016 14:36:59 +0100 From: Igor Mammedov Message-ID: <20160204143659.565a7829@nial.brq.redhat.com> In-Reply-To: <56B34182.5040905@gmail.com> References: <1454586455-10202-1-git-send-email-imammedo@redhat.com> <1454586455-10202-2-git-send-email-imammedo@redhat.com> <56B34182.5040905@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/9] machine: introduce MachineClass.possible_cpu_arch_ids() hook List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum Cc: pbonzini@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, ehabkost@redhat.com On Thu, 4 Feb 2016 14:18:10 +0200 Marcel Apfelbaum wrote: > 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. Then it wouldn't get CPU present status as there is no CPUs at that time. It would also require refatoring of vl.c as machine_init() happens before -smp is parsed. > > Thanks, > Marcel > > > }; > > > > +#define FETCH_CPU_ARCH_ID(array, idx) g_array_index(array, CPUArchId, idx) > > + > > /** > > * MachineState: > > */ > > >