On 08/16/2013 08:35 AM, Andreas Färber wrote:
> Instead of relying on cpu_model, obtain the device tree node label
> per CPU. Use DeviceClass::fw_name when available. This implicitly
> resolves HOST@0 node labels for those CPUs through inheritance.
>
> Whenever DeviceClass::fw_name is not available, derive it from the CPU's
> type name and fill it in for that class with a "PowerPC," prefix for
> PAPR compliance.
I'd rather use the family's @desc instead of CPU class name, would be
simpler and we would not have nodes like "PowerPC,POWER7-family@0" (this is
what I get when comment out dc->fw_name for power7 with my PVR patch, just
to test).
Either way, in what case do you expect that code to work at all? power7,
7+, 8 have fw_name field initialized, what else is really supported for
spapr and requires this workaround?
> As a consequence, spapr_fixup_cpu_dt() can operate on each CPU's fw_name,
> obsoleting sPAPREnvironment::cpu_model, and spapr_create_fdt_skel() can
> drop its cpu_model argument.
>
> Signed-off-by: Prerna Saxena <pre...@linux.vnet.ibm.com>
> Signed-off-by: Andreas Färber <afaer...@suse.de>
> ---
> hw/ppc/spapr.c | 36 ++++++++++++++++--------------------
> include/hw/ppc/spapr.h | 1 -
> 2 files changed, 16 insertions(+), 21 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 16bfab9..6d984dc 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -165,9 +165,8 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment
> *spapr)
> int smt = kvmppc_smt_threads();
> uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
>
> - assert(spapr->cpu_model);
> -
> for (cpu = first_cpu; cpu != NULL; cpu = cpu->next_cpu) {
> + DeviceClass *dc = DEVICE_GET_CLASS(cpu);
> uint32_t associativity[] = {cpu_to_be32(0x5),
> cpu_to_be32(0x0),
> cpu_to_be32(0x0),
> @@ -179,7 +178,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment
> *spapr)
> continue;
> }
>
> - snprintf(cpu_model, 32, "/cpus/%s@%x", spapr->cpu_model,
> + snprintf(cpu_model, 32, "/cpus/%s@%x", dc->fw_name,
> cpu->cpu_index);
>
> offset = fdt_path_offset(fdt, cpu_model);
> @@ -249,8 +248,7 @@ static size_t create_page_sizes_prop(CPUPPCState *env,
> uint32_t *prop,
> } while (0)
>
>
> -static void *spapr_create_fdt_skel(const char *cpu_model,
> - hwaddr initrd_base,
> +static void *spapr_create_fdt_skel(hwaddr initrd_base,
> hwaddr initrd_size,
> hwaddr kernel_size,
> const char *boot_device,
> @@ -266,7 +264,6 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
> char qemu_hypertas_prop[] = "hcall-memop1";
> uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};
> uint32_t interrupt_server_ranges_prop[] = {0, cpu_to_be32(smp_cpus)};
> - char *modelname;
> int i, smt = kvmppc_smt_threads();
> unsigned char vec5[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x80};
>
> @@ -322,18 +319,10 @@ static void *spapr_create_fdt_skel(const char
> *cpu_model,
> _FDT((fdt_property_cell(fdt, "#address-cells", 0x1)));
> _FDT((fdt_property_cell(fdt, "#size-cells", 0x0)));
>
> - modelname = g_strdup(cpu_model);
> -
> - for (i = 0; i < strlen(modelname); i++) {
> - modelname[i] = toupper(modelname[i]);
> - }
> -
> - /* This is needed during FDT finalization */
> - spapr->cpu_model = g_strdup(modelname);
> -
> for (cs = first_cpu; cs != NULL; cs = cs->next_cpu) {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> CPUPPCState *env = &cpu->env;
> + DeviceClass *dc = DEVICE_GET_CLASS(cs);
> PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
> int index = cs->cpu_index;
> uint32_t servers_prop[smp_threads];
> @@ -350,7 +339,17 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
> continue;
> }
>
> - nodename = g_strdup_printf("%s@%x", modelname, index);
> + if (dc->fw_name == NULL) {
> + ObjectClass *oc = OBJECT_CLASS(pcc);
> + const char *typename;
> +
> + typename = object_class_get_name(oc);
> + nodename = g_strndup(typename,
> + strlen(typename) - strlen("-"
> TYPE_POWERPC_CPU));
> + dc->fw_name = g_strdup_printf("PowerPC,%s", nodename);
A bit strange to malloc a string and never free it :)
> + g_free(nodename);
> + }
> + nodename = g_strdup_printf("%s@%x", dc->fw_name, index);
>
> _FDT((fdt_begin_node(fdt, nodename)));
>
> @@ -430,8 +429,6 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
> _FDT((fdt_end_node(fdt)));
> }
>
> - g_free(modelname);
> -
> _FDT((fdt_end_node(fdt)));
>
> /* RTAS */
> @@ -1308,8 +1305,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> &savevm_htab_handlers, spapr);
>
> /* Prepare the device tree */
> - spapr->fdt_skel = spapr_create_fdt_skel(cpu_model,
> - initrd_base, initrd_size,
> + spapr->fdt_skel = spapr_create_fdt_skel(initrd_base, initrd_size,
> kernel_size,
> boot_device, kernel_cmdline,
> spapr->epow_irq);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 9fc1972..b4a7656 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -27,7 +27,6 @@ typedef struct sPAPREnvironment {
> target_ulong entry_point;
> uint32_t next_irq;
> uint64_t rtc_offset;
> - char *cpu_model;
> bool has_graphics;
>
> uint32_t epow_irq;
>
--
Alexey
