Re: [Qemu-ppc] [PATCH v2 3/7] ppc/pnv: Add XSCOM infrastructure

[David Gibson]

On Tue, Sep 06, 2016 at 04:42:01PM +0200, Cédric Le Goater wrote:
> On 09/05/2016 05:39 AM, David Gibson wrote:
> > On Wed, Aug 31, 2016 at 06:34:11PM +0200, Cédric Le Goater wrote:
> >> From: Benjamin Herrenschmidt <address@hidden>
> >>
> >> XSCOM is an interface to a sideband bus provided by the POWER8 chip
> >> pervasive unit, which gives access to a number of facilities in the
> >> chip that are needed by the OPAL firmware and to a lesser extent,
> >> Linux. This is among others how the PCI Host bridges get configured
> >> at boot or how the LPC bus is accessed.
> >>
> >> This provides a simple bus and device type for devices sitting on
> >> XSCOM along with some facilities to optionally generate corresponding
> >> device-tree nodes
> >>
> >> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
> >> [clg: updated for qemu-2.7
> >>       ported on new sPowerNVMachineState which was merged with PnvSystem
> >>       removed TRACE_XSCOM
> >>       fixed checkpatch errors
> >>       replaced assert with error_setg in xscom_realize()
> >>       reworked xscom_create
> >>       introduced the use of the chip_class for chip model contants
> >>       ]
> >> Signed-off-by: Cédric Le Goater <address@hidden>
> >> ---
> >>
> >>  They were some discussions on whether we should use a qemu
> >>  address_space instead of the xscom ranges defined in this patch. 
> >>  I gave it try, it is possible but it brings extra unnecessary calls
> >>  and complexity. I think the current solution is better.
> >>
> >>  hw/ppc/Makefile.objs       |   2 +-
> >>  hw/ppc/pnv.c               |  11 ++
> >>  hw/ppc/pnv_xscom.c         | 408 
> >> +++++++++++++++++++++++++++++++++++++++++++++
> >>  include/hw/ppc/pnv.h       |   2 +
> >>  include/hw/ppc/pnv_xscom.h |  75 +++++++++
> >>  5 files changed, 497 insertions(+), 1 deletion(-)
> >>  create mode 100644 hw/ppc/pnv_xscom.c
> >>  create mode 100644 include/hw/ppc/pnv_xscom.h
> >>
> >> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> >> index 8105db7d5600..f580e5c41413 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
> >>  # IBM PowerNV
> >> -obj-$(CONFIG_POWERNV) += pnv.o
> >> +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.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 06051268e200..a6e7f66b2c0a 100644
> >> --- a/hw/ppc/pnv.c
> >> +++ b/hw/ppc/pnv.c
> >> @@ -39,6 +39,8 @@
> >>  #include "exec/address-spaces.h"
> >>  #include "qemu/cutils.h"
> >>  
> >> +#include "hw/ppc/pnv_xscom.h"
> >> +
> >>  #include <libfdt.h>
> >>  
> >>  #define FDT_ADDR                0x01000000
> >> @@ -103,6 +105,7 @@ static void *powernv_create_fdt(PnvMachineState *pnv,
> >>      char *buf;
> >>      const char plat_compat[] = "qemu,powernv\0ibm,powernv";
> >>      int off;
> >> +    int i;
> >>  
> >>      fdt = g_malloc0(FDT_MAX_SIZE);
> >>      _FDT((fdt_create_empty_tree(fdt, FDT_MAX_SIZE)));
> >> @@ -142,6 +145,11 @@ static void *powernv_create_fdt(PnvMachineState *pnv,
> >>      /* Memory */
> >>      powernv_populate_memory(fdt);
> >>  
> >> +    /* Populate XSCOM for each chip */
> >> +    for (i = 0; i < pnv->num_chips; i++) {
> >> +        xscom_populate_fdt(pnv->chips[i]->xscom, fdt, 0);
> >> +    }
> > 
> > There will be a bunch of per-chip things in the fdt - I suggest a
> > common function to do all the fdt creation for a single chip.  Since
> > you've moved to using the fdt_rw functions exclusively, you don't have
> > the sequence constraints that would have made that awkward previously.
> 
> ok.
> 
> >>      return fdt;
> >>  }
> >>  
> >> @@ -305,6 +313,9 @@ static void pnv_chip_realize(DeviceState *dev, Error 
> >> **errp)
> >>      PnvChip *chip = PNV_CHIP(dev);
> >>      PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> >>  
> >> +    /* Set up XSCOM bus */
> >> +    chip->xscom = xscom_create(chip);
> > 
> > So, this creates the xscom as a new device object, unfortunately doing
> > that from another object's realize() violations QOM lifetime rules as
> > I understand them.  I think - I have to admit I'm pretty confused on
> > this topic myself.
> > 
> > You could construct the scom from chip init, then realize it from chip
> > realize, but I think using object_new() (via qdev_create()) from
> > another object's init also breaks the rules.  You are however allowed
> > to allocate the scom state statically within the chip and use
> > object_initialize() instead, AIUI.
> > 
> > Alternatively.. it might be simpler to just drop the SCOM as a
> > separate device.  I think you could just hang the scom bus directly
> > off the chip object.  IIUC the scom is basically the only chip-level
> > control mechanism, so this does make a certain amount of sense.
> 
> yes. I am exposing more and more stuff of the chip object under the 
> xscom object so we should merge them. I agree. We will still need 
> some XScomDevice of some sort.
> 
> >>      pcc->realize(chip, errp);
> >>  }
> >>  
> >> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> >> new file mode 100644
> >> index 000000000000..7ed3804f4b3a
> >> --- /dev/null
> >> +++ b/hw/ppc/pnv_xscom.c
> >> @@ -0,0 +1,408 @@
> >> +
> >> +/*
> >> + * QEMU PowerNV XSCOM bus definitions
> >> + *
> >> + * Copyright (c) 2010 David Gibson, IBM Corporation <address@hidden>
> >> + * Based on the s390 virtio bus code:
> >> + * Copyright (c) 2009 Alexander Graf <address@hidden>
> >> + *
> >> + * 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/>.
> >> + */
> >> +
> >> +/* TODO: Add some infrastructure for "random stuff" and FIRs that
> >> + * various units might want to deal with without creating actual
> >> + * XSCOM devices.
> >> + *
> >> + * For example, HB LPC XSCOM in the PIBAM
> >> + */
> >> +#include "qemu/osdep.h"
> >> +#include "qapi/error.h"
> >> +#include "hw/hw.h"
> >> +#include "sysemu/sysemu.h"
> >> +#include "hw/boards.h"
> >> +#include "monitor/monitor.h"
> >> +#include "hw/loader.h"
> >> +#include "elf.h"
> >> +#include "hw/sysbus.h"
> >> +#include "sysemu/kvm.h"
> >> +#include "sysemu/device_tree.h"
> >> +#include "hw/ppc/fdt.h"
> >> +
> >> +#include "hw/ppc/pnv_xscom.h"
> >> +
> >> +#include <libfdt.h>
> >> +
> >> +#define TYPE_XSCOM "xscom"
> >> +#define XSCOM(obj) OBJECT_CHECK(XScomState, (obj), TYPE_XSCOM)
> >> +
> >> +#define XSCOM_SIZE        0x800000000ull
> >> +#define XSCOM_BASE(chip)  (0x3fc0000000000ull + ((uint64_t)(chip)) * 
> >> XSCOM_SIZE)
> >> +
> >> +
> >> +typedef struct XScomState {
> >> +    /*< private >*/
> >> +    SysBusDevice parent_obj;
> >> +    /*< public >*/
> >> +
> >> +    MemoryRegion mem;
> >> +    int32_t chip_id;
> > 
> > Merging scom and chip would avoid the duplication of this field too.
> 
> yes.
> 
> >> +    PnvChipClass *chip_class;
> >> +    XScomBus *bus;
> >> +} XScomState;
> >> +
> >> +static uint32_t xscom_to_pcb_addr(uint64_t addr)
> >> +{
> >> +        addr &= (XSCOM_SIZE - 1);
> >> +        return ((addr >> 4) & ~0xfull) | ((addr >> 3) & 0xf);
> >> +}
> >> +
> >> +static void xscom_complete(uint64_t hmer_bits)
> >> +{
> >> +    CPUState *cs = current_cpu;
> >> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> >> +    CPUPPCState *env = &cpu->env;
> >> +
> >> +    cpu_synchronize_state(cs);
> >> +    env->spr[SPR_HMER] |= hmer_bits;
> >> +
> >> +    /* XXX Need a CPU helper to set HMER, also handle gneeration
> >> +     * of HMIs
> >> +     */
> >> +}
> >> +
> >> +static XScomDevice *xscom_find_target(XScomState *s, uint32_t pcb_addr,
> >> +                                      uint32_t *range)
> >> +{
> >> +    BusChild *bc;
> >> +
> >> +    QTAILQ_FOREACH(bc, &s->bus->bus.children, sibling) {
> >> +        DeviceState *qd = bc->child;
> >> +        XScomDevice *xd = XSCOM_DEVICE(qd);
> >> +        unsigned int i;
> >> +
> >> +        for (i = 0; i < MAX_XSCOM_RANGES; i++) {
> >> +            if (xd->ranges[i].addr <= pcb_addr &&
> >> +                (xd->ranges[i].addr + xd->ranges[i].size) > pcb_addr) {
> >> +                *range = i;
> >> +                return xd;
> >> +            }
> >> +        }
> >> +    }
> > 
> > Hmm.. you could set up a SCOM local address space using the
> > infrastructure in memory.c, rather than doing your own dispatch.
> 
> I need to study this option a little deeper. 

So.. something I realized a bit later.

The obvious way of changing XScomDevice to a QOM interface isn't
really compatible with using the address space infrastructure.  You'd
have read/write methods in the interface, and since that's not the
same interface as an MR you'd need to do your own address decode /
dispatch as you do above.

That does suggest an alternative approach though.  You could remove
XScomDevice entirely from QOM existence, and just expose the xscom
address space globally, much like address_space_memory.  The
individual devices could just register their own subregions within it.

I'm not sure if the latter is a good idea, though.

> 
> > 
> >> +    return NULL;
> >> +}
> >> +
> >> +static bool xscom_dispatch_read(XScomState *s, uint32_t pcb_addr,
> >> +                                uint64_t *out_val)
> >> +{
> >> +    uint32_t range, offset;
> >> +    struct XScomDevice *xd = xscom_find_target(s, pcb_addr, &range);
> >> +    XScomDeviceClass *xc;
> >> +
> >> +    if (!xd) {
> >> +        return false;
> >> +    }
> >> +    xc = XSCOM_DEVICE_GET_CLASS(xd);
> >> +    if (!xc->read) {
> >> +        return false;
> >> +    }
> >> +    offset = pcb_addr - xd->ranges[range].addr;
> >> +    return xc->read(xd, range, offset, out_val);
> >> +}
> >> +
> >> +static bool xscom_dispatch_write(XScomState *s, uint32_t pcb_addr, 
> >> uint64_t val)
> >> +{
> >> +    uint32_t range, offset;
> >> +    struct XScomDevice *xd = xscom_find_target(s, pcb_addr, &range);
> >> +    XScomDeviceClass *xc;
> >> +
> >> +    if (!xd) {
> >> +        return false;
> >> +    }
> >> +    xc = XSCOM_DEVICE_GET_CLASS(xd);
> >> +    if (!xc->write) {
> >> +        return false;
> >> +    }
> >> +    offset = pcb_addr - xd->ranges[range].addr;
> >> +    return xc->write(xd, range, offset, val);
> >> +}
> >> +
> >> +static uint64_t xscom_read(void *opaque, hwaddr addr, unsigned width)
> >> +{
> >> +    XScomState *s = opaque;
> >> +    uint32_t pcba = xscom_to_pcb_addr(addr);
> >> +    uint64_t val;
> >> +
> >> +    assert(width == 8);
> >> +
> >> +    /* Handle some SCOMs here before dispatch */
> >> +    switch (pcba) {
> >> +    case 0xf000f:
> >> +        val = s->chip_class->chip_f000f;
> >> +        break;
> >> +    case 0x1010c00:     /* PIBAM FIR */
> >> +    case 0x1010c03:     /* PIBAM FIR MASK */
> >> +    case 0x2020007:     /* ADU stuff */
> >> +    case 0x2020009:     /* ADU stuff */
> >> +    case 0x202000f:     /* ADU stuff */
> >> +        val = 0;
> >> +        break;
> >> +    case 0x2013f00:     /* PBA stuff */
> >> +    case 0x2013f01:     /* PBA stuff */
> >> +    case 0x2013f02:     /* PBA stuff */
> >> +    case 0x2013f03:     /* PBA stuff */
> >> +    case 0x2013f04:     /* PBA stuff */
> >> +    case 0x2013f05:     /* PBA stuff */
> >> +    case 0x2013f06:     /* PBA stuff */
> >> +    case 0x2013f07:     /* PBA stuff */
> >> +        val = 0;
> >> +        break;
> >> +    default:
> >> +        if (!xscom_dispatch_read(s, pcba, &val)) {
> >> +            xscom_complete(HMER_XSCOM_FAIL | HMER_XSCOM_DONE);
> >> +            return 0;
> >> +        }
> >> +    }
> >> +
> >> +    xscom_complete(HMER_XSCOM_DONE);
> >> +    return val;
> >> +}
> >> +
> >> +static void xscom_write(void *opaque, hwaddr addr, uint64_t val,
> >> +                        unsigned width)
> >> +{
> >> +    XScomState *s = opaque;
> >> +    uint32_t pcba = xscom_to_pcb_addr(addr);
> >> +
> >> +    assert(width == 8);
> >> +
> >> +    /* Handle some SCOMs here before dispatch */
> >> +    switch (pcba) {
> >> +        /* We ignore writes to these */
> >> +    case 0xf000f:       /* chip id is RO */
> >> +    case 0x1010c00:     /* PIBAM FIR */
> >> +    case 0x1010c01:     /* PIBAM FIR */
> >> +    case 0x1010c02:     /* PIBAM FIR */
> >> +    case 0x1010c03:     /* PIBAM FIR MASK */
> >> +    case 0x1010c04:     /* PIBAM FIR MASK */
> >> +    case 0x1010c05:     /* PIBAM FIR MASK */
> >> +    case 0x2020007:     /* ADU stuff */
> >> +    case 0x2020009:     /* ADU stuff */
> >> +    case 0x202000f:     /* ADU stuff */
> >> +        break;
> >> +    default:
> >> +        if (!xscom_dispatch_write(s, pcba, val)) {
> >> +            xscom_complete(HMER_XSCOM_FAIL | HMER_XSCOM_DONE);
> >> +            return;
> >> +        }
> >> +    }
> >> +
> >> +    xscom_complete(HMER_XSCOM_DONE);
> >> +}
> >> +
> >> +static const MemoryRegionOps xscom_ops = {
> >> +    .read = xscom_read,
> >> +    .write = xscom_write,
> >> +    .valid.min_access_size = 8,
> >> +    .valid.max_access_size = 8,
> >> +    .impl.min_access_size = 8,
> >> +    .impl.max_access_size = 8,
> >> +    .endianness = DEVICE_BIG_ENDIAN,
> >> +};
> >> +
> >> +static int xscom_init(SysBusDevice *dev)
> >> +{
> >> +    XScomState *s = XSCOM(dev);
> >> +
> >> +    s->chip_id = -1;
> >> +    return 0;
> >> +}
> >> +
> >> +static void xscom_realize(DeviceState *dev, Error **errp)
> >> +{
> >> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> >> +    XScomState *s = XSCOM(dev);
> >> +    char *name;
> >> +
> >> +    if (s->chip_id < 0) {
> >> +        error_setg(errp, "invalid chip id '%d'", s->chip_id);
> >> +        return;
> >> +    }
> >> +    name = g_strdup_printf("xscom-%x", s->chip_id);
> >> +    memory_region_init_io(&s->mem, OBJECT(s), &xscom_ops, s, name, 
> >> XSCOM_SIZE);
> >> +    sysbus_init_mmio(sbd, &s->mem);
> >> +    sysbus_mmio_map(sbd, 0, XSCOM_BASE(s->chip_id));
> >> +}
> >> +
> >> +static Property xscom_properties[] = {
> >> +        DEFINE_PROP_INT32("chip_id", XScomState, chip_id, 0),
> >> +        DEFINE_PROP_END_OF_LIST(),
> >> +};
> >> +
> >> +static void xscom_class_init(ObjectClass *klass, void *data)
> >> +{
> >> +    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> >> +    DeviceClass *dc = DEVICE_CLASS(klass);
> >> +
> >> +    dc->props = xscom_properties;
> >> +    dc->realize = xscom_realize;
> >> +    k->init = xscom_init;
> >> +}
> >> +
> >> +static const TypeInfo xscom_info = {
> >> +    .name          = TYPE_XSCOM,
> >> +    .parent        = TYPE_SYS_BUS_DEVICE,
> >> +    .instance_size = sizeof(XScomState),
> >> +    .class_init    = xscom_class_init,
> >> +};
> >> +
> >> +static void xscom_bus_class_init(ObjectClass *klass, void *data)
> >> +{
> >> +}
> >> +
> >> +static const TypeInfo xscom_bus_info = {
> >> +    .name = TYPE_XSCOM_BUS,
> >> +    .parent = TYPE_BUS,
> >> +    .class_init = xscom_bus_class_init,
> >> +    .instance_size = sizeof(XScomBus),
> >> +};
> >> +
> >> +XScomBus *xscom_create(PnvChip *chip)
> >> +{
> >> +    DeviceState *dev;
> >> +    XScomState *xdev;
> >> +    BusState *qbus;
> >> +    XScomBus *xb;
> >> +    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> >> +
> >> +    dev = qdev_create(NULL, TYPE_XSCOM);
> >> +    qdev_prop_set_uint32(dev, "chip_id", chip->chip_id);
> >> +    qdev_init_nofail(dev);
> >> +
> >> +    /* Create bus on bridge device */
> >> +    qbus = qbus_create(TYPE_XSCOM_BUS, dev, "xscom");
> >> +    xb = DO_UPCAST(XScomBus, bus, qbus);
> >> +    xb->chip_id = chip->chip_id;
> >> +    xdev = XSCOM(dev);
> >> +    xdev->bus = xb;
> >> +    xdev->chip_class = pcc;
> >> +
> >> +    return xb;
> >> +}
> >> +
> >> +int xscom_populate_fdt(XScomBus *xb, void *fdt, int root_offset)
> > 
> > What is the root_offset parameter for, since it is always 0?
> 
> yes ...
> 
> >> +{
> >> +    BusChild *bc;
> >> +    char *name;
> >> +    const char compat[] = "ibm,power8-xscom\0ibm,xscom";
> >> +    uint64_t reg[] = { cpu_to_be64(XSCOM_BASE(xb->chip_id)),
> >> +                       cpu_to_be64(XSCOM_SIZE) };
> >> +    int xscom_offset;
> >> +
> >> +    name = g_strdup_printf("address@hidden", (unsigned long long)
> >> +                           be64_to_cpu(reg[0]));
> > 
> > Nit: in qemu the PRIx64 etc. macros are usually preferred to (unsigned
> > long long) casts of this type.
> 
> sure.
> 
> >> +    xscom_offset = fdt_add_subnode(fdt, root_offset, name);
> >> +    _FDT(xscom_offset);
> >> +    g_free(name);
> >> +    _FDT((fdt_setprop_cell(fdt, xscom_offset, "ibm,chip-id", 
> >> xb->chip_id)));
> >> +    _FDT((fdt_setprop_cell(fdt, xscom_offset, "#address-cells", 1)));
> >> +    _FDT((fdt_setprop_cell(fdt, xscom_offset, "#size-cells", 1)));
> >> +    _FDT((fdt_setprop(fdt, xscom_offset, "reg", reg, sizeof(reg))));
> >> +    _FDT((fdt_setprop(fdt, xscom_offset, "compatible", compat,
> >> +                      sizeof(compat))));
> >> +    _FDT((fdt_setprop(fdt, xscom_offset, "scom-controller", NULL, 0)));
> >> +
> >> +    QTAILQ_FOREACH(bc, &xb->bus.children, sibling) {
> >> +        DeviceState *qd = bc->child;
> >> +        XScomDevice *xd = XSCOM_DEVICE(qd);
> >> +        XScomDeviceClass *xc = XSCOM_DEVICE_GET_CLASS(xd);
> >> +        uint32_t reg[MAX_XSCOM_RANGES * 2];
> >> +        unsigned int i, sz = 0;
> >> +        void *cp, *p;
> >> +        int child_offset;
> >> +
> >> +        /* Some XSCOM slaves may not be represented in the DT */
> >> +        if (!xc->dt_name) {
> >> +            continue;
> >> +        }
> >> +        name = g_strdup_printf("address@hidden", xc->dt_name, 
> >> xd->ranges[0].addr);
> >> +        child_offset = fdt_add_subnode(fdt, xscom_offset, name);
> >> +        _FDT(child_offset);
> >> +        g_free(name);
> >> +        for (i = 0; i < MAX_XSCOM_RANGES; i++) {
> >> +            if (xd->ranges[i].size == 0) {
> >> +                break;
> >> +            }
> >> +            reg[sz++] = cpu_to_be32(xd->ranges[i].addr);
> >> +            reg[sz++] = cpu_to_be32(xd->ranges[i].size);
> >> +        }
> >> +        _FDT((fdt_setprop(fdt, child_offset, "reg", reg, sz * 4)));
> > 
> > Use of fdt_appendprop() might make this a little neater.
> 
> ok.
> 
> >> +        if (xc->devnode) {
> >> +            _FDT((xc->devnode(xd, fdt, child_offset)));
> >> +        }
> >> +#define MAX_COMPATIBLE_PROP     1024
> >> +        cp = p = g_malloc0(MAX_COMPATIBLE_PROP);
> >> +        i = 0;
> >> +        while ((p - cp) < MAX_COMPATIBLE_PROP) {
> >> +            int l;
> >> +            if (xc->dt_compatible[i] == NULL) {
> >> +                break;
> >> +            }
> >> +            l = strlen(xc->dt_compatible[i]);
> >> +            if (l >= (MAX_COMPATIBLE_PROP - i)) {
> >> +                break;
> >> +            }
> >> +            strcpy(p, xc->dt_compatible[i++]);
> >> +            p += l + 1;
> >> +        }
> >> +        _FDT((fdt_setprop(fdt, child_offset, "compatible", cp, p - cp)));
> > 
> > Might it be easier to just require the individual scom device to set
> > the compatible property from its ->devnode callback?  That way it can
> > just
> >     const char compat = "foo\0bar\0baz";
> >     fdt_setprop(..., compat, sizeof(compat));
> > 
> > and avoid this fiddling around with arrays.
> 
> Yes. xscom_populate_fdt() will shrink quite a bit. This is a good idea.
> 
> Thanks,
> 
> C.
> 
> >> +    }
> >> +
> >> +    return 0;
> >> +}
> >> +
> >> +static int xscom_qdev_init(DeviceState *qdev)
> >> +{
> >> +    XScomDevice *xdev = (XScomDevice *)qdev;
> >> +    XScomDeviceClass *xc = XSCOM_DEVICE_GET_CLASS(xdev);
> >> +
> >> +    if (xc->init) {
> >> +        return xc->init(xdev);
> >> +    }
> >> +    return 0;
> >> +}
> >> +
> >> +static void xscom_device_class_init(ObjectClass *klass, void *data)
> >> +{
> >> +    DeviceClass *k = DEVICE_CLASS(klass);
> >> +    k->init = xscom_qdev_init;
> >> +    k->bus_type = TYPE_XSCOM_BUS;
> >> +}
> >> +
> >> +static const TypeInfo xscom_dev_info = {
> >> +    .name = TYPE_XSCOM_DEVICE,
> >> +    .parent = TYPE_DEVICE,
> >> +    .instance_size = sizeof(XScomDevice),
> >> +    .abstract = true,
> >> +    .class_size = sizeof(XScomDeviceClass),
> >> +    .class_init = xscom_device_class_init,
> >> +};
> >> +
> >> +static void xscom_register_types(void)
> >> +{
> >> +    type_register_static(&xscom_info);
> >> +    type_register_static(&xscom_bus_info);
> >> +    type_register_static(&xscom_dev_info);
> >> +}
> >> +
> >> +type_init(xscom_register_types)
> >> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> >> index 1f32573dedff..bc6e1f80096b 100644
> >> --- a/include/hw/ppc/pnv.h
> >> +++ b/include/hw/ppc/pnv.h
> >> @@ -35,12 +35,14 @@ typedef enum PnvChipType {
> >>      PNV_CHIP_P8NVL, /* AKA Naples */
> >>  } PnvChipType;
> >>  
> >> +typedef struct XScomBus XScomBus;
> >>  typedef struct PnvChip {
> >>      /*< private >*/
> >>      SysBusDevice parent_obj;
> >>  
> >>      /*< public >*/
> >>      uint32_t     chip_id;
> >> +    XScomBus     *xscom;
> >>  } PnvChip;
> >>  
> >>  typedef struct PnvChipClass {
> >> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
> >> new file mode 100644
> >> index 000000000000..386ad21c5aa5
> >> --- /dev/null
> >> +++ b/include/hw/ppc/pnv_xscom.h
> >> @@ -0,0 +1,75 @@
> >> +#ifndef _HW_XSCOM_H
> >> +#define _HW_XSCOM_H
> >> +/*
> >> + * QEMU PowerNV XSCOM bus definitions
> >> + *
> >> + * Copyright (c) 2010 David Gibson <address@hidden>, IBM Corp.
> >> + * Based on the s390 virtio bus definitions:
> >> + * Copyright (c) 2009 Alexander Graf <address@hidden>
> >> + *
> >> + * 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 <hw/ppc/pnv.h>
> >> +
> >> +#define TYPE_XSCOM_DEVICE "xscom-device"
> >> +#define XSCOM_DEVICE(obj) \
> >> +     OBJECT_CHECK(XScomDevice, (obj), TYPE_XSCOM_DEVICE)
> >> +#define XSCOM_DEVICE_CLASS(klass) \
> >> +     OBJECT_CLASS_CHECK(XScomDeviceClass, (klass), TYPE_XSCOM_DEVICE)
> >> +#define XSCOM_DEVICE_GET_CLASS(obj) \
> >> +     OBJECT_GET_CLASS(XScomDeviceClass, (obj), TYPE_XSCOM_DEVICE)
> >> +
> >> +#define TYPE_XSCOM_BUS "xscom-bus"
> >> +#define XSCOM_BUS(obj) OBJECT_CHECK(XScomBus, (obj), TYPE_XSCOM_BUS)
> >> +
> >> +typedef struct XScomDevice XScomDevice;
> >> +typedef struct XScomBus XScomBus;
> >> +
> >> +typedef struct XScomDeviceClass {
> >> +    DeviceClass parent_class;
> >> +
> >> +    const char *dt_name;
> >> +    const char **dt_compatible;
> >> +    int (*init)(XScomDevice *dev);
> >> +    int (*devnode)(XScomDevice *dev, void *fdt, int offset);
> >> +
> >> +    /* Actual XScom accesses */
> >> +    bool (*read)(XScomDevice *dev, uint32_t range, uint32_t offset,
> >> +                 uint64_t *out_val);
> >> +    bool (*write)(XScomDevice *dev, uint32_t range, uint32_t offset,
> >> +                  uint64_t val);
> >> +} XScomDeviceClass;
> >> +
> >> +typedef struct XScomRange {
> >> +    uint32_t addr;
> >> +    uint32_t size;
> >> +} XScomRange;
> >> +
> >> +struct XScomDevice {
> >> +    DeviceState qdev;
> >> +#define MAX_XSCOM_RANGES 4
> >> +    struct XScomRange ranges[MAX_XSCOM_RANGES];
> >> +};
> >> +
> >> +struct XScomBus {
> >> +    BusState bus;
> >> +    uint32_t chip_id;
> >> +};
> >> +
> >> +extern XScomBus *xscom_create(PnvChip *chip);
> >> +extern int xscom_populate_fdt(XScomBus *xscom, void *fdt, int offset);
> >> +
> >> +
> >> +#endif /* _HW_XSCOM_H */
> > 
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature