Re: [Qemu-ppc] [PATCH 08/25] spapr: introduce a skeleton for the XIVE interrupt controller

[David Gibson]

On Wed, Nov 29, 2017 at 12:49:04PM +0100, Greg Kurz wrote:
> On Thu, 23 Nov 2017 14:29:38 +0100
> Cédric Le Goater <address@hidden> wrote:
> 
> > The XIVE interrupt controller uses a set of tables to redirect exception
> > from event sources to CPU threads. The Interrupt Virtualization Entry (IVE)
> > table, also known as Event Assignment Structure (EAS), is one them.
> > 
> > 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.
> > 
> > The IVE stores Event Queue data associated with a source. The lookups
> > are performed when the source is configured or when an event is
> > triggered.
> > 
> > Signed-off-by: Cédric Le Goater <address@hidden>
> > ---
> >  default-configs/ppc64-softmmu.mak |   1 +
> >  hw/intc/Makefile.objs             |   1 +
> >  hw/intc/spapr_xive.c              | 165 
> > ++++++++++++++++++++++++++++++++++++++
> >  hw/intc/xive-internal.h           |  50 ++++++++++++
> >  include/hw/ppc/spapr_xive.h       |  44 ++++++++++
> >  5 files changed, 261 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..b2fc3007c85f
> > --- /dev/null
> > +++ b/hw/intc/spapr_xive.c
> > @@ -0,0 +1,165 @@
> > +/*
> > + * QEMU PowerPC sPAPR XIVE model
> > + *
> > + * Copyright (c) 2017, IBM Corporation.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License, version 2, as
> 
> version 2 or (at your option) any later version.
> 
> > + * published by the Free Software Foundation.
> > + *
> > + * This program 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 General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +#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));
> > +    }
> > +}
> > +
> > +void spapr_xive_reset(void *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_malloc0(xive->nr_irqs * sizeof(XiveIVE));
> 
> Even if it isn't documented, AFAIK current recommended practice is to do:

Yeah, that's a good idea - protects against integer overflows if
nothing else.

>     xive->ivt = g_new0(XiveIVE, xive->nr_irqs);
> 
> > +
> > +    qemu_register_reset(spapr_xive_reset, dev);
> 
> Shouldn't you set dc->reset in spapr_xive_class_init() instead ?

Yeah, that.

-- 
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