From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:46537) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7k2q-0004IL-IC for qemu-devel@nongnu.org; Wed, 14 Mar 2012 04:59:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S7k2o-0000Y4-1Y for qemu-devel@nongnu.org; Wed, 14 Mar 2012 04:58:56 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:10860) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7k2n-0000Si-OB for qemu-devel@nongnu.org; Wed, 14 Mar 2012 04:58:53 -0400 Received: from euspt1 (mailout1.w1.samsung.com [210.118.77.11]) by mailout1.w1.samsung.com (iPlanet Messaging Server 5.2 Patch 2 (built Jul 14 2004)) with ESMTP id <0M0V00JP0A9U19@mailout1.w1.samsung.com> for qemu-devel@nongnu.org; Wed, 14 Mar 2012 08:58:42 +0000 (GMT) Received: from [106.109.8.162] by spt1.w1.samsung.com (iPlanet Messaging Server 5.2 Patch 2 (built Jul 14 2004)) with ESMTPA id <0M0V00DQ8A9ZAL@spt1.w1.samsung.com> for qemu-devel@nongnu.org; Wed, 14 Mar 2012 08:58:48 +0000 (GMT) Date: Wed, 14 Mar 2012 12:58:46 +0400 From: Igor Mitsyanko In-reply-to: <4F5F8AE3.9080302@suse.de> Message-id: <4F605DC6.9050601@samsung.com> MIME-version: 1.0 Content-type: multipart/alternative; boundary="Boundary_(ID_S7otK3KOQf9JIRuEsaROSg)" References: <1330893156-26569-1-git-send-email-afaerber@suse.de> <1331398436-20761-1-git-send-email-afaerber@suse.de> <1331398436-20761-3-git-send-email-afaerber@suse.de> <4F5F3E07.9070706@samsung.com> <4F5F8AE3.9080302@suse.de> Subject: Re: [Qemu-devel] [PATCH RFC v4 02/20] target-arm: Introduce QOM ARMCPUClass Reply-To: i.mitsyanko@samsung.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= Cc: Peter Maydell , Paul Brook , qemu-devel@nongnu.org, Anthony Liguori , Dmitry Solodkiy This is a multi-part message in MIME format. --Boundary_(ID_S7otK3KOQf9JIRuEsaROSg) Content-type: text/plain; format=flowed; charset=UTF-8 Content-transfer-encoding: QUOTED-PRINTABLE On 03/13/2012 09:58 PM, Andreas F=C3=A4rber wrote: > Am 13.03.2012 13:31, schrieb Igor Mitsyanko: >> On 03/10/2012 08:53 PM, Andreas F=C3=A4rber wrote: >>> diff --git a/target-arm/cpu.c b/target-arm/cpu.c >>> new file mode 100644 >>> index 0000000..dabc094 >>> --- /dev/null >>> +++ b/target-arm/cpu.c > [...] >>> +static void cpu_register(const ARMCPUInfo *info) >>> +{ >>> + TypeInfo type =3D { >>> + .name =3D info->name, >>> + .parent =3D TYPE_ARM_CPU, >>> + .instance_size =3D sizeof(ARMCPU), >>> + .class_size =3D sizeof(ARMCPUClass), >>> + .class_init =3D arm_cpu_class_init, >>> + .class_data =3D (void *)info, >>> + }; >> Are non-initialized members guaranteed to be zero here? > I thought so for the C99-style struct initialization... I never ran= into > crashes while testing. Do we need static to be safe? > >>> + type_register_static(&type); >>> +} >>> + >> Probably should be type_register() here in case these two will act= ually >> differ in the future. > My thinking was we don't need it here because the data (esp. string= s) > are not dynamically allocated. By comparison, I used type_register(= ) for > -cpudef in target-i386, I believe. > > But I really guess it's a bug that they're just an alias right now!= ;) > Object.h states we need to use type_register_static() only when TypeI= nfo=20 exists for the lifetime of Type. As I understand this, TypeInfo must have *static const* storage= =20 qualifier when used with type_register_static() and can have automati= c=20 storage qualifier when used with type_register(). Maybe in the future type_register_static() will just copy a pointer t= o=20 string variables of passed TypeInfo instead of g_strdup() them, then your code wouldn't work as= =20 expected. >> If this information is of any help, we've got no problems when emu= lating >> ARM-based Exynos boards in QEMU with this whole patchset applied. > Thanks a lot for testing! > > Have you thought about how to QOM'ify your boards? Mid-term I'd lik= e to > see an "exynos4210" object with the CPUs on it - maybe "cpu[0]" and > "cpu[1]" child properties? Or "core[x]"? I had played with the sh77= 50 a > bit on my branch but like the arm926 it's a single-core. > > Andreas > Yes, we've done some work on this, but we are waiting to see how you= =20 want to proceed with SoC QOM type, and if you're going to introduce it at all. Personally I ha= ve=20 no idea what members and methods a general SoC type could have. And another question, should we somehow exploit a fact that majority = of=20 peripheral devices on SoCs share i/o ports with each other and only a handful of them are= =20 actually active at the same time. Currently we instantiate all SoC devices even if half of them a= re=20 not used in emulated configuration. Maybe we should dynamically=20 create/destroy devices depending on runtime GPIO configuration, or make all devices a board's child instead of SoC's= =20 child. Not sure that it's worth an effort though. As for CPU's object, Peter's idea with CortexA9 and core1, core2 chil= ds=20 looks good to me. --=20 Mitsyanko Igor ASWG, Moscow R&D center, Samsung Electronics email: i.mitsyanko@samsung.com --Boundary_(ID_S7otK3KOQf9JIRuEsaROSg) Content-type: text/html; charset=UTF-8 Content-transfer-encoding: QUOTED-PRINTABLE

