From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48920) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UxEEz-0000my-DX for qemu-devel@nongnu.org; Thu, 11 Jul 2013 06:36:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UxEEy-0005Q2-4w for qemu-devel@nongnu.org; Thu, 11 Jul 2013 06:36:49 -0400 Received: from cantor2.suse.de ([195.135.220.15]:39389 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UxEEx-0005Nd-R9 for qemu-devel@nongnu.org; Thu, 11 Jul 2013 06:36:48 -0400 Message-ID: <51DE8ABA.7030605@suse.de> Date: Thu, 11 Jul 2013 12:36:42 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <20130711114759.191f9ed0@nial.usersys.redhat.com> <20130711103115.GB24118@redhat.com> In-Reply-To: <20130711103115.GB24118@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH qom-next v2 3/5] target-arm: Use parent classes for reset + realize List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , peter.crosthwaite@xilinx.com Cc: Igor Mammedov , aliguori@us.ibm.com, qemu-devel@nongnu.org, hutao@cn.fujitsu.com Am 11.07.2013 12:31, schrieb Michael S. Tsirkin: > On Thu, Jul 11, 2013 at 11:47:59AM +0200, Igor Mammedov wrote: >> On Thu, 11 Jul 2013 11:47:16 +1000 >> peter.crosthwaite@xilinx.com wrote: >> >>> From: Peter Crosthwaite >>> >>> ARMCPUClass is only needed for parent-class abstract function access. >>> Just use parent classes for reset and realize access and remove >>> ARMCPUClass completely. >>> >>> Signed-off-by: Peter Crosthwaite >>> --- >>> >>> target-arm/cpu-qom.h | 20 -------------------- >>> target-arm/cpu.c | 16 +++++++--------- >>> 2 files changed, 7 insertions(+), 29 deletions(-) >>> >>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h >>> index ef6261f..bdad93a 100644 >>> --- a/target-arm/cpu-qom.h >>> +++ b/target-arm/cpu-qom.h >>> @@ -24,28 +24,8 @@ >>> =20 >>> #define TYPE_ARM_CPU "arm-cpu" >>> =20 >>> -#define ARM_CPU_CLASS(klass) \ >>> - OBJECT_CLASS_CHECK(ARMCPUClass, (klass), TYPE_ARM_CPU) >>> #define ARM_CPU(obj) \ >>> OBJECT_CHECK(ARMCPU, (obj), TYPE_ARM_CPU) >>> -#define ARM_CPU_GET_CLASS(obj) \ >>> - OBJECT_GET_CLASS(ARMCPUClass, (obj), TYPE_ARM_CPU) >>> - >>> -/** >>> - * ARMCPUClass: >>> - * @parent_realize: The parent class' realize handler. >>> - * @parent_reset: The parent class' reset handler. >>> - * >>> - * An ARM CPU model. >>> - */ >>> -typedef struct ARMCPUClass { >>> - /*< private >*/ >>> - CPUClass parent_class; >>> - /*< public >*/ >>> - >>> - DeviceRealize parent_realize; >>> - void (*parent_reset)(CPUState *cpu); >>> -} ARMCPUClass; >>> =20 >>> /** >>> * ARMCPU: >>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c >>> index ed53df8..ad5ec7b 100644 >>> --- a/target-arm/cpu.c >>> +++ b/target-arm/cpu.c >>> @@ -60,7 +60,8 @@ static void cp_reg_reset(gpointer key, gpointer val= ue, gpointer opaque) >>> static void arm_cpu_reset(CPUState *s) >>> { >>> ARMCPU *cpu =3D ARM_CPU(s); >>> - ARMCPUClass *acc =3D ARM_CPU_GET_CLASS(cpu); >>> + CPUClass *cc_parent =3D >>> + CPU_CLASS(object_class_get_parent_by_name(TYPE_ARM_CPU))= ; >> Maybe object_class_get_parent_of_type() would be less confusing? >> >> This usage assumes that parent of TYPE_ARM_CPU is TYPE_CPU and if >> another TYPE_X added between them, it might break if TYPE_X doesn't >> re-implement this logic in its reset. >=20 > If what's needed is TYPE_CPU, you can just look up the class by name. My suggestion was to hide the implementation details behind an ARM_CPU_GET_PARENT_CLASS(obj) macro. That would at the same time allow to later switch to an iterative approach as seen in v1 without a whole lot of refactoring. Andreas >=20 >> Could reset be modeled like DEVICE.instance_init() instead? Then >> no explicit access to parent from child would be needed and it >> still leaves possibility to override resets if parent->child >> propagation order is not desirable for a particular device. >> >>> CPUARMState *env =3D &cpu->env; >>> =20 >>> if (qemu_loglevel_mask(CPU_LOG_RESET)) { >>> @@ -68,7 +69,7 @@ static void arm_cpu_reset(CPUState *s) >>> log_cpu_state(env, 0); >>> } >>> =20 >>> - acc->parent_reset(s); >>> + cc_parent->reset(s); >>> =20 >>> memset(env, 0, offsetof(CPUARMState, breakpoints)); >>> g_hash_table_foreach(cpu->cp_regs, cp_reg_reset, cpu); >>> @@ -158,7 +159,8 @@ static void arm_cpu_finalizefn(Object *obj) >>> static void arm_cpu_realizefn(DeviceState *dev, Error **errp) >>> { >>> ARMCPU *cpu =3D ARM_CPU(dev); >>> - ARMCPUClass *acc =3D ARM_CPU_GET_CLASS(dev); >>> + DeviceClass *dc_parent =3D >>> + DEVICE_CLASS(object_class_get_parent_by_name(TYPE_ARM_CP= U)); >>> CPUARMState *env =3D &cpu->env; >>> =20 >>> /* Some features automatically imply others: */ >>> @@ -209,7 +211,7 @@ static void arm_cpu_realizefn(DeviceState *dev, E= rror **errp) >>> =20 >>> cpu_reset(CPU(cpu)); >>> =20 >>> - acc->parent_realize(dev, errp); >>> + dc_parent->realize(dev, errp); >>> } >>> =20 >>> /* CPU models */ >>> @@ -803,14 +805,11 @@ static const ARMCPUInfo arm_cpus[] =3D { >>> =20 >>> static void arm_cpu_class_init(ObjectClass *oc, void *data) >>> { >>> - ARMCPUClass *acc =3D ARM_CPU_CLASS(oc); >>> - CPUClass *cc =3D CPU_CLASS(acc); >>> + CPUClass *cc =3D CPU_CLASS(oc); >>> DeviceClass *dc =3D DEVICE_CLASS(oc); >>> =20 >>> - acc->parent_realize =3D dc->realize; >>> dc->realize =3D arm_cpu_realizefn; >>> =20 >>> - acc->parent_reset =3D cc->reset; >>> cc->reset =3D arm_cpu_reset; >>> =20 >>> cc->class_by_name =3D arm_cpu_class_by_name; >>> @@ -839,7 +838,6 @@ static const TypeInfo arm_cpu_type_info =3D { >>> .instance_init =3D arm_cpu_initfn, >>> .instance_finalize =3D arm_cpu_finalizefn, >>> .abstract =3D true, >>> - .class_size =3D sizeof(ARMCPUClass), >>> .class_init =3D arm_cpu_class_init, >>> }; >>> =20 --=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