Re: [PATCH v6 2/3] hw/ppc: Add N1 chiplet model

[Chalapathi V]

On 28-11-2023 12:18, Cédric Le Goater wrote:
> On 11/27/23 18:13, Chalapathi V wrote:
> > The N1 chiplet handle the high speed i/o traffic over PCIe and others.
> > The N1 chiplet consists of PowerBus Fabric controller,
> > nest Memory Management Unit, chiplet control unit and more.
> >
> > This commit creates a N1 chiplet model and initialize and realize the
> > pervasive chiplet model where chiplet control registers are implemented.
> >
> > This commit also implement the read/write method for the powerbus scom
> > registers
> >
> > Signed-off-by: Chalapathi V <chalapathi.v@linux.ibm.com>
> > ---
> >   include/hw/ppc/pnv_n1_chiplet.h |  35 +++++++
> >   include/hw/ppc/pnv_xscom.h      |   6 ++
> >   hw/ppc/pnv_n1_chiplet.c         | 171 ++++++++++++++++++++++++++++++++
> >   hw/ppc/meson.build              |   1 +
> >   4 files changed, 213 insertions(+)
> >   create mode 100644 include/hw/ppc/pnv_n1_chiplet.h
> >   create mode 100644 hw/ppc/pnv_n1_chiplet.c
> >
> > diff --git a/include/hw/ppc/pnv_n1_chiplet.h 
> > b/include/hw/ppc/pnv_n1_chiplet.h
> > new file mode 100644
> > index 0000000000..3c42ada7f4
> > --- /dev/null
> > +++ b/include/hw/ppc/pnv_n1_chiplet.h
> > @@ -0,0 +1,35 @@
> > +/*
> > + * QEMU PowerPC N1 chiplet model
> > + *
> > + * Copyright (c) 2023, IBM Corporation.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + *
> > + * This code is licensed under the GPL version 2 or later. See the
> > + * COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#ifndef PPC_PNV_N1_CHIPLET_H
> > +#define PPC_PNV_N1_CHIPLET_H
> > +
> > +#include "hw/ppc/pnv_nest_pervasive.h"
> > +
> > +#define TYPE_PNV_N1_CHIPLET "pnv-N1-chiplet"
> > +#define PNV_N1_CHIPLET(obj) OBJECT_CHECK(PnvN1Chiplet, (obj), 
> > TYPE_PNV_N1_CHIPLET)
> > +
> > +typedef struct pb_scom {
> > +    uint64_t mode;
> > +    uint64_t hp_mode2_curr;
> > +} pb_scom;
>
> Please use CamelCase coding style.
Sure. Thank You
> > +
> > +typedef struct PnvN1Chiplet {
> > +    DeviceState parent;
> > +    MemoryRegion xscom_pb_eq_regs;
> > +    MemoryRegion xscom_pb_es_regs;
>
> the MemoryRegion are generally called _mr, _iomem.
Sure. Updated in V7. Thank You.
> > +    /* common pervasive chiplet unit */
> > +    PnvNestChipletPervasive nest_pervasive;
> > +    pb_scom eq[8];
> > +    pb_scom es[4];
>
> are these arrays the registers ?
Yes, These are array of pb_scom EQ and ES registers. For now just 
PB_SCOM_EQ0_HP_MODE2_CURR and PB_SCOM_ES3_MODE registers are modeled.
> > +} PnvN1Chiplet;
> > +#endif /*PPC_PNV_N1_CHIPLET_H */
> > diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
> > index 3e15706dec..535ae1dab0 100644
> > --- a/include/hw/ppc/pnv_xscom.h
> > +++ b/include/hw/ppc/pnv_xscom.h
> > @@ -173,6 +173,12 @@ struct PnvXScomInterfaceClass {
> >   #define PNV10_XSCOM_N1_CHIPLET_CTRL_REGS_BASE      0x3000000
> >   #define PNV10_XSCOM_CHIPLET_CTRL_REGS_SIZE         0x400
> >   +#define PNV10_XSCOM_N1_PB_SCOM_EQ_BASE      0x3011000
> > +#define PNV10_XSCOM_N1_PB_SCOM_EQ_SIZE      0x200
> > +
> > +#define PNV10_XSCOM_N1_PB_SCOM_ES_BASE      0x3011300
> > +#define PNV10_XSCOM_N1_PB_SCOM_ES_SIZE      0x100
> > +
> >   #define PNV10_XSCOM_PEC_NEST_BASE  0x3011800 /* index goes 
> > downwards ... */
> >   #define PNV10_XSCOM_PEC_NEST_SIZE  0x100
> >   diff --git a/hw/ppc/pnv_n1_chiplet.c b/hw/ppc/pnv_n1_chiplet.c
> > new file mode 100644
> > index 0000000000..8e4c21dbf6
> > --- /dev/null
> > +++ b/hw/ppc/pnv_n1_chiplet.c
> > @@ -0,0 +1,171 @@
> > +/*
> > + * QEMU PowerPC N1 chiplet model
> > + *
> > + * Copyright (c) 2023, IBM Corporation.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + *
> > + * This code is licensed under the GPL version 2 or later. See the
> > + * COPYING file in the top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/log.h"
> > +#include "hw/qdev-properties.h"
> > +#include "hw/ppc/pnv.h"
> > +#include "hw/ppc/pnv_xscom.h"
> > +#include "hw/ppc/pnv_n1_chiplet.h"
> > +#include "hw/ppc/pnv_nest_pervasive.h"
> > +
> > +/*
> > + * The n1 chiplet contains chiplet control unit,
> > + * PowerBus/RaceTrack/Bridge logic, nest Memory Management Unit(nMMU)
> > + * and more.
> > + *
> > + * In this model Nest1 chiplet control registers are modelled via 
> > common
> > + * nest pervasive model and few PowerBus racetrack registers are 
> > modelled.
> > + */
> > +
> > +#define PB_SCOM_EQ0_HP_MODE2_CURR      0xe
> > +#define PB_SCOM_ES3_MODE               0x8a
> > +
> > +static uint64_t pnv_n1_chiplet_pb_scom_eq_read(void *opaque, hwaddr 
> > addr,
> > +                                                  unsigned size)
> > +{
> > +    PnvN1Chiplet *n1_chiplet = PNV_N1_CHIPLET(opaque);
> > +    int reg = addr >> 3;
> > +    uint64_t val = ~0ull;
> > +
> > +    switch (reg) {
> > +    case PB_SCOM_EQ0_HP_MODE2_CURR:
> > +        val = n1_chiplet->eq[0].hp_mode2_curr;
> > +        break;
> > +    default:
> > +        qemu_log_mask(LOG_UNIMP, "%s: Invalid xscom read at 0x%" 
> > PRIx32 "\n",
> > +                      __func__, reg);
> > +    }
> > +    return val;
> > +}
> > +
> > +static void pnv_n1_chiplet_pb_scom_eq_write(void *opaque, hwaddr addr,
> > +                                               uint64_t val, 
> > unsigned size)
> > +{
> > +    PnvN1Chiplet *n1_chiplet = PNV_N1_CHIPLET(opaque);
> > +    int reg = addr >> 3;
> > +
> > +    switch (reg) {
> > +    case PB_SCOM_EQ0_HP_MODE2_CURR:
> > +        n1_chiplet->eq[0].hp_mode2_curr = val;
> > +        break;
> > +    default:
> > +        qemu_log_mask(LOG_UNIMP, "%s: Invalid xscom write at 0x%" 
> > PRIx32 "\n",
> > +                      __func__, reg);
> > +    }
> > +}
> > +
> > +static const MemoryRegionOps pnv_n1_chiplet_pb_scom_eq_ops = {
> > +    .read = pnv_n1_chiplet_pb_scom_eq_read,
> > +    .write = pnv_n1_chiplet_pb_scom_eq_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 uint64_t pnv_n1_chiplet_pb_scom_es_read(void *opaque, hwaddr 
> > addr,
> > +                                          unsigned size)
> > +{
> > +    PnvN1Chiplet *n1_chiplet = PNV_N1_CHIPLET(opaque);
> > +    int reg = addr >> 3;
> > +    uint64_t val = ~0ull;
> > +
> > +    switch (reg) {
> > +    case PB_SCOM_ES3_MODE:
> > +        val = n1_chiplet->es[3].mode;
> > +        break;
> > +    default:
> > +        qemu_log_mask(LOG_UNIMP, "%s: Invalid xscom read at 0x%" 
> > PRIx32 "\n",
> > +                      __func__, reg);
> > +    }
> > +    return val;
> > +}
> > +
> > +static void pnv_n1_chiplet_pb_scom_es_write(void *opaque, hwaddr addr,
> > +                                               uint64_t val, 
> > unsigned size)
> > +{
> > +    PnvN1Chiplet *n1_chiplet = PNV_N1_CHIPLET(opaque);
> > +    int reg = addr >> 3;
> > +
> > +    switch (reg) {
> > +    case PB_SCOM_ES3_MODE:
> > +        n1_chiplet->es[3].mode = val;
> > +        break;
> > +    default:
> > +        qemu_log_mask(LOG_UNIMP, "%s: Invalid xscom write at 0x%" 
> > PRIx32 "\n",
> > +                      __func__, reg);
> > +    }
> > +}
> > +
> > +static const MemoryRegionOps pnv_n1_chiplet_pb_scom_es_ops = {
> > +    .read = pnv_n1_chiplet_pb_scom_es_read,
> > +    .write = pnv_n1_chiplet_pb_scom_es_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 void pnv_n1_chiplet_realize(DeviceState *dev, Error **errp)
> > +{
> > +    PnvN1Chiplet *n1_chiplet = PNV_N1_CHIPLET(dev);
> > +
> > +    /* Initialize and realize nest pervasive common chiplet model */
> > +    object_initialize_child(OBJECT(n1_chiplet), 
> > "nest_pervasive_common",
> > +                            &n1_chiplet->nest_pervasive,
> > +                            TYPE_PNV_NEST_PERVASIVE);
>
> object_initialize_child() should be called from an instance_init handler.
>
>
> > + object_property_set_str(OBJECT(&n1_chiplet->nest_pervasive),
> > +                                   "parent-obj-name", "n1_chiplet", 
> > errp);
>
> is it that important to name the xscom region  ?
No. not needed. I have removed this property in V7. Thank You.
> Thanks,
>
> C.
>
>
>
> > +    if (!qdev_realize(DEVICE(&n1_chiplet->nest_pervasive), NULL, 
> > errp)) {
> > +        return;
> > +    }
> > +
> > +    /* Nest1 chiplet power bus EQ xscom region */
> > +    pnv_xscom_region_init(&n1_chiplet->xscom_pb_eq_regs, 
> > OBJECT(n1_chiplet),
> > +                          &pnv_n1_chiplet_pb_scom_eq_ops, n1_chiplet,
> > +                          "xscom-n1_chiplet-pb-scom-eq-regs",
> > +                          PNV10_XSCOM_N1_PB_SCOM_EQ_SIZE);
> > +
> > +    /* Nest1 chiplet power bus ES xscom region */
> > +    pnv_xscom_region_init(&n1_chiplet->xscom_pb_es_regs, 
> > OBJECT(n1_chiplet),
> > +                          &pnv_n1_chiplet_pb_scom_es_ops, n1_chiplet,
> > +                          "xscom-n1_chiplet-pb-scom-es-regs",
> > +                          PNV10_XSCOM_N1_PB_SCOM_ES_SIZE);
> > +}
> > +
> > +static void pnv_n1_chiplet_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->desc = "PowerNV n1 chiplet";
> > +    dc->realize = pnv_n1_chiplet_realize;
> > +}
> > +
> > +static const TypeInfo pnv_n1_chiplet_info = {
> > +    .name          = TYPE_PNV_N1_CHIPLET,
> > +    .parent        = TYPE_DEVICE,
> > +    .instance_size = sizeof(PnvN1Chiplet),
> > +    .class_init    = pnv_n1_chiplet_class_init,
> > +    .interfaces    = (InterfaceInfo[]) {
> > +        { TYPE_PNV_XSCOM_INTERFACE },
> > +        { }
> > +    }
> > +};
> > +
> > +static void pnv_n1_chiplet_register_types(void)
> > +{
> > +    type_register_static(&pnv_n1_chiplet_info);
> > +}
> > +
> > +type_init(pnv_n1_chiplet_register_types);
> > diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> > index d6f6f94fcc..256e453c0c 100644
> > --- a/hw/ppc/meson.build
> > +++ b/hw/ppc/meson.build
> > @@ -52,6 +52,7 @@ ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
> >     'pnv_homer.c',
> >     'pnv_pnor.c',
> >     'pnv_nest_pervasive.c',
> > +  'pnv_n1_chiplet.c',
> >   ))
> >   # PowerPC 4xx boards
> >   ppc_ss.add(when: 'CONFIG_PPC405', if_true: files(