From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37723) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScvDz-0006Li-Eg for qemu-devel@nongnu.org; Fri, 08 Jun 2012 05:11:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ScvDx-000626-Df for qemu-devel@nongnu.org; Fri, 08 Jun 2012 05:11:19 -0400 Received: from cantor2.suse.de ([195.135.220.15]:60705 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ScvDx-0005zf-3y for qemu-devel@nongnu.org; Fri, 08 Jun 2012 05:11:17 -0400 Message-ID: <4FD1C1AF.2020208@suse.de> Date: Fri, 08 Jun 2012 11:11:11 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1337742502-28565-1-git-send-email-afaerber@suse.de> <1337742502-28565-5-git-send-email-afaerber@suse.de> <20120608082006.GA22512@thinkpad.mammed.net> In-Reply-To: <20120608082006.GA22512@thinkpad.mammed.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH qom-next 04/59] pc: Add CPU as /machine/cpu[n] List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: Jan Kiszka , Anthony Liguori , Paolo Bonzini , qemu-devel@nongnu.org, Igor Mammedov Am 08.06.2012 10:20, schrieb Igor Mammedov: > On Wed, May 23, 2012 at 05:07:27AM +0200, Andreas F=E4rber wrote: >> Using the cpu_index, give the X86CPU a canonical path. >> This must be done before initializing the APIC. >> >> Signed-off-by: Igor Mammedov >> Signed-off-by: Andreas F=E4rber >> --- >> hw/pc.c | 12 ++++++++++++ >> 1 files changed, 12 insertions(+), 0 deletions(-) >> >> diff --git a/hw/pc.c b/hw/pc.c >> index 4167782..e9d7e05 100644 >> --- a/hw/pc.c >> +++ b/hw/pc.c >> @@ -945,6 +945,8 @@ static X86CPU *pc_new_cpu(const char *cpu_model) >> { >> X86CPU *cpu; >> CPUX86State *env; >> + char *name; >> + Error *error =3D NULL; >> =20 >> cpu =3D cpu_x86_init(cpu_model); >> if (cpu =3D=3D NULL) { >> @@ -952,6 +954,16 @@ static X86CPU *pc_new_cpu(const char *cpu_model) >> exit(1); >> } >> env =3D &cpu->env; >> + >> + name =3D g_strdup_printf("cpu[%d]", env->cpu_index); >> + object_property_add_child(OBJECT(qdev_get_machine()), name, >> + OBJECT(cpu), &error); > This call might be too late. This series here is mostly not going to go through qom-next btw, it is just based on qom-next, so it's not too late to discuss such issues. :) > Imagine if before this call a property/child of this CPU > would set link on on it. Then it would assert in object_property_set_li= nk -> > object_get_canonical_path since CPU would not have parent a that time. > Wouldn't it better to make it child in CPU's initfn? This way CPU objec= t > could be used as a value for link anywhere once it's been created. I've seen that issue in your series. This is touching on a core QOM question: Can we link<> during initfn? That's what you're trying to do for the APIC and why this may be too late in your case. I believe the answer to that question must be no. >>From what I understand about the x86 modeling, the only case this matters is if you hot-unplug CPU 0, right? Question is, what happens with the APIC then? I would guess the remaining n-1 CPUs still want to access the APIC - then it would need to stay and if CPU 0 is hot-replugged it would not need to be recreated but reconnected. The alternative would be that CPU 0 cannot be hot-unplugged at all, in which case the APIC would be irrelevant to hot-plugging and the remodelling would be moot; or all remaining CPUs would suddenly loose the APIC feature on hot-unplug of CPU 0. Again, I don't know how this works in hardware. Another factor that is making this slightly difficult is that there are three APIC subclasses. Currently they all have an instance_size of sizeof(APICCommonState) so it could be created in-place if it actually is a part (child<>) of the CPU wrt hot-plug. Creating objects with object_new() in QOM instance_init is forbidden. Also I have a broader view than the PC in mind: Depending on whether you have a mainboard with CPU sockets or some SoC or module, you may desire different modelings. My above modeling is for a PC, in hw/pc.c, using /machine/cpu[n]. For a QSeven module the parent would be the Container or type for the module, e.g. /machine/qseven/cpu[n]. Another example might be AMD Fusion. Therefore I think that tying the modeling to the CPU initfn is conceptually wrong. Maybe this CPU hot-plug business would be a good topic for a KVM call? Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg