From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58744) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TlX2Q-0005vg-Bm for qemu-devel@nongnu.org; Wed, 19 Dec 2012 22:43:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TlX2L-0005i9-P4 for qemu-devel@nongnu.org; Wed, 19 Dec 2012 22:43:14 -0500 Date: Thu, 20 Dec 2012 14:38:33 +1100 From: David Gibson Message-ID: <20121220033833.GA31560@truffula.fritz.box> References: <1354588937-27122-1-git-send-email-david@gibson.dropbear.id.au> <1354588937-27122-12-git-send-email-david@gibson.dropbear.id.au> <4725D8CD-CF04-4405-BA8F-0AA99857A95E@suse.de> <20121217023253.GC32451@truffula.fritz.box> <32055E46-52FC-42A3-AC2B-46EBD0409239@suse.de> <20121217230031.GA8639@truffula.fritz.box> <89E6E981-6248-4F17-BA9B-258180DAA304@suse.de> <20121219043430.GH8639@truffula.fritz.box> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="mYCpIKhGyMATD0i+" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 11/13] pseries: Fixes and enhancements to L1 cache properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: "qemu-ppc@nongnu.org" , "qemu-devel@nongnu.org" --mYCpIKhGyMATD0i+ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Dec 19, 2012 at 11:40:09PM +0100, Alexander Graf wrote: >=20 > On 19.12.2012, at 05:34, David Gibson wrote: >=20 > > On Tue, Dec 18, 2012 at 12:49:02AM +0100, Alexander Graf wrote: > >>=20 > >> On 18.12.2012, at 00:00, David Gibson wrote: > >>=20 > >>> On Mon, Dec 17, 2012 at 11:10:12AM +0100, Alexander Graf wrote: > >>>>=20 > >>>>=20 > >>>> On 17.12.2012, at 03:32, David Gibson = wrote: > >>>>=20 > >>>>> On Thu, Dec 13, 2012 at 01:50:25PM +0100, Alexander Graf wrote: > >>>>>>=20 > >>>>>> On 04.12.2012, at 03:42, David Gibson wrote: > >>>>>>=20 > >>>>>>> PAPR requires that the device tree's CPU nodes have several prope= rties > >>>>>>> with information about the L1 cache. We created two of these > >>>>>>> properties, but with incorrect names - "[id]cache-block-size" ins= tead > >>>>>>> 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 sa= me > >>>>>>> values as the block size properties in all current cases). We al= so > >>>>>>> add the [id]-cache-size properties. The latter requires some ext= ra > >>>>>>> infrastructure in the general target-ppc code to (optionally) set= the > >>>>>>> cache sizes for various CPUs. We obtain the published values eit= her > >>>>>>> from there, or from the host when KVM is in use. > >>>>>>>=20 > >>>>>>> Signed-off-by: David Gibson > >>>>>>> --- > >>>>>>> hw/spapr.c | 20 ++++++++++++++++++-- > >>>>>>> target-ppc/cpu.h | 1 + > >>>>>>> target-ppc/kvm.c | 10 ++++++++++ > >>>>>>> target-ppc/kvm_ppc.h | 12 ++++++++++++ > >>>>>>> target-ppc/translate_init.c | 4 ++++ > >>>>>>> 5 files changed, 45 insertions(+), 2 deletions(-) > >>>>>>>=20 > >>>>>>> diff --git a/hw/spapr.c b/hw/spapr.c > >>>>>>> index d23aa9d..3bacf2f 100644 > >>>>>>> --- a/hw/spapr.c > >>>>>>> +++ b/hw/spapr.c > >>>>>>> @@ -315,6 +315,10 @@ static void *spapr_create_fdt_skel(const cha= r *cpu_model, > >>>>>>> 0xffffffff, 0xffffffff}; > >>>>>>> uint32_t tbfreq =3D kvm_enabled() ? kvmppc_get_tbfreq() : TI= MEBASE_FREQ; > >>>>>>> uint32_t cpufreq =3D kvm_enabled() ? kvmppc_get_clockfreq() = : 1000000000; > >>>>>>> + int dcache_size =3D kvm_enabled() ? kvmppc_get_dcache_si= ze() > >>>>>>> + : env->l1_dcache_size; > >>>>>>> + int icache_size =3D kvm_enabled() ? kvmppc_get_icache_si= ze() > >>>>>>> + : env->l1_icache_size; > >>>>>>=20 > >>>>>> By default with KVM we use -cpu host, right? So we already should > >>>>>> get the correct cache sizes for the CPU you're on. > >>>>>=20 > >>>>> Um.. sort of. The first problem with that is that I only just added > >>>>> the cache size information to qemu, so only a few CPUs currently > >>>>> populate that information. Using the host info means we can get the > >>>>> right information even for CPUs that don't yet have cache info in > >>>>> qemu. > >>>>>=20 > >>>>>> Imagine we would support the compatibility feature where you could > >>>>>> run with -cpu POWER6 on a POWER7 machine. Would exposing the POWER6 > >>>>>> cache size rather than the host's make any real difference to the > >>>>>> guest? Or would it work nevertheless? > >>>>>=20 > >>>>> The second problem is that there may be circumstances where the > >>>>> cache size is altered from the normal size for the cpu. Running in > >>>>> POWER6 compat mode > >>>>=20 > >>>> Well, either we want to be compatible or we don't :). If we run with > >>>> -cpu POWER6 we want to generate the same dt as we did on a POWER6 > >>>> system itself. > >>>=20 > >>> Hrm. Ok. > >>>=20 > >>> So, the remaining difficulty I have with that is that for -cpu HOST we > >>> should still take the cache sizes from the host, but that can't easily > >>> be done because they're only stored in the env, not the cpu_def. > >>=20 > >> Can we set a bit somewhere that allows us to do a sanity check > >> later? After all, the values coming from the host and the values in > >> the populated env really should just be identical for -cpu > >> host. Every time they're not, it's simply a bug that needs to be > >> reported. > >=20 > > That works. Although it's not obvious where to put the check and > > fixup. kvmppc_fixup_cpu() seems like the obvious place, but that's no > > good because it's called before the per-cpu-type init function, which > > is what populates the expected cachesize values. >=20 > That's a real shame. Any reason we don't run it after the init > function? Fixup indicates that it fixes things up after they > happened, not before :). Well, the very first thing that kvmppc_fixup_cpu() did, when I first implemented it is to adjust the cpu numbers so we get the right SMT behaviour under KVM. I was concerned that later parts of the initialization might use the cpu_index. --=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 --mYCpIKhGyMATD0i+ Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iEYEARECAAYFAlDSiDkACgkQaILKxv3ab8YTqgCfRQbYIlLBoQWab7pnMyGlZCjr Dn0AnRpglw1lPC3Hxn3zjZtO5VjamxRl =P2rA -----END PGP SIGNATURE----- --mYCpIKhGyMATD0i+--