[v4,06/20] ppc/pnv: add XSCOM infrastructure

On a real POWER8 system, the Pervasive Interconnect Bus (PIB) serves
as a backbone to connect different units of the system. The host
firmware connects to the PIB through a bridge unit, the
Alter-Display-Unit (ADU), which gives him access to all the chiplets
on the PCB network (Pervasive Connect Bus), the PIB acting as the root
of this network.

XSCOM (serial communication) is the interface to the sideband bus
provided by the POWER8 pervasive unit to read and write to chiplets
resources. This is needed by the host firmware, OPAL 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.

To represent the ADU of a real system, we introduce a specific
AddressSpace to dispatch XSCOM accesses to the targeted chiplets. The
translation of an XSCOM address into a PCB register address is
slightly different between the P9 and the P8. This is handled before
the dispatch using a 8byte alignment for all.

To customize the device tree, a QOM InterfaceClass, PnvXScomInterface,
is provided with a populate() handler. The chip populates the device
tree by simply looping on its children. Therefore, each model needing
custom nodes should not forget to declare itself as a child at
instantiation time.

Based on previous work done by :
      Benjamin Herrenschmidt <benh@kernel.crashing.org>

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes since v3:

 - reworked the model to dispatch addresses to the memory regions
   using pcb_addr << 3, which is a no-op for the P9. The benefit is
   that all the address translation work can be done before dispatch
   and the conversion handlers in the chip and in the xscom interface
   are gone.
   
 - removed the proxy PnnXscom object and extended the PnvChip object
   with an address space for XSCOM and its associated memory region.
   
 - changed the read/write handlers in the address space to use
   address_space_stq() and address_space_ldq()
   
 - introduced 'fake' default read/write handlers to handle 'core'
   registers. We can add a real device model when more work needs to
   be done under these.
   
 - fixed an issue with the monitor doing read/write in the XSCOM
   address space. When under the monitor, we don't have a cpu to
   update the HMER SPR. That might need more work in the long term.
   
 - introduced a xscom base field to hold the xscom base address as
   it is different on P9

 - renamed the devnode() handler to populate()

 Changes since v2:

 - QOMified the model.
 
 - all mappings in main memory space are now gathered in
   pnv_chip_realize() as done on other architectures.
   
 - removed XScomBus. The parenthood is established through the QOM
   model
   
 - replaced the XScomDevice with an InterfaceClass : PnvXScomInterface. 
 - introduced an XSCOM address space to dispatch accesses to the
   chiplets

 hw/ppc/Makefile.objs       |   2 +-
 hw/ppc/pnv.c               |  25 +++++
 hw/ppc/pnv_xscom.c         | 262 +++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/pnv.h       |  15 +++
 include/hw/ppc/pnv_xscom.h |  47 ++++++++
 5 files changed, 350 insertions(+), 1 deletion(-)
 create mode 100644 hw/ppc/pnv_xscom.c
 create mode 100644 include/hw/ppc/pnv_xscom.h

