From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50355) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YmoK0-0001hV-Qm for qemu-devel@nongnu.org; Mon, 27 Apr 2015 15:04:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YmoJv-0001PD-6W for qemu-devel@nongnu.org; Mon, 27 Apr 2015 15:04:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60894) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YmoJv-0001P5-1C for qemu-devel@nongnu.org; Mon, 27 Apr 2015 15:03:55 -0400 Date: Mon, 27 Apr 2015 21:03:51 +0200 From: "Michael S. Tsirkin" Message-ID: <20150427210338-mutt-send-email-mst@redhat.com> References: <1426024655-17561-2-git-send-email-ehabkost@redhat.com> <1426610796-27439-1-git-send-email-afaerber@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1426610796-27439-1-git-send-email-afaerber@suse.de> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for-next] pc: Ensure non-zero CPU ref count after attaching to ICC bus List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: Paolo Bonzini , Richard Henderson , qemu-devel@nongnu.org, ehabkost@redhat.com On Tue, Mar 17, 2015 at 05:46:36PM +0100, Andreas F=E4rber wrote: > Setting the parent bus of a device increases its ref count, which we > ultimately want to level out. However it is only safe to do so after th= e > last reference to the device in local code, as qom-set or similar opera= tions > might decrease the ref count. >=20 > Therefore move the object_unref() from pc_new_cpu() into its callers. >=20 > The APIC operations on the last CPU in pc_cpus_init() are still potenti= ally > insecure, but that is beyond the scope of this code movement. >=20 > Signed-off-by: Andreas F=E4rber Acked-by: Michael S. Tsirkin Feel free to merge through the x86 tree. > --- > hw/i386/pc.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) >=20 > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 4b46c29..84aa174 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1007,7 +1007,6 @@ static X86CPU *pc_new_cpu(const char *cpu_model, = int64_t apic_id, > } > =20 > qdev_set_parent_bus(DEVICE(cpu), qdev_get_child_bus(icc_bridge, "i= cc")); > - object_unref(OBJECT(cpu)); > =20 > object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_er= r); > object_property_set_bool(OBJECT(cpu), true, "realized", &local_err= ); > @@ -1026,7 +1025,9 @@ static const char *current_cpu_model; > void pc_hot_add_cpu(const int64_t id, Error **errp) > { > DeviceState *icc_bridge; > + X86CPU *cpu; > int64_t apic_id =3D x86_cpu_apic_id_from_index(id); > + Error *local_err =3D NULL; > =20 > if (id < 0) { > error_setg(errp, "Invalid CPU id: %" PRIi64, id); > @@ -1054,7 +1055,12 @@ void pc_hot_add_cpu(const int64_t id, Error **er= rp) > =20 > icc_bridge =3D DEVICE(object_resolve_path_type("icc-bridge", > TYPE_ICC_BRIDGE, NULL= )); > - pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp); > + cpu =3D pc_new_cpu(current_cpu_model, apic_id, icc_bridge, &local_= err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + object_unref(OBJECT(cpu)); > } > =20 > void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) > @@ -1088,6 +1094,7 @@ void pc_cpus_init(const char *cpu_model, DeviceSt= ate *icc_bridge) > error_report_err(error); > exit(1); > } > + object_unref(OBJECT(cpu)); > } > =20 > /* map APIC MMIO area if CPU has APIC */ > --=20 > 2.1.4