On Thu, Mar 16, 2017 at 02:52:17PM +0100, Cédric Le Goater wrote: > On 03/15/2017 07:16 AM, David Gibson wrote: > > On Wed, Mar 08, 2017 at 11:52:49AM +0100, Cédric Le Goater wrote: > >> From: Benjamin Herrenschmidt <b...@kernel.crashing.org> > >> > >> The PSI (Processor Service Interface) Controller is one of the engines > >> of the "Bridge" unit which connects the different interfaces to the > >> Power Processor. > >> > >> This adds just enough of the PSI bridge to handle various on-chip and > >> the one external interrupt. The rest of PSI has to do with the link to > >> the IBM FSP service processor which we don't plan to emulate (not used > >> on OpenPower machines). > >> > >> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org> > >> [clg: - updated for qemu-2.9 > >> - changed the XSCOM interface to fit new model > >> - QOMified the model > >> - reworked set_xive and worked around a skiboot bug > >> - removed the 'psi_mmio_to_xscom' mapping array ] > >> Signed-off-by: Cédric Le Goater <c...@kaod.org> > >> --- > >> hw/ppc/Makefile.objs | 2 +- > >> hw/ppc/pnv.c | 35 ++- > >> hw/ppc/pnv_psi.c | 583 > >> +++++++++++++++++++++++++++++++++++++++++++++ > >> include/hw/ppc/pnv.h | 8 + > >> include/hw/ppc/pnv_psi.h | 61 +++++ > >> include/hw/ppc/pnv_xscom.h | 3 + > >> 6 files changed, 685 insertions(+), 7 deletions(-) > >> create mode 100644 hw/ppc/pnv_psi.c > >> create mode 100644 include/hw/ppc/pnv_psi.h > >> > >> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > >> index 001293423c8d..dc19ee17fa57 100644 > >> --- a/hw/ppc/Makefile.objs > >> +++ b/hw/ppc/Makefile.objs > >> @@ -6,7 +6,7 @@ obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o > >> spapr_rtas.o > >> obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o > >> obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o > >> # IBM PowerNV > >> -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o > >> +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o > >> ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) > >> obj-y += spapr_pci_vfio.o > >> endif > >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > >> index 0ae11cc3a2ca..85b00bf235c6 100644 > >> --- a/hw/ppc/pnv.c > >> +++ b/hw/ppc/pnv.c > >> @@ -356,15 +356,22 @@ static void ppc_powernv_reset(void) > >> * have a CPLD that will collect the SerIRQ and shoot them as a > >> * single level interrupt to the P8 chip. So let's setup a hook > >> * for doing just that. > >> - * > >> - * Note: The actual interrupt input isn't emulated yet, this will > >> - * come with the PSI bridge model. > >> */ > >> static void pnv_lpc_isa_irq_handler_cpld(void *opaque, int n, int level) > >> { > >> - /* We don't yet emulate the PSI bridge which provides the external > >> - * interrupt, so just drop interrupts on the floor > >> - */ > >> + PnvMachineState *pnv = POWERNV_MACHINE(qdev_get_machine()); > >> + uint32_t old_state = pnv->cpld_irqstate; > >> + PnvChip *chip = opaque; > >> + > >> + if (level) { > >> + pnv->cpld_irqstate |= 1u << n; > >> + } else { > >> + pnv->cpld_irqstate &= ~(1u << n); > >> + } > >> + if (pnv->cpld_irqstate != old_state) { > >> + pnv_psi_irq_set(&chip->psi, PSIHB_IRQ_EXTERNAL, > >> + pnv->cpld_irqstate != 0); > >> + } > >> } > >> > >> static void pnv_lpc_isa_irq_handler(void *opaque, int n, int level) > >> @@ -702,6 +709,11 @@ static void pnv_chip_init(Object *obj) > >> > >> object_initialize(&chip->lpc, sizeof(chip->lpc), TYPE_PNV_LPC); > >> object_property_add_child(obj, "lpc", OBJECT(&chip->lpc), NULL); > >> + > >> + object_initialize(&chip->psi, sizeof(chip->psi), TYPE_PNV_PSI); > >> + object_property_add_child(obj, "psi", OBJECT(&chip->psi), NULL); > >> + object_property_add_const_link(OBJECT(&chip->psi), "xics", > >> + OBJECT(qdev_get_machine()), > >> &error_abort); > >> } > >> > >> static void pnv_chip_icp_realize(PnvChip *chip, Error **errp) > >> @@ -722,6 +734,8 @@ static void pnv_chip_realize(DeviceState *dev, Error > >> **errp) > >> char *typename = pnv_core_typename(pcc->cpu_model); > >> size_t typesize = object_type_get_instance_size(typename); > >> int i, core_hwid; > >> + MachineState *machine = MACHINE(qdev_get_machine()); > >> + PnvMachineState *pnv = POWERNV_MACHINE(machine); > >> > >> if (!object_class_by_name(typename)) { > >> error_setg(errp, "Unable to find PowerNV CPU Core '%s'", > >> typename); > >> @@ -797,6 +811,15 @@ static void pnv_chip_realize(DeviceState *dev, Error > >> **errp) > >> } > >> g_free(typename); > >> > >> + > >> + /* Processor Service Interface (PSI) Host Bridge */ > >> + object_property_set_bool(OBJECT(&chip->psi), true, "realized", > >> + &error_fatal); > >> + pnv_xscom_add_subregion(chip, PNV_XSCOM_PSI_BASE, > >> &chip->psi.xscom_regs); > >> + > >> + /* link in the PSI ICS */ > >> + QLIST_INSERT_HEAD(&pnv->ics, &chip->psi.ics, list); > >> + > >> /* Create LPC controller */ > >> object_property_set_bool(OBJECT(&chip->lpc), true, "realized", > >> &error_fatal); > >> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c > >> new file mode 100644 > >> index 000000000000..6ba688aac075 > >> --- /dev/null > >> +++ b/hw/ppc/pnv_psi.c > >> @@ -0,0 +1,583 @@ > >> +/* > >> + * QEMU PowerNV PowerPC PSI interface > >> + * > >> + * 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 "hw/hw.h" > >> +#include "target/ppc/cpu.h" > >> +#include "qemu/log.h" > >> +#include "qapi/error.h" > >> + > >> +#include "exec/address-spaces.h" > >> + > >> +#include "hw/ppc/fdt.h" > >> +#include "hw/ppc/pnv.h" > >> +#include "hw/ppc/pnv_xscom.h" > >> +#include "hw/ppc/pnv_psi.h" > >> + > >> +#include <libfdt.h> > >> + > >> +#define PSIHB_XSCOM_FIR_RW 0x00 > >> +#define PSIHB_XSCOM_FIR_AND 0x01 > >> +#define PSIHB_XSCOM_FIR_OR 0x02 > >> +#define PSIHB_XSCOM_FIRMASK_RW 0x03 > >> +#define PSIHB_XSCOM_FIRMASK_AND 0x04 > >> +#define PSIHB_XSCOM_FIRMASK_OR 0x05 > >> +#define PSIHB_XSCOM_FIRACT0 0x06 > >> +#define PSIHB_XSCOM_FIRACT1 0x07 > >> +#define PSIHB_XSCOM_BAR 0x0a > >> +#define PSIHB_BAR_EN 0x0000000000000001ull > >> +#define PSIHB_XSCOM_FSPBAR 0x0b > >> +#define PSIHB_XSCOM_CR 0x0e > >> +#define PSIHB_CR_FSP_CMD_ENABLE 0x8000000000000000ull > >> +#define PSIHB_CR_FSP_MMIO_ENABLE 0x4000000000000000ull > >> +#define PSIHB_CR_FSP_IRQ_ENABLE 0x1000000000000000ull > >> +#define PSIHB_CR_FSP_ERR_RSP_ENABLE 0x0800000000000000ull > >> +#define PSIHB_CR_PSI_LINK_ENABLE 0x0400000000000000ull > >> +#define PSIHB_CR_FSP_RESET 0x0200000000000000ull > >> +#define PSIHB_CR_PSIHB_RESET 0x0100000000000000ull > >> +#define PSIHB_CR_PSI_IRQ 0x0000800000000000ull > >> +#define PSIHB_CR_FSP_IRQ 0x0000400000000000ull > >> +#define PSIHB_CR_FSP_LINK_ACTIVE 0x0000200000000000ull > >> + /* and more ... */ > >> +#define PSIHB_XSCOM_SEMR 0x0f > >> +#define PSIHB_XSCOM_XIVR_PSI 0x10 > >> +#define PSIHB_XIVR_SERVER_SH 40 > >> +#define PSIHB_XIVR_SERVER_MSK (0xffffull << PSIHB_XIVR_SERVER_SH) > >> +#define PSIHB_XIVR_PRIO_SH 32 > >> +#define PSIHB_XIVR_PRIO_MSK (0xffull << PSIHB_XIVR_PRIO_SH) > >> +#define PSIHB_XIVR_SRC_SH 29 > >> +#define PSIHB_XIVR_SRC_MSK (0x7ull << PSIHB_XIVR_SRC_SH) > >> +#define PSIHB_XIVR_PENDING 0x01000000ull > >> +#define PSIHB_XSCOM_SCR 0x12 > >> +#define PSIHB_XSCOM_CCR 0x13 > >> +#define PSIHB_XSCOM_DMA_UPADD 0x14 > >> +#define PSIHB_XSCOM_IRQ_STAT 0x15 > >> +#define PSIHB_IRQ_STAT_OCC 0x0000001000000000ull > >> +#define PSIHB_IRQ_STAT_FSI 0x0000000800000000ull > >> +#define PSIHB_IRQ_STAT_LPCI2C 0x0000000400000000ull > >> +#define PSIHB_IRQ_STAT_LOCERR 0x0000000200000000ull > >> +#define PSIHB_IRQ_STAT_EXT 0x0000000100000000ull > >> +#define PSIHB_XSCOM_XIVR_OCC 0x16 > >> +#define PSIHB_XSCOM_XIVR_FSI 0x17 > >> +#define PSIHB_XSCOM_XIVR_LPCI2C 0x18 > >> +#define PSIHB_XSCOM_XIVR_LOCERR 0x19 > >> +#define PSIHB_XSCOM_XIVR_EXT 0x1a > >> +#define PSIHB_XSCOM_IRSN 0x1b > >> +#define PSIHB_IRSN_COMP_SH 45 > >> +#define PSIHB_IRSN_COMP_MSK (0x7ffffull << PSIHB_IRSN_COMP_SH) > >> +#define PSIHB_IRSN_IRQ_MUX 0x0000000800000000ull > >> +#define PSIHB_IRSN_IRQ_RESET 0x0000000400000000ull > >> +#define PSIHB_IRSN_DOWNSTREAM_EN 0x0000000200000000ull > >> +#define PSIHB_IRSN_UPSTREAM_EN 0x0000000100000000ull > >> +#define PSIHB_IRSN_COMPMASK_SH 13 > >> +#define PSIHB_IRSN_COMPMASK_MSK (0x7ffffull << > >> PSIHB_IRSN_COMPMASK_SH) > >> + > >> +/* > >> + * These are the values of the registers when accessed through the > >> + * MMIO region. The relation is xscom = (mmio + 0x50) >> 3 > >> + */ > >> +#define PSIHB_MMIO_BAR 0x00 > >> +#define PSIHB_MMIO_FSPBAR 0x08 > >> +#define PSIHB_MMIO_CR 0x20 > >> +#define PSIHB_MMIO_SEMR 0x28 > >> +#define PSIHB_MMIO_XIVR_PSI 0x30 > >> +#define PSIHB_MMIO_SCR 0x40 > >> +#define PSIHB_MMIO_CCR 0x48 > >> +#define PSIHB_MMIO_DMA_UPADD 0x50 > >> +#define PSIHB_MMIO_IRQ_STAT 0x58 > >> +#define PSIHB_MMIO_XIVR_OCC 0x60 > >> +#define PSIHB_MMIO_XIVR_FSI 0x68 > >> +#define PSIHB_MMIO_XIVR_LPCI2C 0x70 > >> +#define PSIHB_MMIO_XIVR_LOCERR 0x78 > >> +#define PSIHB_MMIO_XIVR_EXT 0x80 > >> +#define PSIHB_MMIO_IRSN 0x88 > >> +#define PSIHB_MMIO_MAX 0x100 > >> + > >> +static void pnv_psi_set_bar(PnvPsi *psi, uint64_t bar) > >> +{ > >> + MemoryRegion *sysmem = get_system_memory(); > >> + uint64_t old = psi->regs[PSIHB_XSCOM_BAR]; > >> + > >> + psi->regs[PSIHB_XSCOM_BAR] = bar & 0x0003fffffff00001; > >> + > >> + /* Update MR, always remove it first */ > >> + if (old & PSIHB_BAR_EN) { > >> + memory_region_del_subregion(sysmem, &psi->regs_mr); > >> + } > >> + /* Then add it back if needed */ > >> + if (bar & PSIHB_BAR_EN) { > >> + uint64_t addr = bar & 0x0003fffffff00000; > >> + memory_region_add_subregion(sysmem, addr, &psi->regs_mr); > >> + } > >> +} > >> + > >> +static void pnv_psi_update_fsp_mr(PnvPsi *psi) > >> +{ > >> + /* XXX Update FSP MR if/when we support FSP BAR */ > >> +} > >> + > >> +static void pnv_psi_set_cr(PnvPsi *psi, uint64_t cr) > >> +{ > >> + uint64_t old = psi->regs[PSIHB_XSCOM_CR]; > >> + > >> + psi->regs[PSIHB_XSCOM_CR] = cr & 0x0003ffff00000000; > >> + > >> + /* Check some bit changes */ > >> + if ((old ^ psi->regs[PSIHB_XSCOM_CR]) & PSIHB_CR_FSP_MMIO_ENABLE) { > >> + pnv_psi_update_fsp_mr(psi); > >> + } > >> +} > >> + > >> +static void pnv_psi_set_irsn(PnvPsi *psi, uint64_t val) > >> +{ > >> + uint32_t offset; > >> + ICSState *ics = &psi->ics; > >> + > >> + /* In this model we ignore the up/down enable bits for now > >> + * as SW doesn't use them (other than setting them at boot). > >> + * We ignore IRQ_MUX, its meaning isn't clear and we don't use > >> + * it and finally we ignore reset (XXX fix that ?) > >> + */ > >> + psi->regs[PSIHB_XSCOM_IRSN] = val & (PSIHB_IRSN_COMP_MSK | > >> + PSIHB_IRSN_IRQ_MUX | > >> + PSIHB_IRSN_DOWNSTREAM_EN | > >> + PSIHB_IRSN_DOWNSTREAM_EN | > >> + PSIHB_IRSN_DOWNSTREAM_EN); > >> + > >> + /* We ignore the compare mask as well, our ICS emulation is too > >> + * simplistic to make any use if it, and we extract the offset > >> + * from the compare value > >> + */ > >> + offset = (val & PSIHB_IRSN_COMP_MSK) >> PSIHB_IRSN_COMP_SH; > >> + ics->offset = offset; > >> +} > >> + > >> +static bool pnv_psi_irq_bits(PnvPsi *psi, PnvPsiIrq irq, > >> + uint32_t *out_xivr_reg, > >> + uint32_t *out_stat_reg, > >> + uint64_t *out_stat_bit) > > > > Your PnvPsiIrq values are arbitrary and contiguous AFACT. Why not > > just have a lookup table for this info, instead of a giant switch > > statement? > > Well, I agree but at the same time, we are not gaining much in terms > of lines by using an array,
Hmm.. seems like an ~ x5 line reduction to me... And, IMO, easier to read. > and we have to check for boundaries which > the switch provide. The question would be different if we had more > parameters. So let's keep it that way. > > >> +{ > >> + switch (irq) { > >> + case PSIHB_IRQ_PSI: > >> + *out_xivr_reg = PSIHB_XSCOM_XIVR_PSI; > >> + *out_stat_reg = PSIHB_XSCOM_CR; > >> + *out_stat_bit = PSIHB_CR_PSI_IRQ; > >> + break; > >> + case PSIHB_IRQ_FSP: > >> + *out_xivr_reg = PSIHB_XSCOM_XIVR_PSI; > >> + *out_stat_reg = PSIHB_XSCOM_CR; > >> + *out_stat_bit = PSIHB_CR_FSP_IRQ; > >> + break; > >> + case PSIHB_IRQ_OCC: > >> + *out_xivr_reg = PSIHB_XSCOM_XIVR_OCC; > >> + *out_stat_reg = PSIHB_XSCOM_IRQ_STAT; > >> + *out_stat_bit = PSIHB_IRQ_STAT_OCC; > >> + break; > >> + case PSIHB_IRQ_FSI: > >> + *out_xivr_reg = PSIHB_XSCOM_XIVR_FSI; > >> + *out_stat_reg = PSIHB_XSCOM_IRQ_STAT; > >> + *out_stat_bit = PSIHB_IRQ_STAT_FSI; > >> + break; > >> + case PSIHB_IRQ_LPC_I2C: > >> + *out_xivr_reg = PSIHB_XSCOM_XIVR_LPCI2C; > >> + *out_stat_reg = PSIHB_XSCOM_IRQ_STAT; > >> + *out_stat_bit = PSIHB_IRQ_STAT_LPCI2C; > >> + break; > >> + case PSIHB_IRQ_LOCAL_ERR: > >> + *out_xivr_reg = PSIHB_XSCOM_XIVR_LOCERR; > >> + *out_stat_reg = PSIHB_XSCOM_IRQ_STAT; > >> + *out_stat_bit = PSIHB_IRQ_STAT_LOCERR; > >> + break; > >> + case PSIHB_IRQ_EXTERNAL: > >> + *out_xivr_reg = PSIHB_XSCOM_XIVR_EXT; > >> + *out_stat_reg = PSIHB_XSCOM_IRQ_STAT; > >> + *out_stat_bit = PSIHB_IRQ_STAT_EXT; > >> + break; > >> + default: > >> + return false; > >> + } > >> + return true; > >> +} > >> + > >> +void pnv_psi_irq_set(PnvPsi *psi, PnvPsiIrq irq, bool state) > >> +{ > >> + ICSState *ics = &psi->ics; > >> + uint32_t xivr_reg; > >> + uint32_t stat_reg; > >> + uint64_t stat_bit; > >> + uint32_t src; > >> + bool masked; > >> + > >> + if (!pnv_psi_irq_bits(psi, irq, &xivr_reg, &stat_reg, &stat_bit)) { > >> + qemu_log_mask(LOG_GUEST_ERROR, "PSI: Unsupported irq %d\n", irq); > >> + return; > >> + } > >> + > >> + src = (psi->regs[xivr_reg] & PSIHB_XIVR_SRC_MSK) >> PSIHB_XIVR_SRC_SH; > >> + masked = (psi->regs[xivr_reg] & PSIHB_XIVR_PRIO_MSK) == > >> PSIHB_XIVR_PRIO_MSK; > >> + if (state) { > >> + psi->regs[stat_reg] |= stat_bit; > >> + /* XXX optimization: check mask here. That means re-evaluating > >> + * when unmasking, thus TODO > >> + */ > >> + qemu_irq_raise(ics->qirqs[src]); > >> + } else { > >> + psi->regs[stat_reg] &= ~stat_bit; > >> + > >> + /* FSP and PSI are muxed so don't lower if either still set */ > >> + if (stat_reg != PSIHB_XSCOM_CR || > >> + !(psi->regs[stat_reg] & (PSIHB_CR_PSI_IRQ | > >> PSIHB_CR_FSP_IRQ))) { > >> + qemu_irq_lower(ics->qirqs[src]); > >> + } else { > >> + state = true; > >> + } > >> + } > > > > It might be cleaner to just revaluate the irq level from scratch here, > > and set the level, rather than doing this complicated dance to work > > out if it has changed. > > OK. I need to take a closer look at that. > > >> + > >> + /* XXX Note about the emulation of the pending bit: This isn't > >> + * entirely correct. The pending bit should be cleared when the > >> + * EOI has been received. However, we don't have callbacks on > >> + * EOI (especially not under KVM) so no way to emulate that > >> + * properly, so instead we just set that bit as the logical > >> + * "output" of the XIVR (ie pending & !masked) > >> + * XXX TODO: Also update it on set_xivr > >> + */ > >> + if (state && !masked) { > >> + psi->regs[xivr_reg] |= PSIHB_XIVR_PENDING; > >> + } else { > >> + psi->regs[xivr_reg] &= ~PSIHB_XIVR_PENDING; > >> + } > >> +} > >> + > >> +static void pnv_psi_set_xivr(PnvPsi *psi, uint32_t reg, uint64_t val) > >> +{ > >> + ICSState *ics = &psi->ics; > >> + uint16_t server; > >> + uint8_t prio; > >> + uint8_t src; > >> + int icp_index; > >> + > >> + psi->regs[reg] = (psi->regs[reg] & PSIHB_XIVR_PENDING) | > >> + (val & (PSIHB_XIVR_SERVER_MSK | > >> + PSIHB_XIVR_PRIO_MSK | > >> + PSIHB_XIVR_SRC_MSK)); > >> + val = psi->regs[reg]; > >> + server = (val & PSIHB_XIVR_SERVER_MSK) >> PSIHB_XIVR_SERVER_SH; > >> + prio = (val & PSIHB_XIVR_PRIO_MSK) >> PSIHB_XIVR_PRIO_SH; > >> + src = (val & PSIHB_XIVR_SRC_MSK) >> PSIHB_XIVR_SRC_SH; > >> + if (src > PSIHB_IRQ_EXTERNAL) { > >> + /* XXX Generate error ? */ > >> + return; > >> + } > >> + > >> + /* > >> + * Linux fills the irq xivr with the hw processor id plus the > >> + * link bits. shift back to get something valid. > >> + */ > >> + server >>= 2; > >> + > >> + /* > >> + * When skiboot initializes PSIHB, it fills the xives with > >> + * server=0, prio=0xff, but we don't have a CPU with a pir=0. So > >> + * skip that case. > >> + */ > >> + if (prio != 0xff) { > >> + icp_index = xics_get_cpu_index_by_pir(server); > >> + assert(icp_index != -1); > >> + } else { > >> + if (server) { > >> + qemu_log_mask(LOG_GUEST_ERROR, "PSI: bogus server %d for IRQ > >> %d\n", > >> + server, src); > >> + } > >> + icp_index = server; > >> + } > > > > This logic doesn't seem like it belongs here. You've received a > > server number, seems like you should just pass it on to the xics code, > > and if there can be an error, have that detect it. > > I have removed all of that in a private patch. This is tracking > an issue in skiboot which did not clear correctly the PSI xive. > It is partially resolved in recent version but I still see some > warnings whe the guest reboots. Ok. Workarounds for firmware bugs are fine, but they need a detailed comment so other people have a hope of understanding why they're there. Including stating outright that it is a bug workaround, rather than expected firmware behaviour. > > >> + > >> + /* Now because of source remapping, weird things can happen > >> + * if you change the source number dynamically, our simple ICS > >> + * doesn't deal with remapping. So we just poke a different > >> + * ICS entry based on what source number was written. This will > >> + * do for now but a more accurate implementation would instead > >> + * use a fixed server/prio and a remapper of the generated irq. > >> + */ > >> + ics_simple_write_xive(ics, src, icp_index, prio, prio); > >> +} > >> + > >> +static uint64_t pnv_psi_reg_read(PnvPsi *psi, uint32_t offset, bool mmio) > >> +{ > >> + uint64_t val = 0xffffffffffffffffull; > >> + > >> + switch (offset) { > >> + case PSIHB_XSCOM_FIR_RW: > >> + case PSIHB_XSCOM_FIRACT0: > >> + case PSIHB_XSCOM_FIRACT1: > >> + case PSIHB_XSCOM_BAR: > >> + case PSIHB_XSCOM_FSPBAR: > >> + case PSIHB_XSCOM_CR: > >> + case PSIHB_XSCOM_XIVR_PSI: > >> + case PSIHB_XSCOM_XIVR_OCC: > >> + case PSIHB_XSCOM_XIVR_FSI: > >> + case PSIHB_XSCOM_XIVR_LPCI2C: > >> + case PSIHB_XSCOM_XIVR_LOCERR: > >> + case PSIHB_XSCOM_XIVR_EXT: > >> + case PSIHB_XSCOM_IRQ_STAT: > >> + case PSIHB_XSCOM_SEMR: > >> + case PSIHB_XSCOM_DMA_UPADD: > >> + case PSIHB_XSCOM_IRSN: > >> + val = psi->regs[offset]; > >> + break; > >> + default: > >> + qemu_log_mask(LOG_UNIMP, "PSI Unimplemented register: Ox%" PRIx32 > >> "\n", > >> + offset); > > > > As noted elsewhere, tracepoints are more usual than qemu_log() these > > days. But either way, this really should have a distinguishable > > message from the one in the write path. > > OK. Will add a read/write statement. > > >> + } > >> + return val; > >> +} > >> + > >> +static void pnv_psi_reg_write(PnvPsi *psi, uint32_t offset, uint64_t val, > >> + bool mmio) > >> +{ > >> + switch (offset) { > >> + case PSIHB_XSCOM_FIR_RW: > >> + case PSIHB_XSCOM_FIRACT0: > >> + case PSIHB_XSCOM_FIRACT1: > >> + case PSIHB_XSCOM_SEMR: > >> + case PSIHB_XSCOM_DMA_UPADD: > >> + psi->regs[offset] = val; > >> + break; > >> + case PSIHB_XSCOM_FIR_OR: > >> + psi->regs[PSIHB_XSCOM_FIR_RW] |= val; > >> + break; > >> + case PSIHB_XSCOM_FIR_AND: > >> + psi->regs[PSIHB_XSCOM_FIR_RW] &= val; > >> + break; > >> + case PSIHB_XSCOM_BAR: > >> + /* Only XSCOM can write this one */ > >> + if (!mmio) { > >> + pnv_psi_set_bar(psi, val); > >> + } > >> + break; > >> + case PSIHB_XSCOM_FSPBAR: > >> + psi->regs[PSIHB_XSCOM_BAR] = val & 0x0003ffff00000000; > > > > Should that be PSIHB_XSCOM_FSPBAR? > > yes ... > > >> + pnv_psi_update_fsp_mr(psi); > >> + break; > >> + case PSIHB_XSCOM_CR: > >> + pnv_psi_set_cr(psi, val); > >> + break; > >> + case PSIHB_XSCOM_SCR: > >> + pnv_psi_set_cr(psi, psi->regs[PSIHB_XSCOM_CR] | val); > >> + break; > >> + case PSIHB_XSCOM_CCR: > >> + pnv_psi_set_cr(psi, psi->regs[PSIHB_XSCOM_CR] & ~val); > >> + break; > >> + case PSIHB_XSCOM_XIVR_PSI: > >> + case PSIHB_XSCOM_XIVR_OCC: > >> + case PSIHB_XSCOM_XIVR_FSI: > >> + case PSIHB_XSCOM_XIVR_LPCI2C: > >> + case PSIHB_XSCOM_XIVR_LOCERR: > >> + case PSIHB_XSCOM_XIVR_EXT: > >> + pnv_psi_set_xivr(psi, offset, val); > >> + break; > >> + case PSIHB_XSCOM_IRQ_STAT: > >> + /* Read only, should we generate an error ? */ > >> + break; > >> + case PSIHB_XSCOM_IRSN: > >> + pnv_psi_set_irsn(psi, val); > >> + break; > >> + default: > >> + qemu_log_mask(LOG_UNIMP, "PSI Unimplemented register: Ox%" PRIx32 > >> "\n", > >> + offset); > >> + } > >> +} > >> + > >> +static inline uint32_t psi_mmio_to_xscom(hwaddr addr) > >> +{ > >> + return (addr >> 3) + PSIHB_XSCOM_BAR; > >> +} > >> + > >> +static uint64_t pnv_psi_mmio_read(void *opaque, hwaddr addr, unsigned > >> size) > >> +{ > >> + return pnv_psi_reg_read(opaque, psi_mmio_to_xscom(addr), true); > >> +} > >> + > >> +static void pnv_psi_mmio_write(void *opaque, hwaddr addr, > >> + uint64_t val, unsigned size) > >> +{ > >> + pnv_psi_reg_write(opaque, psi_mmio_to_xscom(addr), val, true); > >> +} > >> + > >> +static const MemoryRegionOps psi_mmio_ops = { > >> + .read = pnv_psi_mmio_read, > >> + .write = pnv_psi_mmio_write, > >> + .endianness = DEVICE_BIG_ENDIAN, > >> + .valid = { > >> + .min_access_size = 8, > >> + .max_access_size = 8, > >> + }, > >> + .impl = { > >> + .min_access_size = 8, > >> + .max_access_size = 8, > >> + }, > >> +}; > >> + > >> +static uint64_t pnv_psi_xscom_read(void *opaque, hwaddr addr, unsigned > >> size) > >> +{ > >> + PnvPsi *psi = PNV_PSI(opaque); > >> + uint32_t offset = addr >> 3; > >> + > >> + return pnv_psi_reg_read(psi, offset, false); > >> +} > >> + > >> +static void pnv_psi_xscom_write(void *opaque, hwaddr addr, > >> + uint64_t val, unsigned size) > >> +{ > >> + PnvPsi *psi = PNV_PSI(opaque); > >> + uint32_t offset = addr >> 3; > >> + > >> + pnv_psi_reg_write(psi, offset, val, false); > >> +} > >> + > >> +static const MemoryRegionOps pnv_psi_xscom_ops = { > >> + .read = pnv_psi_xscom_read, > >> + .write = pnv_psi_xscom_write, > >> + .valid.min_access_size = 8, > >> + .valid.max_access_size = 8, > >> + .impl.min_access_size = 8, > >> + .impl.max_access_size = 8, > > > > Consistent nesting format of the two MemoryRegionOps would be good. > > OK. > > >> + .endianness = DEVICE_BIG_ENDIAN, > >> +}; > >> + > >> +static void pnv_psi_init(Object *obj) > >> +{ > >> + PnvPsi *psi = PNV_PSI(obj); > >> + > >> + object_initialize(&psi->ics, sizeof(psi->ics), TYPE_ICS_SIMPLE); > >> + qdev_set_parent_bus(DEVICE(&psi->ics), sysbus_get_default()); > >> + object_property_add_child(obj, "ics-psi", OBJECT(&psi->ics), NULL); > >> +} > >> + > >> +static void pnv_psi_realize(DeviceState *dev, Error **errp) > >> +{ > >> + PnvPsi *psi = PNV_PSI(dev); > >> + ICSState *ics = &psi->ics; > >> + Object *obj; > >> + Error *err = NULL, *local_err = NULL; > >> + unsigned int i; > >> + > >> + /* Initialize MMIO region */ > >> + memory_region_init_io(&psi->regs_mr, OBJECT(dev), &psi_mmio_ops, psi, > >> + "psihb", PNV_PSIHB_BAR_SIZE); > >> + > >> + /* Default BAR. Use object properties ? */ > >> + pnv_psi_set_bar(psi, PNV_PSIHB_BAR | PSIHB_BAR_EN); > >> + > >> + /* Default sources in XIVR */ > >> + psi->regs[PSIHB_XSCOM_XIVR_PSI] = PSIHB_XIVR_PRIO_MSK | > >> + (0ull << PSIHB_XIVR_SRC_SH); > >> + psi->regs[PSIHB_XSCOM_XIVR_OCC] = PSIHB_XIVR_PRIO_MSK | > >> + (1ull << PSIHB_XIVR_SRC_SH); > >> + psi->regs[PSIHB_XSCOM_XIVR_FSI] = PSIHB_XIVR_PRIO_MSK | > >> + (2ull << PSIHB_XIVR_SRC_SH); > >> + psi->regs[PSIHB_XSCOM_XIVR_LPCI2C] = PSIHB_XIVR_PRIO_MSK | > >> + (3ull << PSIHB_XIVR_SRC_SH); > >> + psi->regs[PSIHB_XSCOM_XIVR_LOCERR] = PSIHB_XIVR_PRIO_MSK | > >> + (4ull << PSIHB_XIVR_SRC_SH); > >> + psi->regs[PSIHB_XSCOM_XIVR_EXT] = PSIHB_XIVR_PRIO_MSK | > >> + (5ull << PSIHB_XIVR_SRC_SH); > >> + > >> + /* get XICSFabric from chip */ > >> + obj = object_property_get_link(OBJECT(dev), "xics", &err); > >> + if (!obj) { > >> + error_setg(errp, "%s: required link 'xics' not found: %s", > >> + __func__, error_get_pretty(err)); > >> + return; > >> + } > >> + > >> + /* > >> + * PSI interrupt control source > >> + */ > >> + object_property_set_int(OBJECT(ics), PSI_NUM_INTERRUPTS, "nr-irqs", > >> &err); > >> + object_property_add_const_link(OBJECT(ics), "xics", obj, &err); > >> + object_property_set_bool(OBJECT(ics), true, "realized", &local_err); > >> + error_propagate(&err, local_err); > >> + if (err) { > >> + error_propagate(errp, err); > >> + return; > >> + } > >> + > >> + for (i = 0; i < ics->nr_irqs; i++) { > >> + ics_set_irq_type(ics, i, true); > >> + } > >> + > >> + /* XScom region for PSI registers */ > >> + pnv_xscom_region_init(&psi->xscom_regs, OBJECT(dev), > >> &pnv_psi_xscom_ops, > >> + psi, "xscom-psi", PNV_XSCOM_PSI_SIZE); > >> +} > >> + > >> +static int pnv_psi_populate(PnvXScomInterface *dev, void *fdt, int > >> xscom_offset) > >> +{ > >> + const char compat[] = "ibm,power8-psihb-x\0ibm,psihb-x"; > >> + char *name; > >> + int offset; > >> + uint32_t lpc_pcba = PNV_XSCOM_PSI_BASE; > >> + uint32_t reg[] = { > >> + cpu_to_be32(lpc_pcba), > >> + cpu_to_be32(PNV_XSCOM_PSI_SIZE) > >> + }; > >> + > >> + name = g_strdup_printf("psihb@%x", lpc_pcba); > >> + offset = fdt_add_subnode(fdt, xscom_offset, name); > >> + _FDT(offset); > >> + g_free(name); > >> + > >> + _FDT((fdt_setprop(fdt, offset, "reg", reg, sizeof(reg)))); > >> + > >> + _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 2))); > >> + _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 1))); > >> + _FDT((fdt_setprop(fdt, offset, "compatible", compat, > >> + sizeof(compat)))); > >> + return 0; > >> +} > >> + > >> + > >> +static void pnv_psi_class_init(ObjectClass *klass, void *data) > >> +{ > >> + DeviceClass *dc = DEVICE_CLASS(klass); > >> + PnvXScomInterfaceClass *xdc = PNV_XSCOM_INTERFACE_CLASS(klass); > >> + > >> + xdc->populate = pnv_psi_populate; > >> + > >> + dc->realize = pnv_psi_realize; > >> +} > >> + > >> +static const TypeInfo pnv_psi_info = { > >> + .name = TYPE_PNV_PSI, > >> + .parent = TYPE_DEVICE, > > > > Since the PSI has an MMIO presence, it probably should be a > > SysBusDevice, rather than a raw descendent of TYPE_DEVICE. > > Yes indeed. > > So I will resend the patchset with just the XICS part. I want to > take a look at pnv_psi_irq_set() first. > > Thanks, > > C. > > >> + .instance_size = sizeof(PnvPsi), > >> + .instance_init = pnv_psi_init, > >> + .class_init = pnv_psi_class_init, > >> + .interfaces = (InterfaceInfo[]) { > >> + { TYPE_PNV_XSCOM_INTERFACE }, > >> + { } > >> + } > >> +}; > >> + > >> +static void pnv_psi_register_types(void) > >> +{ > >> + type_register_static(&pnv_psi_info); > >> +} > >> + > >> +type_init(pnv_psi_register_types) > >> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h > >> index f11215ea31f2..f93ec32603b7 100644 > >> --- a/include/hw/ppc/pnv.h > >> +++ b/include/hw/ppc/pnv.h > >> @@ -23,6 +23,7 @@ > >> #include "hw/sysbus.h" > >> #include "hw/ppc/pnv_lpc.h" > >> #include "hw/ppc/xics.h" > >> +#include "hw/ppc/pnv_psi.h" > >> > >> #define TYPE_PNV_CHIP "powernv-chip" > >> #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP) > >> @@ -58,6 +59,7 @@ typedef struct PnvChip { > >> MemoryRegion icp_mmio; > >> > >> PnvLpcController lpc; > >> + PnvPsi psi; > >> } PnvChip; > >> > >> typedef struct PnvChipClass { > >> @@ -119,6 +121,8 @@ typedef struct PnvMachineState { > >> ICPState *icps; > >> uint32_t nr_servers; > >> QLIST_HEAD(, ICSState) ics; > >> + > >> + uint32_t cpld_irqstate; > >> } PnvMachineState; > >> > >> #define PNV_FDT_ADDR 0x01000000 > >> @@ -150,4 +154,8 @@ typedef struct PnvMachineState { > >> #define PNV_ICP_BASE(chip) 0x0003ffff80000000ull > >> #define PNV_ICP_SIZE 0x0000000000100000ull > >> > >> +#define PNV_PSIHB_BAR 0x0003fffe80000000ull > >> +#define PNV_PSIHB_BAR_SIZE 0x0000000000100000ull > >> + > >> + > >> #endif /* _PPC_PNV_H */ > >> diff --git a/include/hw/ppc/pnv_psi.h b/include/hw/ppc/pnv_psi.h > >> new file mode 100644 > >> index 000000000000..ac3c5f8362e3 > >> --- /dev/null > >> +++ b/include/hw/ppc/pnv_psi.h > >> @@ -0,0 +1,61 @@ > >> +/* > >> + * QEMU PowerPC PowerNV PSI controller > >> + * > >> + * 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_PSI_H > >> +#define _PPC_PNV_PSI_H > >> + > >> +#define TYPE_PNV_PSI "pnv-psi" > >> +#define PNV_PSI(obj) \ > >> + OBJECT_CHECK(PnvPsi, (obj), TYPE_PNV_PSI) > >> + > >> +#define PSIHB_XSCOM_MAX 0x20 > >> + > >> +typedef struct XICSState XICSState; > >> + > >> +typedef struct PnvPsi { > >> + DeviceState parent; > >> + > >> + MemoryRegion regs_mr; > >> + > >> + /* FSP region not supported */ > >> + /* MemoryRegion fsp_mr; */ > >> + > >> + /* Interrupt generation */ > >> + ICSState ics; > >> + > >> + /* Registers */ > >> + uint64_t regs[PSIHB_XSCOM_MAX]; > >> + > >> + MemoryRegion xscom_regs; > >> +} PnvPsi; > >> + > >> +typedef enum PnvPsiIrq { > >> + PSIHB_IRQ_PSI, /* internal use only */ > >> + PSIHB_IRQ_FSP, /* internal use only */ > >> + PSIHB_IRQ_OCC, > >> + PSIHB_IRQ_FSI, > >> + PSIHB_IRQ_LPC_I2C, > >> + PSIHB_IRQ_LOCAL_ERR, > >> + PSIHB_IRQ_EXTERNAL, > >> +} PnvPsiIrq; > >> + > >> +#define PSI_NUM_INTERRUPTS 6 > >> + > >> +extern void pnv_psi_irq_set(PnvPsi *psi, PnvPsiIrq irq, bool state); > >> + > >> +#endif /* _PPC_PNV_PSI_H */ > >> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h > >> index 0faa1847bf13..2938abd74955 100644 > >> --- a/include/hw/ppc/pnv_xscom.h > >> +++ b/include/hw/ppc/pnv_xscom.h > >> @@ -60,6 +60,9 @@ typedef struct PnvXScomInterfaceClass { > >> #define PNV_XSCOM_LPC_BASE 0xb0020 > >> #define PNV_XSCOM_LPC_SIZE 0x4 > >> > >> +#define PNV_XSCOM_PSI_BASE 0x2010900 > >> +#define PNV_XSCOM_PSI_SIZE 0x20 > >> + > >> extern void pnv_xscom_realize(PnvChip *chip, Error **errp); > >> extern int pnv_xscom_populate(PnvChip *chip, void *fdt, int offset); > >> > > > -- 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