From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49895) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UHktv-0007Uw-Tp for qemu-devel@nongnu.org; Mon, 18 Mar 2013 20:59:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UHkto-0007pA-CV for qemu-devel@nongnu.org; Mon, 18 Mar 2013 20:59:39 -0400 Date: Tue, 19 Mar 2013 11:52:49 +1100 From: David Gibson Message-ID: <20130319005249.GN9402@truffula.fritz.box> 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> <5146F614.5060406@suse.de> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="GOzekVbrLdOLv44p" Content-Disposition: inline In-Reply-To: <5146F614.5060406@suse.de> 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: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: "qemu-ppc@nongnu.org list:PowerPC" , qemu-devel qemu-devel --GOzekVbrLdOLv44p Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Mar 18, 2013 at 12:10:12PM +0100, Andreas F=E4rber wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 >=20 > 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. >=20 > 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. >=20 > As I just replied to Alex, the question to ask is whether you want the > user to fiddle with this value or not. User to fiddle, probably not. But I was thinking of submodel specific conditionals in the init_proc function. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --GOzekVbrLdOLv44p Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iEYEARECAAYFAlFHtuEACgkQaILKxv3ab8ZRWQCeOvLrjgZG40rpBkTxp2ZmNoKC Pa8An0J6LS3CznnJorA6AFMotT8arYnc =ejOS -----END PGP SIGNATURE----- --GOzekVbrLdOLv44p--