From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36222) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Znlz5-0001G3-5D for qemu-devel@nongnu.org; Sun, 18 Oct 2015 07:18:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Znlz1-0005WR-Rp for qemu-devel@nongnu.org; Sun, 18 Oct 2015 07:18:39 -0400 Received: from mail-wi0-x22b.google.com ([2a00:1450:400c:c05::22b]:37762) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Znlz1-0005WF-I9 for qemu-devel@nongnu.org; Sun, 18 Oct 2015 07:18:35 -0400 Received: by wicfv8 with SMTP id fv8so42393790wic.0 for ; Sun, 18 Oct 2015 04:18:35 -0700 (PDT) References: <1444878732-12626-1-git-send-email-zhugh.fnst@cn.fujitsu.com> From: Marcel Apfelbaum Message-ID: <56238007.80001@gmail.com> Date: Sun, 18 Oct 2015 14:18:31 +0300 MIME-Version: 1.0 In-Reply-To: <1444878732-12626-1-git-send-email-zhugh.fnst@cn.fujitsu.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RESEND PATCH v3] i386: keep cpu_model field in MachineState uptodate Reply-To: marcel@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhu Guihua , qemu-devel@nongnu.org, ehabkost@redhat.com, mst@redhat.com, pbonzini@redhat.com On 10/15/2015 06:12 AM, Zhu Guihua wrote: > Update cpu_model in MachineState for i386, so that the field can be used > for cpu hotplug, instead of using a static variable. > > This patch is rebased on the latest master. > > Signed-off-by: Zhu Guihua > Reviewed-by: Eduardo Habkost > --- > v3: > -use PCMachineState in pc_cpus_init() instead MachineState > > v2: > -transfer MachineState from all pc_cpus_init() callers > --- > hw/i386/pc.c | 17 ++++++++--------- > hw/i386/pc_piix.c | 2 +- > hw/i386/pc_q35.c | 2 +- > include/hw/i386/pc.h | 2 +- > 4 files changed, 11 insertions(+), 12 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 682867a..208f553 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1077,11 +1077,10 @@ out: > return cpu; > } > > -static const char *current_cpu_model; > - > void pc_hot_add_cpu(const int64_t id, Error **errp) > { > X86CPU *cpu; > + MachineState *machine = MACHINE(qdev_get_machine()); > int64_t apic_id = x86_cpu_apic_id_from_index(id); > Error *local_err = NULL; > > @@ -1109,7 +1108,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp) > return; > } > > - cpu = pc_new_cpu(current_cpu_model, apic_id, &local_err); > + cpu = pc_new_cpu(machine->cpu_model, apic_id, &local_err);t Hi, I am not going to "stop" this patch and I do agree with what is trying to do. What I still don't get is if we are "allowed" to directly access QOM object's private fields outside the implementation C file. This is why we have some wrappers in include/hw/boards.h when we access machine's fields. Just wanted to raise the question, other than that (for what is worth): Reviewed-by: Marcel Apfelbaum Thanks, Marcel > if (local_err) { > error_t whapropagate(errp, local_err); > return; > @@ -1117,22 +1116,22 @@ void pc_hot_add_cpu(const int64_t id, Error **errp) > object_unref(OBJECT(cpu)); > } > > -void pc_cpus_init(const char *cpu_model) > +void pc_cpus_init(PCMachineState *pcms) > { > int i; > X86CPU *cpu = NULL; > + MachineState *machine = MACHINE(pcms); > Error *error = NULL; > unsigned long apic_id_limit; > > /* init CPUs */ > - if (cpu_model == NULL) { > + if (machine->cpu_model == NULL) { > #ifdef TARGET_X86_64 > - cpu_model = "qemu64"; > + machine->cpu_model = "qemu64"; > #else > - cpu_model = "qemu32"; > + machine->cpu_model = "qemu32"; > #endif > } > - current_cpu_model = cpu_model; > > apic_id_limit = pc_apic_id_limit(max_cpus); > if (apic_id_limit > ACPI_CPU_HOTPLUG_ID_LIMIT) { > @@ -1142,7 +1141,7 @@ void pc_cpus_init(const char *cpu_model) > } > > for (i = 0; i < smp_cpus; i++) { > - cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), > + cpu = pc_new_cpu(machine->cpu_model, x86_cpu_apic_id_from_index(i), > &error); > if (error) { > error_report_err(error); > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index ae7bbeb..0eacde1 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -139,7 +139,7 @@ static void pc_init1(MachineState *machine, > exit(1); > } > > - pc_cpus_init(machine->cpu_model); > + pc_cpus_init(pcms); > > if (kvm_enabled() && kvmclock_enabled) { > kvmclock_create(); > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 19e6670..3744abd 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -128,7 +128,7 @@ static void pc_q35_init(MachineState *machine) > exit(1); > } > > - pc_cpus_init(machine->cpu_model); > + pc_cpus_init(pcms); > pc_acpi_init("q35-acpi-dsdt.aml"); > > kvmclock_create(); > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 0503485..c5961d7 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -168,7 +168,7 @@ bool pc_machine_is_smm_enabled(PCMachineState *pcms); > void pc_register_ferr_irq(qemu_irq irq); > void pc_acpi_smi_interrupt(void *opaque, int irq, int level); > > -void pc_cpus_init(const char *cpu_model); > +void pc_cpus_init(PCMachineState *pcms); > void pc_hot_add_cpu(const int64_t id, Error **errp); > void pc_acpi_init(const char *default_dsdt); > >