On 03/13/2012 09:58 PM, Andreas F=C3=A4rber wrote: 
Am 13.03.2012 13:31, schrieb Igor Mitsyanko:

On 03/10/2012 08:53 PM, Andreas F=C3=A4rber wr=
ote:

diff --git a/target-arm/cpu.c b/target-arm/c=
pu.c
new file mode 100644
index 0000000..dabc094
--- /dev/null
+++ b/target-arm/cpu.c

[...]

+static void cpu_register(const ARMCPUInfo *=
info)
+{
+    TypeInfo type =3D {
+        .name =3D info->name,
+        .parent =3D TYPE_ARM_CPU,
+        .instance_size =3D sizeof(ARMCPU),
+        .class_size =3D sizeof(ARMCPUClass),
+        .class_init =3D arm_cpu_class_init,
+        .class_data =3D (void *)info,
+    };

Are non-initialized members guaranteed to be zero here?

I thought so for the C99-style struct initialization... I never ran i=
nto
crashes while testing. Do we need static to be safe?


+    type_register_static(&type);
+}
+

Probably should be type_register() here in case these two will actual=
ly
differ in the future.

My thinking was we don't need it here because the data (esp. strings)
are not dynamically allocated. By comparison, I used type_register() =
for
-cpudef in target-i386, I believe.

But I really guess it's a bug that they're just an alias right now! ;=
)

Object.h states we need to use type_register_static() only when TypeInfo exists for the lifetime of
Type. As I understand this, TypeInfo must have static const storage qualifier when used with type_register_static() and can h= ave automatic storage qualifier when used with type_register().
Maybe in the future type_register_static() will just copy a point= er to string variables of passed
TypeInfo instead of g_strdup() them, then your code wouldn't work= as expected.

If this information is of any help, we've got =
no problems when emulating
ARM-based Exynos boards in QEMU with this whole patchset applied.

Thanks a lot for testing!

Have you thought about how to QOM'ify your boards? Mid-term I'd like =
to
see an "exynos4210" object with the CPUs on it - maybe "cpu[0]" and
"cpu[1]" child properties? Or "core[x]"? I had played with the sh7750=
 a
bit on my branch but like the arm926 it's a single-core.

Andreas
Yes, we've done some work on this, but we are waiting to see how = you want to proceed with SoC
QOM type, and if you're going to introduce it at all. Personally = I have no idea what members and
methods a general SoC type could have.
And another question, should we somehow exploit a fact that major= ity of peripheral devices on
SoCs share i/o ports with each other and only a handful of them a= re actually active at the same
time. Currently we instantiate all SoC devices even if half of th= em are not used in emulated configuration. Maybe we should dynamical= ly create/destroy devices depending on runtime GPIO
configuration, or make all devices a board's child instead of SoC= 's child. Not sure that it's worth an effort though.

As for CPU's object, Peter's idea with CortexA9 and core1, core2 childs looks good to me. 
--=20
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsyanko@samsung.com
--Boundary_(ID_S7otK3KOQf9JIRuEsaROSg)--