From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58880) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UHXxJ-0001gn-Ss for qemu-devel@nongnu.org; Mon, 18 Mar 2013 07:10:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UHXxG-0002S3-Ma for qemu-devel@nongnu.org; Mon, 18 Mar 2013 07:10:17 -0400 Message-ID: <5146F614.5060406@suse.de> Date: Mon, 18 Mar 2013 12:10:12 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1363226008-26639-1-git-send-email-david@gibson.dropbear.id.au> <1363226008-26639-4-git-send-email-david@gibson.dropbear.id.au> <20130316071007.GA9402@truffula.fritz.box> In-Reply-To: <20130316071007.GA9402@truffula.fritz.box> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/5] pseries: Fixes and enhancements to L1 cache properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: "qemu-ppc@nongnu.org list:PowerPC" , Alexander Graf , qemu-devel qemu-devel -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 16.03.2013 08:10, schrieb David Gibson: > On Fri, Mar 15, 2013 at 01:27:09PM +0100, Alexander Graf wrote: >> On 14.03.2013, at 02:53, David Gibson wrote: >>=20 >>> PAPR requires that the device tree's CPU nodes have several >>> properties with information about the L1 cache. We already >>> create two of these properties, but with incorrect names - >>> "[id]cache-block-size" instead of "[id]-cache-block-size" (note >>> the extra hyphen). >>>=20 >>> We were also missing some of the required cache properties. >>> This patch adds the [id]-cache-line-size properties (which have >>> the same values as the block size properties in all current >>> cases). We also add the [id]-cache-size properties. >>>=20 >>> Adding the cache sizes requires some extra infrastructure in >>> the general target-ppc code to (optionally) set the cache sizes >>> for various CPUs. The CPU family descriptions in >>> translate_init.c can set these sizes - this patch adds correct >>> information for POWER7, I'm leaving other CPU types to people >>> who have a physical example to verify against. In addition, >>> for -cpu host we take the values advertised by the host (if >>> available) and use those to override the information based on >>> PVR. >>>=20 >>> Signed-off-by: David Gibson ---=20 >>> hw/ppc/spapr.c | 20 ++++++++++++++++++--=20 >>> target-ppc/cpu.h | 1 + target-ppc/kvm.c >>> | 39 +++++++++++++++++++++++++++++++++++++++=20 >>> target-ppc/translate_init.c | 4 ++++ 4 files changed, 62 >>> insertions(+), 2 deletions(-) >>>=20 >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index >>> 9a13697..7293082 100644 --- a/hw/ppc/spapr.c +++ >>> b/hw/ppc/spapr.c @@ -333,10 +333,26 @@ static void >>> *spapr_create_fdt_skel(const char *cpu_model,=20 >>> _FDT((fdt_property_string(fdt, "device_type", "cpu"))); >>>=20 >>> _FDT((fdt_property_cell(fdt, "cpu-version", >>> env->spr[SPR_PVR]))); - _FDT((fdt_property_cell(fdt, >>> "dcache-block-size", + _FDT((fdt_property_cell(fdt, >>> "d-cache-block-size", env->dcache_line_size))); - >>> _FDT((fdt_property_cell(fdt, "icache-block-size", + >>> _FDT((fdt_property_cell(fdt, "d-cache-line-size", + >>> env->dcache_line_size))); + _FDT((fdt_property_cell(fdt, >>> "i-cache-block-size", + >>> env->icache_line_size))); + _FDT((fdt_property_cell(fdt, >>> "i-cache-line-size", env->icache_line_size))); + + if >>> (env->l1_dcache_size) { + >>> _FDT((fdt_property_cell(fdt, "d-cache-size", >>> env->l1_dcache_size))); + } else { + >>> fprintf(stderr, "Warning: Unknown L1 dcache size for cpu\n"); + >>> } + if (env->l1_icache_size) { + >>> _FDT((fdt_property_cell(fdt, "i-cache-size", >>> env->l1_icache_size))); + } else { + >>> fprintf(stderr, "Warning: Unknown L1 icache size for cpu\n"); + >>> } >>=20 >> The L1 sizes should come from the class, not env, right? >> Andreas, any ideas on this? >=20 > Well.. initially I was going to put them in class. But then it=20 > occurred to me that the class represents a family of similar CPUs, > not a single precise CPU model. Total cache sizes are the sort of > thing that could easily vary between minor revisions, although I > don't know if they have in practice. Actually that is irrelevant: As it stands, we can do neither model-specific instance_init nor class_init due to the old POWERPC_DEF_SVR() macros. Adding support for either seems equally invasive. As I just replied to Alex, the question to ask is whether you want the user to fiddle with this value or not. Andreas > So I thought it was safer to put these in env. - --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=EF=BF=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=EF=BF=B6rffer; HRB 16746 AG N=EF= =BF=BCrnberg -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) iQIcBAEBAgAGBQJRRvYUAAoJEPou0S0+fgE/TuUP/2ZqXQw9klNYkkKfeA8lml8R jX6NsdIGDxAh8BQMea5YEVfTUUqHa5ygO4HMSBT42GvWR4gY7lmDO08d2FrtO3Oz ybWf1OubcNm3rp9vwtGKKWxWAiMBSjv9KqAR2gYd305e15ADr3wzRPRPyUtOXO6r ute+snXu3rI49wCngAF3zBhQnq+HLyUcLpMn/ss/p0086e5LaPhPjEjUDRoKLED6 xqjG3x65rknpWdRPWtpZJHEhPfuDBKpZTS0XdPUkBQ48F2D0yZ3SbRHfIjnikEtV G43RXBYAUK449HmJY0/hz5W51Nm81f1Sg04uXDLsG9K7mBNUcw/TxgjzW+7ByVyk +zjCHsC0aimX7owIgMPNd8wtWXmxs4JxdC7icjDtZUNKUSFHC6plMPInlZdIkTU5 brJq02I16lQCYHfnFkFdUM+pwV7r4B0FtymirEX3G8l3n5ddoFmj4tUmLPdjxcHL Dzn5LMOGNsvL0hqf5eW5JCcq4FMGIYKkeo839+p2u4nnoYSpNIB7mseZzzTTGEYH HLMTXRijpX9Po65fMgPkiyv9Mv3w2N7UjjP2E9a2VJk2oRsEMO0GXrV3OUJBD64U 3qmB+xvjLgijrDvlYTWvZtOhqIRuqjtS+La2qxtbWE8Mf3YYZ1UxVFPd82qQf0zw luXozSihIlcx55xtw9Lr =3DejJS -----END PGP SIGNATURE-----