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

[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 <address@hidden>
> > ---
> > 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.  So I thought it was safer to put these in
env.

-- 
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

signature.asc
Description: Digital signature