From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54787) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUIMU-0005Mc-Ac for qemu-devel@nongnu.org; Mon, 22 Apr 2013 11:09:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UUIMS-0002fa-39 for qemu-devel@nongnu.org; Mon, 22 Apr 2013 11:08:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46184) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UUIMR-0002fT-S1 for qemu-devel@nongnu.org; Mon, 22 Apr 2013 11:08:56 -0400 Date: Mon, 22 Apr 2013 17:08:42 +0200 From: Igor Mammedov Message-ID: <20130422170842.07536008@nial.usersys.redhat.com> In-Reply-To: <51753996.5050202@suse.de> References: <1366063976-4909-1-git-send-email-imammedo@redhat.com> <1366063976-4909-12-git-send-email-imammedo@redhat.com> <51753996.5050202@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 11/16] introduce ICC bus/device/bridge 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, mst@redhat.com, jan.kiszka@siemens.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, aderumier@odiso.com, lcapitulino@redhat.com, jfrei@linux.vnet.ibm.com, yang.z.zhang@intel.com, pbonzini@redhat.com, lig.fnst@cn.fujitsu.com, rth@twiddle.net On Mon, 22 Apr 2013 15:22:30 +0200 Andreas F=E4rber wrote: > Am 16.04.2013 00:12, schrieb Igor Mammedov: > > ... to provide hotplug-able bus. > >=20 > > * icc-bridge will serve as a parent for icc-bus and provide > > mmio mapping services to child icc-devices. > > * icc-device will replace SysBusDevice as a parent of APIC > > and IOAPIC devices. > >=20 > > Signed-off-by: Igor Mammedov > > --- > > v2: > > * Rebase on top the last HW reorganization series. > > * Move hw/icc_bus.c into hw/cpu/ and hw/icc_bus.h include/hw/i386/ >=20 > http://wiki.qemu.org/QOMConventions >=20 > Comments below... >=20 [...] > > + > > +static void icc_device_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *k =3D DEVICE_CLASS(klass); > > + > > + k->init =3D icc_device_init; >=20 > Please use k->realize for any new base device type or type derived from > SysBusDevice. >=20 > If there's nothing to do at this level, you can just provide a dummy > realize function here to override Device's and override it (without > saving the parent's dummy function) in IOAPIC or wherever you need it. > I'll try it, and if changes isn't big, I'll merge into this patch. > > + k->bus_type =3D TYPE_ICC_BUS; > > +} > > + > > +static const TypeInfo icc_device_info =3D { > > + .name =3D TYPE_ICC_DEVICE, > > + .parent =3D TYPE_DEVICE, > > + .abstract =3D true, > > + .instance_size =3D sizeof(ICCDevice), > > + .class_size =3D sizeof(ICCDeviceClass), > > + .class_init =3D icc_device_class_init, > > +}; > > + > > +typedef struct ICCBridgeState { > > + SysBusDevice busdev; >=20 > parent_obj please - makes it clear that it is a QOM struct and flags > accidental uses of obsolete SysBus macros. done >=20 > > +} ICCBridgeState; > > +#define ICC_BRIGDE(obj) OBJECT_CHECK(ICCBridgeState, (obj), > > TYPE_ICC_BRIDGE) + > > + > > +static void icc_bridge_initfn(Object *obj) > > +{ > > + qbus_create(TYPE_ICC_BUS, DEVICE(obj), "icc-bus"); >=20 > qbus_create_inplace()? done =20 [...] >=20 > BTW Anthony recently suggested to drop the "fn" where there's no init > vs. initfn name conflicts; I don't mind either way. changed to init =20 [...] >=20 > /*< private >*/ >=20 > > + BusState qbus; >=20 > parent_obj done >=20 > /*< public >*/ >=20 > ...since it is in a header. >=20 > > +} ICCBus; > > +#define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS) > > + > > +typedef struct ICCDevice { > > + DeviceState qdev; >=20 > dito s/qdev/parent_obj/ will affect APIC and IOAPIC patches make-ing them even bigger and more difficult to review distracting from real changes. it would be better to send follow-up patch, that does trivial QOM conversio= ns afterwards. I'm not sure that making ICCDevice.qdev private would be good performance w= ise since it would require using DEVICE() dynamic cast in apic_check_pic(). I'd like to keep this as is for now and put cleanup patch on TODO list for 1.6 to get rid of DO_UPCAST() macro in apic/ioapic files. >=20 > Who becomes the MAINTAINER of ICC? Perhaps me?? >=20 > Regards, > Andreas >=20