On 09/21/2016 03:51 AM, David Gibson wrote: > On Thu, Sep 15, 2016 at 02:45:55PM +0200, Cédric Le Goater wrote: >> This is largy inspired by sPAPRCPUCore with some simplification, no >> hotplug for instance. But the differences are small and the objects >> could possibly be merged. >> >> A set of PnvCore objects is added to the PnvChip and the device >> tree is populated looping on these cores. >> >> Real HW cpu ids are now generated depending on the chip cpu model, the >> chip id and a core mask. This id is stored in CPUState->cpu_index and >> in PnvCore->pir. It is used to populate the device tree. > > Ok, as noted elsewhere, I think you need to disassociate the PIR value > from the cpu_index. It may be a little less elegant, but it'll make > your life much easier in the short and medium term.
yes I agree. cpu_index was just easy to use mostly because of the ICPs indexes but I am changing that. Hopefully, at the end, the code should not use cpu_dt_id (this is the case right now) nor cpu_index. > Apart from that, this looks pretty good, though I have one suggested > cleanup below. > >> Signed-off-by: Cédric Le Goater <c...@kaod.org> >> --- >> >> Changes since v2: >> >> - added P9 support >> - used error_fatal instead of error_abort when setting the chip >> properties >> - replaced num_cores by nr_cores >> - removed gservers properties that were unused on powernv. >> - used a 'void *' instead of a 'PnvCore *' to hold core Objects of >> potentially different size. >> - qom: linked the core Objects to the chip >> - moved device tree creation under powernv_populate_chip() >> - added a 'pir' property' for ease of use >> >> Changes since v1: >> >> - changed name to PnvCore >> - changed PnvChip core array type to a 'PnvCore *cores' >> - introduced real cpu hw ids using a core mask from the chip >> - reworked powernv_create_core_node() which populates the device tree >> - added missing "ibm,pa-features" property >> - smp_cpus representing threads, used smp_cores instead to create the >> cores in the chip. >> - removed the use of ppc_get_vcpu_dt_id() >> - added "POWER8E" and "POWER8NVL" cpu models to exercice the >> PnvChipClass >> >> hw/ppc/Makefile.objs | 2 +- >> hw/ppc/pnv.c | 186 >> ++++++++++++++++++++++++++++++++++++++++++++++ >> hw/ppc/pnv_core.c | 184 >> +++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/ppc/pnv.h | 3 + >> include/hw/ppc/pnv_core.h | 48 ++++++++++++ >> 5 files changed, 422 insertions(+), 1 deletion(-) >> create mode 100644 hw/ppc/pnv_core.c >> create mode 100644 include/hw/ppc/pnv_core.h >> >> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs >> index 8105db7d5600..f8c7d1db9ade 100644 >> --- a/hw/ppc/Makefile.objs >> +++ b/hw/ppc/Makefile.objs >> @@ -6,7 +6,7 @@ obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o >> spapr_rtas.o >> obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o >> obj-$(CONFIG_PSERIES) += spapr_cpu_core.o >> # IBM PowerNV >> -obj-$(CONFIG_POWERNV) += pnv.o >> +obj-$(CONFIG_POWERNV) += pnv.o pnv_core.o >> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) >> obj-y += spapr_pci_vfio.o >> endif >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index f4c125503249..7bc98f15f14b 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -27,6 +27,7 @@ >> #include "hw/ppc/fdt.h" >> #include "hw/ppc/ppc.h" >> #include "hw/ppc/pnv.h" >> +#include "hw/ppc/pnv_core.h" >> #include "hw/loader.h" >> #include "exec/address-spaces.h" >> #include "qemu/cutils.h" >> @@ -74,14 +75,162 @@ static void powernv_populate_memory_node(void *fdt, int >> chip_id, hwaddr start, >> _FDT((fdt_setprop_cell(fdt, off, "ibm,chip-id", chip_id))); >> } >> >> +static int get_cpus_node(void *fdt) >> +{ >> + int cpus_offset = fdt_path_offset(fdt, "/cpus"); >> + >> + if (cpus_offset < 0) { >> + cpus_offset = fdt_add_subnode(fdt, fdt_path_offset(fdt, "/"), >> + "cpus"); >> + if (cpus_offset) { >> + _FDT((fdt_setprop_cell(fdt, cpus_offset, "#address-cells", >> 0x1))); >> + _FDT((fdt_setprop_cell(fdt, cpus_offset, "#size-cells", 0x0))); >> + } >> + } >> + _FDT(cpus_offset); >> + return cpus_offset; >> +} >> + >> +/* >> + * The PowerNV cores (and threads) need to use real HW ids and not an >> + * incremental index like it has been done on other platforms. This HW >> + * id is stored in the CPU PIR, it is used to create cpu nodes in the >> + * device tree, used in XSCOM to address cores and in interrupt >> + * servers. >> + */ >> +static void powernv_create_core_node(PnvChip *chip, PnvCore *pc, void *fdt) >> +{ >> + CPUState *cs = CPU(DEVICE(pc->threads)); >> + DeviceClass *dc = DEVICE_GET_CLASS(cs); >> + PowerPCCPU *cpu = POWERPC_CPU(cs); >> + int smt_threads = ppc_get_compat_smt_threads(cpu); >> + CPUPPCState *env = &cpu->env; >> + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs); >> + uint32_t servers_prop[smt_threads]; >> + int i; >> + uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40), >> + 0xffffffff, 0xffffffff}; >> + uint32_t tbfreq = PNV_TIMEBASE_FREQ; >> + uint32_t cpufreq = 1000000000; >> + uint32_t page_sizes_prop[64]; >> + size_t page_sizes_prop_size; >> + const uint8_t pa_features[] = { 24, 0, >> + 0xf6, 0x3f, 0xc7, 0xc0, 0x80, 0xf0, >> + 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, >> + 0x00, 0x00, 0x00, 0x00, 0x80, 0x00, >> + 0x80, 0x00, 0x80, 0x00, 0x80, 0x00 }; >> + int offset; >> + char *nodename; >> + int cpus_offset = get_cpus_node(fdt); >> + >> + nodename = g_strdup_printf("%s@%x", dc->fw_name, pc->pir); >> + offset = fdt_add_subnode(fdt, cpus_offset, nodename); >> + _FDT(offset); >> + g_free(nodename); >> + >> + _FDT((fdt_setprop_cell(fdt, offset, "ibm,chip-id", chip->chip_id))); >> + >> + _FDT((fdt_setprop_cell(fdt, offset, "reg", pc->pir))); >> + _FDT((fdt_setprop_cell(fdt, offset, "ibm,pir", pc->pir))); >> + _FDT((fdt_setprop_string(fdt, offset, "device_type", "cpu"))); >> + >> + _FDT((fdt_setprop_cell(fdt, offset, "cpu-version", env->spr[SPR_PVR]))); >> + _FDT((fdt_setprop_cell(fdt, offset, "d-cache-block-size", >> + env->dcache_line_size))); >> + _FDT((fdt_setprop_cell(fdt, offset, "d-cache-line-size", >> + env->dcache_line_size))); >> + _FDT((fdt_setprop_cell(fdt, offset, "i-cache-block-size", >> + env->icache_line_size))); >> + _FDT((fdt_setprop_cell(fdt, offset, "i-cache-line-size", >> + env->icache_line_size))); >> + >> + if (pcc->l1_dcache_size) { >> + _FDT((fdt_setprop_cell(fdt, offset, "d-cache-size", >> + pcc->l1_dcache_size))); >> + } else { >> + error_report("Warning: Unknown L1 dcache size for cpu"); >> + } >> + if (pcc->l1_icache_size) { >> + _FDT((fdt_setprop_cell(fdt, offset, "i-cache-size", >> + pcc->l1_icache_size))); >> + } else { >> + error_report("Warning: Unknown L1 icache size for cpu"); >> + } >> + >> + _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq))); >> + _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq))); >> + _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr))); >> + _FDT((fdt_setprop_string(fdt, offset, "status", "okay"))); >> + _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0))); >> + >> + if (env->spr_cb[SPR_PURR].oea_read) { >> + _FDT((fdt_setprop(fdt, offset, "ibm,purr", NULL, 0))); >> + } >> + >> + if (env->mmu_model & POWERPC_MMU_1TSEG) { >> + _FDT((fdt_setprop(fdt, offset, "ibm,processor-segment-sizes", >> + segs, sizeof(segs)))); >> + } >> + >> + /* Advertise VMX/VSX (vector extensions) if available >> + * 0 / no property == no vector extensions >> + * 1 == VMX / Altivec available >> + * 2 == VSX available */ >> + if (env->insns_flags & PPC_ALTIVEC) { >> + uint32_t vmx = (env->insns_flags2 & PPC2_VSX) ? 2 : 1; >> + >> + _FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", vmx))); >> + } >> + >> + /* Advertise DFP (Decimal Floating Point) if available >> + * 0 / no property == no DFP >> + * 1 == DFP available */ >> + if (env->insns_flags2 & PPC2_DFP) { >> + _FDT((fdt_setprop_cell(fdt, offset, "ibm,dfp", 1))); >> + } >> + >> + page_sizes_prop_size = ppc_create_page_sizes_prop(env, page_sizes_prop, >> + sizeof(page_sizes_prop)); >> + if (page_sizes_prop_size) { >> + _FDT((fdt_setprop(fdt, offset, "ibm,segment-page-sizes", >> + page_sizes_prop, page_sizes_prop_size))); >> + } >> + >> + _FDT((fdt_setprop(fdt, offset, "ibm,pa-features", >> + pa_features, sizeof(pa_features)))); >> + >> + if (cpu->cpu_version) { >> + _FDT((fdt_setprop_cell(fdt, offset, "cpu-version", >> cpu->cpu_version))); >> + } >> + >> + /* Build interrupt servers properties */ >> + for (i = 0; i < smt_threads; i++) { >> + servers_prop[i] = cpu_to_be32(pc->pir + i); >> + } >> + _FDT((fdt_setprop(fdt, offset, "ibm,ppc-interrupt-server#s", >> + servers_prop, sizeof(servers_prop)))); >> +} >> + >> static void powernv_populate_chip(PnvChip *chip, void *fdt) >> { >> + PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip); >> + char *typename = pnv_core_typename(pcc->cpu_model); >> + size_t typesize = object_type_get_instance_size(typename); >> + int i; >> + >> + for (i = 0; i < chip->nr_cores; i++) { >> + PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize); >> + >> + powernv_create_core_node(chip, pnv_core, fdt); >> + } >> + >> /* Put all the memory in one node on chip 0 until we find a way to >> * specify different ranges for each chip >> */ >> if (chip->chip_id == 0) { >> powernv_populate_memory_node(fdt, chip->chip_id, 0, ram_size); >> } >> + g_free(typename); >> } >> >> static void *powernv_create_fdt(PnvMachineState *pnv, >> @@ -382,10 +531,47 @@ static void pnv_chip_realize(DeviceState *dev, Error >> **errp) >> { >> PnvChip *chip = PNV_CHIP(dev); >> PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip); >> + char *typename = pnv_core_typename(pcc->cpu_model); >> + size_t typesize = object_type_get_instance_size(typename); >> + int i, core_hwid; >> + >> + if (!object_class_by_name(typename)) { >> + error_setg(errp, "Unable to find PowerNV CPU Core '%s'", typename); >> + return; >> + } >> >> /* Early checks on the core settings */ >> pnv_chip_core_sanitize(chip); >> >> + chip->cores = g_malloc0(typesize * chip->nr_cores); >> + >> + for (i = 0, core_hwid = 0; (core_hwid < sizeof(chip->cores_mask) * 8) >> + && (i < chip->nr_cores); core_hwid++) { >> + char core_name[32]; >> + void *pnv_core = chip->cores + i * typesize; >> + >> + if (!(chip->cores_mask & (1ull << core_hwid))) { >> + continue; >> + } >> + >> + object_initialize(pnv_core, typesize, typename); >> + snprintf(core_name, sizeof(core_name), "core[%d]", core_hwid); >> + object_property_add_child(OBJECT(chip), core_name, OBJECT(pnv_core), >> + &error_fatal); >> + object_property_set_int(OBJECT(pnv_core), smp_threads, "nr-threads", >> + &error_fatal); >> + object_property_set_int(OBJECT(pnv_core), core_hwid, >> + CPU_CORE_PROP_CORE_ID, &error_fatal); >> + object_property_set_int(OBJECT(pnv_core), >> + pcc->core_pir(chip, core_hwid), >> + "pir", &error_fatal); >> + object_property_set_bool(OBJECT(pnv_core), true, "realized", >> + &error_fatal); >> + object_unref(OBJECT(pnv_core)); >> + i++; >> + } >> + g_free(typename); >> + >> if (pcc->realize) { >> pcc->realize(chip, errp); >> } >> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c >> new file mode 100644 >> index 000000000000..6fed5a208536 >> --- /dev/null >> +++ b/hw/ppc/pnv_core.c >> @@ -0,0 +1,184 @@ >> +/* >> + * QEMU PowerPC PowerNV CPU Core model >> + * >> + * Copyright (c) 2016, IBM Corporation. >> + * >> + * This library is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU Lesser General Public License >> + * as published by the Free Software Foundation; either version 2 of >> + * the License, or (at your option) any later version. >> + * >> + * This library is distributed in the hope that it will be useful, but >> + * WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + * Lesser General Public License for more details. >> + * >> + * You should have received a copy of the GNU Lesser General Public >> + * License along with this library; if not, see >> <http://www.gnu.org/licenses/>. >> + */ >> +#include "qemu/osdep.h" >> +#include "sysemu/sysemu.h" >> +#include "qapi/error.h" >> +#include "target-ppc/cpu.h" >> +#include "hw/ppc/ppc.h" >> +#include "hw/ppc/pnv.h" >> +#include "hw/ppc/pnv_core.h" >> + >> +static void powernv_cpu_reset(void *opaque) >> +{ >> + PowerPCCPU *cpu = opaque; >> + CPUState *cs = CPU(cpu); >> + CPUPPCState *env = &cpu->env; >> + >> + cpu_reset(cs); >> + >> + /* >> + * FIXME: cpu_index is non contiguous but xics native requires it >> + * to find its icp >> + */ >> + env->spr[SPR_PIR] = cs->cpu_index; >> + env->spr[SPR_HIOR] = 0; >> + env->gpr[3] = POWERNV_FDT_ADDR; >> + env->nip = 0x10; >> + env->msr |= MSR_HVB; >> +} >> + >> +static void powernv_cpu_init(PowerPCCPU *cpu, Error **errp) >> +{ >> + CPUPPCState *env = &cpu->env; >> + >> + /* Set time-base frequency to 512 MHz */ >> + cpu_ppc_tb_init(env, PNV_TIMEBASE_FREQ); >> + >> + /* MSR[IP] doesn't exist nowadays */ >> + env->msr_mask &= ~(1 << 6); > > If MSR[IP] is gone, shouldn't that be reflected in the MSR masks > stored for the actual vcpu models, rather than imposed from the > machine or core code? > >> + qemu_register_reset(powernv_cpu_reset, cpu); >> + powernv_cpu_reset(cpu); >> +} >> + >> +static void pnv_core_realize_child(Object *child, Error **errp) >> +{ >> + Error *local_err = NULL; >> + CPUState *cs = CPU(child); >> + PowerPCCPU *cpu = POWERPC_CPU(cs); >> + >> + object_property_set_bool(child, true, "realized", &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return; >> + } >> + >> + powernv_cpu_init(cpu, &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return; >> + } >> +} >> + >> +static void pnv_core_realize(DeviceState *dev, Error **errp) >> +{ >> + PnvCore *pc = PNV_CORE(OBJECT(dev)); >> + CPUCore *cc = CPU_CORE(OBJECT(dev)); >> + PnvCoreClass *pcc = PNV_CORE_GET_CLASS(OBJECT(dev)); >> + const char *typename = object_class_get_name(pcc->cpu_oc); >> + size_t size = object_type_get_instance_size(typename); >> + Error *local_err = NULL; >> + void *obj; >> + int i, j; >> + char name[32]; >> + >> + pc->threads = g_malloc0(size * cc->nr_threads); >> + for (i = 0; i < cc->nr_threads; i++) { >> + CPUState *cs; >> + >> + obj = pc->threads + i * size; >> + >> + object_initialize(obj, size, typename); >> + cs = CPU(obj); >> + /* >> + * FIXME: cpu_index is non contiguous but xics native requires >> + * it to find its icp >> + */ >> + cs->cpu_index = pc->pir + i; >> + snprintf(name, sizeof(name), "thread[%d]", i); >> + object_property_add_child(OBJECT(pc), name, obj, &local_err); >> + if (local_err) { >> + goto err; >> + } >> + object_unref(obj); >> + } >> + >> + for (j = 0; j < cc->nr_threads; j++) { >> + obj = pc->threads + j * size; >> + >> + pnv_core_realize_child(obj, &local_err); >> + if (local_err) { >> + goto err; >> + } >> + } >> + return; >> + >> +err: >> + while (--i >= 0) { >> + obj = pc->threads + i * size; >> + object_unparent(obj); >> + } >> + g_free(pc->threads); >> + error_propagate(errp, local_err); >> +} >> + >> +static Property pnv_core_properties[] = { >> + DEFINE_PROP_UINT32("pir", PnvCore, pir, 0), >> + DEFINE_PROP_END_OF_LIST(), >> +}; >> + >> +static void pnv_core_class_init(ObjectClass *oc, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(oc); >> + PnvCoreClass *pcc = PNV_CORE_CLASS(oc); >> + >> + dc->realize = pnv_core_realize; >> + dc->props = pnv_core_properties; >> + pcc->cpu_oc = cpu_class_by_name(TYPE_POWERPC_CPU, data); >> +} >> + >> +static const TypeInfo pnv_core_info = { >> + .name = TYPE_PNV_CORE, >> + .parent = TYPE_CPU_CORE, >> + .instance_size = sizeof(PnvCore), >> + .class_size = sizeof(PnvCoreClass), >> + .abstract = true, >> +}; >> + >> +/* >> + * Grow this list or merge with SPAPRCoreInfo which is very similar >> + */ >> +static const char *pnv_core_models[] = { >> + "POWER8E", "POWER8", "POWER8NVL", "POWER9" >> +}; >> + >> +static void pnv_core_register_types(void) >> +{ >> + int i ; >> + >> + type_register_static(&pnv_core_info); >> + for (i = 0; i < ARRAY_SIZE(pnv_core_models); ++i) { >> + TypeInfo ti = { >> + .parent = TYPE_PNV_CORE, >> + .instance_size = sizeof(PnvCore), >> + .class_init = pnv_core_class_init, >> + .class_data = (void *) pnv_core_models[i], >> + }; >> + ti.name = pnv_core_typename(pnv_core_models[i]); >> + type_register(&ti); >> + g_free((void *)ti.name); >> + } >> +} > > I think you might be able to use a single chip type table and > construct both the chip types and the corresponding core types from > it. Indeed and It would be better to keep them in sync. I will look into that. Thanks, C. >> + >> +type_init(pnv_core_register_types) >> + >> +char *pnv_core_typename(const char *model) >> +{ >> + return g_strdup_printf(TYPE_PNV_CORE "-%s", model); >> +} >> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h >> index 2bd2294ac2a3..262faa59a75f 100644 >> --- a/include/hw/ppc/pnv.h >> +++ b/include/hw/ppc/pnv.h >> @@ -45,6 +45,7 @@ typedef struct PnvChip { >> >> uint32_t nr_cores; >> uint64_t cores_mask; >> + void *cores; >> } PnvChip; >> >> typedef struct PnvChipClass { >> @@ -119,4 +120,6 @@ typedef struct PnvMachineState { >> >> #define POWERNV_FDT_ADDR 0x01000000 >> >> +#define PNV_TIMEBASE_FREQ 512000000ULL >> + >> #endif /* _PPC_PNV_H */ >> diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h >> new file mode 100644 >> index 000000000000..a151e281c017 >> --- /dev/null >> +++ b/include/hw/ppc/pnv_core.h >> @@ -0,0 +1,48 @@ >> +/* >> + * QEMU PowerPC PowerNV CPU Core model >> + * >> + * Copyright (c) 2016, IBM Corporation. >> + * >> + * This library is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU Lesser General Public License >> + * as published by the Free Software Foundation; either version 2 of >> + * the License, or (at your option) any later version. >> + * >> + * This library is distributed in the hope that it will be useful, but >> + * WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + * Lesser General Public License for more details. >> + * >> + * You should have received a copy of the GNU Lesser General Public >> + * License along with this library; if not, see >> <http://www.gnu.org/licenses/>. >> + */ >> +#ifndef _PPC_PNV_CORE_H >> +#define _PPC_PNV_CORE_H >> + >> +#include "hw/cpu/core.h" >> + >> +#define TYPE_PNV_CORE "powernv-cpu-core" >> +#define PNV_CORE(obj) \ >> + OBJECT_CHECK(PnvCore, (obj), TYPE_PNV_CORE) >> +#define PNV_CORE_CLASS(klass) \ >> + OBJECT_CLASS_CHECK(PnvCoreClass, (klass), TYPE_PNV_CORE) >> +#define PNV_CORE_GET_CLASS(obj) \ >> + OBJECT_GET_CLASS(PnvCoreClass, (obj), TYPE_PNV_CORE) >> + >> +typedef struct PnvCore { >> + /*< private >*/ >> + CPUCore parent_obj; >> + >> + /*< public >*/ >> + void *threads; >> + uint32_t pir; >> +} PnvCore; >> + >> +typedef struct PnvCoreClass { >> + DeviceClass parent_class; >> + ObjectClass *cpu_oc; >> +} PnvCoreClass; >> + >> +extern char *pnv_core_typename(const char *model); >> + >> +#endif /* _PPC_PNV_CORE_H */ >