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

[Cédric Le Goater]

On 10/09/2019 11:31, Balamuruhan S wrote:
> 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 <address@hidden>
>>> ---
>>>  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 ?

Ok. Then it makes sense to have a 'MemoryRegionOps *' in PnvOCCClass.



Reviewed-by: Cédric Le Goater <address@hidden>

Thanks,

C.

> 
> -- 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;
>>>  
>>>
>>
>