On Mon, Mar 18, 2013 at 12:10:12PM +0100, Andreas Färber wrote:
> -----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 <da...@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.
User to fiddle, probably not. But I was thinking of submodel specific
conditionals in the init_proc function.
--
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
