On Wed, Dec 20, 2017 at 08:38:41AM +0100, Cédric Le Goater wrote: > On 12/20/2017 06:09 AM, David Gibson wrote: > > On Sat, Dec 09, 2017 at 09:43:21AM +0100, Cédric Le Goater wrote: > >> With the POWER9 processor comes a new interrupt controller called > >> XIVE. It is composed of three sub-engines : > >> > >> - Interrupt Virtualization Source Engine (IVSE). These are in PHBs, > >> in the main controller for the IPIS and in the PSI host > >> bridge. They are configured to feed the IVRE with events. > >> > >> - Interrupt Virtualization Routing Engine (IVRE). Their job is to > >> match an event source with a Notification Virtualization Target > >> (NVT), a priority and an Event Queue (EQ) to determine if a > >> Virtual Processor can handle the event. > >> > >> - Interrupt Virtualization Presentation Engine (IVPE). It maintains > >> the interrupt state of each hardware thread and present the > >> notification as an external exception. > >> > >> Each of the engines uses a set of internal tables to redirect > >> exceptions from event sources to CPU threads. The first table we > >> introduce is the Interrupt Virtualization Entry (IVE) table, part of > >> the virtualization engine in charge of routing events. It associates > >> event sources (IRQ numbers) to event queues which will forward, or > >> not, the event notification to the presentation controller. > >> > >> The XIVE model is designed to make use of the full range of the IRQ > >> number space and does not use an offset like the XICS mode does. > >> Hence, the IVE table is directly indexed by the IRQ number. > >> > >> Signed-off-by: Cédric Le Goater <c...@kaod.org> > > > > As you've suggested in yourself, I think we might need to more > > explicitly model the different components of the XIVE system. As part > > of that, I think you need to be clearer in this base skeleton about > > exactly what component your XIVE object represents.
Sorry it's been so long since I looked at these. > ok. The base skeleton is the IVRE, the central engine handling > the routing. > > > If the answer is "the overall thing" > > Yes, it is more or less that currently. > > The sPAPRXive object models the source engine and the routing > engine in one object. Yeah, I suspect we don't want that. Although it might seem simpler in the spapr case, at least at first glance, I think it will cause us problems later. At the very least, it's likely to make it harder to share code between the spapr and powernv case. I think it will also make for more confusion about exactly what things belong where. > I have merged these for simplicity and because the interrupt > controller has an internal source for the interrupts of the "IPI" > type, which are used for the CPU IPIs but also for other generic > interrupts, like the OpenCAPI ones. The XIVE sPAPR interface is > also much simpler than the baremetal one, all the tables are > maintained in the hypervisor, so this choice made some sense. > > But since, I have started the PowerNV model and I am duplicating > a lot of code to handle the triggering and the MMIOs in the > different sources. So I am not convinced anymore. Nevertheless, > the overall routing logic is the same even if some the tables > are not located in QEMU anymore, but in the machine memory. > > The sPAPRXiveNVT models some of the CPU presenter engine. It > holds the virtual CPU interrupt states when not dispatched on > a real HW thread. Real world is more complex. There are "CAM" > lines in the HW threads which are compared to find a matching > candidate. But I don't think we need to anything more complex > than today unless we want to support KVM under TCG ... > > > I suspect that's not what you > > want - I had one of those for XICs which proved to be a mistake > > (eventually replaced by the XICSFabric interface). > > The XICSFabric would be the main Xive object. The interface > between the sources and the routing engine is hidden in sPAPR, > we can use a simple function call : > > spapr_xive_irq(pnv->xive, irq); > > we could get rid of the qirqs but they are required for XICS. I don't quite follow, but this doesn't sound right. > PowerNV uses MMIOs to notify an event and it makes the modeling > somewhat easier. Each controller model has a notify port address > register on which a interrupt number is written to forward an > event to the routing engine. So it is a simple store. > > I don't know why there is a different notify port address per > source, may be for extra filtering at the routing engine level. > > > Changing the model later isn't impossible, but doing so without > > breaking migration can be a real pain, so I think it's worth a > > reasonable effort to try and get it right initially. > > I completely agree. > > This is why I have started the PnvXive model to challenge the > current PAPR design. I have hacked a bunch of patches for XIVE, > LPC, PSI, OCC and basic PPC support which boot a PowerNV P9 up to > petitboot. It would look better with a source object, but the > location of the PQ bits is a bit problematic. It highly depends > on the controller. The main controller uses tables in the hypervisor > memory. The PSIHB controller has its own bits. I suppose it is > the same for PHB4. I need to take a closer look at how we could > have a common source object. Ok, sounds like a good idea. > > The most important part is KVM support and how we expose the > MMIO region. We need to make progress on that topic. > > Thanks, > > C. > > > >> --- > >> > >> Changes since v1 : > >> > >> - used g_new0 instead of g_malloc0 > >> - removed VMSTATE_STRUCT_VARRAY_UINT32_ALLOC > >> - introduced a device reset handler. the object needs to be parented > >> to sysbus when created. > >> - renamed spapr_xive_irq_set to spapr_xive_irq_enable > >> - renamed spapr_xive_irq_unset to spapr_xive_irq_disable > >> - moved the PPC_BIT macros under target/ppc/cpu.h > >> - shrinked file copyright header > >> > >> default-configs/ppc64-softmmu.mak | 1 + > >> hw/intc/Makefile.objs | 1 + > >> hw/intc/spapr_xive.c | 156 > >> ++++++++++++++++++++++++++++++++++++++ > >> hw/intc/xive-internal.h | 41 ++++++++++ > >> include/hw/ppc/spapr_xive.h | 35 +++++++++ > >> 5 files changed, 234 insertions(+) > >> create mode 100644 hw/intc/spapr_xive.c > >> create mode 100644 hw/intc/xive-internal.h > >> create mode 100644 include/hw/ppc/spapr_xive.h > >> > >> diff --git a/default-configs/ppc64-softmmu.mak > >> b/default-configs/ppc64-softmmu.mak > >> index d1b3a6dd50f8..4a7f6a0696de 100644 > >> --- a/default-configs/ppc64-softmmu.mak > >> +++ b/default-configs/ppc64-softmmu.mak > >> @@ -56,6 +56,7 @@ CONFIG_SM501=y > >> CONFIG_XICS=$(CONFIG_PSERIES) > >> CONFIG_XICS_SPAPR=$(CONFIG_PSERIES) > >> CONFIG_XICS_KVM=$(call land,$(CONFIG_PSERIES),$(CONFIG_KVM)) > >> +CONFIG_XIVE_SPAPR=$(CONFIG_PSERIES) > >> # For PReP > >> CONFIG_SERIAL_ISA=y > >> CONFIG_MC146818RTC=y > >> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs > >> index ae358569a155..49e13e7aeeee 100644 > >> --- a/hw/intc/Makefile.objs > >> +++ b/hw/intc/Makefile.objs > >> @@ -35,6 +35,7 @@ obj-$(CONFIG_SH4) += sh_intc.o > >> obj-$(CONFIG_XICS) += xics.o > >> obj-$(CONFIG_XICS_SPAPR) += xics_spapr.o > >> obj-$(CONFIG_XICS_KVM) += xics_kvm.o > >> +obj-$(CONFIG_XIVE_SPAPR) += spapr_xive.o > >> obj-$(CONFIG_POWERNV) += xics_pnv.o > >> obj-$(CONFIG_ALLWINNER_A10_PIC) += allwinner-a10-pic.o > >> obj-$(CONFIG_S390_FLIC) += s390_flic.o > >> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c > >> new file mode 100644 > >> index 000000000000..e6e8841add17 > >> --- /dev/null > >> +++ b/hw/intc/spapr_xive.c > >> @@ -0,0 +1,156 @@ > >> +/* > >> + * QEMU PowerPC sPAPR XIVE interrupt controller model > >> + * > >> + * Copyright (c) 2017, IBM Corporation. > >> + * > >> + * This code is licensed under the GPL version 2 or later. See the > >> + * COPYING file in the top-level directory. > >> + */ > >> + > >> +#include "qemu/osdep.h" > >> +#include "qemu/log.h" > >> +#include "qapi/error.h" > >> +#include "target/ppc/cpu.h" > >> +#include "sysemu/cpus.h" > >> +#include "sysemu/dma.h" > >> +#include "monitor/monitor.h" > >> +#include "hw/ppc/spapr_xive.h" > >> + > >> +#include "xive-internal.h" > >> + > >> +/* > >> + * Main XIVE object > >> + */ > >> + > >> +void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon) > >> +{ > >> + int i; > >> + > >> + for (i = 0; i < xive->nr_irqs; i++) { > >> + XiveIVE *ive = &xive->ivt[i]; > >> + > >> + if (!(ive->w & IVE_VALID)) { > >> + continue; > >> + } > >> + > >> + monitor_printf(mon, " %4x %s %08x %08x\n", i, > >> + ive->w & IVE_MASKED ? "M" : " ", > >> + (int) GETFIELD(IVE_EQ_INDEX, ive->w), > >> + (int) GETFIELD(IVE_EQ_DATA, ive->w)); > >> + } > >> +} > >> + > >> +static void spapr_xive_reset(DeviceState *dev) > >> +{ > >> + sPAPRXive *xive = SPAPR_XIVE(dev); > >> + int i; > >> + > >> + /* Mask all valid IVEs in the IRQ number space. */ > >> + for (i = 0; i < xive->nr_irqs; i++) { > >> + XiveIVE *ive = &xive->ivt[i]; > >> + if (ive->w & IVE_VALID) { > >> + ive->w |= IVE_MASKED; > >> + } > >> + } > >> +} > >> + > >> +static void spapr_xive_realize(DeviceState *dev, Error **errp) > >> +{ > >> + sPAPRXive *xive = SPAPR_XIVE(dev); > >> + > >> + if (!xive->nr_irqs) { > >> + error_setg(errp, "Number of interrupt needs to be greater 0"); > >> + return; > >> + } > >> + > >> + /* Allocate the IVT (Interrupt Virtualization Table) */ > >> + xive->ivt = g_new0(XiveIVE, xive->nr_irqs); > >> +} > >> + > >> +static const VMStateDescription vmstate_spapr_xive_ive = { > >> + .name = TYPE_SPAPR_XIVE "/ive", > >> + .version_id = 1, > >> + .minimum_version_id = 1, > >> + .fields = (VMStateField []) { > >> + VMSTATE_UINT64(w, XiveIVE), > >> + VMSTATE_END_OF_LIST() > >> + }, > >> +}; > >> + > >> +static bool vmstate_spapr_xive_needed(void *opaque) > >> +{ > >> + /* TODO check machine XIVE support */ > >> + return true; > >> +} > >> + > >> +static const VMStateDescription vmstate_spapr_xive = { > >> + .name = TYPE_SPAPR_XIVE, > >> + .version_id = 1, > >> + .minimum_version_id = 1, > >> + .needed = vmstate_spapr_xive_needed, > >> + .fields = (VMStateField[]) { > >> + VMSTATE_UINT32_EQUAL(nr_irqs, sPAPRXive, NULL), > >> + VMSTATE_STRUCT_VARRAY_UINT32(ivt, sPAPRXive, nr_irqs, 1, > >> + vmstate_spapr_xive_ive, XiveIVE), > >> + VMSTATE_END_OF_LIST() > >> + }, > >> +}; > >> + > >> +static Property spapr_xive_properties[] = { > >> + DEFINE_PROP_UINT32("nr-irqs", sPAPRXive, nr_irqs, 0), > >> + DEFINE_PROP_END_OF_LIST(), > >> +}; > >> + > >> +static void spapr_xive_class_init(ObjectClass *klass, void *data) > >> +{ > >> + DeviceClass *dc = DEVICE_CLASS(klass); > >> + > >> + dc->realize = spapr_xive_realize; > >> + dc->reset = spapr_xive_reset; > >> + dc->props = spapr_xive_properties; > >> + dc->desc = "sPAPR XIVE interrupt controller"; > >> + dc->vmsd = &vmstate_spapr_xive; > >> +} > >> + > >> +static const TypeInfo spapr_xive_info = { > >> + .name = TYPE_SPAPR_XIVE, > >> + .parent = TYPE_SYS_BUS_DEVICE, > >> + .instance_size = sizeof(sPAPRXive), > >> + .class_init = spapr_xive_class_init, > >> +}; > >> + > >> +static void spapr_xive_register_types(void) > >> +{ > >> + type_register_static(&spapr_xive_info); > >> +} > >> + > >> +type_init(spapr_xive_register_types) > >> + > >> +XiveIVE *spapr_xive_get_ive(sPAPRXive *xive, uint32_t lisn) > >> +{ > >> + return lisn < xive->nr_irqs ? &xive->ivt[lisn] : NULL; > >> +} > >> + > >> +bool spapr_xive_irq_enable(sPAPRXive *xive, uint32_t lisn) > >> +{ > >> + XiveIVE *ive = spapr_xive_get_ive(xive, lisn); > >> + > >> + if (!ive) { > >> + return false; > >> + } > >> + > >> + ive->w |= IVE_VALID; > >> + return true; > >> +} > >> + > >> +bool spapr_xive_irq_disable(sPAPRXive *xive, uint32_t lisn) > >> +{ > >> + XiveIVE *ive = spapr_xive_get_ive(xive, lisn); > >> + > >> + if (!ive) { > >> + return false; > >> + } > >> + > >> + ive->w &= ~IVE_VALID; > >> + return true; > >> +} > >> diff --git a/hw/intc/xive-internal.h b/hw/intc/xive-internal.h > >> new file mode 100644 > >> index 000000000000..132b71a6daf0 > >> --- /dev/null > >> +++ b/hw/intc/xive-internal.h > >> @@ -0,0 +1,41 @@ > >> +/* > >> + * QEMU PowerPC XIVE interrupt controller model > >> + * > >> + * Copyright (c) 2016-2017, IBM Corporation. > >> + * > >> + * This code is licensed under the GPL version 2 or later. See the > >> + * COPYING file in the top-level directory. > >> + */ > >> + > >> +#ifndef _INTC_XIVE_INTERNAL_H > >> +#define _INTC_XIVE_INTERNAL_H > >> + > >> +/* Utilities to manipulate these (originaly from OPAL) */ > >> +#define MASK_TO_LSH(m) (__builtin_ffsl(m) - 1) > >> +#define GETFIELD(m, v) (((v) & (m)) >> MASK_TO_LSH(m)) > >> +#define SETFIELD(m, v, val) \ > >> + (((v) & ~(m)) | ((((typeof(v))(val)) << MASK_TO_LSH(m)) & (m))) > >> + > >> +/* IVE/EAS > >> + * > >> + * One per interrupt source. Targets that interrupt to a given EQ > >> + * and provides the corresponding logical interrupt number (EQ data) > >> + * > >> + * We also map this structure to the escalation descriptor inside > >> + * an EQ, though in that case the valid and masked bits are not used. > >> + */ > >> +typedef struct XiveIVE { > >> + /* Use a single 64-bit definition to make it easier to > >> + * perform atomic updates > >> + */ > >> + uint64_t w; > >> +#define IVE_VALID PPC_BIT(0) > >> +#define IVE_EQ_BLOCK PPC_BITMASK(4, 7) /* Destination EQ block# > >> */ > >> +#define IVE_EQ_INDEX PPC_BITMASK(8, 31) /* Destination EQ index > >> */ > >> +#define IVE_MASKED PPC_BIT(32) /* Masked */ > >> +#define IVE_EQ_DATA PPC_BITMASK(33, 63) /* Data written to the > >> EQ */ > >> +} XiveIVE; > >> + > >> +XiveIVE *spapr_xive_get_ive(sPAPRXive *xive, uint32_t lisn); > >> + > >> +#endif /* _INTC_XIVE_INTERNAL_H */ > >> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h > >> new file mode 100644 > >> index 000000000000..5b1f78e06a1e > >> --- /dev/null > >> +++ b/include/hw/ppc/spapr_xive.h > >> @@ -0,0 +1,35 @@ > >> +/* > >> + * QEMU PowerPC sPAPR XIVE interrupt controller model > >> + * > >> + * Copyright (c) 2017, IBM Corporation. > >> + * > >> + * This code is licensed under the GPL version 2 or later. See the > >> + * COPYING file in the top-level directory. > >> + */ > >> + > >> +#ifndef PPC_SPAPR_XIVE_H > >> +#define PPC_SPAPR_XIVE_H > >> + > >> +#include <hw/sysbus.h> > >> + > >> +typedef struct sPAPRXive sPAPRXive; > >> +typedef struct XiveIVE XiveIVE; > >> + > >> +#define TYPE_SPAPR_XIVE "spapr-xive" > >> +#define SPAPR_XIVE(obj) OBJECT_CHECK(sPAPRXive, (obj), TYPE_SPAPR_XIVE) > >> + > >> +struct sPAPRXive { > >> + SysBusDevice parent; > >> + > >> + /* Properties */ > >> + uint32_t nr_irqs; > >> + > >> + /* XIVE internal tables */ > >> + XiveIVE *ivt; > >> +}; > >> + > >> +bool spapr_xive_irq_enable(sPAPRXive *xive, uint32_t lisn); > >> +bool spapr_xive_irq_disable(sPAPRXive *xive, uint32_t lisn); > >> +void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon); > >> + > >> +#endif /* PPC_SPAPR_XIVE_H */ > > > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature