From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34272) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVKHj-0004J9-AU for qemu-devel@nongnu.org; Tue, 10 Mar 2015 09:33:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVKHe-00041n-C9 for qemu-devel@nongnu.org; Tue, 10 Mar 2015 09:33:23 -0400 Received: from cantor2.suse.de ([195.135.220.15]:36560 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVKHe-00041f-5r for qemu-devel@nongnu.org; Tue, 10 Mar 2015 09:33:18 -0400 Message-ID: <54FEF29D.9070405@suse.de> Date: Tue, 10 Mar 2015 14:33:17 +0100 From: =?windows-1252?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1425576411-32639-1-git-send-email-ehabkost@redhat.com> <1425576411-32639-2-git-send-email-ehabkost@redhat.com> <54FEEFF9.9040307@suse.de> <20150310133057.GB3513@thinpad.lan.raisama.net> In-Reply-To: <20150310133057.GB3513@thinpad.lan.raisama.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/1] target-i386: Remove icc_bridge parameter from cpu_x86_create() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: zhugh.fnst@cn.fujitsu.com, "Michael S. Tsirkin" , qemu-devel@nongnu.org, tangchen@cn.fujitsu.com, Gu Zheng , isimatu.yasuaki@jp.fujitsu.com, Igor Mammedov , chen.fan.fnst@cn.fujitsu.com, Paolo Bonzini , anshul.makkar@profitbricks.com Am 10.03.2015 um 14:30 schrieb Eduardo Habkost: > On Tue, Mar 10, 2015 at 02:22:01PM +0100, Andreas F=E4rber wrote: >> Am 05.03.2015 um 18:26 schrieb Eduardo Habkost: >>> Instead of passing icc_bridge from the PC initialization code to >>> cpu_x86_create(), make the PC initialization code attach the CPU to >>> icc_bridge. >>> >>> The only difference here is that icc_bridge attachment will now be do= ne >>> after x86_cpu_parse_featurestr() is called. But this shouldn't make a= ny >>> difference, as property setters shouldn't depend on icc_bridge. >>> >>> Signed-off-by: Eduardo Habkost >>> --- >>> hw/i386/pc.c | 6 +++++- >>> target-i386/cpu.c | 14 ++------------ >>> target-i386/cpu.h | 3 +-- >>> 3 files changed, 8 insertions(+), 15 deletions(-) >>> >>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >>> index ed54d93..66b9fa6 100644 >>> --- a/hw/i386/pc.c >>> +++ b/hw/i386/pc.c >>> @@ -995,12 +995,16 @@ static X86CPU *pc_new_cpu(const char *cpu_model= , int64_t apic_id, >>> X86CPU *cpu; >>> Error *local_err =3D NULL; >>> =20 >>> - cpu =3D cpu_x86_create(cpu_model, icc_bridge, &local_err); >>> + cpu =3D cpu_x86_create(cpu_model, &local_err); >>> if (local_err !=3D NULL) { >>> error_propagate(errp, local_err); >>> return NULL; >>> } >>> =20 >>> + assert(icc_bridge); >> >> On second thoughts, why are you asserting here rather than setting err= p? >> Just add an out: below and goto out, like I did. >> >> On startup it doesn't matter much, but for hot-add asserting would not >> be so nice. >=20 > Because not having icc_bus passed as argument would be a coding error. >=20 > Also, I have no idea what kind of things would break if we destroy a CP= U > after cpu_exec_init() was already called in instance_init. Then do it before cpu_x86_create()! :) Also, every memory allocation failure can result in an assertion (which is why I'm trying to cut down on their number). Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Felix Imend=F6rffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG N=FCrnberg)