Re: [Qemu-devel] [PATCH v4 20/20] ppc/pnv: add support for POWER9 LPC Controller

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 18780 bytes --]

On Mon, Oct 03, 2016 at 09:24:56AM +0200, Cédric Le Goater wrote:
> The LPC Controller on POWER9 is very similar to the one found on
> POWER8 but accesses are now done via on MMIOs, without the XSCOM and
> ECCB logic. The device tree is populated differently so we add a
> PnvLpcClass and a couple of methods for the purpose.
> 
> LPC interrupts routing is missing. This is Work In Progress but it
> gives us a P9 console and, more important, a first idea of what
> changes we should consider for the following models.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ppc/pnv.c             |  32 +++---
>  hw/ppc/pnv_lpc.c         | 255 ++++++++++++++++++++++++++++++++++++++++++++---
>  include/hw/ppc/pnv.h     |   9 +-
>  include/hw/ppc/pnv_lpc.h |  33 ++++++
>  4 files changed, 304 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 5b70ccf66fac..d5ef4f8ddb53 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -237,6 +237,8 @@ static void powernv_populate_chip(PnvChip *chip, void *fdt)
>          xics_native_populate_icp(chip, fdt, 0, pnv_core->pir, smt);
>      }
>  
> +    pnv_lpc_populate(chip, fdt, 0);
> +
>      /* Put all the memory in one node on chip 0 until we find a way to
>       * specify different ranges for each chip
>       */
> @@ -355,7 +357,7 @@ static void pnv_lpc_isa_irq_handler(void *opaque, int n, int level)
>  
>  static ISABus *pnv_isa_create(PnvChip *chip)
>  {
> -    PnvLpcController *lpc = &chip->lpc;
> +    PnvLpcController *lpc = chip->lpc;
>      ISABus *isa_bus;
>      qemu_irq *irqs;
>      PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> @@ -653,9 +655,6 @@ static void pnv_chip_init(Object *obj)
>  
>      chip->xscom_base = pcc->xscom_base;
>  
> -    object_initialize(&chip->lpc, sizeof(chip->lpc), TYPE_PNV_LPC);
> -    object_property_add_child(obj, "lpc", OBJECT(&chip->lpc), NULL);
> -
>      object_initialize(&chip->xics, sizeof(chip->xics), TYPE_XICS_NATIVE);
>      object_property_add_child(obj, "xics", OBJECT(&chip->xics), NULL);
>  
> @@ -667,11 +666,6 @@ static void pnv_chip_init(Object *obj)
>      object_property_add_const_link(OBJECT(&chip->occ), "psi",
>                                     OBJECT(&chip->psi), &error_abort);
>  
> -    /*
> -     * The LPC controller needs PSI to generate interrupts
> -     */
> -    object_property_add_const_link(OBJECT(&chip->lpc), "psi",
> -                                   OBJECT(&chip->psi), &error_abort);
>  }
>  
>  static void pnv_chip_realize(DeviceState *dev, Error **errp)
> @@ -757,10 +751,24 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
>      xics_insert_ics(XICS_COMMON(&chip->xics), &chip->psi.ics);
>  
>      /* Create LPC controller */
> -    object_property_set_bool(OBJECT(&chip->lpc), true, "realized",
> +    typename = g_strdup_printf(TYPE_PNV_LPC "-%s", pcc->cpu_model);
> +    chip->lpc = PNV_LPC(object_new(typename));

Urgh.  I think doing object_new() from realize is frowned upon.
Unfortunately, I don't know what the right way to do this is :/.

> +    object_property_add_child(OBJECT(chip), "lpc", OBJECT(chip->lpc), NULL);
> +
> +    /* The LPC controller needs PSI to generate interrupts  */
> +    object_property_add_const_link(OBJECT(chip->lpc), "psi",
> +                                   OBJECT(&chip->psi), &error_abort);
> +    object_property_set_bool(OBJECT(chip->lpc), true, "realized",
>                               &error_fatal);
> -    memory_region_add_subregion(&chip->xscom, PNV_XSCOM_LPC_BASE << 3,
> -                                &chip->lpc.xscom_regs);
> +
> +    if (PNV_CHIP_GET_CLASS(chip)->chip_type != PNV_CHIP_POWER9) {
> +        memory_region_add_subregion(&chip->xscom, PNV_XSCOM_LPC_BASE << 3,
> +                                    &chip->lpc->xscom_regs);
> +    } else {
> +        memory_region_add_subregion(get_system_memory(), PNV_POWER9_LPCM_BASE,
> +                                    &chip->lpc->xscom_regs);
> +    }
> +
>  
>      /* Create the simplified OCC model */
>      object_property_set_bool(OBJECT(&chip->occ), true, "realized",
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index 8b78b0a1e414..93ccc656120c 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -28,6 +28,7 @@
>  #include "hw/ppc/fdt.h"
>  
>  #include <libfdt.h>
> +#include "exec/address-spaces.h"
>  
>  enum {
>      ECCB_CTL    = 0,
> @@ -91,6 +92,88 @@ enum {
>  #define LPC_HC_REGS_OPB_SIZE    0x00001000
>  
>  
> +int pnv_lpc_populate(PnvChip *chip, void *fdt, int offset)
> +{
> +    PnvLpcClass *plc = PNV_LPC_GET_CLASS(chip->lpc);
> +
> +    if (plc->populate) {
> +        return plc->populate(chip, fdt, offset);
> +    }
> +    return 0;
> +}
> +
> +static int pnv_lpc_power9_populate(PnvChip *chip, void *fdt, int root_offset)
> +{
> +    const char compat[] = "ibm,power9-lpcm-opb\0simple-bus";
> +    const char lpc_compat[] = "ibm,power9-lpc\0ibm,lpc";
> +    char *name;
> +    int offset, lpcm_offset;
> +    /* should depend on a MMIO base address of the chip */
> +    uint64_t lpcm_addr = PNV_POWER9_LPCM_BASE;
> +    /* TODO: build the ranges properly, array of :
> +     *              { offset, addr hi, addr low, size }
> +     */
> +    uint32_t ranges[4] = { 0,
> +                           cpu_to_be32(lpcm_addr >> 32),
> +                           cpu_to_be32((uint32_t)lpcm_addr),
> +                           cpu_to_be32(PNV_POWER9_LPCM_SIZE / 2),
> +    };
> +    uint32_t reg[2];
> +
> +    /*
> +     * OPB bus
> +     */
> +    name = g_strdup_printf("lpcm-opb@%"PRIx64, lpcm_addr);
> +    lpcm_offset = fdt_add_subnode(fdt, root_offset, name);
> +    _FDT(lpcm_offset);
> +    g_free(name);
> +    _FDT((fdt_setprop_cell(fdt, lpcm_offset, "#address-cells", 1)));
> +    _FDT((fdt_setprop_cell(fdt, lpcm_offset, "#size-cells", 1)));
> +    _FDT((fdt_setprop(fdt, lpcm_offset, "compatible", compat, sizeof(compat))));
> +    _FDT((fdt_setprop_cell(fdt, lpcm_offset, "ibm,chip-id", chip->chip_id)));
> +   _FDT((fdt_setprop(fdt, lpcm_offset, "ranges", ranges, sizeof(ranges))));
> +
> +    /*
> +     * OPB Master registers
> +     */
> +    name = g_strdup_printf("opb-master@%x", LPC_OPB_REGS_OPB_ADDR);
> +    offset = fdt_add_subnode(fdt, lpcm_offset, name);
> +    _FDT(offset);
> +    g_free(name);
> +
> +    reg[0] = cpu_to_be32(LPC_OPB_REGS_OPB_ADDR);
> +    reg[1] = cpu_to_be32(LPC_OPB_REGS_OPB_SIZE);
> +    _FDT((fdt_setprop(fdt, offset, "reg", reg, sizeof(reg))));
> +    _FDT((fdt_setprop_string(fdt, offset, "compatible",
> +                             "ibm,power9-lpcm-opb-master")));
> +
> +    /*
> +     * LPC Host Controller registers
> +     */
> +    name = g_strdup_printf("lpc-controller@%x", LPC_HC_REGS_OPB_ADDR);
> +    offset = fdt_add_subnode(fdt, lpcm_offset, name);
> +    _FDT(offset);
> +    g_free(name);
> +
> +    reg[0] = cpu_to_be32(LPC_HC_REGS_OPB_ADDR);
> +    reg[1] = cpu_to_be32(LPC_HC_REGS_OPB_SIZE);
> +    _FDT((fdt_setprop(fdt, offset, "reg", reg, sizeof(reg))));
> +    _FDT((fdt_setprop_string(fdt, offset, "compatible",
> +                             "ibm,power9-lpc-controller")));
> +
> +    name = g_strdup_printf("lpc@%x", LPC_FW_OPB_ADDR);
> +    offset = fdt_add_subnode(fdt, lpcm_offset, name);
> +    _FDT(offset);
> +    g_free(name);
> +    _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 2)));
> +    _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 1)));
> +    _FDT((fdt_setprop(fdt, offset, "primary", NULL, 0)));
> +    _FDT((fdt_setprop(fdt, offset, "compatible", lpc_compat,
> +                      sizeof(lpc_compat))));
> +
> +    return 0;
> +}
> +
>  /*
>   * TODO: the "primary" cell should only be added on chip 0. This is
>   * how skiboot chooses the default LPC controller on multichip
> @@ -99,7 +182,8 @@ enum {
>   * It would be easly done if we can change the populate() interface to
>   * replace the PnvXScomInterface parameter by a PnvChip one
>   */
> -static int pnv_lpc_populate(PnvXScomInterface *dev, void *fdt, int xscom_offset)
> +static int pnv_lpc_power8_populate(PnvXScomInterface *dev, void *fdt,
> +                                   int xscom_offset)
>  {
>      const char compat[] = "ibm,power8-lpc\0ibm,lpc";
>      char *name;
> @@ -249,6 +333,74 @@ static const MemoryRegionOps pnv_lpc_xscom_ops = {
>      .endianness = DEVICE_BIG_ENDIAN,
>  };
>  
> +static uint64_t pnv_lpc_mmio_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    PnvLpcController *lpc = PNV_LPC(opaque);
> +    uint64_t val = 0;
> +    uint32_t opb_addr = addr & ECCB_CTL_ADDR_MASK;
> +    MemTxResult result;
> +
> +    switch (size) {
> +    case 4:
> +        val = address_space_ldl(&lpc->opb_as, opb_addr, MEMTXATTRS_UNSPECIFIED,
> +                                &result);
> +        break;
> +    case 1:
> +        val = address_space_ldub(&lpc->opb_as, opb_addr, MEMTXATTRS_UNSPECIFIED,
> +                                 &result);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "OPB read failed at @0x%"
> +                      HWADDR_PRIx " invalid size %d\n", addr, size);
> +        return 0;
> +    }
> +
> +    if (result != MEMTX_OK) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "OPB read failed at @0x%"
> +                      HWADDR_PRIx "\n", addr);
> +    }
> +
> +    return val;
> +}
> +
> +static void pnv_lpc_mmio_write(void *opaque, hwaddr addr,
> +                                uint64_t val, unsigned size)
> +{
> +    PnvLpcController *lpc = PNV_LPC(opaque);
> +    uint32_t opb_addr = addr & ECCB_CTL_ADDR_MASK;
> +    MemTxResult result;
> +
> +    switch (size) {
> +    case 4:
> +        address_space_stl(&lpc->opb_as, opb_addr, val, MEMTXATTRS_UNSPECIFIED,
> +                          &result);
> +         break;
> +    case 1:
> +        address_space_stb(&lpc->opb_as, opb_addr, val, MEMTXATTRS_UNSPECIFIED,
> +                          &result);
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR, "OPB write failed at @0x%"
> +                      HWADDR_PRIx " invalid size %d\n", addr, size);
> +        return;
> +    }
> +
> +    if (result != MEMTX_OK) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "OPB write failed at @0x%"
> +                      HWADDR_PRIx "\n", addr);
> +    }
> +}
> +
> +static const MemoryRegionOps pnv_lpc_mmio_ops = {
> +    .read = pnv_lpc_mmio_read,
> +    .write = pnv_lpc_mmio_write,
> +    .impl = {
> +        .min_access_size = 1,
> +        .max_access_size = 4,
> +    },
> +    .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
>  void pnv_lpc_eval_irqs(PnvLpcController *lpc)
>  {
>      bool lpc_to_opb_irq = false;
> @@ -426,9 +578,29 @@ static const MemoryRegionOps opb_master_ops = {
>      },
>  };
>  
> +static void pnv_lpc_power8_realize(DeviceState *dev, Error **errp)
> +{
> +    PnvLpcController *lpc = PNV_LPC(dev);
> +
> +    /* XScom region for LPC registers */
> +    memory_region_init_io(&lpc->xscom_regs, OBJECT(dev),
> +                          &pnv_lpc_xscom_ops, lpc, "xscom-lpc",
> +                          PNV_XSCOM_LPC_SIZE << 3);
> +}
> +
> +static void pnv_lpc_power9_realize(DeviceState *dev, Error **errp)
> +{
> +    PnvLpcController *lpc = PNV_LPC(dev);
> +
> +    /* Initialize MMIO region */
> +    memory_region_init_io(&lpc->xscom_regs, OBJECT(dev), &pnv_lpc_mmio_ops, lpc,
> +                          "lpcm", PNV_POWER9_LPCM_SIZE);
> +}
> +
>  static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>  {
>      PnvLpcController *lpc = PNV_LPC(dev);
> +    PnvLpcClass *plc = PNV_LPC_GET_CLASS(dev);
>      Object *obj;
>      Error *error = NULL;
>  
> @@ -470,11 +642,6 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>      memory_region_add_subregion(&lpc->opb_mr, LPC_HC_REGS_OPB_ADDR,
>                                  &lpc->lpc_hc_regs);
>  
> -    /* XScom region for LPC registers */
> -    memory_region_init_io(&lpc->xscom_regs, OBJECT(dev),
> -                          &pnv_lpc_xscom_ops, lpc, "xscom-lpc",
> -                          PNV_XSCOM_LPC_SIZE << 3);
> -
>      /* get PSI object from chip */
>      obj = object_property_get_link(OBJECT(dev), "psi", &error);
>      if (!obj) {
> @@ -483,32 +650,96 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>      lpc->psi = PNV_PSI(obj);
> +
> +    plc->realize(dev, errp);
>  }
>  
> -static void pnv_lpc_class_init(ObjectClass *klass, void *data)
> +static void pnv_lpc_power8_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PnvXScomInterfaceClass *xdc = PNV_XSCOM_INTERFACE_CLASS(klass);
> +    PnvLpcClass *plc = PNV_LPC_CLASS(klass);
>  
> -    xdc->populate = pnv_lpc_populate;
> +    xdc->populate = pnv_lpc_power8_populate;
>  
>      dc->realize = pnv_lpc_realize;
> +    plc->realize = pnv_lpc_power8_realize;
>  }
>  
> -static const TypeInfo pnv_lpc_info = {
> -    .name          = TYPE_PNV_LPC,
> -    .parent        = TYPE_DEVICE,
> +static const TypeInfo pnv_lpc_power8e_info = {
> +    .name          = TYPE_PNV_LPC_POWER8E,
> +    .parent        = TYPE_PNV_LPC,
>      .instance_size = sizeof(PnvLpcController),
> -    .class_init    = pnv_lpc_class_init,
> +    .class_init    = pnv_lpc_power8_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_PNV_XSCOM_INTERFACE },
> +        { }
> +    }
> +};
> +
> +static const TypeInfo pnv_lpc_power8_info = {
> +    .name          = TYPE_PNV_LPC_POWER8,
> +    .parent        = TYPE_PNV_LPC,
> +    .instance_size = sizeof(PnvLpcController),
> +    .class_init    = pnv_lpc_power8_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_PNV_XSCOM_INTERFACE },
> +        { }
> +    }
> +};
> +
> +static const TypeInfo pnv_lpc_power8nvl_info = {
> +    .name          = TYPE_PNV_LPC_POWER8NVL,
> +    .parent        = TYPE_PNV_LPC,
> +    .instance_size = sizeof(PnvLpcController),
> +    .class_init    = pnv_lpc_power8_class_init,
>      .interfaces = (InterfaceInfo[]) {
>          { TYPE_PNV_XSCOM_INTERFACE },
>          { }
>      }
>  };
>  
> +static void pnv_lpc_power9_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PnvLpcClass *plc = PNV_LPC_CLASS(klass);
> +
> +    dc->realize = pnv_lpc_realize;
> +    plc->realize = pnv_lpc_power9_realize;
> +    plc->populate = pnv_lpc_power9_populate;
> +}
> +
> +static const TypeInfo pnv_lpc_power9_info = {
> +    .name          = TYPE_PNV_LPC_POWER9,
> +    .parent        = TYPE_PNV_LPC,
> +    .instance_size = sizeof(PnvLpcController),
> +    .class_init    = pnv_lpc_power9_class_init,
> +};
> +
> +static void pnv_lpc_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = pnv_lpc_realize;
> +    dc->desc = "PowerNV LPC Controller";
> +}
> +
> +static const TypeInfo pnv_lpc_info = {
> +    .name          = TYPE_PNV_LPC,
> +    .parent        = TYPE_DEVICE,
> +    .class_init    = pnv_lpc_class_init,
> +    .class_size    = sizeof(PnvLpcClass),
> +    .abstract      = true,
> +};
> +
> +
>  static void pnv_lpc_register_types(void)
>  {
>      type_register_static(&pnv_lpc_info);
> +    type_register_static(&pnv_lpc_power8e_info);
> +    type_register_static(&pnv_lpc_power8_info);
> +    type_register_static(&pnv_lpc_power8nvl_info);
> +    type_register_static(&pnv_lpc_power9_info);
>  }
>  
>  type_init(pnv_lpc_register_types)
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index ed3316501326..361af311a92e 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -57,7 +57,7 @@ typedef struct PnvChip {
>      uint64_t  cores_mask;
>      void      *cores;
>  
> -    PnvLpcController lpc;
> +    PnvLpcController *lpc;
>      XICSNative   xics;
>      PnvPsiController psi;
>      PnvOCC           occ;
> @@ -154,4 +154,11 @@ typedef struct PnvMachineState {
>  #define PNV_PSIHB_BAR_SIZE    0x0000000000100000ull
>  
>  
> +/*
> + * POWER9 MMIO regions
> + */
> +#define PNV_POWER9_MMIO_BASE   0x006000000000000ull
> +#define PNV_POWER9_LPCM_BASE   (PNV_POWER9_MMIO_BASE + 0x30000000000ull)
> +#define PNV_POWER9_LPCM_SIZE   0x100000000ull
> +
>  #endif /* _PPC_PNV_H */
> diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
> index fc348dca50ca..510fab2fd115 100644
> --- a/include/hw/ppc/pnv_lpc.h
> +++ b/include/hw/ppc/pnv_lpc.h
> @@ -22,8 +22,13 @@
>  #define TYPE_PNV_LPC "pnv-lpc"
>  #define PNV_LPC(obj) \
>       OBJECT_CHECK(PnvLpcController, (obj), TYPE_PNV_LPC)
> +#define PNV_LPC_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(PnvLpcClass, (klass), TYPE_PNV_LPC)
> +#define PNV_LPC_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(PnvLpcClass, (obj), TYPE_PNV_LPC)
>  
>  typedef struct PnvPsiController PnvPsiController;
> +typedef struct PnvChip PnvChip;
>  
>  typedef struct PnvLpcController {
>      DeviceState parent;
> @@ -68,7 +73,35 @@ typedef struct PnvLpcController {
>      MemoryRegion xscom_regs;
>  } PnvLpcController;
>  
> +typedef struct PnvLpcClass {
> +    DeviceClass parent_class;
> +
> +    int (*populate)(PnvChip *chip, void *fdt, int offset);
> +    void (*realize)(DeviceState *dev, Error **errp);
> +} PnvLpcClass;
> +
> +
> +#define TYPE_PNV_LPC_POWER8E TYPE_PNV_LPC "-POWER8E"
> +#define PNV_LPC_POWER8E(obj) \
> +    OBJECT_CHECK(PnvLpc, (obj), TYPE_PNV_LPC_POWER8E)
> +
> +#define TYPE_PNV_LPC_POWER8 TYPE_PNV_LPC "-POWER8"
> +#define PNV_LPC_POWER8(obj) \
> +    OBJECT_CHECK(PnvLpc, (obj), TYPE_PNV_LPC_POWER8)
> +
> +#define TYPE_PNV_LPC_POWER8NVL TYPE_PNV_LPC "-POWER8NVL"
> +#define PNV_LPC_POWER8NVL(obj) \
> +    OBJECT_CHECK(PnvLpc, (obj), TYPE_PNV_LPC_POWER8NVL)
> +
> +#define TYPE_PNV_LPC_POWER9 TYPE_PNV_LPC "-POWER9"
> +#define PNV_LPC_POWER9(obj) \
> +    OBJECT_CHECK(PnvLpc, (obj), TYPE_PNV_LPC_POWER9)
> +
> +
> +
>  #define   LPC_HC_IRQ_SERIRQ0            0x80000000 /* all bits down to ... */
>  void pnv_lpc_eval_irqs(PnvLpcController *lpc);
>  
> +int pnv_lpc_populate(PnvChip *chip, void *fdt, int root_offset);
> +
>  #endif /* _PPC_PNV_LPC_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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]