| Message ID | 20190807071445.4109-4-bala24@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Enhancing Qemu MMIO emulation with scripting interface | expand |
On 07/08/2019 09:14, Balamuruhan S wrote: > Add mmio callback functions to enable homer/occ common area > to emulate pstate table, occ-sensors, slw, occ static and > dynamic values for Power8 and Power9 chips. It also works for > multiple chips as offset remains the same whereas the base > address are handled appropriately while initializing device > tree. > > currently skiboot disables the homer/occ code path with > `QUIRK_NO_PBA`, this quirk have to be removed in skiboot > for it to use this infrastructure. I think this patch can come before the others as it is adding support without the python extra facilities. Some comments below, > Signed-off-by: Hariharan T.S <hari@linux.vnet.ibm.com> > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com> > --- > hw/ppc/Makefile.objs | 2 +- > hw/ppc/pnv_homer.c | 185 +++++++++++++++++++++++++++++++++++++++++++++ > include/hw/ppc/pnv.h | 14 ++++ > include/hw/ppc/pnv_homer.h | 41 ++++++++++ > 4 files changed, 241 insertions(+), 1 deletion(-) > create mode 100644 hw/ppc/pnv_homer.c > create mode 100644 include/hw/ppc/pnv_homer.h > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > index 9da93af905..7260b4a96c 100644 > --- a/hw/ppc/Makefile.objs > +++ b/hw/ppc/Makefile.objs > @@ -7,7 +7,7 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o > obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o > obj-$(CONFIG_SPAPR_RNG) += spapr_rng.o > # IBM PowerNV > -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o > +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o pnv_homer.o add an extra line. > ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) > obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o > endif > diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c > new file mode 100644 > index 0000000000..73a94856d0 > --- /dev/null > +++ b/hw/ppc/pnv_homer.c > @@ -0,0 +1,185 @@ > +/* > + * QEMU PowerPC PowerNV Homer and OCC common area region > + * > + * Copyright (c) 2019, 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/hw_accel.h" > +#include "sysemu/cpus.h" > +#include "hw/ppc/pnv.h" > + > +static bool core_max_array(hwaddr addr) > +{ > + char *cpu_type; > + hwaddr core_max_base = 0xe2819; What is this representing ? > + MachineState *ms = MACHINE(qdev_get_machine()); > + cpu_type = strstr(ms->cpu_type, "power8"); you need to get this information some other way. The PnvChip should have it. > + if (cpu_type) > + core_max_base = 0x1f8810; It could be a PnvChipClass value. > + for (int i = 0; i <= ms->smp.cores; i++) > + if (addr == (core_max_base + i)) > + return true; > + return false; > +} > +static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width) > +{ > + switch (addr) { We should be using defines for the case statements below. Are we accessing one or more structures which are mapped at specific addresses ? If so I would define them in this file and change the memory ops to use well known offsets. Are these structures the same on P9 and P8 ? Are there default values ? May be we could use a reset handler in this case. > + case 0xe2006: /* max pstate ultra turbo */ > + case 0xe2018: /* pstate id for 0 */ > + case 0x1f8001: /* P8 occ pstate version */ > + case 0x1f8003: /* P8 pstate min */ > + case 0x1f8010: /* P8 pstate id for 0 */ > + return 0; > + case 0xe2000: /* occ data area */ > + case 0xe2002: /* occ_role master/slave*/ > + case 0xe2004: /* pstate nom */ > + case 0xe2005: /* pstate turbo */ > + case 0xe2020: /* pstate id for 1 */ > + case 0xe2818: /* pstate ultra turbo */ > + case 0xe2b85: /* opal dynamic data (runtime) */ > + case 0x1f8000: /* P8 occ pstate valid */ > + case 0x1f8002: /* P8 throttle */ > + case 0x1f8004: /* P8 pstate nom */ > + case 0x1f8005: /* P8 pstate turbo */ > + case 0x1f8012: /* vdd voltage identifier */ > + case 0x1f8013: /* vcs voltage identifier */ > + case 0x1f8018: /* P8 pstate id for 1 */ > + return 1; > + case 0xe2003: /* pstate min (2 as pstate min) */ > + case 0xe2028: /* pstate id for 2 */ > + case 0x1f8006: /* P8 pstate ultra turbo */ > + case 0x1f8020: /* P8 pstate id for 2 */ > + return 2; > + case 0xe2001: /* major version */ > + return 0x90; > + /* 3000 khz frequency for 0, 1, and 2 pstates */ > + case 0xe201c: > + case 0xe2024: > + case 0xe202c: > + /* P8 frequency for 0, 1, and 2 pstates */ > + case 0x1f8014: > + case 0x1f801c: > + case 0x1f8024: > + return 3000; > + case 0x0: /* homer base */ > + case 0xe2008: /* occ data area + 8 */ > + case 0x1f8008: /* P8 occ data area + 8 */ > + case 0x200008: /* homer base access to get homer image pointer*/ > + return 0x1000000000000000; > + } > + /* pstate table core max array */ > + if (core_max_array(addr)) > + return 1; I don't understand what the core_max_array is returning > + return 0; > +} > + > +static void homer_write(void *opaque, hwaddr addr, uint64_t val, > + unsigned width) > +{ > + /* callback function defined to homer write */ > + return; > +} > + > +const MemoryRegionOps pnv_homer_ops = { > + .read = homer_read, > + .write = homer_write, > + .valid.min_access_size = 1, > + .valid.max_access_size = 8, > + .impl.min_access_size = 1, > + .impl.max_access_size = 8, > + .endianness = DEVICE_BIG_ENDIAN, > +}; > + > +static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width) > +{ > + switch (addr) { > + /* > + * occ-sensor sanity check that asserts the sensor > + * header block > + */ Same comments as above. > + case 0x580000: /* occ sensor data block */ > + case 0x580001: /* valid */ > + case 0x580002: /* version */ > + case 0x580004: /* reading_version */ > + case 0x580008: /* nr_sensors */ > + case 0x580010: /* names_offset */ > + case 0x580014: /* reading_ping_offset */ > + case 0x58000c: /* reading_pong_offset */ > + case 0x580023: /* structure_type */ > + return 1; > + case 0x58000d: /* name length */ > + return 0x30; > + case 0x580022: /* occ sensor loc core */ > + return 0x0040; > + case 0x580003: /* occ sensor type power */ > + return 0x0080; > + case 0x580005: /* sensor name */ > + return 0x1000; > + case 0x58001e: /* HWMON_SENSORS_MASK */ > + case 0x580020: > + return 0x8e00; > + case 0x0: /* P8 slw base access for slw image size */ > + return 0x1000000000000000; > + } > + return 0; > +} > + > +static void occ_common_area_write(void *opaque, hwaddr addr, uint64_t val, > + unsigned width) > +{ > + /* callback function defined to occ common area write */ > + return; > +} > + > +const MemoryRegionOps pnv_occ_common_area_ops = { > + .read = occ_common_area_read, > + .write = occ_common_area_write, > + .valid.min_access_size = 1, > + .valid.max_access_size = 8, > + .impl.min_access_size = 1, > + .impl.max_access_size = 8, > + .endianness = DEVICE_BIG_ENDIAN, > +}; Why aren't you using the PnvOCC model ? > +void pnv_occ_common_area_realize(PnvChip *chip, Error **errp) > +{ > + SysBusDevice *sbd = SYS_BUS_DEVICE(chip); > + sbd->num_mmio = PNV_OCC_COMMON_AREA_SYSBUS; > + char *occ_common_area; > + > + /* occ common area */ > + occ_common_area = g_strdup_printf("occ-common-area-%x", chip->chip_id); > + memory_region_init_io(&chip->occ_common_area_mmio, OBJECT(chip), > + &pnv_occ_common_area_ops, chip, occ_common_area, > + PNV_OCC_COMMON_AREA_SIZE); > + sysbus_init_mmio(sbd, &chip->occ_common_area_mmio); > + g_free(occ_common_area); > +} May be this "device" deserves a PnvHomer model, one for P8 and one for P9. > +void pnv_homer_realize(PnvChip *chip, Error **errp) > +{ > + SysBusDevice *sbd = SYS_BUS_DEVICE(chip); > + sbd->num_mmio = PNV_HOMER_SYSBUS; > + char *homer; > + > + /* homer region */ > + homer = g_strdup_printf("homer-%x", chip->chip_id); > + memory_region_init_io(&chip->homer_mmio, OBJECT(chip), &pnv_homer_ops, > + chip, homer, PNV_HOMER_SIZE); > + sysbus_init_mmio(sbd, &chip->homer_mmio); > + g_free(homer); > +} > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h > index fb123edc4e..6464e32892 100644 > --- a/include/hw/ppc/pnv.h > +++ b/include/hw/ppc/pnv.h > @@ -28,6 +28,7 @@ > #include "hw/ppc/pnv_occ.h" > #include "hw/ppc/pnv_xive.h" > #include "hw/ppc/pnv_core.h" > +#include "hw/ppc/pnv_homer.h" > > #define TYPE_PNV_CHIP "pnv-chip" > #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP) > @@ -36,6 +37,13 @@ > #define PNV_CHIP_GET_CLASS(obj) \ > OBJECT_GET_CLASS(PnvChipClass, (obj), TYPE_PNV_CHIP) > > +enum SysBusNum { > + PNV_XSCOM_SYSBUS, > + PNV_ICP_SYSBUS, > + PNV_HOMER_SYSBUS, > + PNV_OCC_COMMON_AREA_SYSBUS, > +}; What is this ? > typedef enum PnvChipType { > PNV_CHIP_POWER8E, /* AKA Murano (default) */ > PNV_CHIP_POWER8, /* AKA Venice */ > @@ -56,6 +64,8 @@ typedef struct PnvChip { > uint64_t cores_mask; > void *cores; > > + MemoryRegion homer_mmio; > + MemoryRegion occ_common_area_mmio; > MemoryRegion xscom_mmio; > MemoryRegion xscom; > AddressSpace xscom_as; > @@ -191,6 +201,10 @@ static inline bool pnv_is_power9(PnvMachineState *pnv) > void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt); > void pnv_bmc_powerdown(IPMIBmc *bmc); > > +extern void pnv_occ_common_area_realize(PnvChip *chip, Error **errp); > +extern void pnv_homer_realize(PnvChip *chip, Error **errp); > + > + > /* > * POWER8 MMIO base addresses > */ > diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h > new file mode 100644 > index 0000000000..0fe6469abe > --- /dev/null > +++ b/include/hw/ppc/pnv_homer.h > @@ -0,0 +1,41 @@ > +/* > + * QEMU PowerPC PowerNV Homer and occ common area definitions > + * > + * Copyright (c) 2019, 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_HOMER_H > +#define _PPC_PNV_HOMER_H > + > +#include "qom/object.h" > + > +/* > + * HOMER region size 4M per OCC (1 OCC is defined per chip in struct PnvChip) > + * so chip_num can be used to offset between HOMER region from its base address > + */ > +#define PNV_HOMER_SIZE 0x300000 > +#define PNV_OCC_COMMON_AREA_SIZE 0x700000 > + > +#define PNV_HOMER_BASE(chip) \ > + (0x7ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE) > +#define PNV_OCC_COMMON_AREA(chip) \ > + (0x7fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE) > + > +#define PNV9_HOMER_BASE(chip) \ > + (0x203ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE) > +#define PNV9_OCC_COMMON_AREA(chip) \ > + (0x203fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE) > + > +#endif /* _PPC_PNV_HOMER_H */ >
On Wed, Aug 07, 2019 at 09:54:55AM +0200, Cédric Le Goater wrote: > On 07/08/2019 09:14, Balamuruhan S wrote: > > Add mmio callback functions to enable homer/occ common area > > to emulate pstate table, occ-sensors, slw, occ static and > > dynamic values for Power8 and Power9 chips. It also works for > > multiple chips as offset remains the same whereas the base > > address are handled appropriately while initializing device > > tree. > > > > currently skiboot disables the homer/occ code path with > > `QUIRK_NO_PBA`, this quirk have to be removed in skiboot > > for it to use this infrastructure. > > > I think this patch can come before the others as it is adding > support without the python extra facilities. Okay sure. > > Some comments below, > > > Signed-off-by: Hariharan T.S <hari@linux.vnet.ibm.com> > > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com> > > --- > > hw/ppc/Makefile.objs | 2 +- > > hw/ppc/pnv_homer.c | 185 +++++++++++++++++++++++++++++++++++++++++++++ > > include/hw/ppc/pnv.h | 14 ++++ > > include/hw/ppc/pnv_homer.h | 41 ++++++++++ > > 4 files changed, 241 insertions(+), 1 deletion(-) > > create mode 100644 hw/ppc/pnv_homer.c > > create mode 100644 include/hw/ppc/pnv_homer.h > > > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > > index 9da93af905..7260b4a96c 100644 > > --- a/hw/ppc/Makefile.objs > > +++ b/hw/ppc/Makefile.objs > > @@ -7,7 +7,7 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o > > obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o > > obj-$(CONFIG_SPAPR_RNG) += spapr_rng.o > > # IBM PowerNV > > -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o > > +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o pnv_homer.o > > add an extra line. I will do that. > > > ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) > > obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o > > endif > > diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c > > new file mode 100644 > > index 0000000000..73a94856d0 > > --- /dev/null > > +++ b/hw/ppc/pnv_homer.c > > @@ -0,0 +1,185 @@ > > +/* > > + * QEMU PowerPC PowerNV Homer and OCC common area region > > + * > > + * Copyright (c) 2019, 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/hw_accel.h" > > +#include "sysemu/cpus.h" > > +#include "hw/ppc/pnv.h" > > + > > +static bool core_max_array(hwaddr addr) > > +{ > > + char *cpu_type; > > + hwaddr core_max_base = 0xe2819; > > What is this representing ? > > > + MachineState *ms = MACHINE(qdev_get_machine()); > > + cpu_type = strstr(ms->cpu_type, "power8"); > > you need to get this information some other way. The PnvChip should have it. okay I will use it, I think you mean PnvChipType chip_type. > > > + if (cpu_type) > > + core_max_base = 0x1f8810; > > It could be a PnvChipClass value. or should we also consider it also as macro ? > > > + for (int i = 0; i <= ms->smp.cores; i++) > > + if (addr == (core_max_base + i)) > > + return true; > > + return false; > > +} > > > > +static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width) > > +{ > > + switch (addr) { > > We should be using defines for the case statements below. > > Are we accessing one or more structures which are mapped at specific > addresses ? If so I would define them in this file and change the > memory ops to use well known offsets. okay, but lot of defines have to be done, will that be fine ? > > Are these structures the same on P9 and P8 ? no, stuctures are different on P9 and P8, occ pstate table version 0x90 is for P9 and 0x01, 0x02 is for P8. I have mentioned P8 specific offsets in comments and rest are P9. > > Are there default values ? May be we could use a reset handler > in this case. yes, I tried to return default expected values for skiboot. I am not aware of reset handler, where can I check it ? > > > + case 0xe2006: /* max pstate ultra turbo */ > > + case 0xe2018: /* pstate id for 0 */ > > + case 0x1f8001: /* P8 occ pstate version */ > > + case 0x1f8003: /* P8 pstate min */ > > + case 0x1f8010: /* P8 pstate id for 0 */ > > + return 0; > > + case 0xe2000: /* occ data area */ > > + case 0xe2002: /* occ_role master/slave*/ > > + case 0xe2004: /* pstate nom */ > > + case 0xe2005: /* pstate turbo */ > > + case 0xe2020: /* pstate id for 1 */ > > + case 0xe2818: /* pstate ultra turbo */ > > + case 0xe2b85: /* opal dynamic data (runtime) */ > > + case 0x1f8000: /* P8 occ pstate valid */ > > + case 0x1f8002: /* P8 throttle */ > > + case 0x1f8004: /* P8 pstate nom */ > > + case 0x1f8005: /* P8 pstate turbo */ > > + case 0x1f8012: /* vdd voltage identifier */ > > + case 0x1f8013: /* vcs voltage identifier */ > > + case 0x1f8018: /* P8 pstate id for 1 */ > > + return 1; > > + case 0xe2003: /* pstate min (2 as pstate min) */ > > + case 0xe2028: /* pstate id for 2 */ > > + case 0x1f8006: /* P8 pstate ultra turbo */ > > + case 0x1f8020: /* P8 pstate id for 2 */ > > + return 2; > > + case 0xe2001: /* major version */ > > + return 0x90; > > + /* 3000 khz frequency for 0, 1, and 2 pstates */ > > + case 0xe201c: > > + case 0xe2024: > > + case 0xe202c: > > + /* P8 frequency for 0, 1, and 2 pstates */ > > + case 0x1f8014: > > + case 0x1f801c: > > + case 0x1f8024: > > + return 3000; > > + case 0x0: /* homer base */ > > + case 0xe2008: /* occ data area + 8 */ > > + case 0x1f8008: /* P8 occ data area + 8 */ > > + case 0x200008: /* homer base access to get homer image pointer*/ > > + return 0x1000000000000000; > > + } > > + /* pstate table core max array */ > > + if (core_max_array(addr)) > > + return 1; > > I don't understand what the core_max_array is returning core_max_array defaults to nominal pstate 1 as maximum sustainable pstate. please advise if you think it is not right. > > > + return 0; > > +} > > + > > +static void homer_write(void *opaque, hwaddr addr, uint64_t val, > > + unsigned width) > > +{ > > + /* callback function defined to homer write */ > > + return; > > +} > > + > > +const MemoryRegionOps pnv_homer_ops = { > > + .read = homer_read, > > + .write = homer_write, > > + .valid.min_access_size = 1, > > + .valid.max_access_size = 8, > > + .impl.min_access_size = 1, > > + .impl.max_access_size = 8, > > + .endianness = DEVICE_BIG_ENDIAN, > > +}; > > + > > +static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width) > > +{ > > + switch (addr) { > > + /* > > + * occ-sensor sanity check that asserts the sensor > > + * header block > > + */ > > Same comments as above. okay, will do. > > > + case 0x580000: /* occ sensor data block */ > > + case 0x580001: /* valid */ > > + case 0x580002: /* version */ > > + case 0x580004: /* reading_version */ > > + case 0x580008: /* nr_sensors */ > > + case 0x580010: /* names_offset */ > > + case 0x580014: /* reading_ping_offset */ > > + case 0x58000c: /* reading_pong_offset */ > > + case 0x580023: /* structure_type */ > > + return 1; > > + case 0x58000d: /* name length */ > > + return 0x30; > > + case 0x580022: /* occ sensor loc core */ > > + return 0x0040; > > + case 0x580003: /* occ sensor type power */ > > + return 0x0080; > > + case 0x580005: /* sensor name */ > > + return 0x1000; > > + case 0x58001e: /* HWMON_SENSORS_MASK */ > > + case 0x580020: > > + return 0x8e00; > > + case 0x0: /* P8 slw base access for slw image size */ > > + return 0x1000000000000000; > > + } > > + return 0; > > +} > > + > > +static void occ_common_area_write(void *opaque, hwaddr addr, uint64_t val, > > + unsigned width) > > +{ > > + /* callback function defined to occ common area write */ > > + return; > > +} > > + > > +const MemoryRegionOps pnv_occ_common_area_ops = { > > + .read = occ_common_area_read, > > + .write = occ_common_area_write, > > + .valid.min_access_size = 1, > > + .valid.max_access_size = 8, > > + .impl.min_access_size = 1, > > + .impl.max_access_size = 8, > > + .endianness = DEVICE_BIG_ENDIAN, > > +}; > > > Why aren't you using the PnvOCC model ? I was not aware of how to use it here, I will check it to make use of it. > > > +void pnv_occ_common_area_realize(PnvChip *chip, Error **errp) > > +{ > > + SysBusDevice *sbd = SYS_BUS_DEVICE(chip); > > + sbd->num_mmio = PNV_OCC_COMMON_AREA_SYSBUS; > > + char *occ_common_area; > > + > > + /* occ common area */ > > + occ_common_area = g_strdup_printf("occ-common-area-%x", chip->chip_id); > > + memory_region_init_io(&chip->occ_common_area_mmio, OBJECT(chip), > > + &pnv_occ_common_area_ops, chip, occ_common_area, > > + PNV_OCC_COMMON_AREA_SIZE); > > + sysbus_init_mmio(sbd, &chip->occ_common_area_mmio); > > + g_free(occ_common_area); > > +} > > > May be this "device" deserves a PnvHomer model, one for P8 and one for P9. :+1: > > > +void pnv_homer_realize(PnvChip *chip, Error **errp) > > +{ > > + SysBusDevice *sbd = SYS_BUS_DEVICE(chip); > > + sbd->num_mmio = PNV_HOMER_SYSBUS; > > + char *homer; > > + > > + /* homer region */ > > + homer = g_strdup_printf("homer-%x", chip->chip_id); > > + memory_region_init_io(&chip->homer_mmio, OBJECT(chip), &pnv_homer_ops, > > + chip, homer, PNV_HOMER_SIZE); > > + sysbus_init_mmio(sbd, &chip->homer_mmio); > > + g_free(homer); > > +} > > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h > > index fb123edc4e..6464e32892 100644 > > --- a/include/hw/ppc/pnv.h > > +++ b/include/hw/ppc/pnv.h > > @@ -28,6 +28,7 @@ > > #include "hw/ppc/pnv_occ.h" > > #include "hw/ppc/pnv_xive.h" > > #include "hw/ppc/pnv_core.h" > > +#include "hw/ppc/pnv_homer.h" > > > > #define TYPE_PNV_CHIP "pnv-chip" > > #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP) > > @@ -36,6 +37,13 @@ > > #define PNV_CHIP_GET_CLASS(obj) \ > > OBJECT_GET_CLASS(PnvChipClass, (obj), TYPE_PNV_CHIP) > > > > +enum SysBusNum { > > + PNV_XSCOM_SYSBUS, > > + PNV_ICP_SYSBUS, > > + PNV_HOMER_SYSBUS, > > + PNV_OCC_COMMON_AREA_SYSBUS, > > +}; > > What is this ? enumeration for SysBusDevice device init, it is needed for mapping to work, please correct me if it is not the right way. > > > > typedef enum PnvChipType { > > PNV_CHIP_POWER8E, /* AKA Murano (default) */ > > PNV_CHIP_POWER8, /* AKA Venice */ > > @@ -56,6 +64,8 @@ typedef struct PnvChip { > > uint64_t cores_mask; > > void *cores; > > > > + MemoryRegion homer_mmio; > > + MemoryRegion occ_common_area_mmio; > > MemoryRegion xscom_mmio; > > MemoryRegion xscom; > > AddressSpace xscom_as; > > @@ -191,6 +201,10 @@ static inline bool pnv_is_power9(PnvMachineState *pnv) > > void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt); > > void pnv_bmc_powerdown(IPMIBmc *bmc); > > > > +extern void pnv_occ_common_area_realize(PnvChip *chip, Error **errp); > > +extern void pnv_homer_realize(PnvChip *chip, Error **errp); > > + > > + > > /* > > * POWER8 MMIO base addresses > > */ > > diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h > > new file mode 100644 > > index 0000000000..0fe6469abe > > --- /dev/null > > +++ b/include/hw/ppc/pnv_homer.h > > @@ -0,0 +1,41 @@ > > +/* > > + * QEMU PowerPC PowerNV Homer and occ common area definitions > > + * > > + * Copyright (c) 2019, 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_HOMER_H > > +#define _PPC_PNV_HOMER_H > > + > > +#include "qom/object.h" > > + > > +/* > > + * HOMER region size 4M per OCC (1 OCC is defined per chip in struct PnvChip) > > + * so chip_num can be used to offset between HOMER region from its base address > > + */ > > +#define PNV_HOMER_SIZE 0x300000 > > +#define PNV_OCC_COMMON_AREA_SIZE 0x700000 > > + > > +#define PNV_HOMER_BASE(chip) \ > > + (0x7ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE) > > +#define PNV_OCC_COMMON_AREA(chip) \ > > + (0x7fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE) > > + > > +#define PNV9_HOMER_BASE(chip) \ > > + (0x203ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE) > > +#define PNV9_OCC_COMMON_AREA(chip) \ > > + (0x203fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE) > > + > > +#endif /* _PPC_PNV_HOMER_H */ > > >
On 07/08/2019 12:07, Balamuruhan S wrote: > On Wed, Aug 07, 2019 at 09:54:55AM +0200, Cédric Le Goater wrote: >> On 07/08/2019 09:14, Balamuruhan S wrote: >>> Add mmio callback functions to enable homer/occ common area >>> to emulate pstate table, occ-sensors, slw, occ static and >>> dynamic values for Power8 and Power9 chips. It also works for >>> multiple chips as offset remains the same whereas the base >>> address are handled appropriately while initializing device >>> tree. >>> >>> currently skiboot disables the homer/occ code path with >>> `QUIRK_NO_PBA`, this quirk have to be removed in skiboot >>> for it to use this infrastructure. >> >> >> I think this patch can come before the others as it is adding >> support without the python extra facilities. > > Okay sure. > >> >> Some comments below, >> >>> Signed-off-by: Hariharan T.S <hari@linux.vnet.ibm.com> >>> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com> >>> --- >>> hw/ppc/Makefile.objs | 2 +- >>> hw/ppc/pnv_homer.c | 185 +++++++++++++++++++++++++++++++++++++++++++++ >>> include/hw/ppc/pnv.h | 14 ++++ >>> include/hw/ppc/pnv_homer.h | 41 ++++++++++ >>> 4 files changed, 241 insertions(+), 1 deletion(-) >>> create mode 100644 hw/ppc/pnv_homer.c >>> create mode 100644 include/hw/ppc/pnv_homer.h >>> >>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs >>> index 9da93af905..7260b4a96c 100644 >>> --- a/hw/ppc/Makefile.objs >>> +++ b/hw/ppc/Makefile.objs >>> @@ -7,7 +7,7 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o >>> obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o >>> obj-$(CONFIG_SPAPR_RNG) += spapr_rng.o >>> # IBM PowerNV >>> -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o >>> +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o pnv_homer.o >> >> add an extra line. > > I will do that. > >> >>> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) >>> obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o >>> endif >>> diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c >>> new file mode 100644 >>> index 0000000000..73a94856d0 >>> --- /dev/null >>> +++ b/hw/ppc/pnv_homer.c >>> @@ -0,0 +1,185 @@ >>> +/* >>> + * QEMU PowerPC PowerNV Homer and OCC common area region >>> + * >>> + * Copyright (c) 2019, 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/hw_accel.h" >>> +#include "sysemu/cpus.h" >>> +#include "hw/ppc/pnv.h" >>> + >>> +static bool core_max_array(hwaddr addr) >>> +{ >>> + char *cpu_type; >>> + hwaddr core_max_base = 0xe2819; >> >> What is this representing ? >> >>> + MachineState *ms = MACHINE(qdev_get_machine()); >>> + cpu_type = strstr(ms->cpu_type, "power8"); >> >> you need to get this information some other way. The PnvChip should have it. > > okay I will use it, I think you mean PnvChipType chip_type. you will need to fetch this information from some model instance or class, PnvChip or a new one. >> >>> + if (cpu_type) >>> + core_max_base = 0x1f8810; >> >> It could be a PnvChipClass value. > > or should we also consider it also as macro ? it's a chip constant ? see the pnv_chip_*_class_init routine. >> >>> + for (int i = 0; i <= ms->smp.cores; i++) >>> + if (addr == (core_max_base + i)) >>> + return true; >>> + return false; >>> +} >> >> >>> +static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width) >>> +{ >>> + switch (addr) { >> >> We should be using defines for the case statements below. >> >> Are we accessing one or more structures which are mapped at specific >> addresses ? If so I would define them in this file and change the >> memory ops to use well known offsets. > > okay, but lot of defines have to be done, will that be fine ? Not a problem. You can copy the definitions and the structures from skiboot and adapt them to QEMU coding style. Look at the other Pnv models PSI, XIVE, LPC. > >> >> Are these structures the same on P9 and P8 ? > > no, stuctures are different on P9 and P8, occ pstate table version 0x90 is for > P9 and 0x01, 0x02 is for P8. These are class constants. most probably a new PnvXScom/PnvHomer or PnvOCC > I have mentioned P8 specific offsets in comments and rest are P9. > >> >> Are there default values ? May be we could use a reset handler >> in this case. > > yes, I tried to return default expected values for skiboot. I am > not aware of reset handler, where can I check it ? You need to introduce a new model and add a reset handler for it, or use an existing one. I am not sure we need a PnvHomer model but we might need to add PnvXscom and add the HOMER stuff into it. >>> + case 0xe2006: /* max pstate ultra turbo */ >>> + case 0xe2018: /* pstate id for 0 */ >>> + case 0x1f8001: /* P8 occ pstate version */ >>> + case 0x1f8003: /* P8 pstate min */ >>> + case 0x1f8010: /* P8 pstate id for 0 */ >>> + return 0; >>> + case 0xe2000: /* occ data area */ >>> + case 0xe2002: /* occ_role master/slave*/ >>> + case 0xe2004: /* pstate nom */ >>> + case 0xe2005: /* pstate turbo */ >>> + case 0xe2020: /* pstate id for 1 */ >>> + case 0xe2818: /* pstate ultra turbo */ >>> + case 0xe2b85: /* opal dynamic data (runtime) */ >>> + case 0x1f8000: /* P8 occ pstate valid */ >>> + case 0x1f8002: /* P8 throttle */ >>> + case 0x1f8004: /* P8 pstate nom */ >>> + case 0x1f8005: /* P8 pstate turbo */ >>> + case 0x1f8012: /* vdd voltage identifier */ >>> + case 0x1f8013: /* vcs voltage identifier */ >>> + case 0x1f8018: /* P8 pstate id for 1 */ >>> + return 1; >>> + case 0xe2003: /* pstate min (2 as pstate min) */ >>> + case 0xe2028: /* pstate id for 2 */ >>> + case 0x1f8006: /* P8 pstate ultra turbo */ >>> + case 0x1f8020: /* P8 pstate id for 2 */ >>> + return 2; >>> + case 0xe2001: /* major version */ >>> + return 0x90; >>> + /* 3000 khz frequency for 0, 1, and 2 pstates */ >>> + case 0xe201c: >>> + case 0xe2024: >>> + case 0xe202c: >>> + /* P8 frequency for 0, 1, and 2 pstates */ >>> + case 0x1f8014: >>> + case 0x1f801c: >>> + case 0x1f8024: >>> + return 3000; >>> + case 0x0: /* homer base */ >>> + case 0xe2008: /* occ data area + 8 */ >>> + case 0x1f8008: /* P8 occ data area + 8 */ >>> + case 0x200008: /* homer base access to get homer image pointer*/ >>> + return 0x1000000000000000; >>> + } >>> + /* pstate table core max array */ >>> + if (core_max_array(addr)) >>> + return 1; >> >> I don't understand what the core_max_array is returning > > core_max_array defaults to nominal pstate 1 as maximum sustainable > pstate. please advise if you think it is not right. I still do not understand what this is about :/ > >> >>> + return 0; >>> +} >>> + >>> +static void homer_write(void *opaque, hwaddr addr, uint64_t val, >>> + unsigned width) >>> +{ >>> + /* callback function defined to homer write */ >>> + return; >>> +} >>> + >>> +const MemoryRegionOps pnv_homer_ops = { >>> + .read = homer_read, >>> + .write = homer_write, >>> + .valid.min_access_size = 1, >>> + .valid.max_access_size = 8, >>> + .impl.min_access_size = 1, >>> + .impl.max_access_size = 8, >>> + .endianness = DEVICE_BIG_ENDIAN, >>> +}; >>> + >>> +static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width) >>> +{ >>> + switch (addr) { >>> + /* >>> + * occ-sensor sanity check that asserts the sensor >>> + * header block >>> + */ >> >> Same comments as above. > > okay, will do. >> >>> + case 0x580000: /* occ sensor data block */ >>> + case 0x580001: /* valid */ >>> + case 0x580002: /* version */ >>> + case 0x580004: /* reading_version */ >>> + case 0x580008: /* nr_sensors */ >>> + case 0x580010: /* names_offset */ >>> + case 0x580014: /* reading_ping_offset */ >>> + case 0x58000c: /* reading_pong_offset */ >>> + case 0x580023: /* structure_type */ >>> + return 1; >>> + case 0x58000d: /* name length */ >>> + return 0x30; >>> + case 0x580022: /* occ sensor loc core */ >>> + return 0x0040; >>> + case 0x580003: /* occ sensor type power */ >>> + return 0x0080; >>> + case 0x580005: /* sensor name */ >>> + return 0x1000; >>> + case 0x58001e: /* HWMON_SENSORS_MASK */ >>> + case 0x580020: >>> + return 0x8e00; >>> + case 0x0: /* P8 slw base access for slw image size */ >>> + return 0x1000000000000000; >>> + } >>> + return 0; >>> +} >>> + >>> +static void occ_common_area_write(void *opaque, hwaddr addr, uint64_t val, >>> + unsigned width) >>> +{ >>> + /* callback function defined to occ common area write */ >>> + return; >>> +} >>> + >>> +const MemoryRegionOps pnv_occ_common_area_ops = { >>> + .read = occ_common_area_read, >>> + .write = occ_common_area_write, >>> + .valid.min_access_size = 1, >>> + .valid.max_access_size = 8, >>> + .impl.min_access_size = 1, >>> + .impl.max_access_size = 8, >>> + .endianness = DEVICE_BIG_ENDIAN, >>> +}; >> >> >> Why aren't you using the PnvOCC model ? > > I was not aware of how to use it here, I will check it to make use > of it. Yes. Check how the machine and the chips are defined in pnv.c and then the other logic units, LPC, XIVE, OCC, etc. The XSCOM bus does not have a model of its own because it is limited to an address space. We might need to introduce a PnvXscom model and put the HOMER info there. > >> >>> +void pnv_occ_common_area_realize(PnvChip *chip, Error **errp) >>> +{ >>> + SysBusDevice *sbd = SYS_BUS_DEVICE(chip); >>> + sbd->num_mmio = PNV_OCC_COMMON_AREA_SYSBUS; >>> + char *occ_common_area; >>> + >>> + /* occ common area */ >>> + occ_common_area = g_strdup_printf("occ-common-area-%x", chip->chip_id); >>> + memory_region_init_io(&chip->occ_common_area_mmio, OBJECT(chip), >>> + &pnv_occ_common_area_ops, chip, occ_common_area, >>> + PNV_OCC_COMMON_AREA_SIZE); >>> + sysbus_init_mmio(sbd, &chip->occ_common_area_mmio); >>> + g_free(occ_common_area); >>> +} >> >> >> May be this "device" deserves a PnvHomer model, one for P8 and one for P9. > > :+1: For OCC stuff, please try to extend the OCC model with a new region mapping the structures of the SRAM that the chip uses for sensors, pstate, etc. >> >>> +void pnv_homer_realize(PnvChip *chip, Error **errp) >>> +{ >>> + SysBusDevice *sbd = SYS_BUS_DEVICE(chip); >>> + sbd->num_mmio = PNV_HOMER_SYSBUS; >>> + char *homer; >>> + >>> + /* homer region */ >>> + homer = g_strdup_printf("homer-%x", chip->chip_id); >>> + memory_region_init_io(&chip->homer_mmio, OBJECT(chip), &pnv_homer_ops, >>> + chip, homer, PNV_HOMER_SIZE); >>> + sysbus_init_mmio(sbd, &chip->homer_mmio); >>> + g_free(homer); >>> +} >>> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h >>> index fb123edc4e..6464e32892 100644 >>> --- a/include/hw/ppc/pnv.h >>> +++ b/include/hw/ppc/pnv.h >>> @@ -28,6 +28,7 @@ >>> #include "hw/ppc/pnv_occ.h" >>> #include "hw/ppc/pnv_xive.h" >>> #include "hw/ppc/pnv_core.h" >>> +#include "hw/ppc/pnv_homer.h" >>> >>> #define TYPE_PNV_CHIP "pnv-chip" >>> #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP) >>> @@ -36,6 +37,13 @@ >>> #define PNV_CHIP_GET_CLASS(obj) \ >>> OBJECT_GET_CLASS(PnvChipClass, (obj), TYPE_PNV_CHIP) >>> >>> +enum SysBusNum { >>> + PNV_XSCOM_SYSBUS, >>> + PNV_ICP_SYSBUS, >>> + PNV_HOMER_SYSBUS, >>> + PNV_OCC_COMMON_AREA_SYSBUS, >>> +}; >> >> What is this ? > > enumeration for SysBusDevice device init, it is needed for mapping to > work, please correct me if it is not the right way. I think you can add the HOMER memory region in a new PnvXscom model. the SRAM memory region can go in the OCC model. We need to check how all these models interacts. Thanks, C. > >> >> >>> typedef enum PnvChipType { >>> PNV_CHIP_POWER8E, /* AKA Murano (default) */ >>> PNV_CHIP_POWER8, /* AKA Venice */ >>> @@ -56,6 +64,8 @@ typedef struct PnvChip { >>> uint64_t cores_mask; >>> void *cores; >>> >>> + MemoryRegion homer_mmio; >>> + MemoryRegion occ_common_area_mmio; >>> MemoryRegion xscom_mmio; >>> MemoryRegion xscom; >>> AddressSpace xscom_as; >>> @@ -191,6 +201,10 @@ static inline bool pnv_is_power9(PnvMachineState *pnv) >>> void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt); >>> void pnv_bmc_powerdown(IPMIBmc *bmc); >>> >>> +extern void pnv_occ_common_area_realize(PnvChip *chip, Error **errp); >>> +extern void pnv_homer_realize(PnvChip *chip, Error **errp); >>> + >>> + >>> /* >>> * POWER8 MMIO base addresses >>> */ >>> diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h >>> new file mode 100644 >>> index 0000000000..0fe6469abe >>> --- /dev/null >>> +++ b/include/hw/ppc/pnv_homer.h >>> @@ -0,0 +1,41 @@ >>> +/* >>> + * QEMU PowerPC PowerNV Homer and occ common area definitions >>> + * >>> + * Copyright (c) 2019, 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_HOMER_H >>> +#define _PPC_PNV_HOMER_H >>> + >>> +#include "qom/object.h" >>> + >>> +/* >>> + * HOMER region size 4M per OCC (1 OCC is defined per chip in struct PnvChip) >>> + * so chip_num can be used to offset between HOMER region from its base address >>> + */ >>> +#define PNV_HOMER_SIZE 0x300000 >>> +#define PNV_OCC_COMMON_AREA_SIZE 0x700000 >>> + >>> +#define PNV_HOMER_BASE(chip) \ >>> + (0x7ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE) >>> +#define PNV_OCC_COMMON_AREA(chip) \ >>> + (0x7fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE) >>> + >>> +#define PNV9_HOMER_BASE(chip) \ >>> + (0x203ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE) >>> +#define PNV9_OCC_COMMON_AREA(chip) \ >>> + (0x203fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE) >>> + >>> +#endif /* _PPC_PNV_HOMER_H */ >>> >> >
On Wed, Aug 07, 2019 at 09:54:55AM +0200, Cédric Le Goater wrote: > On 07/08/2019 09:14, Balamuruhan S wrote: > > Add mmio callback functions to enable homer/occ common area > > to emulate pstate table, occ-sensors, slw, occ static and > > dynamic values for Power8 and Power9 chips. It also works for > > multiple chips as offset remains the same whereas the base > > address are handled appropriately while initializing device > > tree. > > > > currently skiboot disables the homer/occ code path with > > `QUIRK_NO_PBA`, this quirk have to be removed in skiboot > > for it to use this infrastructure. > > > I think this patch can come before the others as it is adding > support without the python extra facilities. Right. In fact it seems to me having it as an entirely separate series would be preferable. I don't think we want to tie review of a basic OCC extension to to the frankly not all that palatable idea of adding arbitrary scripting into the MMIO path. > > Some comments below, > > > Signed-off-by: Hariharan T.S <hari@linux.vnet.ibm.com> > > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com> > > --- > > hw/ppc/Makefile.objs | 2 +- > > hw/ppc/pnv_homer.c | 185 +++++++++++++++++++++++++++++++++++++++++++++ > > include/hw/ppc/pnv.h | 14 ++++ > > include/hw/ppc/pnv_homer.h | 41 ++++++++++ > > 4 files changed, 241 insertions(+), 1 deletion(-) > > create mode 100644 hw/ppc/pnv_homer.c > > create mode 100644 include/hw/ppc/pnv_homer.h > > > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > > index 9da93af905..7260b4a96c 100644 > > --- a/hw/ppc/Makefile.objs > > +++ b/hw/ppc/Makefile.objs > > @@ -7,7 +7,7 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o > > obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o > > obj-$(CONFIG_SPAPR_RNG) += spapr_rng.o > > # IBM PowerNV > > -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o > > +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o pnv_homer.o > > add an extra line. > > > ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) > > obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o > > endif > > diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c > > new file mode 100644 > > index 0000000000..73a94856d0 > > --- /dev/null > > +++ b/hw/ppc/pnv_homer.c > > @@ -0,0 +1,185 @@ > > +/* > > + * QEMU PowerPC PowerNV Homer and OCC common area region > > + * > > + * Copyright (c) 2019, 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/hw_accel.h" > > +#include "sysemu/cpus.h" > > +#include "hw/ppc/pnv.h" > > + > > +static bool core_max_array(hwaddr addr) > > +{ > > + char *cpu_type; > > + hwaddr core_max_base = 0xe2819; > > What is this representing ? > > > + MachineState *ms = MACHINE(qdev_get_machine()); > > + cpu_type = strstr(ms->cpu_type, "power8"); > > you need to get this information some other way. The PnvChip should have it. > > > + if (cpu_type) > > + core_max_base = 0x1f8810; > > It could be a PnvChipClass value. > > > + for (int i = 0; i <= ms->smp.cores; i++) > > + if (addr == (core_max_base + i)) > > + return true; > > + return false; > > +} > > > > +static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width) > > +{ > > + switch (addr) { > > We should be using defines for the case statements below. > > Are we accessing one or more structures which are mapped at specific > addresses ? If so I would define them in this file and change the > memory ops to use well known offsets. > > Are these structures the same on P9 and P8 ? > > Are there default values ? May be we could use a reset handler > in this case. > > > + case 0xe2006: /* max pstate ultra turbo */ > > + case 0xe2018: /* pstate id for 0 */ > > + case 0x1f8001: /* P8 occ pstate version */ > > + case 0x1f8003: /* P8 pstate min */ > > + case 0x1f8010: /* P8 pstate id for 0 */ > > + return 0; > > + case 0xe2000: /* occ data area */ > > + case 0xe2002: /* occ_role master/slave*/ > > + case 0xe2004: /* pstate nom */ > > + case 0xe2005: /* pstate turbo */ > > + case 0xe2020: /* pstate id for 1 */ > > + case 0xe2818: /* pstate ultra turbo */ > > + case 0xe2b85: /* opal dynamic data (runtime) */ > > + case 0x1f8000: /* P8 occ pstate valid */ > > + case 0x1f8002: /* P8 throttle */ > > + case 0x1f8004: /* P8 pstate nom */ > > + case 0x1f8005: /* P8 pstate turbo */ > > + case 0x1f8012: /* vdd voltage identifier */ > > + case 0x1f8013: /* vcs voltage identifier */ > > + case 0x1f8018: /* P8 pstate id for 1 */ > > + return 1; > > + case 0xe2003: /* pstate min (2 as pstate min) */ > > + case 0xe2028: /* pstate id for 2 */ > > + case 0x1f8006: /* P8 pstate ultra turbo */ > > + case 0x1f8020: /* P8 pstate id for 2 */ > > + return 2; > > + case 0xe2001: /* major version */ > > + return 0x90; > > + /* 3000 khz frequency for 0, 1, and 2 pstates */ > > + case 0xe201c: > > + case 0xe2024: > > + case 0xe202c: > > + /* P8 frequency for 0, 1, and 2 pstates */ > > + case 0x1f8014: > > + case 0x1f801c: > > + case 0x1f8024: > > + return 3000; > > + case 0x0: /* homer base */ > > + case 0xe2008: /* occ data area + 8 */ > > + case 0x1f8008: /* P8 occ data area + 8 */ > > + case 0x200008: /* homer base access to get homer image pointer*/ > > + return 0x1000000000000000; > > + } > > + /* pstate table core max array */ > > + if (core_max_array(addr)) > > + return 1; > > I don't understand what the core_max_array is returning > > > + return 0; > > +} > > + > > +static void homer_write(void *opaque, hwaddr addr, uint64_t val, > > + unsigned width) > > +{ > > + /* callback function defined to homer write */ > > + return; > > +} > > + > > +const MemoryRegionOps pnv_homer_ops = { > > + .read = homer_read, > > + .write = homer_write, > > + .valid.min_access_size = 1, > > + .valid.max_access_size = 8, > > + .impl.min_access_size = 1, > > + .impl.max_access_size = 8, > > + .endianness = DEVICE_BIG_ENDIAN, > > +}; > > + > > +static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width) > > +{ > > + switch (addr) { > > + /* > > + * occ-sensor sanity check that asserts the sensor > > + * header block > > + */ > > Same comments as above. > > > + case 0x580000: /* occ sensor data block */ > > + case 0x580001: /* valid */ > > + case 0x580002: /* version */ > > + case 0x580004: /* reading_version */ > > + case 0x580008: /* nr_sensors */ > > + case 0x580010: /* names_offset */ > > + case 0x580014: /* reading_ping_offset */ > > + case 0x58000c: /* reading_pong_offset */ > > + case 0x580023: /* structure_type */ > > + return 1; > > + case 0x58000d: /* name length */ > > + return 0x30; > > + case 0x580022: /* occ sensor loc core */ > > + return 0x0040; > > + case 0x580003: /* occ sensor type power */ > > + return 0x0080; > > + case 0x580005: /* sensor name */ > > + return 0x1000; > > + case 0x58001e: /* HWMON_SENSORS_MASK */ > > + case 0x580020: > > + return 0x8e00; > > + case 0x0: /* P8 slw base access for slw image size */ > > + return 0x1000000000000000; > > + } > > + return 0; > > +} > > + > > +static void occ_common_area_write(void *opaque, hwaddr addr, uint64_t val, > > + unsigned width) > > +{ > > + /* callback function defined to occ common area write */ > > + return; > > +} > > + > > +const MemoryRegionOps pnv_occ_common_area_ops = { > > + .read = occ_common_area_read, > > + .write = occ_common_area_write, > > + .valid.min_access_size = 1, > > + .valid.max_access_size = 8, > > + .impl.min_access_size = 1, > > + .impl.max_access_size = 8, > > + .endianness = DEVICE_BIG_ENDIAN, > > +}; > > > Why aren't you using the PnvOCC model ? > > > +void pnv_occ_common_area_realize(PnvChip *chip, Error **errp) > > +{ > > + SysBusDevice *sbd = SYS_BUS_DEVICE(chip); > > + sbd->num_mmio = PNV_OCC_COMMON_AREA_SYSBUS; > > + char *occ_common_area; > > + > > + /* occ common area */ > > + occ_common_area = g_strdup_printf("occ-common-area-%x", chip->chip_id); > > + memory_region_init_io(&chip->occ_common_area_mmio, OBJECT(chip), > > + &pnv_occ_common_area_ops, chip, occ_common_area, > > + PNV_OCC_COMMON_AREA_SIZE); > > + sysbus_init_mmio(sbd, &chip->occ_common_area_mmio); > > + g_free(occ_common_area); > > +} > > > May be this "device" deserves a PnvHomer model, one for P8 and one for P9. > > > +void pnv_homer_realize(PnvChip *chip, Error **errp) > > +{ > > + SysBusDevice *sbd = SYS_BUS_DEVICE(chip); > > + sbd->num_mmio = PNV_HOMER_SYSBUS; > > + char *homer; > > + > > + /* homer region */ > > + homer = g_strdup_printf("homer-%x", chip->chip_id); > > + memory_region_init_io(&chip->homer_mmio, OBJECT(chip), &pnv_homer_ops, > > + chip, homer, PNV_HOMER_SIZE); > > + sysbus_init_mmio(sbd, &chip->homer_mmio); > > + g_free(homer); > > +} > > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h > > index fb123edc4e..6464e32892 100644 > > --- a/include/hw/ppc/pnv.h > > +++ b/include/hw/ppc/pnv.h > > @@ -28,6 +28,7 @@ > > #include "hw/ppc/pnv_occ.h" > > #include "hw/ppc/pnv_xive.h" > > #include "hw/ppc/pnv_core.h" > > +#include "hw/ppc/pnv_homer.h" > > > > #define TYPE_PNV_CHIP "pnv-chip" > > #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP) > > @@ -36,6 +37,13 @@ > > #define PNV_CHIP_GET_CLASS(obj) \ > > OBJECT_GET_CLASS(PnvChipClass, (obj), TYPE_PNV_CHIP) > > > > +enum SysBusNum { > > + PNV_XSCOM_SYSBUS, > > + PNV_ICP_SYSBUS, > > + PNV_HOMER_SYSBUS, > > + PNV_OCC_COMMON_AREA_SYSBUS, > > +}; > > What is this ? > > > > typedef enum PnvChipType { > > PNV_CHIP_POWER8E, /* AKA Murano (default) */ > > PNV_CHIP_POWER8, /* AKA Venice */ > > @@ -56,6 +64,8 @@ typedef struct PnvChip { > > uint64_t cores_mask; > > void *cores; > > > > + MemoryRegion homer_mmio; > > + MemoryRegion occ_common_area_mmio; > > MemoryRegion xscom_mmio; > > MemoryRegion xscom; > > AddressSpace xscom_as; > > @@ -191,6 +201,10 @@ static inline bool pnv_is_power9(PnvMachineState *pnv) > > void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt); > > void pnv_bmc_powerdown(IPMIBmc *bmc); > > > > +extern void pnv_occ_common_area_realize(PnvChip *chip, Error **errp); > > +extern void pnv_homer_realize(PnvChip *chip, Error **errp); > > + > > + > > /* > > * POWER8 MMIO base addresses > > */ > > diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h > > new file mode 100644 > > index 0000000000..0fe6469abe > > --- /dev/null > > +++ b/include/hw/ppc/pnv_homer.h > > @@ -0,0 +1,41 @@ > > +/* > > + * QEMU PowerPC PowerNV Homer and occ common area definitions > > + * > > + * Copyright (c) 2019, 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_HOMER_H > > +#define _PPC_PNV_HOMER_H > > + > > +#include "qom/object.h" > > + > > +/* > > + * HOMER region size 4M per OCC (1 OCC is defined per chip in struct PnvChip) > > + * so chip_num can be used to offset between HOMER region from its base address > > + */ > > +#define PNV_HOMER_SIZE 0x300000 > > +#define PNV_OCC_COMMON_AREA_SIZE 0x700000 > > + > > +#define PNV_HOMER_BASE(chip) \ > > + (0x7ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE) > > +#define PNV_OCC_COMMON_AREA(chip) \ > > + (0x7fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE) > > + > > +#define PNV9_HOMER_BASE(chip) \ > > + (0x203ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE) > > +#define PNV9_OCC_COMMON_AREA(chip) \ > > + (0x203fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE) > > + > > +#endif /* _PPC_PNV_HOMER_H */ > > >
On 8/9/19 10:14 AM, David Gibson wrote: > On Wed, Aug 07, 2019 at 09:54:55AM +0200, Cédric Le Goater wrote: >> On 07/08/2019 09:14, Balamuruhan S wrote: >>> Add mmio callback functions to enable homer/occ common area >>> to emulate pstate table, occ-sensors, slw, occ static and >>> dynamic values for Power8 and Power9 chips. It also works for >>> multiple chips as offset remains the same whereas the base >>> address are handled appropriately while initializing device >>> tree. >>> >>> currently skiboot disables the homer/occ code path with >>> `QUIRK_NO_PBA`, this quirk have to be removed in skiboot >>> for it to use this infrastructure. >> >> I think this patch can come before the others as it is adding >> support without the python extra facilities. > Right. In fact it seems to me having it as an entirely separate > series would be preferable. I don't think we want to tie review of a > basic OCC extension to to the frankly not all that palatable idea of > adding arbitrary scripting into the MMIO path. sure, I will send them as separate series. But the idea is to demonstrate how the scripting interface can be added and to leverage it and I can get feedback/suggestions to correct/improve it. > >> Some comments below, >> >>> Signed-off-by: Hariharan T.S <hari@linux.vnet.ibm.com> >>> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com> >>> --- >>> hw/ppc/Makefile.objs | 2 +- >>> hw/ppc/pnv_homer.c | 185 +++++++++++++++++++++++++++++++++++++++++++++ >>> include/hw/ppc/pnv.h | 14 ++++ >>> include/hw/ppc/pnv_homer.h | 41 ++++++++++ >>> 4 files changed, 241 insertions(+), 1 deletion(-) >>> create mode 100644 hw/ppc/pnv_homer.c >>> create mode 100644 include/hw/ppc/pnv_homer.h >>> >>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs >>> index 9da93af905..7260b4a96c 100644 >>> --- a/hw/ppc/Makefile.objs >>> +++ b/hw/ppc/Makefile.objs >>> @@ -7,7 +7,7 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o >>> obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o >>> obj-$(CONFIG_SPAPR_RNG) += spapr_rng.o >>> # IBM PowerNV >>> -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o >>> +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o pnv_homer.o >> add an extra line. >> >>> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) >>> obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o >>> endif >>> diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c >>> new file mode 100644 >>> index 0000000000..73a94856d0 >>> --- /dev/null >>> +++ b/hw/ppc/pnv_homer.c >>> @@ -0,0 +1,185 @@ >>> +/* >>> + * QEMU PowerPC PowerNV Homer and OCC common area region >>> + * >>> + * Copyright (c) 2019, 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/hw_accel.h" >>> +#include "sysemu/cpus.h" >>> +#include "hw/ppc/pnv.h" >>> + >>> +static bool core_max_array(hwaddr addr) >>> +{ >>> + char *cpu_type; >>> + hwaddr core_max_base = 0xe2819; >> What is this representing ? >> >>> + MachineState *ms = MACHINE(qdev_get_machine()); >>> + cpu_type = strstr(ms->cpu_type, "power8"); >> you need to get this information some other way. The PnvChip should have it. >> >>> + if (cpu_type) >>> + core_max_base = 0x1f8810; >> It could be a PnvChipClass value. >> >>> + for (int i = 0; i <= ms->smp.cores; i++) >>> + if (addr == (core_max_base + i)) >>> + return true; >>> + return false; >>> +} >> >>> +static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width) >>> +{ >>> + switch (addr) { >> We should be using defines for the case statements below. >> >> Are we accessing one or more structures which are mapped at specific >> addresses ? If so I would define them in this file and change the >> memory ops to use well known offsets. >> >> Are these structures the same on P9 and P8 ? >> >> Are there default values ? May be we could use a reset handler >> in this case. >> >>> + case 0xe2006: /* max pstate ultra turbo */ >>> + case 0xe2018: /* pstate id for 0 */ >>> + case 0x1f8001: /* P8 occ pstate version */ >>> + case 0x1f8003: /* P8 pstate min */ >>> + case 0x1f8010: /* P8 pstate id for 0 */ >>> + return 0; >>> + case 0xe2000: /* occ data area */ >>> + case 0xe2002: /* occ_role master/slave*/ >>> + case 0xe2004: /* pstate nom */ >>> + case 0xe2005: /* pstate turbo */ >>> + case 0xe2020: /* pstate id for 1 */ >>> + case 0xe2818: /* pstate ultra turbo */ >>> + case 0xe2b85: /* opal dynamic data (runtime) */ >>> + case 0x1f8000: /* P8 occ pstate valid */ >>> + case 0x1f8002: /* P8 throttle */ >>> + case 0x1f8004: /* P8 pstate nom */ >>> + case 0x1f8005: /* P8 pstate turbo */ >>> + case 0x1f8012: /* vdd voltage identifier */ >>> + case 0x1f8013: /* vcs voltage identifier */ >>> + case 0x1f8018: /* P8 pstate id for 1 */ >>> + return 1; >>> + case 0xe2003: /* pstate min (2 as pstate min) */ >>> + case 0xe2028: /* pstate id for 2 */ >>> + case 0x1f8006: /* P8 pstate ultra turbo */ >>> + case 0x1f8020: /* P8 pstate id for 2 */ >>> + return 2; >>> + case 0xe2001: /* major version */ >>> + return 0x90; >>> + /* 3000 khz frequency for 0, 1, and 2 pstates */ >>> + case 0xe201c: >>> + case 0xe2024: >>> + case 0xe202c: >>> + /* P8 frequency for 0, 1, and 2 pstates */ >>> + case 0x1f8014: >>> + case 0x1f801c: >>> + case 0x1f8024: >>> + return 3000; >>> + case 0x0: /* homer base */ >>> + case 0xe2008: /* occ data area + 8 */ >>> + case 0x1f8008: /* P8 occ data area + 8 */ >>> + case 0x200008: /* homer base access to get homer image pointer*/ >>> + return 0x1000000000000000; >>> + } >>> + /* pstate table core max array */ >>> + if (core_max_array(addr)) >>> + return 1; >> I don't understand what the core_max_array is returning >> >>> + return 0; >>> +} >>> + >>> +static void homer_write(void *opaque, hwaddr addr, uint64_t val, >>> + unsigned width) >>> +{ >>> + /* callback function defined to homer write */ >>> + return; >>> +} >>> + >>> +const MemoryRegionOps pnv_homer_ops = { >>> + .read = homer_read, >>> + .write = homer_write, >>> + .valid.min_access_size = 1, >>> + .valid.max_access_size = 8, >>> + .impl.min_access_size = 1, >>> + .impl.max_access_size = 8, >>> + .endianness = DEVICE_BIG_ENDIAN, >>> +}; >>> + >>> +static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width) >>> +{ >>> + switch (addr) { >>> + /* >>> + * occ-sensor sanity check that asserts the sensor >>> + * header block >>> + */ >> Same comments as above. >> >>> + case 0x580000: /* occ sensor data block */ >>> + case 0x580001: /* valid */ >>> + case 0x580002: /* version */ >>> + case 0x580004: /* reading_version */ >>> + case 0x580008: /* nr_sensors */ >>> + case 0x580010: /* names_offset */ >>> + case 0x580014: /* reading_ping_offset */ >>> + case 0x58000c: /* reading_pong_offset */ >>> + case 0x580023: /* structure_type */ >>> + return 1; >>> + case 0x58000d: /* name length */ >>> + return 0x30; >>> + case 0x580022: /* occ sensor loc core */ >>> + return 0x0040; >>> + case 0x580003: /* occ sensor type power */ >>> + return 0x0080; >>> + case 0x580005: /* sensor name */ >>> + return 0x1000; >>> + case 0x58001e: /* HWMON_SENSORS_MASK */ >>> + case 0x580020: >>> + return 0x8e00; >>> + case 0x0: /* P8 slw base access for slw image size */ >>> + return 0x1000000000000000; >>> + } >>> + return 0; >>> +} >>> + >>> +static void occ_common_area_write(void *opaque, hwaddr addr, uint64_t val, >>> + unsigned width) >>> +{ >>> + /* callback function defined to occ common area write */ >>> + return; >>> +} >>> + >>> +const MemoryRegionOps pnv_occ_common_area_ops = { >>> + .read = occ_common_area_read, >>> + .write = occ_common_area_write, >>> + .valid.min_access_size = 1, >>> + .valid.max_access_size = 8, >>> + .impl.min_access_size = 1, >>> + .impl.max_access_size = 8, >>> + .endianness = DEVICE_BIG_ENDIAN, >>> +}; >> >> Why aren't you using the PnvOCC model ? >> >>> +void pnv_occ_common_area_realize(PnvChip *chip, Error **errp) >>> +{ >>> + SysBusDevice *sbd = SYS_BUS_DEVICE(chip); >>> + sbd->num_mmio = PNV_OCC_COMMON_AREA_SYSBUS; >>> + char *occ_common_area; >>> + >>> + /* occ common area */ >>> + occ_common_area = g_strdup_printf("occ-common-area-%x", chip->chip_id); >>> + memory_region_init_io(&chip->occ_common_area_mmio, OBJECT(chip), >>> + &pnv_occ_common_area_ops, chip, occ_common_area, >>> + PNV_OCC_COMMON_AREA_SIZE); >>> + sysbus_init_mmio(sbd, &chip->occ_common_area_mmio); >>> + g_free(occ_common_area); >>> +} >> >> May be this "device" deserves a PnvHomer model, one for P8 and one for P9. >> >>> +void pnv_homer_realize(PnvChip *chip, Error **errp) >>> +{ >>> + SysBusDevice *sbd = SYS_BUS_DEVICE(chip); >>> + sbd->num_mmio = PNV_HOMER_SYSBUS; >>> + char *homer; >>> + >>> + /* homer region */ >>> + homer = g_strdup_printf("homer-%x", chip->chip_id); >>> + memory_region_init_io(&chip->homer_mmio, OBJECT(chip), &pnv_homer_ops, >>> + chip, homer, PNV_HOMER_SIZE); >>> + sysbus_init_mmio(sbd, &chip->homer_mmio); >>> + g_free(homer); >>> +} >>> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h >>> index fb123edc4e..6464e32892 100644 >>> --- a/include/hw/ppc/pnv.h >>> +++ b/include/hw/ppc/pnv.h >>> @@ -28,6 +28,7 @@ >>> #include "hw/ppc/pnv_occ.h" >>> #include "hw/ppc/pnv_xive.h" >>> #include "hw/ppc/pnv_core.h" >>> +#include "hw/ppc/pnv_homer.h" >>> >>> #define TYPE_PNV_CHIP "pnv-chip" >>> #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP) >>> @@ -36,6 +37,13 @@ >>> #define PNV_CHIP_GET_CLASS(obj) \ >>> OBJECT_GET_CLASS(PnvChipClass, (obj), TYPE_PNV_CHIP) >>> >>> +enum SysBusNum { >>> + PNV_XSCOM_SYSBUS, >>> + PNV_ICP_SYSBUS, >>> + PNV_HOMER_SYSBUS, >>> + PNV_OCC_COMMON_AREA_SYSBUS, >>> +}; >> What is this ? >> >> >>> typedef enum PnvChipType { >>> PNV_CHIP_POWER8E, /* AKA Murano (default) */ >>> PNV_CHIP_POWER8, /* AKA Venice */ >>> @@ -56,6 +64,8 @@ typedef struct PnvChip { >>> uint64_t cores_mask; >>> void *cores; >>> >>> + MemoryRegion homer_mmio; >>> + MemoryRegion occ_common_area_mmio; >>> MemoryRegion xscom_mmio; >>> MemoryRegion xscom; >>> AddressSpace xscom_as; >>> @@ -191,6 +201,10 @@ static inline bool pnv_is_power9(PnvMachineState *pnv) >>> void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt); >>> void pnv_bmc_powerdown(IPMIBmc *bmc); >>> >>> +extern void pnv_occ_common_area_realize(PnvChip *chip, Error **errp); >>> +extern void pnv_homer_realize(PnvChip *chip, Error **errp); >>> + >>> + >>> /* >>> * POWER8 MMIO base addresses >>> */ >>> diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h >>> new file mode 100644 >>> index 0000000000..0fe6469abe >>> --- /dev/null >>> +++ b/include/hw/ppc/pnv_homer.h >>> @@ -0,0 +1,41 @@ >>> +/* >>> + * QEMU PowerPC PowerNV Homer and occ common area definitions >>> + * >>> + * Copyright (c) 2019, 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_HOMER_H >>> +#define _PPC_PNV_HOMER_H >>> + >>> +#include "qom/object.h" >>> + >>> +/* >>> + * HOMER region size 4M per OCC (1 OCC is defined per chip in struct PnvChip) >>> + * so chip_num can be used to offset between HOMER region from its base address >>> + */ >>> +#define PNV_HOMER_SIZE 0x300000 >>> +#define PNV_OCC_COMMON_AREA_SIZE 0x700000 >>> + >>> +#define PNV_HOMER_BASE(chip) \ >>> + (0x7ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE) >>> +#define PNV_OCC_COMMON_AREA(chip) \ >>> + (0x7fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE) >>> + >>> +#define PNV9_HOMER_BASE(chip) \ >>> + (0x203ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE) >>> +#define PNV9_OCC_COMMON_AREA(chip) \ >>> + (0x203fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE) >>> + >>> +#endif /* _PPC_PNV_HOMER_H */ >>>
diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs index 9da93af905..7260b4a96c 100644 --- a/hw/ppc/Makefile.objs +++ b/hw/ppc/Makefile.objs @@ -7,7 +7,7 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o obj-$(CONFIG_SPAPR_RNG) += spapr_rng.o # IBM PowerNV -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o pnv_homer.o ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o endif diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c new file mode 100644 index 0000000000..73a94856d0 --- /dev/null +++ b/hw/ppc/pnv_homer.c @@ -0,0 +1,185 @@ +/* + * QEMU PowerPC PowerNV Homer and OCC common area region + * + * Copyright (c) 2019, 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/hw_accel.h" +#include "sysemu/cpus.h" +#include "hw/ppc/pnv.h" + +static bool core_max_array(hwaddr addr) +{ + char *cpu_type; + hwaddr core_max_base = 0xe2819; + MachineState *ms = MACHINE(qdev_get_machine()); + cpu_type = strstr(ms->cpu_type, "power8"); + if (cpu_type) + core_max_base = 0x1f8810; + for (int i = 0; i <= ms->smp.cores; i++) + if (addr == (core_max_base + i)) + return true; + return false; +} + +static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width) +{ + switch (addr) { + case 0xe2006: /* max pstate ultra turbo */ + case 0xe2018: /* pstate id for 0 */ + case 0x1f8001: /* P8 occ pstate version */ + case 0x1f8003: /* P8 pstate min */ + case 0x1f8010: /* P8 pstate id for 0 */ + return 0; + case 0xe2000: /* occ data area */ + case 0xe2002: /* occ_role master/slave*/ + case 0xe2004: /* pstate nom */ + case 0xe2005: /* pstate turbo */ + case 0xe2020: /* pstate id for 1 */ + case 0xe2818: /* pstate ultra turbo */ + case 0xe2b85: /* opal dynamic data (runtime) */ + case 0x1f8000: /* P8 occ pstate valid */ + case 0x1f8002: /* P8 throttle */ + case 0x1f8004: /* P8 pstate nom */ + case 0x1f8005: /* P8 pstate turbo */ + case 0x1f8012: /* vdd voltage identifier */ + case 0x1f8013: /* vcs voltage identifier */ + case 0x1f8018: /* P8 pstate id for 1 */ + return 1; + case 0xe2003: /* pstate min (2 as pstate min) */ + case 0xe2028: /* pstate id for 2 */ + case 0x1f8006: /* P8 pstate ultra turbo */ + case 0x1f8020: /* P8 pstate id for 2 */ + return 2; + case 0xe2001: /* major version */ + return 0x90; + /* 3000 khz frequency for 0, 1, and 2 pstates */ + case 0xe201c: + case 0xe2024: + case 0xe202c: + /* P8 frequency for 0, 1, and 2 pstates */ + case 0x1f8014: + case 0x1f801c: + case 0x1f8024: + return 3000; + case 0x0: /* homer base */ + case 0xe2008: /* occ data area + 8 */ + case 0x1f8008: /* P8 occ data area + 8 */ + case 0x200008: /* homer base access to get homer image pointer*/ + return 0x1000000000000000; + } + /* pstate table core max array */ + if (core_max_array(addr)) + return 1; + return 0; +} + +static void homer_write(void *opaque, hwaddr addr, uint64_t val, + unsigned width) +{ + /* callback function defined to homer write */ + return; +} + +const MemoryRegionOps pnv_homer_ops = { + .read = homer_read, + .write = homer_write, + .valid.min_access_size = 1, + .valid.max_access_size = 8, + .impl.min_access_size = 1, + .impl.max_access_size = 8, + .endianness = DEVICE_BIG_ENDIAN, +}; + +static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned width) +{ + switch (addr) { + /* + * occ-sensor sanity check that asserts the sensor + * header block + */ + case 0x580000: /* occ sensor data block */ + case 0x580001: /* valid */ + case 0x580002: /* version */ + case 0x580004: /* reading_version */ + case 0x580008: /* nr_sensors */ + case 0x580010: /* names_offset */ + case 0x580014: /* reading_ping_offset */ + case 0x58000c: /* reading_pong_offset */ + case 0x580023: /* structure_type */ + return 1; + case 0x58000d: /* name length */ + return 0x30; + case 0x580022: /* occ sensor loc core */ + return 0x0040; + case 0x580003: /* occ sensor type power */ + return 0x0080; + case 0x580005: /* sensor name */ + return 0x1000; + case 0x58001e: /* HWMON_SENSORS_MASK */ + case 0x580020: + return 0x8e00; + case 0x0: /* P8 slw base access for slw image size */ + return 0x1000000000000000; + } + return 0; +} + +static void occ_common_area_write(void *opaque, hwaddr addr, uint64_t val, + unsigned width) +{ + /* callback function defined to occ common area write */ + return; +} + +const MemoryRegionOps pnv_occ_common_area_ops = { + .read = occ_common_area_read, + .write = occ_common_area_write, + .valid.min_access_size = 1, + .valid.max_access_size = 8, + .impl.min_access_size = 1, + .impl.max_access_size = 8, + .endianness = DEVICE_BIG_ENDIAN, +}; + +void pnv_occ_common_area_realize(PnvChip *chip, Error **errp) +{ + SysBusDevice *sbd = SYS_BUS_DEVICE(chip); + sbd->num_mmio = PNV_OCC_COMMON_AREA_SYSBUS; + char *occ_common_area; + + /* occ common area */ + occ_common_area = g_strdup_printf("occ-common-area-%x", chip->chip_id); + memory_region_init_io(&chip->occ_common_area_mmio, OBJECT(chip), + &pnv_occ_common_area_ops, chip, occ_common_area, + PNV_OCC_COMMON_AREA_SIZE); + sysbus_init_mmio(sbd, &chip->occ_common_area_mmio); + g_free(occ_common_area); +} + +void pnv_homer_realize(PnvChip *chip, Error **errp) +{ + SysBusDevice *sbd = SYS_BUS_DEVICE(chip); + sbd->num_mmio = PNV_HOMER_SYSBUS; + char *homer; + + /* homer region */ + homer = g_strdup_printf("homer-%x", chip->chip_id); + memory_region_init_io(&chip->homer_mmio, OBJECT(chip), &pnv_homer_ops, + chip, homer, PNV_HOMER_SIZE); + sysbus_init_mmio(sbd, &chip->homer_mmio); + g_free(homer); +} diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h index fb123edc4e..6464e32892 100644 --- a/include/hw/ppc/pnv.h +++ b/include/hw/ppc/pnv.h @@ -28,6 +28,7 @@ #include "hw/ppc/pnv_occ.h" #include "hw/ppc/pnv_xive.h" #include "hw/ppc/pnv_core.h" +#include "hw/ppc/pnv_homer.h" #define TYPE_PNV_CHIP "pnv-chip" #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP) @@ -36,6 +37,13 @@ #define PNV_CHIP_GET_CLASS(obj) \ OBJECT_GET_CLASS(PnvChipClass, (obj), TYPE_PNV_CHIP) +enum SysBusNum { + PNV_XSCOM_SYSBUS, + PNV_ICP_SYSBUS, + PNV_HOMER_SYSBUS, + PNV_OCC_COMMON_AREA_SYSBUS, +}; + typedef enum PnvChipType { PNV_CHIP_POWER8E, /* AKA Murano (default) */ PNV_CHIP_POWER8, /* AKA Venice */ @@ -56,6 +64,8 @@ typedef struct PnvChip { uint64_t cores_mask; void *cores; + MemoryRegion homer_mmio; + MemoryRegion occ_common_area_mmio; MemoryRegion xscom_mmio; MemoryRegion xscom; AddressSpace xscom_as; @@ -191,6 +201,10 @@ static inline bool pnv_is_power9(PnvMachineState *pnv) void pnv_dt_bmc_sensors(IPMIBmc *bmc, void *fdt); void pnv_bmc_powerdown(IPMIBmc *bmc); +extern void pnv_occ_common_area_realize(PnvChip *chip, Error **errp); +extern void pnv_homer_realize(PnvChip *chip, Error **errp); + + /* * POWER8 MMIO base addresses */ diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h new file mode 100644 index 0000000000..0fe6469abe --- /dev/null +++ b/include/hw/ppc/pnv_homer.h @@ -0,0 +1,41 @@ +/* + * QEMU PowerPC PowerNV Homer and occ common area definitions + * + * Copyright (c) 2019, 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_HOMER_H +#define _PPC_PNV_HOMER_H + +#include "qom/object.h" + +/* + * HOMER region size 4M per OCC (1 OCC is defined per chip in struct PnvChip) + * so chip_num can be used to offset between HOMER region from its base address + */ +#define PNV_HOMER_SIZE 0x300000 +#define PNV_OCC_COMMON_AREA_SIZE 0x700000 + +#define PNV_HOMER_BASE(chip) \ + (0x7ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE) +#define PNV_OCC_COMMON_AREA(chip) \ + (0x7fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE) + +#define PNV9_HOMER_BASE(chip) \ + (0x203ffd800000ull + ((uint64_t)(chip)->chip_num) * PNV_HOMER_SIZE) +#define PNV9_OCC_COMMON_AREA(chip) \ + (0x203fff800000ull + ((uint64_t)(chip)->chip_num) * PNV_OCC_COMMON_AREA_SIZE) + +#endif /* _PPC_PNV_HOMER_H */