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

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


Alex

> +
>         _FDT((fdt_property_cell(fdt, "timebase-frequency", tbfreq)));
>         _FDT((fdt_property_cell(fdt, "clock-frequency", cpufreq)));
>         _FDT((fdt_property_cell(fdt, "ibm,slb-size", env->slb_nr)));
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 0b2d0b9..793ae0f 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -994,6 +994,7 @@ struct CPUPPCState {
> 
>     int dcache_line_size;
>     int icache_line_size;
> +    int l1_dcache_size, l1_icache_size;
> 
>     /* Those resources are used during exception processing */
>     /* CPU model definition */
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index f5710b7..eaa11a1 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1612,14 +1612,49 @@ static void alter_insns(uint64_t *word, uint64_t 
> flags, bool on)
>     }
> }
> 
> +typedef struct PowerPCHostCPUClass {
> +    /*< private >*/
> +    PowerPCCPUClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +} PowerPCHostCPUClass;
> +
> +#define POWERPC_HOST_CPU_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(PowerPCHostCPUClass, (klass), TYPE_HOST_POWERPC_CPU)
> +#define POWERPC_HOST_CPU_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(PowerPCHostCPUClass, (obj), TYPE_HOST_POWERPC_CPU)
> +
> static void kvmppc_host_cpu_initfn(Object *obj)
> {
>     assert(kvm_enabled());
> }
> 
> +static void kvmppc_host_cpu_realizefn(DeviceState *dev, Error **errp)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(dev);
> +    CPUPPCState *env = &cpu->env;
> +    PowerPCHostCPUClass *hpcc = POWERPC_HOST_CPU_GET_CLASS(dev);
> +    uint32_t val;
> +
> +    (*hpcc->parent_realize)(dev, errp);
> +
> +    val = kvmppc_read_int_cpu_dt("d-cache-size");
> +    if (val != -1) {
> +        env->l1_dcache_size = val;
> +    }
> +
> +    val = kvmppc_read_int_cpu_dt("i-cache-size");
> +    if (val != -1) {
> +        env->l1_icache_size = val;
> +    }
> +}
> +
> static void kvmppc_host_cpu_class_init(ObjectClass *oc, void *data)
> {
> +    PowerPCHostCPUClass *hpcc = POWERPC_HOST_CPU_CLASS(oc);
>     PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
> +    DeviceClass *dc = DEVICE_CLASS(oc);
>     uint32_t vmx = kvmppc_get_vmx();
>     uint32_t dfp = kvmppc_get_dfp();
> 
> @@ -1634,6 +1669,9 @@ static void kvmppc_host_cpu_class_init(ObjectClass *oc, 
> void *data)
>         /* Only override when we know what the host supports */
>         alter_insns(&pcc->insns_flags2, PPC2_DFP, dfp);
>     }
> +
> +    hpcc->parent_realize = dc->realize;
> +    dc->realize = kvmppc_host_cpu_realizefn;
> }
> 
> int kvmppc_fixup_cpu(PowerPCCPU *cpu)
> @@ -1654,6 +1692,7 @@ static int kvm_ppc_register_host_cpu_type(void)
>     TypeInfo type_info = {
>         .name = TYPE_HOST_POWERPC_CPU,
>         .instance_init = kvmppc_host_cpu_initfn,
> +        .class_size = sizeof(PowerPCHostCPUClass),
>         .class_init = kvmppc_host_cpu_class_init,
>     };
>     uint32_t host_pvr = mfpvr();
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 3cf440e..594053e 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -6989,6 +6989,10 @@ static void init_proc_POWER7 (CPUPPCState *env)
>     init_excp_POWER7(env);
>     env->dcache_line_size = 128;
>     env->icache_line_size = 128;
> +
> +    env->l1_dcache_size = 0x8000;
> +    env->l1_icache_size = 0x8000;
> +
>     /* Allocate hardware IRQ controller */
>     ppcPOWER7_irq_init(env);
>     /* Can't find information on what this should be on reset.  This
> -- 
> 1.7.10.4
>