On Thu, Sep 22, 2016 at 10:25:59AM +0200, Cédric Le Goater wrote: > >> @@ -493,6 +525,8 @@ static void pnv_chip_power9_class_init(ObjectClass > >> *klass, void *data) > >> k->chip_cfam_id = 0x100d104980000000ull; /* P9 Nimbus DD1.0 */ > >> k->cores_mask = POWER9_CORE_MASK; > >> k->core_pir = pnv_chip_core_pir_p9; > >> + k->xscom_addr = pnv_chip_xscom_addr_p9; > >> + k->xscom_pcba = pnv_chip_xscom_pcba_p9; > > > > So if you do as BenH (and I) suggested and have the "scom address > > space" actually be addressed by (pcba << 3), I think you can probably > > avoid these. > > I will look at that option again. > > I was trying to untangle a few things at the same time. I have better > view of the problem to solve now. The bus is gone, that's was one > thing. How we map these xscom regions is the next. > > Ben suggested to add some P7/P8 mangling before the dispatch in > the &address_space_xscom. This should make things cleaner. I had > not thought of doing that and this is why I introduced these helpers : > > +uint32_t pnv_xscom_pcba(PnvXScomInterface *dev, uint64_t addr) > +uint64_t pnv_xscom_addr(PnvXScomInterface *dev, uint32_t pcba) > > which I don't really like ... > > but we must make sure that we can do the mapping of the xscom > subregions in the &address_space_xscom using (pcba << 3) > > > > Instead you can handle it in the chip or ADU realize function by either: > > > > P8: * map one big subregion for the ADU into &address_space_memory > > * have the handler for that subregion do the address mangling, > > then redispatch into the xscom address space > > > > P9: * Map the appropriate chunk of the xscom address space > > directly into address_space_memory > > Yes that was my feeling for a better solution but Ben chimed in with the > HMER topic. I need to look at that.
Right. Doesn't change the basic concept though - it just means you need (slightly different) redispatchers for both P8 and P9. > > > >> dc->desc = "PowerNV Chip POWER9"; > >> } > >> > >> @@ -527,6 +561,16 @@ static void pnv_chip_core_sanitize(PnvChip *chip) > >> chip->cores_mask &= pcc->cores_mask; > >> } > >> > >> +static void pnv_chip_init(Object *obj) > >> +{ > >> + PnvChip *chip = PNV_CHIP(obj); > >> + > >> + object_initialize(&chip->xscom, sizeof(chip->xscom), TYPE_PNV_XSCOM); > >> + object_property_add_child(obj, "xscom", OBJECT(&chip->xscom), NULL); > >> + object_property_add_const_link(OBJECT(&chip->xscom), "chip", > >> + OBJECT(chip), &error_abort); > >> +} > >> + > >> static void pnv_chip_realize(DeviceState *dev, Error **errp) > >> { > >> PnvChip *chip = PNV_CHIP(dev); > >> @@ -540,6 +584,12 @@ static void pnv_chip_realize(DeviceState *dev, Error > >> **errp) > >> return; > >> } > >> > >> + /* XSCOM bridge */ > >> + object_property_set_bool(OBJECT(&chip->xscom), true, "realized", > >> + &error_fatal); > >> + sysbus_mmio_map(SYS_BUS_DEVICE(&chip->xscom), 0, > >> + PNV_XSCOM_BASE(chip->chip_id)); > >> + > >> /* Early checks on the core settings */ > >> pnv_chip_core_sanitize(chip); > >> > >> @@ -597,6 +647,7 @@ static const TypeInfo pnv_chip_info = { > >> .name = TYPE_PNV_CHIP, > >> .parent = TYPE_SYS_BUS_DEVICE, > >> .class_init = pnv_chip_class_init, > >> + .instance_init = pnv_chip_init, > >> .class_size = sizeof(PnvChipClass), > >> .abstract = true, > >> }; > >> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c > >> new file mode 100644 > >> index 000000000000..019cd85428de > >> --- /dev/null > >> +++ b/hw/ppc/pnv_xscom.c > >> @@ -0,0 +1,308 @@ > >> +/* > >> + * QEMU PowerPC PowerNV XSCOM bus > >> + * > >> + * 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 "qapi/error.h" > >> +#include "hw/hw.h" > >> +#include "qemu/log.h" > >> +#include "sysemu/kvm.h" > >> +#include "target-ppc/cpu.h" > >> +#include "hw/sysbus.h" > >> + > >> +#include "hw/ppc/fdt.h" > >> +#include "hw/ppc/pnv_xscom.h" > >> +#include "hw/ppc/pnv.h" > >> + > >> +#include <libfdt.h> > >> + > >> +static void xscom_complete(uint64_t hmer_bits) > >> +{ > >> + CPUState *cs = current_cpu; > > > > Hmm.. is current_cpu a safe thing to use in the case of KVM or MTTCG? > > hmm, indeed. I need to look at that. > > >> + PowerPCCPU *cpu = POWERPC_CPU(cs); > >> + CPUPPCState *env = &cpu->env; > >> + > >> + cpu_synchronize_state(cs); > >> + env->spr[SPR_HMER] |= hmer_bits; > >> + > >> + /* XXX Need a CPU helper to set HMER, also handle gneeration > >> + * of HMIs > >> + */ > >> +} > >> + > >> +static bool xscom_dispatch_read(PnvXScom *xscom, hwaddr addr, uint64_t > >> *val) > >> +{ > >> + uint32_t success; > >> + uint8_t data[8]; > >> + > >> + success = !address_space_rw(&xscom->xscom_as, addr, > >> MEMTXATTRS_UNSPECIFIED, > >> + data, 8, false); > >> + *val = (((uint64_t) data[0]) << 56 | > >> + ((uint64_t) data[1]) << 48 | > >> + ((uint64_t) data[2]) << 40 | > >> + ((uint64_t) data[3]) << 32 | > >> + ((uint64_t) data[4]) << 24 | > >> + ((uint64_t) data[5]) << 16 | > >> + ((uint64_t) data[6]) << 8 | > >> + ((uint64_t) data[7])); > > > > AFAICT this is basically assuming data is always encoded BE. With the > > right choice of endian flags on the individual SCOM device > > registrations with the scom address space, I think you should be able > > to avoid this mangling. > > yes. I should but curiously I had to do this, and this works the same on > an intel host or a ppc64 host. Hmm.. I suspect what you actually need is NATIVE_ENDIAN on the individual SCOM devices, with BIG_ENDIAN on the redispatcher region. > >> + return success; > >> +} > >> + > >> +static bool xscom_dispatch_write(PnvXScom *xscom, hwaddr addr, uint64_t > >> val) > >> +{ > >> + uint32_t success; > >> + uint8_t data[8]; > >> + > >> + data[0] = (val >> 56) & 0xff; > >> + data[1] = (val >> 48) & 0xff; > >> + data[2] = (val >> 40) & 0xff; > >> + data[3] = (val >> 32) & 0xff; > >> + data[4] = (val >> 24) & 0xff; > >> + data[5] = (val >> 16) & 0xff; > >> + data[6] = (val >> 8) & 0xff; > >> + data[7] = val & 0xff; > >> + > >> + success = !address_space_rw(&xscom->xscom_as, addr, > >> MEMTXATTRS_UNSPECIFIED, > >> + data, 8, true); > >> + return success; > >> +} > >> + > >> +static uint64_t xscom_read(void *opaque, hwaddr addr, unsigned width) > >> +{ > >> + PnvXScom *s = opaque; > >> + uint32_t pcba = s->chip_class->xscom_pcba(addr); > >> + uint64_t val = 0; > >> + > >> + /* Handle some SCOMs here before dispatch */ > >> + switch (pcba) { > >> + case 0xf000f: > >> + val = s->chip_class->chip_cfam_id; > >> + break; > >> + case 0x1010c00: /* PIBAM FIR */ > >> + case 0x1010c03: /* PIBAM FIR MASK */ > >> + case 0x2020007: /* ADU stuff */ > >> + case 0x2020009: /* ADU stuff */ > >> + case 0x202000f: /* ADU stuff */ > >> + val = 0; > >> + break; > >> + case 0x2013f00: /* PBA stuff */ > >> + case 0x2013f01: /* PBA stuff */ > >> + case 0x2013f02: /* PBA stuff */ > >> + case 0x2013f03: /* PBA stuff */ > >> + case 0x2013f04: /* PBA stuff */ > >> + case 0x2013f05: /* PBA stuff */ > >> + case 0x2013f06: /* PBA stuff */ > >> + case 0x2013f07: /* PBA stuff */ > >> + val = 0; > >> + break; > > > > It'd be theoretically nicer to actually register regions for these > > special case addresses, but handling it here is a reasonable hack to > > get things working quickly for the time being. > > I will make a default region on the whole xscomm address space to catch > these. Ok. > > >> + default: > >> + if (!xscom_dispatch_read(s, addr, &val)) { > >> + qemu_log_mask(LOG_GUEST_ERROR, "XSCOM read failed at @0x%" > >> + HWADDR_PRIx " pcba=0x%08x\n", addr, pcba); > >> + xscom_complete(HMER_XSCOM_FAIL | HMER_XSCOM_DONE); > >> + return 0; > >> + } > >> + } > >> + > >> + xscom_complete(HMER_XSCOM_DONE); > >> + return val; > >> +} > >> + > >> +static void xscom_write(void *opaque, hwaddr addr, uint64_t val, > >> + unsigned width) > >> +{ > >> + PnvXScom *s = opaque; > >> + uint32_t pcba = s->chip_class->xscom_pcba(addr); > >> + > >> + /* Handle some SCOMs here before dispatch */ > >> + switch (pcba) { > >> + /* We ignore writes to these */ > >> + case 0xf000f: /* chip id is RO */ > >> + case 0x1010c00: /* PIBAM FIR */ > >> + case 0x1010c01: /* PIBAM FIR */ > >> + case 0x1010c02: /* PIBAM FIR */ > >> + case 0x1010c03: /* PIBAM FIR MASK */ > >> + case 0x1010c04: /* PIBAM FIR MASK */ > >> + case 0x1010c05: /* PIBAM FIR MASK */ > >> + case 0x2020007: /* ADU stuff */ > >> + case 0x2020009: /* ADU stuff */ > >> + case 0x202000f: /* ADU stuff */ > >> + break; > >> + default: > >> + if (!xscom_dispatch_write(s, addr, val)) { > >> + qemu_log_mask(LOG_GUEST_ERROR, "XSCOM write failed at @0x%" > >> + HWADDR_PRIx " pcba=0x%08x data=0x%" PRIx64 "\n", > >> + addr, pcba, val); > >> + xscom_complete(HMER_XSCOM_FAIL | HMER_XSCOM_DONE); > >> + return; > >> + } > >> + } > >> + > >> + xscom_complete(HMER_XSCOM_DONE); > >> +} > >> + > >> +const MemoryRegionOps pnv_xscom_ops = { > >> + .read = xscom_read, > >> + .write = xscom_write, > >> + .valid.min_access_size = 8, > >> + .valid.max_access_size = 8, > >> + .impl.min_access_size = 8, > >> + .impl.max_access_size = 8, > >> + .endianness = DEVICE_BIG_ENDIAN, > >> +}; > >> + > >> +static void pnv_xscom_realize(DeviceState *dev, Error **errp) > >> +{ > >> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > >> + PnvXScom *s = PNV_XSCOM(dev); > >> + char *name; > >> + Object *obj; > >> + Error *err = NULL; > >> + > >> + obj = object_property_get_link(OBJECT(s), "chip", &err); > >> + if (!obj) { > >> + error_setg(errp, "%s: required link 'chip' not found: %s", > >> + __func__, error_get_pretty(err)); > >> + return; > >> + } > >> + > >> + s->chip_class = PNV_CHIP_GET_CLASS(obj); > >> + s->chip_id = PNV_CHIP(obj)->chip_id; > >> + > >> + if (s->chip_id < 0) { > >> + error_setg(errp, "invalid chip id '%d'", s->chip_id); > >> + return; > >> + } > >> + > >> + name = g_strdup_printf("xscom-%x", s->chip_id); > >> + memory_region_init_io(&s->mem, OBJECT(s), &pnv_xscom_ops, s, name, > >> + PNV_XSCOM_SIZE); > >> + sysbus_init_mmio(sbd, &s->mem); > >> + > >> + memory_region_init(&s->xscom_mr, OBJECT(s), name, PNV_XSCOM_SIZE); > >> + address_space_init(&s->xscom_as, &s->xscom_mr, name); > >> + g_free(name); > >> + > >> +} > >> + > >> +static void pnv_xscom_class_init(ObjectClass *klass, void *data) > >> +{ > >> + DeviceClass *dc = DEVICE_CLASS(klass); > >> + > >> + dc->realize = pnv_xscom_realize; > >> +} > >> + > >> +static const TypeInfo pnv_xscom_info = { > >> + .name = TYPE_PNV_XSCOM, > >> + .parent = TYPE_SYS_BUS_DEVICE, > >> + .instance_size = sizeof(PnvXScom), > >> + .class_init = pnv_xscom_class_init, > >> +}; > >> + > >> +static const TypeInfo pnv_xscom_interface_info = { > >> + .name = TYPE_PNV_XSCOM_INTERFACE, > >> + .parent = TYPE_INTERFACE, > >> + .class_size = sizeof(PnvXScomInterfaceClass), > >> +}; > >> + > >> +static void pnv_xscom_register_types(void) > >> +{ > >> + type_register_static(&pnv_xscom_info); > >> + type_register_static(&pnv_xscom_interface_info); > >> +} > >> + > >> +type_init(pnv_xscom_register_types) > >> + > >> +typedef struct ForeachPopulateArgs { > >> + void *fdt; > >> + int xscom_offset; > >> +} ForeachPopulateArgs; > >> + > >> +static int xscom_populate_child(Object *child, void *opaque) > >> +{ > >> + if (object_dynamic_cast(child, TYPE_PNV_XSCOM_INTERFACE)) { > >> + ForeachPopulateArgs *args = opaque; > >> + PnvXScomInterface *xd = PNV_XSCOM_INTERFACE(child);; > >> + PnvXScomInterfaceClass *xc = PNV_XSCOM_INTERFACE_GET_CLASS(xd); > >> + > >> + if (xc->devnode) { > >> + _FDT((xc->devnode(xd, args->fdt, args->xscom_offset))); > >> + } > >> + } > >> + return 0; > >> +} > >> + > >> +int pnv_xscom_populate_fdt(PnvXScom *adu, void *fdt, int root_offset) > >> +{ > >> + const char compat[] = "ibm,power8-xscom\0ibm,xscom"; > >> + uint64_t reg[] = { cpu_to_be64(PNV_XSCOM_BASE(adu->chip_id)), > >> + cpu_to_be64(PNV_XSCOM_SIZE) }; > >> + int xscom_offset; > >> + ForeachPopulateArgs args; > >> + char *name; > >> + > >> + name = g_strdup_printf("xscom@%" PRIx64, be64_to_cpu(reg[0])); > >> + xscom_offset = fdt_add_subnode(fdt, root_offset, name); > >> + _FDT(xscom_offset); > >> + g_free(name); > >> + _FDT((fdt_setprop_cell(fdt, xscom_offset, "ibm,chip-id", > >> adu->chip_id))); > >> + _FDT((fdt_setprop_cell(fdt, xscom_offset, "#address-cells", 1))); > >> + _FDT((fdt_setprop_cell(fdt, xscom_offset, "#size-cells", 1))); > >> + _FDT((fdt_setprop(fdt, xscom_offset, "reg", reg, sizeof(reg)))); > >> + _FDT((fdt_setprop(fdt, xscom_offset, "compatible", compat, > >> + sizeof(compat)))); > >> + _FDT((fdt_setprop(fdt, xscom_offset, "scom-controller", NULL, 0))); > >> + > >> + args.fdt = fdt; > >> + args.xscom_offset = xscom_offset; > >> + > >> + object_child_foreach(OBJECT(adu), xscom_populate_child, &args); > >> + return 0; > >> +} > >> + > >> +/* > >> + * XScom address translation depends on the chip type and not all > >> + * objects have backlink to it. Here's a helper to handle this case. > >> + * To be improved. > >> + */ > > > > Yeah.. this seems a bit hacky as you say. Passing the individual > > device to these helpers just seems wrong, since it's a chip / machine > > dependent mapping. Does this go away if you use the (pcba << 3) > > encoding? > > Hopefully. the goal is to solve that in v4. > > >> +uint32_t pnv_xscom_pcba(PnvXScomInterface *dev, uint64_t addr) > >> +{ > >> + PnvXScomInterfaceClass *xc = PNV_XSCOM_INTERFACE_GET_CLASS(dev); > >> + > >> + if (!xc->xscom_pcba) { > >> + PnvMachineState *pnv = POWERNV_MACHINE(qdev_get_machine()); > >> + PnvChipClass *pcc = PNV_CHIP_GET_CLASS(pnv->chips[0]); > >> + > >> + xc->xscom_pcba = pcc->xscom_pcba; > >> + } > >> + > >> + return xc->xscom_pcba(addr); > >> +} > >> + > >> +uint64_t pnv_xscom_addr(PnvXScomInterface *dev, uint32_t pcba) > >> +{ > >> + PnvXScomInterfaceClass *xc = PNV_XSCOM_INTERFACE_GET_CLASS(dev); > >> + > >> + if (!xc->xscom_addr) { > >> + PnvMachineState *pnv = POWERNV_MACHINE(qdev_get_machine()); > >> + PnvChipClass *pcc = PNV_CHIP_GET_CLASS(pnv->chips[0]); > >> + > >> + xc->xscom_addr = pcc->xscom_addr; > >> + } > >> + > >> + return xc->xscom_addr(pcba); > >> +} > >> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h > >> index 262faa59a75f..0371710b1882 100644 > >> --- a/include/hw/ppc/pnv.h > >> +++ b/include/hw/ppc/pnv.h > >> @@ -21,6 +21,7 @@ > >> > >> #include "hw/boards.h" > >> #include "hw/sysbus.h" > >> +#include "hw/ppc/pnv_xscom.h" > >> > >> #define TYPE_PNV_CHIP "powernv-chip" > >> #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP) > >> @@ -42,6 +43,7 @@ typedef struct PnvChip { > >> > >> /*< public >*/ > >> uint32_t chip_id; > >> + PnvXScom xscom; > >> > >> uint32_t nr_cores; > >> uint64_t cores_mask; > >> @@ -60,6 +62,8 @@ typedef struct PnvChipClass { > >> > >> void (*realize)(PnvChip *dev, Error **errp); > >> uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id); > >> + uint64_t (*xscom_addr)(uint32_t pcba); > >> + uint32_t (*xscom_pcba)(uint64_t addr); > >> } PnvChipClass; > >> > >> #define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-POWER8E" > >> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h > >> new file mode 100644 > >> index 000000000000..0a03d533db59 > >> --- /dev/null > >> +++ b/include/hw/ppc/pnv_xscom.h > >> @@ -0,0 +1,74 @@ > >> +/* > >> + * QEMU PowerPC PowerNV XSCOM bus definitions > >> + * > >> + * 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_XSCOM_H > >> +#define _PPC_PNV_XSCOM_H > >> + > >> +#include "hw/sysbus.h" > >> + > >> +typedef struct PnvXScomInterface { > >> + Object parent; > >> +} PnvXScomInterface; > >> + > >> +#define TYPE_PNV_XSCOM_INTERFACE "pnv-xscom-interface" > >> +#define PNV_XSCOM_INTERFACE(obj) \ > >> + OBJECT_CHECK(PnvXScomInterface, (obj), TYPE_PNV_XSCOM_INTERFACE) > >> +#define PNV_XSCOM_INTERFACE_CLASS(klass) \ > >> + OBJECT_CLASS_CHECK(PnvXScomInterfaceClass, (klass), \ > >> + TYPE_PNV_XSCOM_INTERFACE) > >> +#define PNV_XSCOM_INTERFACE_GET_CLASS(obj) \ > >> + OBJECT_GET_CLASS(PnvXScomInterfaceClass, (obj), > >> TYPE_PNV_XSCOM_INTERFACE) > >> + > >> +typedef struct PnvXScomInterfaceClass { > >> + InterfaceClass parent; > >> + int (*devnode)(PnvXScomInterface *dev, void *fdt, int offset); > >> + > >> + uint64_t (*xscom_addr)(uint32_t pcba); > >> + uint32_t (*xscom_pcba)(uint64_t addr); > > > > Allowing the SCOM device to override the mapping just seems bogus. > > Surely this should always go via the chip. > > yes. > > >> +} PnvXScomInterfaceClass; > >> + > >> +#define TYPE_PNV_XSCOM "pnv-xscom" > >> +#define PNV_XSCOM(obj) OBJECT_CHECK(PnvXScom, (obj), TYPE_PNV_XSCOM) > >> + > >> +typedef struct PnvChipClass PnvChipClass; > >> + > >> +typedef struct PnvXScom { > >> + /*< private >*/ > >> + SysBusDevice parent_obj; > >> + /*< public >*/ > >> + > >> + MemoryRegion mem; > >> + int32_t chip_id; > >> + PnvChipClass *chip_class; > > > > Just having a pointer to the chip seems simpler than storing both id > > and class here. But I guess it'll go away anyway if you merge the ADU > > into the chip object itself. > > I will merge in v4. > > Thanks, > > C. > > >> + MemoryRegion xscom_mr; > >> + AddressSpace xscom_as; > >> +} PnvXScom; > >> + > >> +#define PNV_XSCOM_SIZE 0x800000000ull > >> +#define PNV_XSCOM_BASE(chip) \ > >> + (0x3fc0000000000ull + ((uint64_t)(chip)) * PNV_XSCOM_SIZE) > >> + > >> +extern int pnv_xscom_populate_fdt(PnvXScom *xscom, void *fdt, int offset); > >> + > >> +/* > >> + * helpers to translate to XScomm PCB addresses > >> + */ > >> +extern uint32_t pnv_xscom_pcba(PnvXScomInterface *dev, uint64_t addr); > >> +extern uint64_t pnv_xscom_addr(PnvXScomInterface *dev, uint32_t pcba); > >> + > >> +#endif /* _PPC_PNV_XSCOM_H */ > > > -- 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: PGP signature