On Mon, Mar 18, 2013 at 11:54:05AM +0100, Andreas Färber wrote:
> Am 15.03.2013 13:27, schrieb Alexander Graf:
> >
> > 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?
>
> Generally speaking,
>
> CPUPPCState: Only if this is used for TCG with an offset from AREG0 (or
> for legacy grouping reasons).
>
> PowerPCCPU: If you ever intend to let the user override this value
> (per-instance) from the command line.
>
> PowerPCCPUClass: If the value is always constant at runtime.
>
> I can't tell from a brief look at this patch which may be the case here.
Ok, on that rationale I'll rework to move it to the class.
--
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
