Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/5] pseries: Fixes and enhancements to L1 cache properties

["Andreas Färber" <afaerber@suse.de>]

-----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:
>> 
>>> 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).
>>> 
>>> 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.
>>> 
>>> 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.
>>> 
>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- 
>>> hw/ppc/spapr.c              |   20 ++++++++++++++++++-- 
>>> target-ppc/cpu.h            |    1 + target-ppc/kvm.c
>>> |   39 +++++++++++++++++++++++++++++++++++++++ 
>>> target-ppc/translate_init.c |    4 ++++ 4 files changed, 62
>>> insertions(+), 2 deletions(-)
>>> 
>>> 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, 
>>> _FDT((fdt_property_string(fdt, "device_type", "cpu")));
>>> 
>>> _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"); +
>>> }
>> 
>> The L1 sizes should come from the class, not env, right?
>> Andreas, any ideas on this?
> 
> Well.. initially I was going to put them in class.  But then it 
> 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.

- -- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nrnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imend￶rffer; HRB 16746 AG Nrnberg
-----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
=ejJS
-----END PGP SIGNATURE-----