On Mon, Oct 03, 2016 at 09:24:42AM +0200, Cédric Le Goater wrote:
> On a real POWER8 system, the Pervasive Interconnect Bus (PIB) serves
> as a backbone to connect different units of the system. The host
> firmware connects to the PIB through a bridge unit, the
> Alter-Display-Unit (ADU), which gives him access to all the chiplets
> on the PCB network (Pervasive Connect Bus), the PIB acting as the root
> of this network.
> 
> XSCOM (serial communication) is the interface to the sideband bus
> provided by the POWER8 pervasive unit to read and write to chiplets
> resources. This is needed by the host firmware, OPAL 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.
> 
> To represent the ADU of a real system, we introduce a specific
> AddressSpace to dispatch XSCOM accesses to the targeted chiplets. The
> translation of an XSCOM address into a PCB register address is
> slightly different between the P9 and the P8. This is handled before
> the dispatch using a 8byte alignment for all.
> 
> To customize the device tree, a QOM InterfaceClass, PnvXScomInterface,
> is provided with a populate() handler. The chip populates the device
> tree by simply looping on its children. Therefore, each model needing
> custom nodes should not forget to declare itself as a child at
> instantiation time.
> 
> Based on previous work done by :
>       Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>  Changes since v3:
> 
>  - reworked the model to dispatch addresses to the memory regions
>    using pcb_addr << 3, which is a no-op for the P9. The benefit is
>    that all the address translation work can be done before dispatch
>    and the conversion handlers in the chip and in the xscom interface
>    are gone.
>    
>  - removed the proxy PnnXscom object and extended the PnvChip object
>    with an address space for XSCOM and its associated memory region.
>    
>  - changed the read/write handlers in the address space to use
>    address_space_stq() and address_space_ldq()
>    
>  - introduced 'fake' default read/write handlers to handle 'core'
>    registers. We can add a real device model when more work needs to
>    be done under these.
>    
>  - fixed an issue with the monitor doing read/write in the XSCOM
>    address space. When under the monitor, we don't have a cpu to
>    update the HMER SPR. That might need more work in the long term.
>    
>  - introduced a xscom base field to hold the xscom base address as
>    it is different on P9
> 
>  - renamed the devnode() handler to populate()
> 
>  Changes since v2:
> 
>  - QOMified the model.
>  
>  - all mappings in main memory space are now gathered in
>    pnv_chip_realize() as done on other architectures.
>    
>  - removed XScomBus. The parenthood is established through the QOM
>    model
>    
>  - replaced the XScomDevice with an InterfaceClass : PnvXScomInterface. 
>  - introduced an XSCOM address space to dispatch accesses to the
>    chiplets
> 
>  hw/ppc/Makefile.objs       |   2 +-
>  hw/ppc/pnv.c               |  25 +++++
>  hw/ppc/pnv_xscom.c         | 262 +++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/pnv.h       |  15 +++
>  include/hw/ppc/pnv_xscom.h |  47 ++++++++
>  5 files changed, 350 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 f8c7d1db9ade..08c213c40684 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 pnv_core.o
> +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.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 2376bb222918..5e19b6880387 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -32,6 +32,8 @@
>  #include "exec/address-spaces.h"
>  #include "qemu/cutils.h"
>  
> +#include "hw/ppc/pnv_xscom.h"
> +
>  #include <libfdt.h>
>  
>  #define FDT_MAX_SIZE            0x00100000
> @@ -218,6 +220,8 @@ static void powernv_populate_chip(PnvChip *chip, void *fdt)
>      size_t typesize = object_type_get_instance_size(typename);
>      int i;
>  
> +    pnv_xscom_populate(chip, fdt, 0);
> +
>      for (i = 0; i < chip->nr_cores; i++) {
>          PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize);
>  
> @@ -450,6 +454,7 @@ static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
>      k->chip_cfam_id = 0x221ef04980000000ull;  /* P8 Murano DD2.1 */
>      k->cores_mask = POWER8E_CORE_MASK;
>      k->core_pir = pnv_chip_core_pir_p8;
> +    k->xscom_base = 0x003fc0000000000ull;
>      dc->desc = "PowerNV Chip POWER8E";
>  }
>  
> @@ -470,6 +475,7 @@ static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
>      k->chip_cfam_id = 0x220ea04980000000ull; /* P8 Venice DD2.0 */
>      k->cores_mask = POWER8_CORE_MASK;
>      k->core_pir = pnv_chip_core_pir_p8;
> +    k->xscom_base = 0x003fc0000000000ull;
>      dc->desc = "PowerNV Chip POWER8";
>  }
>  
> @@ -490,6 +496,7 @@ static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
>      k->chip_cfam_id = 0x120d304980000000ull;  /* P8 Naples DD1.0 */
>      k->cores_mask = POWER8_CORE_MASK;
>      k->core_pir = pnv_chip_core_pir_p8;
> +    k->xscom_base = 0x003fc0000000000ull;
>      dc->desc = "PowerNV Chip POWER8NVL";
>  }
>  
> @@ -510,6 +517,7 @@ static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
>      k->chip_cfam_id = 0x100d104980000000ull; /* P9 Nimbus DD1.0 */
>      k->cores_mask = POWER9_CORE_MASK;
>      k->core_pir = pnv_chip_core_pir_p9;
> +    k->xscom_base = 0x00603fc00000000ull;
>      dc->desc = "PowerNV Chip POWER9";
>  }
>  
> @@ -549,6 +557,14 @@ static void pnv_chip_core_sanitize(PnvChip *chip, Error **errp)
>      }
>  }
>  
> +static void pnv_chip_init(Object *obj)
> +{
> +    PnvChip *chip = PNV_CHIP(obj);
> +    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> +
> +    chip->xscom_base = pcc->xscom_base;

Is there any reason you really need the per-isntance xscom_base as
well as the per-class xscom_base?

If you do have a per-instance value, it seems like you should fold in
the calculation from PNV_XSCOM_BASE() here.

> +}
> +
>  static void pnv_chip_realize(DeviceState *dev, Error **errp)
>  {
>      PnvChip *chip = PNV_CHIP(dev);
> @@ -563,6 +579,14 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>  
> +    /* XSCOM bridge */
> +    pnv_xscom_realize(chip, &error);
> +    if (error) {
> +        error_propagate(errp, error);
> +        return;
> +    }
> +    sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV_XSCOM_BASE(chip));
> +
>      /* Early checks on the core settings */
>      pnv_chip_core_sanitize(chip, &error);
>      if (error) {
> @@ -620,6 +644,7 @@ static const TypeInfo pnv_chip_info = {
>      .name          = TYPE_PNV_CHIP,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .class_init    = pnv_chip_class_init,
> +    .instance_init = pnv_chip_init,
>      .class_size    = sizeof(PnvChipClass),
>      .abstract      = true,
>  };
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> new file mode 100644
> index 000000000000..ce1182d9a13e
> --- /dev/null
> +++ b/hw/ppc/pnv_xscom.c
> @@ -0,0 +1,262 @@
> +/*
> + * QEMU PowerPC PowerNV XSCOM bus
> + *
> + * 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 "qapi/error.h"
> +#include "hw/hw.h"
> +#include "qemu/log.h"
> +#include "sysemu/kvm.h"
> +#include "target-ppc/cpu.h"
> +#include "hw/sysbus.h"
> +
> +#include "hw/ppc/fdt.h"
> +#include "hw/ppc/pnv_xscom.h"
> +#include "hw/ppc/pnv.h"
> +
> +#include <libfdt.h>
> +
> +static void xscom_complete(uint64_t hmer_bits)

I think this should take a cpu parameter instead of assuming
current_cpu.  Of course, the callers will probably just use
current_cpu, but in the callers it's a bit more obvious that
current_cpu is actually the right thing.

> +{
> +    CPUState *cs = current_cpu;
> +
> +    /*
> +     * TODO: When the read/write comes from the monitor, we do not
> +     * have a cpu.
> +     */
> +    if (cs) {
> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
> +        CPUPPCState *env = &cpu->env;
> +
> +        /*
> +         * TODO: Need a CPU helper to set HMER, also handle generation
> +         * of HMIs
> +         */
> +        cpu_synchronize_state(cs);
> +        env->spr[SPR_HMER] |= hmer_bits;
> +    }
> +}
> +
> +static uint32_t pnv_xscom_pcba(PnvChip *chip, uint64_t addr)
> +{
> +    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> +
> +    addr &= (PNV_XSCOM_SIZE - 1);
> +    if (pcc->chip_type == PNV_CHIP_POWER9) {
> +        return addr >> 3;
> +    } else {
> +        return ((addr >> 4) & ~0xfull) | ((addr >> 3) & 0xf);
> +    }
> +}
> +
> +static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
> +{
> +    switch (pcba) {
> +    case 0xf000f:
> +        return PNV_CHIP_GET_CLASS(chip)->chip_cfam_id;
> +    case 0x1010c00:     /* PIBAM FIR */
> +    case 0x1010c03:     /* PIBAM FIR MASK */
> +    case 0x2020007:     /* ADU stuff */
> +    case 0x2020009:     /* ADU stuff */
> +    case 0x202000f:     /* ADU stuff */
> +        return 0;
> +    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 */
> +        return 0;
> +    case 0x2013028:     /* CAPP stuff */
> +    case 0x201302a:     /* CAPP stuff */
> +    case 0x2013801:     /* CAPP stuff */
> +    case 0x2013802:     /* CAPP stuff */
> +        return 0;
> +    default:
> +        return -1;
> +    }
> +}
> +
> +static bool xscom_write_default(PnvChip *chip, uint32_t pcba, uint64_t val)
> +{
> +    /* We ignore writes to these */
> +    switch (pcba) {
> +    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 */
> +        return true;
> +    default:
> +        return false;
> +    }
> +}
> +
> +static uint64_t xscom_read(void *opaque, hwaddr addr, unsigned width)
> +{
> +    PnvChip *chip = opaque;
> +    uint32_t pcba = pnv_xscom_pcba(chip, addr);
> +    uint64_t val = 0;
> +    MemTxResult result;
> +
> +    /* Handle some SCOMs here before dispatch */
> +    val = xscom_read_default(chip, pcba);
> +    if (val != -1) {
> +        goto complete;
> +    }
> +
> +    val = address_space_ldq(&chip->xscom_as, pcba << 3, MEMTXATTRS_UNSPECIFIED,
> +                            &result);
> +    if (result != MEMTX_OK) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "XSCOM read failed at @0x%"
> +                      HWADDR_PRIx " pcba=0x%08x\n", addr, pcba);
> +        xscom_complete(HMER_XSCOM_FAIL | HMER_XSCOM_DONE);
> +        return 0;
> +    }
> +
> +complete:
> +    xscom_complete(HMER_XSCOM_DONE);
> +    return val;
> +}
> +
> +static void xscom_write(void *opaque, hwaddr addr, uint64_t val,
> +                        unsigned width)
> +{
> +    PnvChip *chip = opaque;
> +    uint32_t pcba = pnv_xscom_pcba(chip, addr);
> +    MemTxResult result;
> +
> +    /* Handle some SCOMs here before dispatch */
> +    if (xscom_write_default(chip, pcba, val)) {
> +        goto complete;
> +    }
> +
> +    address_space_stq(&chip->xscom_as, pcba << 3, val, MEMTXATTRS_UNSPECIFIED,
> +                      &result);
> +    if (result != MEMTX_OK) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "XSCOM write failed at @0x%"
> +                      HWADDR_PRIx " pcba=0x%08x data=0x%" PRIx64 "\n",
> +                      addr, pcba, val);
> +        xscom_complete(HMER_XSCOM_FAIL | HMER_XSCOM_DONE);
> +        return;
> +    }
> +
> +complete:
> +    xscom_complete(HMER_XSCOM_DONE);
> +}
> +
> +const MemoryRegionOps pnv_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,
> +};
> +
> +void pnv_xscom_realize(PnvChip *chip, Error **errp)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
> +    char *name;
> +
> +    name = g_strdup_printf("xscom-%x", chip->chip_id);
> +    memory_region_init_io(&chip->xscom_mmio, OBJECT(chip), &pnv_xscom_ops,
> +                          chip, name, PNV_XSCOM_SIZE);
> +    sysbus_init_mmio(sbd, &chip->xscom_mmio);
> +
> +    memory_region_init(&chip->xscom, OBJECT(chip), name, PNV_XSCOM_SIZE);
> +    address_space_init(&chip->xscom_as, &chip->xscom, name);
> +    g_free(name);
> +}
> +
> +static const TypeInfo pnv_xscom_interface_info = {
> +    .name = TYPE_PNV_XSCOM_INTERFACE,
> +    .parent = TYPE_INTERFACE,
> +    .class_size = sizeof(PnvXScomInterfaceClass),
> +};
> +
> +static void pnv_xscom_register_types(void)
> +{
> +    type_register_static(&pnv_xscom_interface_info);
> +}
> +
> +type_init(pnv_xscom_register_types)
> +
> +typedef struct ForeachPopulateArgs {
> +    void *fdt;
> +    int xscom_offset;
> +} ForeachPopulateArgs;
> +
> +static int xscom_populate_child(Object *child, void *opaque)
> +{
> +    if (object_dynamic_cast(child, TYPE_PNV_XSCOM_INTERFACE)) {
> +        ForeachPopulateArgs *args = opaque;
> +        PnvXScomInterface *xd = PNV_XSCOM_INTERFACE(child);
> +        PnvXScomInterfaceClass *xc = PNV_XSCOM_INTERFACE_GET_CLASS(xd);
> +
> +        if (xc->populate) {
> +            _FDT((xc->populate(xd, args->fdt, args->xscom_offset)));
> +        }
> +    }
> +    return 0;
> +}
> +
> +static const char compat_p8[] = "ibm,power8-xscom\0ibm,xscom";
> +static const char compat_p9[] = "ibm,power9-xscom\0ibm,xscom";
> +
> +int pnv_xscom_populate(PnvChip *chip, void *fdt, int root_offset)
> +{
> +    uint64_t reg[] = { cpu_to_be64(PNV_XSCOM_BASE(chip)),
> +                       cpu_to_be64(PNV_XSCOM_SIZE) };
> +    int xscom_offset;
> +    ForeachPopulateArgs args;
> +    char *name;
> +    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> +
> +    name = g_strdup_printf("xscom@%" PRIx64, be64_to_cpu(reg[0]));
> +    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", chip->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))));
> +
> +    if (pcc->chip_type == PNV_CHIP_POWER9) {
> +        _FDT((fdt_setprop(fdt, xscom_offset, "compatible", compat_p9,
> +                          sizeof(compat_p9))));
> +    } else {
> +        _FDT((fdt_setprop(fdt, xscom_offset, "compatible", compat_p8,
> +                          sizeof(compat_p8))));
> +    }
> +
> +    _FDT((fdt_setprop(fdt, xscom_offset, "scom-controller", NULL, 0)));
> +
> +    args.fdt = fdt;
> +    args.xscom_offset = xscom_offset;
> +
> +    object_child_foreach(OBJECT(chip), xscom_populate_child, &args);
> +    return 0;
> +}
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index ed4a360cde3b..f89eddb6e5e0 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -21,6 +21,7 @@
>  
>  #include "hw/boards.h"
>  #include "hw/sysbus.h"
> +#include "hw/ppc/pnv_xscom.h"
>  
>  #define TYPE_PNV_CHIP "powernv-chip"
>  #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP)
> @@ -43,6 +44,11 @@ typedef struct PnvChip {
>      /*< public >*/
>      uint32_t     chip_id;
>  
> +    hwaddr       xscom_base;
> +    MemoryRegion xscom_mmio;
> +    MemoryRegion xscom;
> +    AddressSpace xscom_as;
> +
>      uint32_t  nr_cores;
>      uint64_t  cores_mask;
>      void      *cores;
> @@ -58,6 +64,8 @@ typedef struct PnvChipClass {
>      uint64_t     chip_cfam_id;
>      uint64_t     cores_mask;
>  
> +    hwaddr       xscom_base;
> +
>      uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
>  } PnvChipClass;
>  
> @@ -105,4 +113,11 @@ typedef struct PnvMachineState {
>  
>  #define PNV_TIMEBASE_FREQ           512000000ULL
>  
> +/*
> + * POWER8 MMIO base addresses
> + */
> +#define PNV_XSCOM_SIZE        0x800000000ull
> +#define PNV_XSCOM_BASE(chip)                                            \
> +    (chip->xscom_base + ((uint64_t)(chip)->chip_id) * PNV_XSCOM_SIZE)
> +
>  #endif /* _PPC_PNV_H */
> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
> new file mode 100644
> index 000000000000..f50eb0bc4099
> --- /dev/null
> +++ b/include/hw/ppc/pnv_xscom.h
> @@ -0,0 +1,47 @@
> +/*
> + * QEMU PowerPC PowerNV XSCOM bus definitions
> + *
> + * 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_XSCOM_H
> +#define _PPC_PNV_XSCOM_H
> +
> +#include "qom/object.h"
> +
> +typedef struct PnvChip PnvChip;
> +
> +typedef struct PnvXScomInterface {
> +    Object parent;
> +} PnvXScomInterface;
> +
> +#define TYPE_PNV_XSCOM_INTERFACE "pnv-xscom-interface"
> +#define PNV_XSCOM_INTERFACE(obj) \
> +     OBJECT_CHECK(PnvXScomInterface, (obj), TYPE_PNV_XSCOM_INTERFACE)
> +#define PNV_XSCOM_INTERFACE_CLASS(klass)                \
> +    OBJECT_CLASS_CHECK(PnvXScomInterfaceClass, (klass), \
> +                       TYPE_PNV_XSCOM_INTERFACE)
> +#define PNV_XSCOM_INTERFACE_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(PnvXScomInterfaceClass, (obj), TYPE_PNV_XSCOM_INTERFACE)
> +
> +typedef struct PnvXScomInterfaceClass {
> +    InterfaceClass parent;
> +    int (*populate)(PnvXScomInterface *dev, void *fdt, int offset);
> +} PnvXScomInterfaceClass;
> +
> +extern void pnv_xscom_realize(PnvChip *chip, Error **errp);
> +extern int pnv_xscom_populate(PnvChip *chip, void *fdt, int offset);
> +
> +#endif /* _PPC_PNV_XSCOM_H */

On 10/13/2016 02:41 AM, David Gibson wrote:
> On Mon, Oct 03, 2016 at 09:24:42AM +0200, Cédric Le Goater wrote:
>> On a real POWER8 system, the Pervasive Interconnect Bus (PIB) serves
>> as a backbone to connect different units of the system. The host
>> firmware connects to the PIB through a bridge unit, the
>> Alter-Display-Unit (ADU), which gives him access to all the chiplets
>> on the PCB network (Pervasive Connect Bus), the PIB acting as the root
>> of this network.
>>
>> XSCOM (serial communication) is the interface to the sideband bus
>> provided by the POWER8 pervasive unit to read and write to chiplets
>> resources. This is needed by the host firmware, OPAL 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.
>>
>> To represent the ADU of a real system, we introduce a specific
>> AddressSpace to dispatch XSCOM accesses to the targeted chiplets. The
>> translation of an XSCOM address into a PCB register address is
>> slightly different between the P9 and the P8. This is handled before
>> the dispatch using a 8byte alignment for all.
>>
>> To customize the device tree, a QOM InterfaceClass, PnvXScomInterface,
>> is provided with a populate() handler. The chip populates the device
>> tree by simply looping on its children. Therefore, each model needing
>> custom nodes should not forget to declare itself as a child at
>> instantiation time.
>>
>> Based on previous work done by :
>>       Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>>  Changes since v3:
>>
>>  - reworked the model to dispatch addresses to the memory regions
>>    using pcb_addr << 3, which is a no-op for the P9. The benefit is
>>    that all the address translation work can be done before dispatch
>>    and the conversion handlers in the chip and in the xscom interface
>>    are gone.
>>    
>>  - removed the proxy PnnXscom object and extended the PnvChip object
>>    with an address space for XSCOM and its associated memory region.
>>    
>>  - changed the read/write handlers in the address space to use
>>    address_space_stq() and address_space_ldq()
>>    
>>  - introduced 'fake' default read/write handlers to handle 'core'
>>    registers. We can add a real device model when more work needs to
>>    be done under these.
>>    
>>  - fixed an issue with the monitor doing read/write in the XSCOM
>>    address space. When under the monitor, we don't have a cpu to
>>    update the HMER SPR. That might need more work in the long term.
>>    
>>  - introduced a xscom base field to hold the xscom base address as
>>    it is different on P9
>>
>>  - renamed the devnode() handler to populate()
>>
>>  Changes since v2:
>>
>>  - QOMified the model.
>>  
>>  - all mappings in main memory space are now gathered in
>>    pnv_chip_realize() as done on other architectures.
>>    
>>  - removed XScomBus. The parenthood is established through the QOM
>>    model
>>    
>>  - replaced the XScomDevice with an InterfaceClass : PnvXScomInterface. 
>>  - introduced an XSCOM address space to dispatch accesses to the
>>    chiplets
>>
>>  hw/ppc/Makefile.objs       |   2 +-
>>  hw/ppc/pnv.c               |  25 +++++
>>  hw/ppc/pnv_xscom.c         | 262 +++++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/ppc/pnv.h       |  15 +++
>>  include/hw/ppc/pnv_xscom.h |  47 ++++++++
>>  5 files changed, 350 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 f8c7d1db9ade..08c213c40684 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 pnv_core.o
>> +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.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 2376bb222918..5e19b6880387 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -32,6 +32,8 @@
>>  #include "exec/address-spaces.h"
>>  #include "qemu/cutils.h"
>>  
>> +#include "hw/ppc/pnv_xscom.h"
>> +
>>  #include <libfdt.h>
>>  
>>  #define FDT_MAX_SIZE            0x00100000
>> @@ -218,6 +220,8 @@ static void powernv_populate_chip(PnvChip *chip, void *fdt)
>>      size_t typesize = object_type_get_instance_size(typename);
>>      int i;
>>  
>> +    pnv_xscom_populate(chip, fdt, 0);
>> +
>>      for (i = 0; i < chip->nr_cores; i++) {
>>          PnvCore *pnv_core = PNV_CORE(chip->cores + i * typesize);
>>  
>> @@ -450,6 +454,7 @@ static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
>>      k->chip_cfam_id = 0x221ef04980000000ull;  /* P8 Murano DD2.1 */
>>      k->cores_mask = POWER8E_CORE_MASK;
>>      k->core_pir = pnv_chip_core_pir_p8;
>> +    k->xscom_base = 0x003fc0000000000ull;
>>      dc->desc = "PowerNV Chip POWER8E";
>>  }
>>  
>> @@ -470,6 +475,7 @@ static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
>>      k->chip_cfam_id = 0x220ea04980000000ull; /* P8 Venice DD2.0 */
>>      k->cores_mask = POWER8_CORE_MASK;
>>      k->core_pir = pnv_chip_core_pir_p8;
>> +    k->xscom_base = 0x003fc0000000000ull;
>>      dc->desc = "PowerNV Chip POWER8";
>>  }
>>  
>> @@ -490,6 +496,7 @@ static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
>>      k->chip_cfam_id = 0x120d304980000000ull;  /* P8 Naples DD1.0 */
>>      k->cores_mask = POWER8_CORE_MASK;
>>      k->core_pir = pnv_chip_core_pir_p8;
>> +    k->xscom_base = 0x003fc0000000000ull;
>>      dc->desc = "PowerNV Chip POWER8NVL";
>>  }
>>  
>> @@ -510,6 +517,7 @@ static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
>>      k->chip_cfam_id = 0x100d104980000000ull; /* P9 Nimbus DD1.0 */
>>      k->cores_mask = POWER9_CORE_MASK;
>>      k->core_pir = pnv_chip_core_pir_p9;
>> +    k->xscom_base = 0x00603fc00000000ull;
>>      dc->desc = "PowerNV Chip POWER9";
>>  }
>>  
>> @@ -549,6 +557,14 @@ static void pnv_chip_core_sanitize(PnvChip *chip, Error **errp)
>>      }
>>  }
>>  
>> +static void pnv_chip_init(Object *obj)
>> +{
>> +    PnvChip *chip = PNV_CHIP(obj);
>> +    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>> +
>> +    chip->xscom_base = pcc->xscom_base;
> 
> Is there any reason you really need the per-isntance xscom_base as
> well as the per-class xscom_base?

The per-instance xscom_base is not strictly necessary. It is a chip 
constant relative to the chip id. So I need the chip to calculate it.

> If you do have a per-instance value, it seems like you should fold in
> the calculation from PNV_XSCOM_BASE() here.

But I can make it a little cleaner. We need the value in pnv_chip_realize()
and pnv_xscom_populate() to populate the device tree. something like 
that would do :

 ( PNV_CHIP_GET_CLASS(chip)->xscom_base + ((uint64_t)(chip)->chip_id) * PNV_XSCOM_SIZE)

I want to add a mmio_base because the ICP BAR and the PSIHB BAR depend 
on it. PHB will also and the Centaur if we model this chip one day.

>> +}
>> +
>>  static void pnv_chip_realize(DeviceState *dev, Error **errp)
>>  {
>>      PnvChip *chip = PNV_CHIP(dev);
>> @@ -563,6 +579,14 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
>>          return;
>>      }
>>  
>> +    /* XSCOM bridge */
>> +    pnv_xscom_realize(chip, &error);
>> +    if (error) {
>> +        error_propagate(errp, error);
>> +        return;
>> +    }
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV_XSCOM_BASE(chip));
>> +
>>      /* Early checks on the core settings */
>>      pnv_chip_core_sanitize(chip, &error);
>>      if (error) {
>> @@ -620,6 +644,7 @@ static const TypeInfo pnv_chip_info = {
>>      .name          = TYPE_PNV_CHIP,
>>      .parent        = TYPE_SYS_BUS_DEVICE,
>>      .class_init    = pnv_chip_class_init,
>> +    .instance_init = pnv_chip_init,
>>      .class_size    = sizeof(PnvChipClass),
>>      .abstract      = true,
>>  };
>> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
>> new file mode 100644
>> index 000000000000..ce1182d9a13e
>> --- /dev/null
>> +++ b/hw/ppc/pnv_xscom.c
>> @@ -0,0 +1,262 @@
>> +/*
>> + * QEMU PowerPC PowerNV XSCOM bus
>> + *
>> + * 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 "qapi/error.h"
>> +#include "hw/hw.h"
>> +#include "qemu/log.h"
>> +#include "sysemu/kvm.h"
>> +#include "target-ppc/cpu.h"
>> +#include "hw/sysbus.h"
>> +
>> +#include "hw/ppc/fdt.h"
>> +#include "hw/ppc/pnv_xscom.h"
>> +#include "hw/ppc/pnv.h"
>> +
>> +#include <libfdt.h>
>> +
>> +static void xscom_complete(uint64_t hmer_bits)
> 
> I think this should take a cpu parameter instead of assuming
> current_cpu.  Of course, the callers will probably just use
> current_cpu, but in the callers it's a bit more obvious that
> current_cpu is actually the right thing.

Yes. the 'if (cs)' statement below is good argument for what you
say.

Thanks,

C.


>> +{
>> +    CPUState *cs = current_cpu;
>> +
>> +    /*
>> +     * TODO: When the read/write comes from the monitor, we do not
>> +     * have a cpu.
>> +     */
>> +    if (cs) {
>> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
>> +        CPUPPCState *env = &cpu->env;
>> +
>> +        /*
>> +         * TODO: Need a CPU helper to set HMER, also handle generation
>> +         * of HMIs
>> +         */
>> +        cpu_synchronize_state(cs);
>> +        env->spr[SPR_HMER] |= hmer_bits;
>> +    }
>> +}
>> +
>> +static uint32_t pnv_xscom_pcba(PnvChip *chip, uint64_t addr)
>> +{
>> +    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>> +
>> +    addr &= (PNV_XSCOM_SIZE - 1);
>> +    if (pcc->chip_type == PNV_CHIP_POWER9) {
>> +        return addr >> 3;
>> +    } else {
>> +        return ((addr >> 4) & ~0xfull) | ((addr >> 3) & 0xf);
>> +    }
>> +}
>> +
>> +static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
>> +{
>> +    switch (pcba) {
>> +    case 0xf000f:
>> +        return PNV_CHIP_GET_CLASS(chip)->chip_cfam_id;
>> +    case 0x1010c00:     /* PIBAM FIR */
>> +    case 0x1010c03:     /* PIBAM FIR MASK */
>> +    case 0x2020007:     /* ADU stuff */
>> +    case 0x2020009:     /* ADU stuff */
>> +    case 0x202000f:     /* ADU stuff */
>> +        return 0;
>> +    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 */
>> +        return 0;
>> +    case 0x2013028:     /* CAPP stuff */
>> +    case 0x201302a:     /* CAPP stuff */
>> +    case 0x2013801:     /* CAPP stuff */
>> +    case 0x2013802:     /* CAPP stuff */
>> +        return 0;
>> +    default:
>> +        return -1;
>> +    }
>> +}
>> +
>> +static bool xscom_write_default(PnvChip *chip, uint32_t pcba, uint64_t val)
>> +{
>> +    /* We ignore writes to these */
>> +    switch (pcba) {
>> +    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 */
>> +        return true;
>> +    default:
>> +        return false;
>> +    }
>> +}
>> +
>> +static uint64_t xscom_read(void *opaque, hwaddr addr, unsigned width)
>> +{
>> +    PnvChip *chip = opaque;
>> +    uint32_t pcba = pnv_xscom_pcba(chip, addr);
>> +    uint64_t val = 0;
>> +    MemTxResult result;
>> +
>> +    /* Handle some SCOMs here before dispatch */
>> +    val = xscom_read_default(chip, pcba);
>> +    if (val != -1) {
>> +        goto complete;
>> +    }
>> +
>> +    val = address_space_ldq(&chip->xscom_as, pcba << 3, MEMTXATTRS_UNSPECIFIED,
>> +                            &result);
>> +    if (result != MEMTX_OK) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "XSCOM read failed at @0x%"
>> +                      HWADDR_PRIx " pcba=0x%08x\n", addr, pcba);
>> +        xscom_complete(HMER_XSCOM_FAIL | HMER_XSCOM_DONE);
>> +        return 0;
>> +    }
>> +
>> +complete:
>> +    xscom_complete(HMER_XSCOM_DONE);
>> +    return val;
>> +}
>> +
>> +static void xscom_write(void *opaque, hwaddr addr, uint64_t val,
>> +                        unsigned width)
>> +{
>> +    PnvChip *chip = opaque;
>> +    uint32_t pcba = pnv_xscom_pcba(chip, addr);
>> +    MemTxResult result;
>> +
>> +    /* Handle some SCOMs here before dispatch */
>> +    if (xscom_write_default(chip, pcba, val)) {
>> +        goto complete;
>> +    }
>> +
>> +    address_space_stq(&chip->xscom_as, pcba << 3, val, MEMTXATTRS_UNSPECIFIED,
>> +                      &result);
>> +    if (result != MEMTX_OK) {
>> +        qemu_log_mask(LOG_GUEST_ERROR, "XSCOM write failed at @0x%"
>> +                      HWADDR_PRIx " pcba=0x%08x data=0x%" PRIx64 "\n",
>> +                      addr, pcba, val);
>> +        xscom_complete(HMER_XSCOM_FAIL | HMER_XSCOM_DONE);
>> +        return;
>> +    }
>> +
>> +complete:
>> +    xscom_complete(HMER_XSCOM_DONE);
>> +}
>> +
>> +const MemoryRegionOps pnv_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,
>> +};
>> +
>> +void pnv_xscom_realize(PnvChip *chip, Error **errp)
>> +{
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
>> +    char *name;
>> +
>> +    name = g_strdup_printf("xscom-%x", chip->chip_id);
>> +    memory_region_init_io(&chip->xscom_mmio, OBJECT(chip), &pnv_xscom_ops,
>> +                          chip, name, PNV_XSCOM_SIZE);
>> +    sysbus_init_mmio(sbd, &chip->xscom_mmio);
>> +
>> +    memory_region_init(&chip->xscom, OBJECT(chip), name, PNV_XSCOM_SIZE);
>> +    address_space_init(&chip->xscom_as, &chip->xscom, name);
>> +    g_free(name);
>> +}
>> +
>> +static const TypeInfo pnv_xscom_interface_info = {
>> +    .name = TYPE_PNV_XSCOM_INTERFACE,
>> +    .parent = TYPE_INTERFACE,
>> +    .class_size = sizeof(PnvXScomInterfaceClass),
>> +};
>> +
>> +static void pnv_xscom_register_types(void)
>> +{
>> +    type_register_static(&pnv_xscom_interface_info);
>> +}
>> +
>> +type_init(pnv_xscom_register_types)
>> +
>> +typedef struct ForeachPopulateArgs {
>> +    void *fdt;
>> +    int xscom_offset;
>> +} ForeachPopulateArgs;
>> +
>> +static int xscom_populate_child(Object *child, void *opaque)
>> +{
>> +    if (object_dynamic_cast(child, TYPE_PNV_XSCOM_INTERFACE)) {
>> +        ForeachPopulateArgs *args = opaque;
>> +        PnvXScomInterface *xd = PNV_XSCOM_INTERFACE(child);
>> +        PnvXScomInterfaceClass *xc = PNV_XSCOM_INTERFACE_GET_CLASS(xd);
>> +
>> +        if (xc->populate) {
>> +            _FDT((xc->populate(xd, args->fdt, args->xscom_offset)));
>> +        }
>> +    }
>> +    return 0;
>> +}
>> +
>> +static const char compat_p8[] = "ibm,power8-xscom\0ibm,xscom";
>> +static const char compat_p9[] = "ibm,power9-xscom\0ibm,xscom";
>> +
>> +int pnv_xscom_populate(PnvChip *chip, void *fdt, int root_offset)
>> +{
>> +    uint64_t reg[] = { cpu_to_be64(PNV_XSCOM_BASE(chip)),
>> +                       cpu_to_be64(PNV_XSCOM_SIZE) };
>> +    int xscom_offset;
>> +    ForeachPopulateArgs args;
>> +    char *name;
>> +    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>> +
>> +    name = g_strdup_printf("xscom@%" PRIx64, be64_to_cpu(reg[0]));
>> +    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", chip->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))));
>> +
>> +    if (pcc->chip_type == PNV_CHIP_POWER9) {
>> +        _FDT((fdt_setprop(fdt, xscom_offset, "compatible", compat_p9,
>> +                          sizeof(compat_p9))));
>> +    } else {
>> +        _FDT((fdt_setprop(fdt, xscom_offset, "compatible", compat_p8,
>> +                          sizeof(compat_p8))));
>> +    }
>> +
>> +    _FDT((fdt_setprop(fdt, xscom_offset, "scom-controller", NULL, 0)));
>> +
>> +    args.fdt = fdt;
>> +    args.xscom_offset = xscom_offset;
>> +
>> +    object_child_foreach(OBJECT(chip), xscom_populate_child, &args);
>> +    return 0;
>> +}
>> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
>> index ed4a360cde3b..f89eddb6e5e0 100644
>> --- a/include/hw/ppc/pnv.h
>> +++ b/include/hw/ppc/pnv.h
>> @@ -21,6 +21,7 @@
>>  
>>  #include "hw/boards.h"
>>  #include "hw/sysbus.h"
>> +#include "hw/ppc/pnv_xscom.h"
>>  
>>  #define TYPE_PNV_CHIP "powernv-chip"
>>  #define PNV_CHIP(obj) OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP)
>> @@ -43,6 +44,11 @@ typedef struct PnvChip {
>>      /*< public >*/
>>      uint32_t     chip_id;
>>  
>> +    hwaddr       xscom_base;
>> +    MemoryRegion xscom_mmio;
>> +    MemoryRegion xscom;
>> +    AddressSpace xscom_as;
>> +
>>      uint32_t  nr_cores;
>>      uint64_t  cores_mask;
>>      void      *cores;
>> @@ -58,6 +64,8 @@ typedef struct PnvChipClass {
>>      uint64_t     chip_cfam_id;
>>      uint64_t     cores_mask;
>>  
>> +    hwaddr       xscom_base;
>> +
>>      uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
>>  } PnvChipClass;
>>  
>> @@ -105,4 +113,11 @@ typedef struct PnvMachineState {
>>  
>>  #define PNV_TIMEBASE_FREQ           512000000ULL
>>  
>> +/*
>> + * POWER8 MMIO base addresses
>> + */
>> +#define PNV_XSCOM_SIZE        0x800000000ull
>> +#define PNV_XSCOM_BASE(chip)                                            \
>> +    (chip->xscom_base + ((uint64_t)(chip)->chip_id) * PNV_XSCOM_SIZE)
>> +
>>  #endif /* _PPC_PNV_H */
>> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
>> new file mode 100644
>> index 000000000000..f50eb0bc4099
>> --- /dev/null
>> +++ b/include/hw/ppc/pnv_xscom.h
>> @@ -0,0 +1,47 @@
>> +/*
>> + * QEMU PowerPC PowerNV XSCOM bus definitions
>> + *
>> + * 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_XSCOM_H
>> +#define _PPC_PNV_XSCOM_H
>> +
>> +#include "qom/object.h"
>> +
>> +typedef struct PnvChip PnvChip;
>> +
>> +typedef struct PnvXScomInterface {
>> +    Object parent;
>> +} PnvXScomInterface;
>> +
>> +#define TYPE_PNV_XSCOM_INTERFACE "pnv-xscom-interface"
>> +#define PNV_XSCOM_INTERFACE(obj) \
>> +     OBJECT_CHECK(PnvXScomInterface, (obj), TYPE_PNV_XSCOM_INTERFACE)
>> +#define PNV_XSCOM_INTERFACE_CLASS(klass)                \
>> +    OBJECT_CLASS_CHECK(PnvXScomInterfaceClass, (klass), \
>> +                       TYPE_PNV_XSCOM_INTERFACE)
>> +#define PNV_XSCOM_INTERFACE_GET_CLASS(obj) \
>> +     OBJECT_GET_CLASS(PnvXScomInterfaceClass, (obj), TYPE_PNV_XSCOM_INTERFACE)
>> +
>> +typedef struct PnvXScomInterfaceClass {
>> +    InterfaceClass parent;
>> +    int (*populate)(PnvXScomInterface *dev, void *fdt, int offset);
>> +} PnvXScomInterfaceClass;
>> +
>> +extern void pnv_xscom_realize(PnvChip *chip, Error **errp);
>> +extern int pnv_xscom_populate(PnvChip *chip, void *fdt, int offset);
>> +
>> +#endif /* _PPC_PNV_XSCOM_H */
>

On Mon, Oct 03, Cédric Le Goater wrote:

> +++ b/include/hw/ppc/pnv_xscom.h

> +typedef struct PnvChip PnvChip;

This causes a compile error with gcc 4.5.1:

...
[  552s] In file included from /usr/src/packages/BUILD/qemu-20161104T092624.9226682/hw/ppc/pnv.c:29:0:
[  552s] /usr/src/packages/BUILD/qemu-20161104T092624.9226682/include/hw/ppc/pnv.h:60:3: error: redefinition of typedef 'PnvChip'
[  552s] /usr/src/packages/BUILD/qemu-20161104T092624.9226682/include/hw/ppc/pnv_xscom.h:24:24: note: previous declaration of 'PnvChip' was here
[  552s] make[1]: *** [hw/ppc/pnv.o] Error 1
[  552s] make[1]: Leaving directory `/usr/src/packages/BUILD/qemu-20161104T092624.9226682/ppc64-softmmu'
[  552s] make: *** [subdir-ppc64-softmmu] Error 2
...

Olaf

On 11/07/2016 09:26 AM, Olaf Hering wrote:
> On Mon, Oct 03, Cédric Le Goater wrote:
> 
>> +++ b/include/hw/ppc/pnv_xscom.h
> 
>> +typedef struct PnvChip PnvChip;
> 
> This causes a compile error with gcc 4.5.1:

Ah yes. That happens with "old" compilers. I will remove the extra definition. 

Thanks,

C. 

> ...
> [  552s] In file included from /usr/src/packages/BUILD/qemu-20161104T092624.9226682/hw/ppc/pnv.c:29:0:
> [  552s] /usr/src/packages/BUILD/qemu-20161104T092624.9226682/include/hw/ppc/pnv.h:60:3: error: redefinition of typedef 'PnvChip'
> [  552s] /usr/src/packages/BUILD/qemu-20161104T092624.9226682/include/hw/ppc/pnv_xscom.h:24:24: note: previous declaration of 'PnvChip' was here
> [  552s] make[1]: *** [hw/ppc/pnv.o] Error 1
> [  552s] make[1]: Leaving directory `/usr/src/packages/BUILD/qemu-20161104T092624.9226682/ppc64-softmmu'
> [  552s] make: *** [subdir-ppc64-softmmu] Error 2
> ...
> 
> Olaf
>