Re: [Qemu-devel] [PATCH v1 2/3] hw/ppc/pnv_occ: add sram device model for occ common area

[Balamuruhan S <bala24@linux.ibm.com>]

On Tue, Sep 10, 2019 at 09:19:11AM +0200, Cédric Le Goater wrote:
> On 10/09/2019 09:10, Balamuruhan S wrote:
> > emulate occ common area region with occ sram device model which
> > occ and skiboot uses it to communicate regarding sensors, slw
> > and HWMON in PowerNV emulated host.
> > 
> > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > ---
> >  hw/ppc/pnv.c             |  8 +++++
> >  hw/ppc/pnv_occ.c         | 78 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/hw/ppc/pnv_occ.h |  3 ++
> >  3 files changed, 89 insertions(+)
> > 
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 3f08db7b9e..80338ffe87 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -938,6 +938,10 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
> >          return;
> >      }
> >      pnv_xscom_add_subregion(chip, PNV_XSCOM_OCC_BASE, &chip8->occ.xscom_regs);
> > +
> > +    /* OCC SRAM model */
> > +    memory_region_add_subregion(get_system_memory(), PNV_OCC_COMMON_AREA(chip),
> > +                                &chip8->occ.sram_regs);
> >  }
> >  
> >  static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
> > @@ -1126,6 +1130,10 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> >          return;
> >      }
> >      pnv_xscom_add_subregion(chip, PNV9_XSCOM_OCC_BASE, &chip9->occ.xscom_regs);
> > +
> > +    /* OCC SRAM model */
> > +    memory_region_add_subregion(get_system_memory(), PNV9_OCC_COMMON_AREA(chip),
> > +                                &chip9->occ.sram_regs);
> >  }
> >  
> >  static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
> > diff --git a/hw/ppc/pnv_occ.c b/hw/ppc/pnv_occ.c
> > index 8bead2c930..785653bb67 100644
> > --- a/hw/ppc/pnv_occ.c
> > +++ b/hw/ppc/pnv_occ.c
> > @@ -30,6 +30,24 @@
> >  #define OCB_OCI_OCCMISC_AND     0x4021
> >  #define OCB_OCI_OCCMISC_OR      0x4022
> >  
> > +/* OCC sensors */
> > +#define OCC_SENSOR_DATA_BLOCK_OFFSET          0x580000
> > +#define OCC_SENSOR_DATA_VALID                 0x580001
> > +#define OCC_SENSOR_DATA_VERSION               0x580002
> > +#define OCC_SENSOR_DATA_READING_VERSION       0x580004
> > +#define OCC_SENSOR_DATA_NR_SENSORS            0x580008
> > +#define OCC_SENSOR_DATA_NAMES_OFFSET          0x580010
> > +#define OCC_SENSOR_DATA_READING_PING_OFFSET   0x580014
> > +#define OCC_SENSOR_DATA_READING_PONG_OFFSET   0x58000c
> > +#define OCC_SENSOR_DATA_NAME_LENGTH           0x58000d
> > +#define OCC_SENSOR_NAME_STRUCTURE_TYPE        0x580023
> > +#define OCC_SENSOR_LOC_CORE                   0x580022
> > +#define OCC_SENSOR_LOC_GPU                    0x580020
> > +#define OCC_SENSOR_TYPE_POWER                 0x580003
> > +#define OCC_SENSOR_NAME                       0x580005
> > +#define HWMON_SENSORS_MASK                    0x58001e
> > +#define SLW_IMAGE_BASE                        0x0
> > +
> >  static void pnv_occ_set_misc(PnvOCC *occ, uint64_t val)
> >  {
> >      bool irq_state;
> > @@ -82,6 +100,48 @@ static void pnv_occ_power8_xscom_write(void *opaque, hwaddr addr,
> >      }
> >  }
> >  
> > +static uint64_t pnv_occ_common_area_read(void *opaque, hwaddr addr,
> > +                                         unsigned width)
> > +{
> > +    switch (addr) {
> > +    /*
> > +     * occ-sensor sanity check that asserts the sensor
> > +     * header block
> > +     */
> > +    case OCC_SENSOR_DATA_BLOCK_OFFSET:
> > +    case OCC_SENSOR_DATA_VALID:
> > +    case OCC_SENSOR_DATA_VERSION:
> > +    case OCC_SENSOR_DATA_READING_VERSION:
> > +    case OCC_SENSOR_DATA_NR_SENSORS:
> > +    case OCC_SENSOR_DATA_NAMES_OFFSET:
> > +    case OCC_SENSOR_DATA_READING_PING_OFFSET:
> > +    case OCC_SENSOR_DATA_READING_PONG_OFFSET:
> > +    case OCC_SENSOR_NAME_STRUCTURE_TYPE:
> > +        return 1;
> > +    case OCC_SENSOR_DATA_NAME_LENGTH:
> > +        return 0x30;
> > +    case OCC_SENSOR_LOC_CORE:
> > +        return 0x0040;
> > +    case OCC_SENSOR_TYPE_POWER:
> > +        return 0x0080;
> > +    case OCC_SENSOR_NAME:
> > +        return 0x1000;
> > +    case HWMON_SENSORS_MASK:
> > +    case OCC_SENSOR_LOC_GPU:
> > +        return 0x8e00;
> > +    case SLW_IMAGE_BASE:
> > +        return 0x1000000000000000;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static void pnv_occ_common_area_write(void *opaque, hwaddr addr,
> > +                                             uint64_t val, unsigned width)
> > +{
> > +    /* callback function defined to occ common area write */
> > +    return;
> > +}
> > +
> >  static const MemoryRegionOps pnv_occ_power8_xscom_ops = {
> >      .read = pnv_occ_power8_xscom_read,
> >      .write = pnv_occ_power8_xscom_write,
> > @@ -92,12 +152,24 @@ static const MemoryRegionOps pnv_occ_power8_xscom_ops = {
> >      .endianness = DEVICE_BIG_ENDIAN,
> >  };
> >  
> > +const MemoryRegionOps pnv_occ_sram_ops = {
> > +    .read = pnv_occ_common_area_read,
> > +    .write = pnv_occ_common_area_write,
> > +    .valid.min_access_size = 1,
> > +    .valid.max_access_size = 8,
> > +    .impl.min_access_size = 1,
> > +    .impl.max_access_size = 8,
> > +    .endianness = DEVICE_BIG_ENDIAN,
> > +};
> > +
> >  static void pnv_occ_power8_class_init(ObjectClass *klass, void *data)
> >  {
> >      PnvOCCClass *poc = PNV_OCC_CLASS(klass);
> >  
> >      poc->xscom_size = PNV_XSCOM_OCC_SIZE;
> > +    poc->sram_size = PNV_OCC_COMMON_AREA_SIZE;
> >      poc->xscom_ops = &pnv_occ_power8_xscom_ops;
> > +    poc->sram_ops = &pnv_occ_sram_ops;
> >      poc->psi_irq = PSIHB_IRQ_OCC;
> >  }
> >  
> > @@ -168,7 +240,9 @@ static void pnv_occ_power9_class_init(ObjectClass *klass, void *data)
> >      PnvOCCClass *poc = PNV_OCC_CLASS(klass);
> >  
> >      poc->xscom_size = PNV9_XSCOM_OCC_SIZE;
> > +    poc->sram_size = PNV9_OCC_COMMON_AREA_SIZE;
> >      poc->xscom_ops = &pnv_occ_power9_xscom_ops;
> > +    poc->sram_ops = &pnv_occ_sram_ops;
> >      poc->psi_irq = PSIHB9_IRQ_OCC;
> >  }
> >  
> 
> do we plan to have different OCC SRAM operation per chip model ? 

Cedric, currently I did just tried to handle OCC SRAM access during
boot time and returns skiboot expected default values. I could observe
that P8 does 1 access only for slw image base address, so can we extend
it with per chip model in future ?

-- Bala
> 
> C.  
> 
> > @@ -199,6 +273,10 @@ static void pnv_occ_realize(DeviceState *dev, Error **errp)
> >      /* XScom region for OCC registers */
> >      pnv_xscom_region_init(&occ->xscom_regs, OBJECT(dev), poc->xscom_ops,
> >                            occ, "xscom-occ", poc->xscom_size);
> > +
> > +    /* XScom region for OCC SRAM registers */
> > +    pnv_xscom_region_init(&occ->sram_regs, OBJECT(dev), poc->sram_ops,
> > +                          occ, "occ-common-area", poc->sram_size);
> >  }
> >  
> >  static void pnv_occ_class_init(ObjectClass *klass, void *data)
> > diff --git a/include/hw/ppc/pnv_occ.h b/include/hw/ppc/pnv_occ.h
> > index ed0709bfc0..66b0989be6 100644
> > --- a/include/hw/ppc/pnv_occ.h
> > +++ b/include/hw/ppc/pnv_occ.h
> > @@ -38,6 +38,7 @@ typedef struct PnvOCC {
> >      PnvPsi *psi;
> >  
> >      MemoryRegion xscom_regs;
> > +    MemoryRegion sram_regs;
> >  } PnvOCC;
> >  
> >  #define PNV_OCC_CLASS(klass) \
> > @@ -49,7 +50,9 @@ typedef struct PnvOCCClass {
> >      DeviceClass parent_class;
> >  
> >      int xscom_size;
> > +    int sram_size;
> >      const MemoryRegionOps *xscom_ops;
> > +    const MemoryRegionOps *sram_ops;
> >      int psi_irq;
> >  } PnvOCCClass;
> >  
> > 
>