From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33826) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TK9te-00032G-Nh for qemu-devel@nongnu.org; Fri, 05 Oct 2012 11:33:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TK9td-0001cg-1A for qemu-devel@nongnu.org; Fri, 05 Oct 2012 11:33:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37357) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TK9tc-0001cL-P8 for qemu-devel@nongnu.org; Fri, 05 Oct 2012 11:33:00 -0400 Date: Fri, 5 Oct 2012 17:32:54 +0200 From: Igor Mammedov Message-ID: <20121005173254.0dcbf5a8@nial.usersys.redhat.com> In-Reply-To: <506DB634.70001@suse.de> References: <1349361818-15331-1-git-send-email-imammedo@redhat.com> <506DB634.70001@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] target-i386: initialize APIC at CPU level List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?ISO-8859-1?B?RuRyYmVy?= Cc: aliguori@us.ibm.com, ehabkost@redhat.com, gleb@redhat.com, Don@CloudSwitch.com, qemu-devel@nongnu.org, jan.kiszka@web.de On Thu, 04 Oct 2012 18:15:48 +0200 Andreas F=E4rber wrote: > > + env->apic_state =3D qdev_create(NULL, apic_type); > > + > > + if (env->apic_state =3D=3D NULL) { > > + error_set(errp, QERR_DEVICE_INIT_FAILED, apic_type); > > + return; > > + } > > + > > + object_property_add_child(OBJECT(cpu), "apic", > > + OBJECT(env->apic_state), NULL); > > + qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id); > > + /* TODO: convert to link<> */ > > + qdev_prop_set_ptr(env->apic_state, "cpu_env", env); >=20 > Do we still need env->apic_state and a property on the APIC at all? From > the X86CPU side we can access the "apic" property to retrieve the > pointer, It would introduce too much overhead. For test I've just substituted all us= es of env->apic_state with a func that could serve as get_child(name) +DeviceState *get_apic(CPUX86State *env) { + X86CPU *cpu =3D x86_env_get_cpu(env); + Object *o =3D object_resolve_path_component(OBJECT(cpu), (gchar *)"api= c"); + DeviceState *dev =3D OBJECT_CHECK(DeviceState, o, "device"); + return dev;=20 +} and run qemu under valgrind --tool=3Dcallgrind, booting openbsd for 5 min. hot path get_apic() call count get_apic() incl cost.=20 TCG: pic_irq_request() 7M 44 (8th from top 10) KVM: kvm_arch_post_run() 2M 38 (6th from top 10) NO PATCH: pic_irq_request() 18M 0.8 pic_irq_request() calls get_apic() 3x, kvm_arch_post_run() - 2x times I suppose calling get_apic() only once on hot path could reduce amount of calls but property searching and dynamic cast won't disappear any way. Looks like it's better to leave apic_state as it is. > and from the APIC we should be able to navigate to its parent, > right? while adding object_get_parent(), I've noticed in object.h " * There is no way for a child to determine what its parent is. It is not * a bidirectional relationship. This is by design. " Would be it acceptable to allow child access to parent? ... >=20 > Andreas >=20 Thanks, Igor