[Qemu-devel] [PATCH v1 0/3] add Homer/OCC common area emulation for PowerNV

[Qemu-devel] [PATCH v1 0/3] add Homer/OCC common area emulation for PowerNV

[Balamuruhan S]

Hi All,

This is follow-up patch that implements HOMER and OCC SRAM device
models to emulate homer memory and occ common area access for pstate
table, occ sensors, runtime data and slw.

This version addresses review comments in previous patchset and
breaks it to have separate patch series for Homer and OCC emulation,

https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg00979.html

currently skiboot disables the homer/occ code path with `QUIRK_NO_PBA`,
this quirk have to be removed in skiboot for it to use HOMER and OCC
SRAM device models along with few bug fixes,

https://github.com/balamuruhans/skiboot/commit/a655514d2a730e0372a2faee277d1cf01f71a524
https://github.com/balamuruhans/skiboot/commit/fd3d93d92ec66a7494346d6d24ced7b48264c9a0
https://github.com/balamuruhans/skiboot/commit/165b3829a93bc177c18133945a8cca3a2d701173

changes from v1:
    * reuse PnvOCC device model to implement SRAM device.
    * implement PnvHomer as separate device model.
    * have core max base address as part of PnvHOMERClass.
    * reuse PNV_CHIP_INDEX() instead of introducing new `chip_num`.
    * define all the memory ops access address as macros.
    * few coding style warnings given by checkpatch.pl.

I request for review, comments and suggestions for the changes.

Balamuruhan S (3):
  hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs
  hw/ppc/pnv_occ: add sram device model for occ common area
  hw/ppc/pnv_homer: add PowerNV homer device model

 hw/ppc/Makefile.objs       |   1 +
 hw/ppc/pnv.c               |  87 ++++++++++++---
 hw/ppc/pnv_homer.c         | 258 +++++++++++++++++++++++++++++++++++++++++++++
 hw/ppc/pnv_occ.c           |  78 ++++++++++++++
 hw/ppc/pnv_xscom.c         |  34 +++++-
 include/hw/ppc/pnv.h       |  21 ++++
 include/hw/ppc/pnv_homer.h |  52 +++++++++
 include/hw/ppc/pnv_occ.h   |   3 +
 8 files changed, 513 insertions(+), 21 deletions(-)
 create mode 100644 hw/ppc/pnv_homer.c
 create mode 100644 include/hw/ppc/pnv_homer.h

-- 
2.14.5

[Qemu-devel] [PATCH v1 1/3] hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs

[Balamuruhan S]

During PowerNV boot skiboot populates the device tree by
retrieving base address of homer/occ common area from
PBA BARs and prd ipoll mask by accessing xscom read/write
accesses.

Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
---
 hw/ppc/pnv_xscom.c   | 34 ++++++++++++++++++++++++++++++----
 include/hw/ppc/pnv.h | 18 ++++++++++++++++++
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
index 67aab98fef..f01d788a65 100644
--- a/hw/ppc/pnv_xscom.c
+++ b/hw/ppc/pnv_xscom.c
@@ -36,6 +36,16 @@
 #define PRD_P9_IPOLL_REG_MASK           0x000F0033
 #define PRD_P9_IPOLL_REG_STATUS         0x000F0034
 
+/* PBA BARs */
+#define P8_PBA_BAR0                     0x2013f00
+#define P8_PBA_BAR2                     0x2013f02
+#define P8_PBA_BARMASK0                 0x2013f04
+#define P8_PBA_BARMASK2                 0x2013f06
+#define P9_PBA_BAR0                     0x5012b00
+#define P9_PBA_BAR2                     0x5012b02
+#define P9_PBA_BARMASK0                 0x5012b04
+#define P9_PBA_BARMASK2                 0x5012b06
+
 static void xscom_complete(CPUState *cs, uint64_t hmer_bits)
 {
     /*
@@ -74,6 +84,26 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
     case 0x18002:       /* ECID2 */
         return 0;
 
+    case P9_PBA_BAR0:
+        return PNV9_HOMER_BASE(chip);
+    case P8_PBA_BAR0:
+        return PNV_HOMER_BASE(chip);
+
+    case P9_PBA_BARMASK0: /* P9 homer region size */
+        return PNV9_HOMER_SIZE;
+    case P8_PBA_BARMASK0: /* P8 homer region size */
+        return PNV_HOMER_SIZE;
+
+    case P9_PBA_BAR2: /* P9 occ common area */
+        return PNV9_OCC_COMMON_AREA(chip);
+    case P8_PBA_BAR2: /* P8 occ common area */
+        return PNV_OCC_COMMON_AREA(chip);
+
+    case P9_PBA_BARMASK2: /* P9 occ common area size */
+        return PNV9_OCC_COMMON_AREA_SIZE;
+    case P8_PBA_BARMASK2: /* P8 occ common area size */
+        return PNV_OCC_COMMON_AREA_SIZE;
+
     case 0x1010c00:     /* PIBAM FIR */
     case 0x1010c03:     /* PIBAM FIR MASK */
 
@@ -93,13 +123,9 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
     case 0x2020009:     /* ADU stuff, error register */
     case 0x202000f:     /* ADU stuff, receive status register*/
         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 */
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index fb123edc4e..63a4b7b6a7 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -198,6 +198,16 @@ void pnv_bmc_powerdown(IPMIBmc *bmc);
 #define PNV_XSCOM_BASE(chip)                                            \
     (0x0003fc0000000000ull + ((uint64_t)(chip)->chip_id) * PNV_XSCOM_SIZE)
 
+#define PNV_OCC_COMMON_AREA_SIZE    0x0000000000700000ull
+#define PNV_OCC_COMMON_AREA(chip)                                       \
+    (0x7fff800000ull + ((uint64_t)PNV_CHIP_INDEX(chip) * \
+                         PNV_OCC_COMMON_AREA_SIZE))
+
+#define PNV_HOMER_SIZE              0x0000000000300000ull
+#define PNV_HOMER_BASE(chip)                                            \
+    (0x7ffd800000ull + ((uint64_t)PNV_CHIP_INDEX(chip)) * PNV_HOMER_SIZE)
+
+
 /*
  * XSCOM 0x20109CA defines the ICP BAR:
  *
@@ -256,4 +266,12 @@ void pnv_bmc_powerdown(IPMIBmc *bmc);
 #define PNV9_XSCOM_SIZE              0x0000000400000000ull
 #define PNV9_XSCOM_BASE(chip)        PNV9_CHIP_BASE(chip, 0x00603fc00000000ull)
 
+#define PNV9_OCC_COMMON_AREA_SIZE    0x0000000000700000ull
+#define PNV9_OCC_COMMON_AREA(chip)                                      \
+    (0x203fff800000ull + ((uint64_t)PNV_CHIP_INDEX(chip) * \
+                           PNV9_OCC_COMMON_AREA_SIZE))
+
+#define PNV9_HOMER_SIZE              0x0000000000300000ull
+#define PNV9_HOMER_BASE(chip)                                           \
+    (0x203ffd800000ull + ((uint64_t)PNV_CHIP_INDEX(chip)) * PNV9_HOMER_SIZE)
 #endif /* PPC_PNV_H */
-- 
2.14.5

Re: [Qemu-devel] [PATCH v1 1/3] hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs

[Cédric Le Goater]

On 10/09/2019 09:10, Balamuruhan S wrote:
> During PowerNV boot skiboot populates the device tree by
> retrieving base address of homer/occ common area from
> PBA BARs and prd ipoll mask by accessing xscom read/write
> accesses.
> 
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>

LGTM,

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

Thanks,

C.

> ---
>  hw/ppc/pnv_xscom.c   | 34 ++++++++++++++++++++++++++++++----
>  include/hw/ppc/pnv.h | 18 ++++++++++++++++++
>  2 files changed, 48 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index 67aab98fef..f01d788a65 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -36,6 +36,16 @@
>  #define PRD_P9_IPOLL_REG_MASK           0x000F0033
>  #define PRD_P9_IPOLL_REG_STATUS         0x000F0034
>  
> +/* PBA BARs */
> +#define P8_PBA_BAR0                     0x2013f00
> +#define P8_PBA_BAR2                     0x2013f02
> +#define P8_PBA_BARMASK0                 0x2013f04
> +#define P8_PBA_BARMASK2                 0x2013f06
> +#define P9_PBA_BAR0                     0x5012b00
> +#define P9_PBA_BAR2                     0x5012b02
> +#define P9_PBA_BARMASK0                 0x5012b04
> +#define P9_PBA_BARMASK2                 0x5012b06
> +
>  static void xscom_complete(CPUState *cs, uint64_t hmer_bits)
>  {
>      /*
> @@ -74,6 +84,26 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
>      case 0x18002:       /* ECID2 */
>          return 0;
>  
> +    case P9_PBA_BAR0:
> +        return PNV9_HOMER_BASE(chip);
> +    case P8_PBA_BAR0:
> +        return PNV_HOMER_BASE(chip);
> +
> +    case P9_PBA_BARMASK0: /* P9 homer region size */
> +        return PNV9_HOMER_SIZE;
> +    case P8_PBA_BARMASK0: /* P8 homer region size */
> +        return PNV_HOMER_SIZE;
> +
> +    case P9_PBA_BAR2: /* P9 occ common area */
> +        return PNV9_OCC_COMMON_AREA(chip);
> +    case P8_PBA_BAR2: /* P8 occ common area */
> +        return PNV_OCC_COMMON_AREA(chip);
> +
> +    case P9_PBA_BARMASK2: /* P9 occ common area size */
> +        return PNV9_OCC_COMMON_AREA_SIZE;
> +    case P8_PBA_BARMASK2: /* P8 occ common area size */
> +        return PNV_OCC_COMMON_AREA_SIZE;
> +
>      case 0x1010c00:     /* PIBAM FIR */
>      case 0x1010c03:     /* PIBAM FIR MASK */
>  
> @@ -93,13 +123,9 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
>      case 0x2020009:     /* ADU stuff, error register */
>      case 0x202000f:     /* ADU stuff, receive status register*/
>          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 */
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index fb123edc4e..63a4b7b6a7 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -198,6 +198,16 @@ void pnv_bmc_powerdown(IPMIBmc *bmc);
>  #define PNV_XSCOM_BASE(chip)                                            \
>      (0x0003fc0000000000ull + ((uint64_t)(chip)->chip_id) * PNV_XSCOM_SIZE)
>  
> +#define PNV_OCC_COMMON_AREA_SIZE    0x0000000000700000ull
> +#define PNV_OCC_COMMON_AREA(chip)                                       \
> +    (0x7fff800000ull + ((uint64_t)PNV_CHIP_INDEX(chip) * \
> +                         PNV_OCC_COMMON_AREA_SIZE))
> +
> +#define PNV_HOMER_SIZE              0x0000000000300000ull
> +#define PNV_HOMER_BASE(chip)                                            \
> +    (0x7ffd800000ull + ((uint64_t)PNV_CHIP_INDEX(chip)) * PNV_HOMER_SIZE)
> +
> +
>  /*
>   * XSCOM 0x20109CA defines the ICP BAR:
>   *
> @@ -256,4 +266,12 @@ void pnv_bmc_powerdown(IPMIBmc *bmc);
>  #define PNV9_XSCOM_SIZE              0x0000000400000000ull
>  #define PNV9_XSCOM_BASE(chip)        PNV9_CHIP_BASE(chip, 0x00603fc00000000ull)
>  
> +#define PNV9_OCC_COMMON_AREA_SIZE    0x0000000000700000ull
> +#define PNV9_OCC_COMMON_AREA(chip)                                      \
> +    (0x203fff800000ull + ((uint64_t)PNV_CHIP_INDEX(chip) * \
> +                           PNV9_OCC_COMMON_AREA_SIZE))
> +
> +#define PNV9_HOMER_SIZE              0x0000000000300000ull
> +#define PNV9_HOMER_BASE(chip)                                           \
> +    (0x203ffd800000ull + ((uint64_t)PNV_CHIP_INDEX(chip)) * PNV9_HOMER_SIZE)
>  #endif /* PPC_PNV_H */
> 

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

[Balamuruhan S]

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;
 }
 
@@ -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;
 
-- 
2.14.5

Re: [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 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 ? 

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

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

[Balamuruhan S]

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

Re: [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 <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 ?

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



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

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

[Qemu-devel] [PATCH v1 3/3] hw/ppc/pnv_homer: add PowerNV homer device model

[Balamuruhan S]

add PnvHOMER device model to emulate homer memory access
for pstate table, occ-sensors, slw, occ static and dynamic
values for Power8 and Power9 chips. Fix few coding style
warnings given by checkpatch.pl.

Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
---
 hw/ppc/Makefile.objs       |   1 +
 hw/ppc/pnv.c               |  79 +++++++++++---
 hw/ppc/pnv_homer.c         | 258 +++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/pnv.h       |   3 +
 include/hw/ppc/pnv_homer.h |  52 +++++++++
 5 files changed, 376 insertions(+), 17 deletions(-)
 create mode 100644 hw/ppc/pnv_homer.c
 create mode 100644 include/hw/ppc/pnv_homer.h

diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index 2c4e1c8de0..580bb4f0dd 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -9,6 +9,7 @@ obj-$(CONFIG_PSERIES) += spapr_tpm_proxy.o
 obj-$(CONFIG_SPAPR_RNG) +=  spapr_rng.o
 # IBM PowerNV
 obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
+obj-$(CONFIG_POWERNV) += pnv_homer.o
 ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
 obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o
 endif
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 80338ffe87..ddddcc9bb6 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -187,7 +187,8 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
 
     _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
     _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
-    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
+    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size",
+                           cpu->hash64_opts->slb_size)));
     _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
     _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
 
@@ -200,19 +201,23 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
                            segs, sizeof(segs))));
     }
 
-    /* Advertise VMX/VSX (vector extensions) if available
+    /*
+     * Advertise VMX/VSX (vector extensions) if available
      *   0 / no property == no vector extensions
      *   1               == VMX / Altivec available
-     *   2               == VSX available */
+     *   2               == VSX available
+     */
     if (env->insns_flags & PPC_ALTIVEC) {
         uint32_t vmx = (env->insns_flags2 & PPC2_VSX) ? 2 : 1;
 
         _FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", vmx)));
     }
 
-    /* Advertise DFP (Decimal Floating Point) if available
+    /*
+     * Advertise DFP (Decimal Floating Point) if available
      *   0 / no property == no DFP
-     *   1               == DFP available */
+     *   1               == DFP available
+     */
     if (env->insns_flags2 & PPC2_DFP) {
         _FDT((fdt_setprop_cell(fdt, offset, "ibm,dfp", 1)));
     }
@@ -424,7 +429,8 @@ static int pnv_dt_isa_device(DeviceState *dev, void *opaque)
     return 0;
 }
 
-/* The default LPC bus of a multichip system is on chip 0. It's
+/*
+ * The default LPC bus of a multichip system is on chip 0. It's
  * recognized by the firmware (skiboot) using a "primary" property.
  */
 static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
@@ -442,8 +448,10 @@ static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
     assert(phandle > 0);
     _FDT((fdt_setprop_cell(fdt, isa_offset, "phandle", phandle)));
 
-    /* ISA devices are not necessarily parented to the ISA bus so we
-     * can not use object_child_foreach() */
+    /*
+     * ISA devices are not necessarily parented to the ISA bus so we
+     * can not use object_child_foreach()
+     */
     qbus_walk_children(BUS(pnv->isa_bus), pnv_dt_isa_device, NULL, NULL, NULL,
                        &args);
 }
@@ -545,7 +553,8 @@ static void pnv_reset(MachineState *machine)
 
     qemu_devices_reset();
 
-    /* OpenPOWER systems have a BMC, which can be defined on the
+    /*
+     * OpenPOWER systems have a BMC, which can be defined on the
      * command line with:
      *
      *   -device ipmi-bmc-sim,id=bmc0
@@ -705,7 +714,8 @@ static void pnv_init(MachineState *machine)
 
         pnv->chips[i] = PNV_CHIP(chip);
 
-        /* TODO: put all the memory in one node on chip 0 until we find a
+        /*
+         * TODO: put all the memory in one node on chip 0 until we find a
          * way to specify different ranges for each chip
          */
         if (i == 0) {
@@ -732,8 +742,10 @@ static void pnv_init(MachineState *machine)
     /* Create an RTC ISA device too */
     mc146818_rtc_init(pnv->isa_bus, 2000, NULL);
 
-    /* OpenPOWER systems use a IPMI SEL Event message to notify the
-     * host to powerdown */
+    /*
+     * OpenPOWER systems use a IPMI SEL Event message to notify the
+     * host to powerdown
+     */
     pnv->powerdown_notifier.notify = pnv_powerdown_notify;
     qemu_register_powerdown_notifier(&pnv->powerdown_notifier);
 }
@@ -803,7 +815,8 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
     pnv_cpu->intc = obj;
 }
 
-/* Allowed core identifiers on a POWER8 Processor Chip :
+/*
+ * Allowed core identifiers on a POWER8 Processor Chip :
  *
  * <EX0 reserved>
  *  EX1  - Venice only
@@ -847,6 +860,11 @@ static void pnv_chip_power8_instance_init(Object *obj)
                             TYPE_PNV8_OCC, &error_abort, NULL);
     object_property_add_const_link(OBJECT(&chip8->occ), "psi",
                                    OBJECT(&chip8->psi), &error_abort);
+
+    object_initialize_child(obj, "homer",  &chip8->homer, sizeof(chip8->homer),
+                            TYPE_PNV8_HOMER, &error_abort, NULL);
+    object_property_add_const_link(OBJECT(&chip8->homer), "xics",
+                                   OBJECT(qdev_get_machine()), &error_abort);
 }
 
 static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
@@ -923,8 +941,10 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
                                             (uint64_t) PNV_XSCOM_BASE(chip),
                                             PNV_XSCOM_LPC_BASE);
 
-    /* Interrupt Management Area. This is the memory region holding
-     * all the Interrupt Control Presenter (ICP) registers */
+    /*
+     * Interrupt Management Area. This is the memory region holding
+     * all the Interrupt Control Presenter (ICP) registers
+     */
     pnv_chip_icp_realize(chip8, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
@@ -942,6 +962,16 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
     /* OCC SRAM model */
     memory_region_add_subregion(get_system_memory(), PNV_OCC_COMMON_AREA(chip),
                                 &chip8->occ.sram_regs);
+
+    /* HOMER */
+    object_property_set_bool(OBJECT(&chip8->homer), true, "realized",
+                             &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    memory_region_add_subregion(get_system_memory(), PNV_HOMER_BASE(chip),
+                                &chip8->homer.homer_regs);
 }
 
 static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
@@ -1024,6 +1054,11 @@ static void pnv_chip_power9_instance_init(Object *obj)
                             TYPE_PNV9_OCC, &error_abort, NULL);
     object_property_add_const_link(OBJECT(&chip9->occ), "psi",
                                    OBJECT(&chip9->psi), &error_abort);
+
+    object_initialize_child(obj, "homer",  &chip9->homer, sizeof(chip9->homer),
+                            TYPE_PNV9_HOMER, &error_abort, NULL);
+    object_property_add_const_link(OBJECT(&chip9->homer), "chip", obj,
+                                   &error_abort);
 }
 
 static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error **errp)
@@ -1134,6 +1169,16 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
     /* OCC SRAM model */
     memory_region_add_subregion(get_system_memory(), PNV9_OCC_COMMON_AREA(chip),
                                 &chip9->occ.sram_regs);
+
+    /* HOMER */
+    object_property_set_bool(OBJECT(&chip9->homer), true, "realized",
+                             &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    memory_region_add_subregion(get_system_memory(), PNV9_HOMER_BASE(chip),
+                                &chip9->homer.homer_regs);
 }
 
 static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
@@ -1412,8 +1457,8 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
     mc->init = pnv_init;
     mc->reset = pnv_reset;
     mc->max_cpus = MAX_CPUS;
-    mc->block_default_type = IF_IDE; /* Pnv provides a AHCI device for
-                                      * storage */
+    /* Pnv provides a AHCI device for storage */
+    mc->block_default_type = IF_IDE;
     mc->no_parallel = 1;
     mc->default_boot_order = NULL;
     /*
diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
new file mode 100644
index 0000000000..2886e27176
--- /dev/null
+++ b/hw/ppc/pnv_homer.c
@@ -0,0 +1,258 @@
+/*
+ * QEMU PowerPC PowerNV Emulation of a few HOMER related registers
+ *
+ * Copyright (c) 2019, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/cpus.h"
+#include "hw/ppc/pnv.h"
+#include "hw/qdev-core.h"
+#include "hw/ppc/pnv_homer.h"
+
+
+static bool core_max_array(void *opaque, hwaddr addr)
+{
+    PnvHOMER *homer = PNV_HOMER(opaque);
+    PnvHOMERClass *homer_class = PNV_HOMER_GET_CLASS(homer);
+
+    MachineState *ms = MACHINE(qdev_get_machine());
+
+    for (int i = 0; i <= ms->smp.cores; i++) {
+        if (addr == (homer_class->core_max_base + i)) {
+            return true;
+       }
+    }
+    return false;
+}
+
+/* P8 Pstate table */
+
+#define PNV8_OCC_PSTATE_VERSION          0x1f8001
+#define PNV8_OCC_PSTATE_MIN              0x1f8003
+#define PNV8_OCC_PSTATE_VALID            0x1f8000
+#define PNV8_OCC_PSTATE_THROTTLE         0x1f8002
+#define PNV8_OCC_PSTATE_NOM              0x1f8004
+#define PNV8_OCC_PSTATE_TURBO            0x1f8005
+#define PNV8_OCC_PSTATE_ULTRA_TURBO      0x1f8006
+#define PNV8_OCC_PSTATE_DATA             0x1f8008
+#define PNV8_OCC_PSTATE_ID_ZERO          0x1f8010
+#define PNV8_OCC_PSTATE_ID_ONE           0x1f8018
+#define PNV8_OCC_PSTATE_ID_TWO           0x1f8020
+#define PNV8_OCC_VDD_VOLTAGE_IDENTIFIER  0x1f8012
+#define PNV8_OCC_VCS_VOLTAGE_IDENTIFIER  0x1f8013
+#define PNV8_OCC_PSTATE_ZERO_FREQUENCY   0x1f8014
+#define PNV8_OCC_PSTATE_ONE_FREQUENCY    0x1f801c
+#define PNV8_OCC_PSTATE_TWO_FREQUENCY    0x1f8024
+#define PNV8_CORE_MAX_BASE               0x1f8810
+
+
+static uint64_t pnv_power8_homer_read(void *opaque, hwaddr addr,
+                                      unsigned size)
+{
+    switch (addr) {
+    case PNV8_OCC_PSTATE_VERSION:
+    case PNV8_OCC_PSTATE_MIN:
+    case PNV8_OCC_PSTATE_ID_ZERO:
+        return 0;
+    case PNV8_OCC_PSTATE_VALID:
+    case PNV8_OCC_PSTATE_THROTTLE:
+    case PNV8_OCC_PSTATE_NOM:
+    case PNV8_OCC_PSTATE_TURBO:
+    case PNV8_OCC_PSTATE_ID_ONE:
+    case PNV8_OCC_VDD_VOLTAGE_IDENTIFIER:
+    case PNV8_OCC_VCS_VOLTAGE_IDENTIFIER:
+        return 1;
+    case PNV8_OCC_PSTATE_ULTRA_TURBO:
+    case PNV8_OCC_PSTATE_ID_TWO:
+        return 2;
+    case PNV8_OCC_PSTATE_DATA:
+        return 0x1000000000000000;
+    /* P8 frequency for 0, 1, and 2 pstates */
+    case PNV8_OCC_PSTATE_ZERO_FREQUENCY:
+    case PNV8_OCC_PSTATE_ONE_FREQUENCY:
+    case PNV8_OCC_PSTATE_TWO_FREQUENCY:
+        return 3000;
+    }
+    /* pstate table core max array */
+    if (core_max_array(opaque, addr)) {
+        return 1;
+    }
+    return 0;
+}
+
+static void pnv_power8_homer_write(void *opaque, hwaddr addr,
+                                   uint64_t val, unsigned size)
+{
+    /* callback function defined to homer write */
+    return;
+}
+
+static const MemoryRegionOps pnv_power8_homer_ops = {
+    .read = pnv_power8_homer_read,
+    .write = pnv_power8_homer_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_homer_power8_class_init(ObjectClass *klass, void *data)
+{
+    PnvHOMERClass *homer = PNV_HOMER_CLASS(klass);
+
+    homer->homer_size = PNV_HOMER_SIZE;
+    homer->homer_ops = &pnv_power8_homer_ops;
+    homer->core_max_base = PNV8_CORE_MAX_BASE;
+}
+
+static const TypeInfo pnv_homer_power8_type_info = {
+    .name          = TYPE_PNV8_HOMER,
+    .parent        = TYPE_PNV_HOMER,
+    .instance_size = sizeof(PnvHOMER),
+    .class_init    = pnv_homer_power8_class_init,
+};
+
+/* P9 Pstate table */
+
+#define PNV9_OCC_PSTATE_ID_ZERO          0xe2018
+#define PNV9_OCC_PSTATE_ID_ONE           0xe2020
+#define PNV9_OCC_PSTATE_ID_TWO           0xe2028
+#define PNV9_OCC_PSTATE_DATA             0xe2000
+#define PNV9_OCC_PSTATE_DATA_AREA        0xe2008
+#define PNV9_OCC_PSTATE_MIN              0xe2003
+#define PNV9_OCC_PSTATE_NOM              0xe2004
+#define PNV9_OCC_PSTATE_TURBO            0xe2005
+#define PNV9_OCC_PSTATE_ULTRA_TURBO      0xe2818
+#define PNV9_OCC_MAX_PSTATE_ULTRA_TURBO  0xe2006
+#define PNV9_OCC_PSTATE_MAJOR_VERSION    0xe2001
+#define PNV9_OCC_OPAL_RUNTIME_DATA       0xe2b85
+#define PNV9_CHIP_HOMER_IMAGE_POINTER    0x200008
+#define PNV9_CHIP_HOMER_BASE             0x0
+#define PNV9_OCC_PSTATE_ZERO_FREQUENCY   0xe201c
+#define PNV9_OCC_PSTATE_ONE_FREQUENCY    0xe2024
+#define PNV9_OCC_PSTATE_TWO_FREQUENCY    0xe202c
+#define PNV9_OCC_ROLE_MASTER_OR_SLAVE    0xe2002
+#define PNV9_CORE_MAX_BASE               0xe2819
+
+
+static uint64_t pnv_power9_homer_read(void *opaque, hwaddr addr,
+                                      unsigned size)
+{
+    switch (addr) {
+    case PNV9_OCC_MAX_PSTATE_ULTRA_TURBO:
+    case PNV9_OCC_PSTATE_ID_ZERO:
+        return 0;
+    case PNV9_OCC_PSTATE_DATA:
+    case PNV9_OCC_ROLE_MASTER_OR_SLAVE:
+    case PNV9_OCC_PSTATE_NOM:
+    case PNV9_OCC_PSTATE_TURBO:
+    case PNV9_OCC_PSTATE_ID_ONE:
+    case PNV9_OCC_PSTATE_ULTRA_TURBO:
+    case PNV9_OCC_OPAL_RUNTIME_DATA:
+        return 1;
+    case PNV9_OCC_PSTATE_MIN:
+    case PNV9_OCC_PSTATE_ID_TWO:
+        return 2;
+
+    /* 3000 khz frequency for 0, 1, and 2 pstates */
+    case PNV9_OCC_PSTATE_ZERO_FREQUENCY:
+    case PNV9_OCC_PSTATE_ONE_FREQUENCY:
+    case PNV9_OCC_PSTATE_TWO_FREQUENCY:
+        return 3000;
+    case PNV9_OCC_PSTATE_MAJOR_VERSION:
+        return 0x90;
+    case PNV9_CHIP_HOMER_BASE:
+    case PNV9_OCC_PSTATE_DATA_AREA:
+    case PNV9_CHIP_HOMER_IMAGE_POINTER:
+        return 0x1000000000000000;
+    }
+    /* pstate table core max array */
+    if (core_max_array(opaque, addr)) {
+        return 1;
+    }
+    return 0;
+}
+
+static void pnv_power9_homer_write(void *opaque, hwaddr addr,
+                                   uint64_t val, unsigned size)
+{
+    /* callback function defined to homer write */
+    return;
+}
+
+static const MemoryRegionOps pnv_power9_homer_ops = {
+    .read = pnv_power9_homer_read,
+    .write = pnv_power9_homer_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_homer_power9_class_init(ObjectClass *klass, void *data)
+{
+    PnvHOMERClass *homer = PNV_HOMER_CLASS(klass);
+
+    homer->homer_size = PNV9_HOMER_SIZE;
+    homer->homer_ops = &pnv_power9_homer_ops;
+    homer->core_max_base = PNV9_CORE_MAX_BASE;
+}
+
+static const TypeInfo pnv_homer_power9_type_info = {
+    .name          = TYPE_PNV9_HOMER,
+    .parent        = TYPE_PNV_HOMER,
+    .instance_size = sizeof(PnvHOMER),
+    .class_init    = pnv_homer_power9_class_init,
+};
+
+static void pnv_homer_realize(DeviceState *dev, Error **errp)
+{
+    PnvHOMER *homer = PNV_HOMER(dev);
+    PnvHOMERClass *homer_class = PNV_HOMER_GET_CLASS(homer);
+
+    /* homer region */
+    memory_region_init_io(&homer->homer_regs, OBJECT(dev),
+                          homer_class->homer_ops, homer, "homer-main-memory",
+                          homer_class->homer_size);
+}
+
+static void pnv_homer_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = pnv_homer_realize;
+    dc->desc = "PowerNV HOMER Memory";
+}
+
+static const TypeInfo pnv_homer_type_info = {
+    .name          = TYPE_PNV_HOMER,
+    .parent        = TYPE_DEVICE,
+    .instance_size = sizeof(PnvHOMER),
+    .class_init    = pnv_homer_class_init,
+    .class_size    = sizeof(PnvHOMERClass),
+    .abstract      = true,
+};
+
+static void pnv_homer_register_types(void)
+{
+    type_register_static(&pnv_homer_type_info);
+    type_register_static(&pnv_homer_power8_type_info);
+    type_register_static(&pnv_homer_power9_type_info);
+}
+
+type_init(pnv_homer_register_types);
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 63a4b7b6a7..321d4ca967 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -26,6 +26,7 @@
 #include "hw/ppc/pnv_lpc.h"
 #include "hw/ppc/pnv_psi.h"
 #include "hw/ppc/pnv_occ.h"
+#include "hw/ppc/pnv_homer.h"
 #include "hw/ppc/pnv_xive.h"
 #include "hw/ppc/pnv_core.h"
 
@@ -76,6 +77,7 @@ typedef struct Pnv8Chip {
     PnvLpcController lpc;
     Pnv8Psi      psi;
     PnvOCC       occ;
+    PnvHOMER     homer;
 } Pnv8Chip;
 
 #define TYPE_PNV9_CHIP "pnv9-chip"
@@ -90,6 +92,7 @@ typedef struct Pnv9Chip {
     Pnv9Psi      psi;
     PnvLpcController lpc;
     PnvOCC       occ;
+    PnvHOMER     homer;
 
     uint32_t     nr_quads;
     PnvQuad      *quads;
diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h
new file mode 100644
index 0000000000..3b666a6245
--- /dev/null
+++ b/include/hw/ppc/pnv_homer.h
@@ -0,0 +1,52 @@
+/*
+ * QEMU PowerPC PowerNV Emulation of a few HOMER related registers
+ *
+ * Copyright (c) 2019, 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_HOMER_H
+#define PPC_PNV_HOMER_H
+
+#define TYPE_PNV_HOMER "pnv-homer"
+#define PNV_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV_HOMER)
+#define TYPE_PNV8_HOMER TYPE_PNV_HOMER "-POWER8"
+#define PNV8_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV8_HOMER)
+#define TYPE_PNV9_HOMER TYPE_PNV_HOMER "-POWER9"
+#define PNV9_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV9_HOMER)
+
+typedef struct PnvHOMER {
+    DeviceState parent;
+
+    MemoryRegion homer_regs;
+} PnvHOMER;
+
+#define PNV_HOMER_CLASS(klass)   \
+     OBJECT_CLASS_CHECK(PnvHOMERClass, (klass), TYPE_PNV_HOMER)
+#define PNV_HOMER_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(PnvHOMERClass, (obj), TYPE_PNV_HOMER)
+
+typedef struct PnvHOMERClass {
+    DeviceClass parent_class;
+
+    int homer_size;
+    const MemoryRegionOps *homer_ops;
+
+    hwaddr core_max_base;
+} PnvHOMERClass;
+
+
+
+#endif /* PPC_PNV_HOMER_H */
-- 
2.14.5

Re: [Qemu-devel] [PATCH v1 3/3] hw/ppc/pnv_homer: add PowerNV homer device model

[Cédric Le Goater]

On 10/09/2019 09:10, Balamuruhan S wrote:
> add PnvHOMER device model to emulate homer memory access
> for pstate table, occ-sensors, slw, occ static and dynamic
> values for Power8 and Power9 chips. Fix few coding style
> warnings given by checkpatch.pl.

These are valid changes. I would have put them in another patch but 
I guess this is OK.

Looks good. Some comments below,

> 
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
>  hw/ppc/Makefile.objs       |   1 +
>  hw/ppc/pnv.c               |  79 +++++++++++---
>  hw/ppc/pnv_homer.c         | 258 +++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/pnv.h       |   3 +
>  include/hw/ppc/pnv_homer.h |  52 +++++++++
>  5 files changed, 376 insertions(+), 17 deletions(-)
>  create mode 100644 hw/ppc/pnv_homer.c
>  create mode 100644 include/hw/ppc/pnv_homer.h
> 
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index 2c4e1c8de0..580bb4f0dd 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -9,6 +9,7 @@ obj-$(CONFIG_PSERIES) += spapr_tpm_proxy.o
>  obj-$(CONFIG_SPAPR_RNG) +=  spapr_rng.o
>  # IBM PowerNV
>  obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
> +obj-$(CONFIG_POWERNV) += pnv_homer.o
>  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
>  obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o
>  endif
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 80338ffe87..ddddcc9bb6 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -187,7 +187,8 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>  
>      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
>      _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
> -    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
> +    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size",
> +                           cpu->hash64_opts->slb_size)));
>      _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
>      _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
>  
> @@ -200,19 +201,23 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>                             segs, sizeof(segs))));
>      }
>  
> -    /* Advertise VMX/VSX (vector extensions) if available
> +    /*
> +     * Advertise VMX/VSX (vector extensions) if available
>       *   0 / no property == no vector extensions
>       *   1               == VMX / Altivec available
> -     *   2               == VSX available */
> +     *   2               == VSX available
> +     */
>      if (env->insns_flags & PPC_ALTIVEC) {
>          uint32_t vmx = (env->insns_flags2 & PPC2_VSX) ? 2 : 1;
>  
>          _FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", vmx)));
>      }
>  
> -    /* Advertise DFP (Decimal Floating Point) if available
> +    /*
> +     * Advertise DFP (Decimal Floating Point) if available
>       *   0 / no property == no DFP
> -     *   1               == DFP available */
> +     *   1               == DFP available
> +     */
>      if (env->insns_flags2 & PPC2_DFP) {
>          _FDT((fdt_setprop_cell(fdt, offset, "ibm,dfp", 1)));
>      }
> @@ -424,7 +429,8 @@ static int pnv_dt_isa_device(DeviceState *dev, void *opaque)
>      return 0;
>  }
>  
> -/* The default LPC bus of a multichip system is on chip 0. It's
> +/*
> + * The default LPC bus of a multichip system is on chip 0. It's
>   * recognized by the firmware (skiboot) using a "primary" property.
>   */
>  static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
> @@ -442,8 +448,10 @@ static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
>      assert(phandle > 0);
>      _FDT((fdt_setprop_cell(fdt, isa_offset, "phandle", phandle)));
>  
> -    /* ISA devices are not necessarily parented to the ISA bus so we
> -     * can not use object_child_foreach() */
> +    /*
> +     * ISA devices are not necessarily parented to the ISA bus so we
> +     * can not use object_child_foreach()
> +     */
>      qbus_walk_children(BUS(pnv->isa_bus), pnv_dt_isa_device, NULL, NULL, NULL,
>                         &args);
>  }
> @@ -545,7 +553,8 @@ static void pnv_reset(MachineState *machine)
>  
>      qemu_devices_reset();
>  
> -    /* OpenPOWER systems have a BMC, which can be defined on the
> +    /*
> +     * OpenPOWER systems have a BMC, which can be defined on the
>       * command line with:
>       *
>       *   -device ipmi-bmc-sim,id=bmc0
> @@ -705,7 +714,8 @@ static void pnv_init(MachineState *machine)
>  
>          pnv->chips[i] = PNV_CHIP(chip);
>  
> -        /* TODO: put all the memory in one node on chip 0 until we find a
> +        /*
> +         * TODO: put all the memory in one node on chip 0 until we find a
>           * way to specify different ranges for each chip
>           */
>          if (i == 0) {
> @@ -732,8 +742,10 @@ static void pnv_init(MachineState *machine)
>      /* Create an RTC ISA device too */
>      mc146818_rtc_init(pnv->isa_bus, 2000, NULL);
>  
> -    /* OpenPOWER systems use a IPMI SEL Event message to notify the
> -     * host to powerdown */
> +    /*
> +     * OpenPOWER systems use a IPMI SEL Event message to notify the
> +     * host to powerdown
> +     */
>      pnv->powerdown_notifier.notify = pnv_powerdown_notify;
>      qemu_register_powerdown_notifier(&pnv->powerdown_notifier);
>  }
> @@ -803,7 +815,8 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
>      pnv_cpu->intc = obj;
>  }
>  
> -/* Allowed core identifiers on a POWER8 Processor Chip :
> +/*
> + * Allowed core identifiers on a POWER8 Processor Chip :
>   *
>   * <EX0 reserved>
>   *  EX1  - Venice only
> @@ -847,6 +860,11 @@ static void pnv_chip_power8_instance_init(Object *obj)
>                              TYPE_PNV8_OCC, &error_abort, NULL);
>      object_property_add_const_link(OBJECT(&chip8->occ), "psi",
>                                     OBJECT(&chip8->psi), &error_abort);
> +
> +    object_initialize_child(obj, "homer",  &chip8->homer, sizeof(chip8->homer),
> +                            TYPE_PNV8_HOMER, &error_abort, NULL);
> +    object_property_add_const_link(OBJECT(&chip8->homer), "xics",
> +                                   OBJECT(qdev_get_machine()), &error_abort);

Why is there a XICS fabric link ? Does HOMER need to generate interrupts ? 

>  }
>  
>  static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
> @@ -923,8 +941,10 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>                                              (uint64_t) PNV_XSCOM_BASE(chip),
>                                              PNV_XSCOM_LPC_BASE);
>  
> -    /* Interrupt Management Area. This is the memory region holding
> -     * all the Interrupt Control Presenter (ICP) registers */
> +    /*
> +     * Interrupt Management Area. This is the memory region holding
> +     * all the Interrupt Control Presenter (ICP) registers
> +     */
>      pnv_chip_icp_realize(chip8, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> @@ -942,6 +962,16 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>      /* OCC SRAM model */
>      memory_region_add_subregion(get_system_memory(), PNV_OCC_COMMON_AREA(chip),
>                                  &chip8->occ.sram_regs);
> +
> +    /* HOMER */
> +    object_property_set_bool(OBJECT(&chip8->homer), true, "realized",
> +                             &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +    memory_region_add_subregion(get_system_memory(), PNV_HOMER_BASE(chip),
> +                                &chip8->homer.homer_regs);
>  }
>  
>  static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
> @@ -1024,6 +1054,11 @@ static void pnv_chip_power9_instance_init(Object *obj)
>                              TYPE_PNV9_OCC, &error_abort, NULL);
>      object_property_add_const_link(OBJECT(&chip9->occ), "psi",
>                                     OBJECT(&chip9->psi), &error_abort);
> +
> +    object_initialize_child(obj, "homer",  &chip9->homer, sizeof(chip9->homer),
> +                            TYPE_PNV9_HOMER, &error_abort, NULL);
> +    object_property_add_const_link(OBJECT(&chip9->homer), "chip", obj,
> +                                   &error_abort);

Does HOMER need the chip ? It is not used but we might want to in the 
core_max_array() ? 

>  }
>  
>  static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error **errp)
> @@ -1134,6 +1169,16 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>      /* OCC SRAM model */
>      memory_region_add_subregion(get_system_memory(), PNV9_OCC_COMMON_AREA(chip),
>                                  &chip9->occ.sram_regs);
> +
> +    /* HOMER */
> +    object_property_set_bool(OBJECT(&chip9->homer), true, "realized",
> +                             &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +    memory_region_add_subregion(get_system_memory(), PNV9_HOMER_BASE(chip),
> +                                &chip9->homer.homer_regs);
>  }
>  
>  static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
> @@ -1412,8 +1457,8 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
>      mc->init = pnv_init;
>      mc->reset = pnv_reset;
>      mc->max_cpus = MAX_CPUS;
> -    mc->block_default_type = IF_IDE; /* Pnv provides a AHCI device for
> -                                      * storage */
> +    /* Pnv provides a AHCI device for storage */
> +    mc->block_default_type = IF_IDE;
>      mc->no_parallel = 1;
>      mc->default_boot_order = NULL;
>      /*
> diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
> new file mode 100644
> index 0000000000..2886e27176
> --- /dev/null
> +++ b/hw/ppc/pnv_homer.c
> @@ -0,0 +1,258 @@
> +/*
> + * QEMU PowerPC PowerNV Emulation of a few HOMER related registers
> + *
> + * Copyright (c) 2019, IBM Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation.
> + *
> + * This program 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "sysemu/cpus.h"
> +#include "hw/ppc/pnv.h"
> +#include "hw/qdev-core.h"
> +#include "hw/ppc/pnv_homer.h"
> +
> +
> +static bool core_max_array(void *opaque, hwaddr addr)
> +{
> +    PnvHOMER *homer = PNV_HOMER(opaque);
> +    PnvHOMERClass *homer_class = PNV_HOMER_GET_CLASS(homer);
> +
> +    MachineState *ms = MACHINE(qdev_get_machine());

Do you need the whole machine or only the chip ?  

> +
> +    for (int i = 0; i <= ms->smp.cores; i++) {

This declaration might not compile on some compilers used to generate QEMU.

> +        if (addr == (homer_class->core_max_base + i)) {
> +            return true;
> +       }
> +    }
> +    return false;
> +}
> +
> +/* P8 Pstate table */
> +
> +#define PNV8_OCC_PSTATE_VERSION          0x1f8001
> +#define PNV8_OCC_PSTATE_MIN              0x1f8003
> +#define PNV8_OCC_PSTATE_VALID            0x1f8000
> +#define PNV8_OCC_PSTATE_THROTTLE         0x1f8002
> +#define PNV8_OCC_PSTATE_NOM              0x1f8004
> +#define PNV8_OCC_PSTATE_TURBO            0x1f8005
> +#define PNV8_OCC_PSTATE_ULTRA_TURBO      0x1f8006
> +#define PNV8_OCC_PSTATE_DATA             0x1f8008
> +#define PNV8_OCC_PSTATE_ID_ZERO          0x1f8010
> +#define PNV8_OCC_PSTATE_ID_ONE           0x1f8018
> +#define PNV8_OCC_PSTATE_ID_TWO           0x1f8020
> +#define PNV8_OCC_VDD_VOLTAGE_IDENTIFIER  0x1f8012
> +#define PNV8_OCC_VCS_VOLTAGE_IDENTIFIER  0x1f8013
> +#define PNV8_OCC_PSTATE_ZERO_FREQUENCY   0x1f8014
> +#define PNV8_OCC_PSTATE_ONE_FREQUENCY    0x1f801c
> +#define PNV8_OCC_PSTATE_TWO_FREQUENCY    0x1f8024
> +#define PNV8_CORE_MAX_BASE               0x1f8810
> +
> +
> +static uint64_t pnv_power8_homer_read(void *opaque, hwaddr addr,
> +                                      unsigned size)
> +{
> +    switch (addr) {
> +    case PNV8_OCC_PSTATE_VERSION:
> +    case PNV8_OCC_PSTATE_MIN:
> +    case PNV8_OCC_PSTATE_ID_ZERO:
> +        return 0;
> +    case PNV8_OCC_PSTATE_VALID:
> +    case PNV8_OCC_PSTATE_THROTTLE:
> +    case PNV8_OCC_PSTATE_NOM:
> +    case PNV8_OCC_PSTATE_TURBO:
> +    case PNV8_OCC_PSTATE_ID_ONE:
> +    case PNV8_OCC_VDD_VOLTAGE_IDENTIFIER:
> +    case PNV8_OCC_VCS_VOLTAGE_IDENTIFIER:
> +        return 1;
> +    case PNV8_OCC_PSTATE_ULTRA_TURBO:
> +    case PNV8_OCC_PSTATE_ID_TWO:
> +        return 2;
> +    case PNV8_OCC_PSTATE_DATA:
> +        return 0x1000000000000000;
> +    /* P8 frequency for 0, 1, and 2 pstates */
> +    case PNV8_OCC_PSTATE_ZERO_FREQUENCY:
> +    case PNV8_OCC_PSTATE_ONE_FREQUENCY:
> +    case PNV8_OCC_PSTATE_TWO_FREQUENCY:
> +        return 3000;
> +    }
> +    /* pstate table core max array */
> +    if (core_max_array(opaque, addr)) {
> +        return 1;
> +    }
> +    return 0;
> +}
> +
> +static void pnv_power8_homer_write(void *opaque, hwaddr addr,
> +                                   uint64_t val, unsigned size)
> +{
> +    /* callback function defined to homer write */
> +    return;
> +}
> +
> +static const MemoryRegionOps pnv_power8_homer_ops = {
> +    .read = pnv_power8_homer_read,
> +    .write = pnv_power8_homer_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_homer_power8_class_init(ObjectClass *klass, void *data)
> +{
> +    PnvHOMERClass *homer = PNV_HOMER_CLASS(klass);
> +
> +    homer->homer_size = PNV_HOMER_SIZE;
> +    homer->homer_ops = &pnv_power8_homer_ops;
> +    homer->core_max_base = PNV8_CORE_MAX_BASE;
> +}
> +
> +static const TypeInfo pnv_homer_power8_type_info = {
> +    .name          = TYPE_PNV8_HOMER,
> +    .parent        = TYPE_PNV_HOMER,
> +    .instance_size = sizeof(PnvHOMER),
> +    .class_init    = pnv_homer_power8_class_init,
> +};
> +
> +/* P9 Pstate table */
> +

no version ? 

> +#define PNV9_OCC_PSTATE_ID_ZERO          0xe2018
> +#define PNV9_OCC_PSTATE_ID_ONE           0xe2020
> +#define PNV9_OCC_PSTATE_ID_TWO           0xe2028
> +#define PNV9_OCC_PSTATE_DATA             0xe2000
> +#define PNV9_OCC_PSTATE_DATA_AREA        0xe2008
> +#define PNV9_OCC_PSTATE_MIN              0xe2003
> +#define PNV9_OCC_PSTATE_NOM              0xe2004
> +#define PNV9_OCC_PSTATE_TURBO            0xe2005
> +#define PNV9_OCC_PSTATE_ULTRA_TURBO      0xe2818
> +#define PNV9_OCC_MAX_PSTATE_ULTRA_TURBO  0xe2006
> +#define PNV9_OCC_PSTATE_MAJOR_VERSION    0xe2001
> +#define PNV9_OCC_OPAL_RUNTIME_DATA       0xe2b85
> +#define PNV9_CHIP_HOMER_IMAGE_POINTER    0x200008
> +#define PNV9_CHIP_HOMER_BASE             0x0
> +#define PNV9_OCC_PSTATE_ZERO_FREQUENCY   0xe201c
> +#define PNV9_OCC_PSTATE_ONE_FREQUENCY    0xe2024
> +#define PNV9_OCC_PSTATE_TWO_FREQUENCY    0xe202c
> +#define PNV9_OCC_ROLE_MASTER_OR_SLAVE    0xe2002
> +#define PNV9_CORE_MAX_BASE               0xe2819
> +
> +
> +static uint64_t pnv_power9_homer_read(void *opaque, hwaddr addr,
> +                                      unsigned size)
> +{
> +    switch (addr) {
> +    case PNV9_OCC_MAX_PSTATE_ULTRA_TURBO:
> +    case PNV9_OCC_PSTATE_ID_ZERO:
> +        return 0;
> +    case PNV9_OCC_PSTATE_DATA:
> +    case PNV9_OCC_ROLE_MASTER_OR_SLAVE:
> +    case PNV9_OCC_PSTATE_NOM:
> +    case PNV9_OCC_PSTATE_TURBO:
> +    case PNV9_OCC_PSTATE_ID_ONE:
> +    case PNV9_OCC_PSTATE_ULTRA_TURBO:
> +    case PNV9_OCC_OPAL_RUNTIME_DATA:
> +        return 1;
> +    case PNV9_OCC_PSTATE_MIN:
> +    case PNV9_OCC_PSTATE_ID_TWO:
> +        return 2;
> +
> +    /* 3000 khz frequency for 0, 1, and 2 pstates */
> +    case PNV9_OCC_PSTATE_ZERO_FREQUENCY:
> +    case PNV9_OCC_PSTATE_ONE_FREQUENCY:
> +    case PNV9_OCC_PSTATE_TWO_FREQUENCY:
> +        return 3000;
> +    case PNV9_OCC_PSTATE_MAJOR_VERSION:
> +        return 0x90;
> +    case PNV9_CHIP_HOMER_BASE:
> +    case PNV9_OCC_PSTATE_DATA_AREA:
> +    case PNV9_CHIP_HOMER_IMAGE_POINTER:
> +        return 0x1000000000000000;
> +    }
> +    /* pstate table core max array */
> +    if (core_max_array(opaque, addr)) {
> +        return 1;
> +    }
> +    return 0;
> +}
> +
> +static void pnv_power9_homer_write(void *opaque, hwaddr addr,
> +                                   uint64_t val, unsigned size)
> +{
> +    /* callback function defined to homer write */
> +    return;
> +}
> +
> +static const MemoryRegionOps pnv_power9_homer_ops = {
> +    .read = pnv_power9_homer_read,
> +    .write = pnv_power9_homer_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_homer_power9_class_init(ObjectClass *klass, void *data)
> +{
> +    PnvHOMERClass *homer = PNV_HOMER_CLASS(klass);
> +
> +    homer->homer_size = PNV9_HOMER_SIZE;
> +    homer->homer_ops = &pnv_power9_homer_ops;
> +    homer->core_max_base = PNV9_CORE_MAX_BASE;
> +}
> +
> +static const TypeInfo pnv_homer_power9_type_info = {
> +    .name          = TYPE_PNV9_HOMER,
> +    .parent        = TYPE_PNV_HOMER,
> +    .instance_size = sizeof(PnvHOMER),
> +    .class_init    = pnv_homer_power9_class_init,
> +};
> +
> +static void pnv_homer_realize(DeviceState *dev, Error **errp)
> +{
> +    PnvHOMER *homer = PNV_HOMER(dev);
> +    PnvHOMERClass *homer_class = PNV_HOMER_GET_CLASS(homer)
hmrc would be shorter.

> +
> +    /* homer region */
> +    memory_region_init_io(&homer->homer_regs, OBJECT(dev),
> +                          homer_class->homer_ops, homer, "homer-main-memory",
> +                          homer_class->homer_size);
> +}
> +
> +static void pnv_homer_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = pnv_homer_realize;
> +    dc->desc = "PowerNV HOMER Memory";
> +}
> +
> +static const TypeInfo pnv_homer_type_info = {
> +    .name          = TYPE_PNV_HOMER,
> +    .parent        = TYPE_DEVICE,
> +    .instance_size = sizeof(PnvHOMER),
> +    .class_init    = pnv_homer_class_init,
> +    .class_size    = sizeof(PnvHOMERClass),
> +    .abstract      = true,
> +};
> +
> +static void pnv_homer_register_types(void)
> +{
> +    type_register_static(&pnv_homer_type_info);
> +    type_register_static(&pnv_homer_power8_type_info);
> +    type_register_static(&pnv_homer_power9_type_info);
> +}
> +
> +type_init(pnv_homer_register_types);
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 63a4b7b6a7..321d4ca967 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -26,6 +26,7 @@
>  #include "hw/ppc/pnv_lpc.h"
>  #include "hw/ppc/pnv_psi.h"
>  #include "hw/ppc/pnv_occ.h"
> +#include "hw/ppc/pnv_homer.h"
>  #include "hw/ppc/pnv_xive.h"
>  #include "hw/ppc/pnv_core.h"
>  
> @@ -76,6 +77,7 @@ typedef struct Pnv8Chip {
>      PnvLpcController lpc;
>      Pnv8Psi      psi;
>      PnvOCC       occ;
> +    PnvHOMER     homer;
>  } Pnv8Chip;
>  
>  #define TYPE_PNV9_CHIP "pnv9-chip"
> @@ -90,6 +92,7 @@ typedef struct Pnv9Chip {
>      Pnv9Psi      psi;
>      PnvLpcController lpc;
>      PnvOCC       occ;
> +    PnvHOMER     homer;
>  
>      uint32_t     nr_quads;
>      PnvQuad      *quads;
> diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h
> new file mode 100644
> index 0000000000..3b666a6245
> --- /dev/null
> +++ b/include/hw/ppc/pnv_homer.h
> @@ -0,0 +1,52 @@
> +/*
> + * QEMU PowerPC PowerNV Emulation of a few HOMER related registers
> + *
> + * Copyright (c) 2019, 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_HOMER_H
> +#define PPC_PNV_HOMER_H
> +
> +#define TYPE_PNV_HOMER "pnv-homer"
> +#define PNV_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV_HOMER)
> +#define TYPE_PNV8_HOMER TYPE_PNV_HOMER "-POWER8"
> +#define PNV8_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV8_HOMER)
> +#define TYPE_PNV9_HOMER TYPE_PNV_HOMER "-POWER9"
> +#define PNV9_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV9_HOMER)
> +
> +typedef struct PnvHOMER {
> +    DeviceState parent;
> +
> +    MemoryRegion homer_regs;

the homer_ prefix is not useful.

> +} PnvHOMER;
> +
> +#define PNV_HOMER_CLASS(klass)   \
> +     OBJECT_CLASS_CHECK(PnvHOMERClass, (klass), TYPE_PNV_HOMER)
> +#define PNV_HOMER_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(PnvHOMERClass, (obj), TYPE_PNV_HOMER)
> +
> +typedef struct PnvHOMERClass {
> +    DeviceClass parent_class;
> +
> +    int homer_size;
> +    const MemoryRegionOps *homer_ops;


the homer_ prefix is not useful.

> +
> +    hwaddr core_max_base;
> +} PnvHOMERClass;
> +
> +
> +
> +#endif /* PPC_PNV_HOMER_H */
> 

Re: [Qemu-devel] [PATCH v1 3/3] hw/ppc/pnv_homer: add PowerNV homer device model

[Balamuruhan S]

On Tue, Sep 10, 2019 at 09:46:16AM +0200, Cédric Le Goater wrote:
> On 10/09/2019 09:10, Balamuruhan S wrote:
> > add PnvHOMER device model to emulate homer memory access
> > for pstate table, occ-sensors, slw, occ static and dynamic
> > values for Power8 and Power9 chips. Fix few coding style
> > warnings given by checkpatch.pl.
> 
> These are valid changes. I would have put them in another patch but 
> I guess this is OK.
> 
> Looks good. Some comments below,

Thanks Cedric for review, I can have them in separate commit in the next
version by fixing some of your review comments below,

> 
> > 
> > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > ---
> >  hw/ppc/Makefile.objs       |   1 +
> >  hw/ppc/pnv.c               |  79 +++++++++++---
> >  hw/ppc/pnv_homer.c         | 258 +++++++++++++++++++++++++++++++++++++++++++++
> >  include/hw/ppc/pnv.h       |   3 +
> >  include/hw/ppc/pnv_homer.h |  52 +++++++++
> >  5 files changed, 376 insertions(+), 17 deletions(-)
> >  create mode 100644 hw/ppc/pnv_homer.c
> >  create mode 100644 include/hw/ppc/pnv_homer.h
> > 
> > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> > index 2c4e1c8de0..580bb4f0dd 100644
> > --- a/hw/ppc/Makefile.objs
> > +++ b/hw/ppc/Makefile.objs
> > @@ -9,6 +9,7 @@ obj-$(CONFIG_PSERIES) += spapr_tpm_proxy.o
> >  obj-$(CONFIG_SPAPR_RNG) +=  spapr_rng.o
> >  # IBM PowerNV
> >  obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
> > +obj-$(CONFIG_POWERNV) += pnv_homer.o
> >  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> >  obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o
> >  endif
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 80338ffe87..ddddcc9bb6 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -187,7 +187,8 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
> >  
> >      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
> >      _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
> > -    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
> > +    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size",
> > +                           cpu->hash64_opts->slb_size)));
> >      _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
> >      _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
> >  
> > @@ -200,19 +201,23 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
> >                             segs, sizeof(segs))));
> >      }
> >  
> > -    /* Advertise VMX/VSX (vector extensions) if available
> > +    /*
> > +     * Advertise VMX/VSX (vector extensions) if available
> >       *   0 / no property == no vector extensions
> >       *   1               == VMX / Altivec available
> > -     *   2               == VSX available */
> > +     *   2               == VSX available
> > +     */
> >      if (env->insns_flags & PPC_ALTIVEC) {
> >          uint32_t vmx = (env->insns_flags2 & PPC2_VSX) ? 2 : 1;
> >  
> >          _FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", vmx)));
> >      }
> >  
> > -    /* Advertise DFP (Decimal Floating Point) if available
> > +    /*
> > +     * Advertise DFP (Decimal Floating Point) if available
> >       *   0 / no property == no DFP
> > -     *   1               == DFP available */
> > +     *   1               == DFP available
> > +     */
> >      if (env->insns_flags2 & PPC2_DFP) {
> >          _FDT((fdt_setprop_cell(fdt, offset, "ibm,dfp", 1)));
> >      }
> > @@ -424,7 +429,8 @@ static int pnv_dt_isa_device(DeviceState *dev, void *opaque)
> >      return 0;
> >  }
> >  
> > -/* The default LPC bus of a multichip system is on chip 0. It's
> > +/*
> > + * The default LPC bus of a multichip system is on chip 0. It's
> >   * recognized by the firmware (skiboot) using a "primary" property.
> >   */
> >  static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
> > @@ -442,8 +448,10 @@ static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
> >      assert(phandle > 0);
> >      _FDT((fdt_setprop_cell(fdt, isa_offset, "phandle", phandle)));
> >  
> > -    /* ISA devices are not necessarily parented to the ISA bus so we
> > -     * can not use object_child_foreach() */
> > +    /*
> > +     * ISA devices are not necessarily parented to the ISA bus so we
> > +     * can not use object_child_foreach()
> > +     */
> >      qbus_walk_children(BUS(pnv->isa_bus), pnv_dt_isa_device, NULL, NULL, NULL,
> >                         &args);
> >  }
> > @@ -545,7 +553,8 @@ static void pnv_reset(MachineState *machine)
> >  
> >      qemu_devices_reset();
> >  
> > -    /* OpenPOWER systems have a BMC, which can be defined on the
> > +    /*
> > +     * OpenPOWER systems have a BMC, which can be defined on the
> >       * command line with:
> >       *
> >       *   -device ipmi-bmc-sim,id=bmc0
> > @@ -705,7 +714,8 @@ static void pnv_init(MachineState *machine)
> >  
> >          pnv->chips[i] = PNV_CHIP(chip);
> >  
> > -        /* TODO: put all the memory in one node on chip 0 until we find a
> > +        /*
> > +         * TODO: put all the memory in one node on chip 0 until we find a
> >           * way to specify different ranges for each chip
> >           */
> >          if (i == 0) {
> > @@ -732,8 +742,10 @@ static void pnv_init(MachineState *machine)
> >      /* Create an RTC ISA device too */
> >      mc146818_rtc_init(pnv->isa_bus, 2000, NULL);
> >  
> > -    /* OpenPOWER systems use a IPMI SEL Event message to notify the
> > -     * host to powerdown */
> > +    /*
> > +     * OpenPOWER systems use a IPMI SEL Event message to notify the
> > +     * host to powerdown
> > +     */
> >      pnv->powerdown_notifier.notify = pnv_powerdown_notify;
> >      qemu_register_powerdown_notifier(&pnv->powerdown_notifier);
> >  }
> > @@ -803,7 +815,8 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
> >      pnv_cpu->intc = obj;
> >  }
> >  
> > -/* Allowed core identifiers on a POWER8 Processor Chip :
> > +/*
> > + * Allowed core identifiers on a POWER8 Processor Chip :
> >   *
> >   * <EX0 reserved>
> >   *  EX1  - Venice only
> > @@ -847,6 +860,11 @@ static void pnv_chip_power8_instance_init(Object *obj)
> >                              TYPE_PNV8_OCC, &error_abort, NULL);
> >      object_property_add_const_link(OBJECT(&chip8->occ), "psi",
> >                                     OBJECT(&chip8->psi), &error_abort);
> > +
> > +    object_initialize_child(obj, "homer",  &chip8->homer, sizeof(chip8->homer),
> > +                            TYPE_PNV8_HOMER, &error_abort, NULL);
> > +    object_property_add_const_link(OBJECT(&chip8->homer), "xics",
> > +                                   OBJECT(qdev_get_machine()), &error_abort);
> 
> Why is there a XICS fabric link ? Does HOMER need to generate interrupts ? 

I am sorry, it is not required, I will remove it.

> 
> >  }
> >  
> >  static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
> > @@ -923,8 +941,10 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
> >                                              (uint64_t) PNV_XSCOM_BASE(chip),
> >                                              PNV_XSCOM_LPC_BASE);
> >  
> > -    /* Interrupt Management Area. This is the memory region holding
> > -     * all the Interrupt Control Presenter (ICP) registers */
> > +    /*
> > +     * Interrupt Management Area. This is the memory region holding
> > +     * all the Interrupt Control Presenter (ICP) registers
> > +     */
> >      pnv_chip_icp_realize(chip8, &local_err);
> >      if (local_err) {
> >          error_propagate(errp, local_err);
> > @@ -942,6 +962,16 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
> >      /* OCC SRAM model */
> >      memory_region_add_subregion(get_system_memory(), PNV_OCC_COMMON_AREA(chip),
> >                                  &chip8->occ.sram_regs);
> > +
> > +    /* HOMER */
> > +    object_property_set_bool(OBJECT(&chip8->homer), true, "realized",
> > +                             &local_err);
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        return;
> > +    }
> > +    memory_region_add_subregion(get_system_memory(), PNV_HOMER_BASE(chip),
> > +                                &chip8->homer.homer_regs);
> >  }
> >  
> >  static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
> > @@ -1024,6 +1054,11 @@ static void pnv_chip_power9_instance_init(Object *obj)
> >                              TYPE_PNV9_OCC, &error_abort, NULL);
> >      object_property_add_const_link(OBJECT(&chip9->occ), "psi",
> >                                     OBJECT(&chip9->psi), &error_abort);
> > +
> > +    object_initialize_child(obj, "homer",  &chip9->homer, sizeof(chip9->homer),
> > +                            TYPE_PNV9_HOMER, &error_abort, NULL);
> > +    object_property_add_const_link(OBJECT(&chip9->homer), "chip", obj,
> > +                                   &error_abort);
> 
> Does HOMER need the chip ? It is not used but we might want to in the 
> core_max_array() ? 

sorry, no it is not required, I will remove it.

> 
> >  }
> >  
> >  static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error **errp)
> > @@ -1134,6 +1169,16 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> >      /* OCC SRAM model */
> >      memory_region_add_subregion(get_system_memory(), PNV9_OCC_COMMON_AREA(chip),
> >                                  &chip9->occ.sram_regs);
> > +
> > +    /* HOMER */
> > +    object_property_set_bool(OBJECT(&chip9->homer), true, "realized",
> > +                             &local_err);
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        return;
> > +    }
> > +    memory_region_add_subregion(get_system_memory(), PNV9_HOMER_BASE(chip),
> > +                                &chip9->homer.homer_regs);
> >  }
> >  
> >  static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
> > @@ -1412,8 +1457,8 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
> >      mc->init = pnv_init;
> >      mc->reset = pnv_reset;
> >      mc->max_cpus = MAX_CPUS;
> > -    mc->block_default_type = IF_IDE; /* Pnv provides a AHCI device for
> > -                                      * storage */
> > +    /* Pnv provides a AHCI device for storage */
> > +    mc->block_default_type = IF_IDE;
> >      mc->no_parallel = 1;
> >      mc->default_boot_order = NULL;
> >      /*
> > diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
> > new file mode 100644
> > index 0000000000..2886e27176
> > --- /dev/null
> > +++ b/hw/ppc/pnv_homer.c
> > @@ -0,0 +1,258 @@
> > +/*
> > + * QEMU PowerPC PowerNV Emulation of a few HOMER related registers
> > + *
> > + * Copyright (c) 2019, IBM Corporation.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License, version 2, as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program 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 General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "sysemu/cpus.h"
> > +#include "hw/ppc/pnv.h"
> > +#include "hw/qdev-core.h"
> > +#include "hw/ppc/pnv_homer.h"
> > +
> > +
> > +static bool core_max_array(void *opaque, hwaddr addr)
> > +{
> > +    PnvHOMER *homer = PNV_HOMER(opaque);
> > +    PnvHOMERClass *homer_class = PNV_HOMER_GET_CLASS(homer);
> > +
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> 
> Do you need the whole machine or only the chip ?  

yes, I see it is for active cores in the chip, I can use `nr_cores`
defined in PnvChip.

> 
> > +
> > +    for (int i = 0; i <= ms->smp.cores; i++) {
> 
> This declaration might not compile on some compilers used to generate QEMU.

I will declare at first and use it, Thanks for correcting it.

> 
> > +        if (addr == (homer_class->core_max_base + i)) {
> > +            return true;
> > +       }
> > +    }
> > +    return false;
> > +}
> > +
> > +/* P8 Pstate table */
> > +
> > +#define PNV8_OCC_PSTATE_VERSION          0x1f8001
> > +#define PNV8_OCC_PSTATE_MIN              0x1f8003
> > +#define PNV8_OCC_PSTATE_VALID            0x1f8000
> > +#define PNV8_OCC_PSTATE_THROTTLE         0x1f8002
> > +#define PNV8_OCC_PSTATE_NOM              0x1f8004
> > +#define PNV8_OCC_PSTATE_TURBO            0x1f8005
> > +#define PNV8_OCC_PSTATE_ULTRA_TURBO      0x1f8006
> > +#define PNV8_OCC_PSTATE_DATA             0x1f8008
> > +#define PNV8_OCC_PSTATE_ID_ZERO          0x1f8010
> > +#define PNV8_OCC_PSTATE_ID_ONE           0x1f8018
> > +#define PNV8_OCC_PSTATE_ID_TWO           0x1f8020
> > +#define PNV8_OCC_VDD_VOLTAGE_IDENTIFIER  0x1f8012
> > +#define PNV8_OCC_VCS_VOLTAGE_IDENTIFIER  0x1f8013
> > +#define PNV8_OCC_PSTATE_ZERO_FREQUENCY   0x1f8014
> > +#define PNV8_OCC_PSTATE_ONE_FREQUENCY    0x1f801c
> > +#define PNV8_OCC_PSTATE_TWO_FREQUENCY    0x1f8024
> > +#define PNV8_CORE_MAX_BASE               0x1f8810
> > +
> > +
> > +static uint64_t pnv_power8_homer_read(void *opaque, hwaddr addr,
> > +                                      unsigned size)
> > +{
> > +    switch (addr) {
> > +    case PNV8_OCC_PSTATE_VERSION:
> > +    case PNV8_OCC_PSTATE_MIN:
> > +    case PNV8_OCC_PSTATE_ID_ZERO:
> > +        return 0;
> > +    case PNV8_OCC_PSTATE_VALID:
> > +    case PNV8_OCC_PSTATE_THROTTLE:
> > +    case PNV8_OCC_PSTATE_NOM:
> > +    case PNV8_OCC_PSTATE_TURBO:
> > +    case PNV8_OCC_PSTATE_ID_ONE:
> > +    case PNV8_OCC_VDD_VOLTAGE_IDENTIFIER:
> > +    case PNV8_OCC_VCS_VOLTAGE_IDENTIFIER:
> > +        return 1;
> > +    case PNV8_OCC_PSTATE_ULTRA_TURBO:
> > +    case PNV8_OCC_PSTATE_ID_TWO:
> > +        return 2;
> > +    case PNV8_OCC_PSTATE_DATA:
> > +        return 0x1000000000000000;
> > +    /* P8 frequency for 0, 1, and 2 pstates */
> > +    case PNV8_OCC_PSTATE_ZERO_FREQUENCY:
> > +    case PNV8_OCC_PSTATE_ONE_FREQUENCY:
> > +    case PNV8_OCC_PSTATE_TWO_FREQUENCY:
> > +        return 3000;
> > +    }
> > +    /* pstate table core max array */
> > +    if (core_max_array(opaque, addr)) {
> > +        return 1;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static void pnv_power8_homer_write(void *opaque, hwaddr addr,
> > +                                   uint64_t val, unsigned size)
> > +{
> > +    /* callback function defined to homer write */
> > +    return;
> > +}
> > +
> > +static const MemoryRegionOps pnv_power8_homer_ops = {
> > +    .read = pnv_power8_homer_read,
> > +    .write = pnv_power8_homer_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_homer_power8_class_init(ObjectClass *klass, void *data)
> > +{
> > +    PnvHOMERClass *homer = PNV_HOMER_CLASS(klass);
> > +
> > +    homer->homer_size = PNV_HOMER_SIZE;
> > +    homer->homer_ops = &pnv_power8_homer_ops;
> > +    homer->core_max_base = PNV8_CORE_MAX_BASE;
> > +}
> > +
> > +static const TypeInfo pnv_homer_power8_type_info = {
> > +    .name          = TYPE_PNV8_HOMER,
> > +    .parent        = TYPE_PNV_HOMER,
> > +    .instance_size = sizeof(PnvHOMER),
> > +    .class_init    = pnv_homer_power8_class_init,
> > +};
> > +
> > +/* P9 Pstate table */
> > +
> 
> no version ? 

PNV9_OCC_PSTATE_MAJOR_VERSION is the P9 pstate version.

> 
> > +#define PNV9_OCC_PSTATE_ID_ZERO          0xe2018
> > +#define PNV9_OCC_PSTATE_ID_ONE           0xe2020
> > +#define PNV9_OCC_PSTATE_ID_TWO           0xe2028
> > +#define PNV9_OCC_PSTATE_DATA             0xe2000
> > +#define PNV9_OCC_PSTATE_DATA_AREA        0xe2008
> > +#define PNV9_OCC_PSTATE_MIN              0xe2003
> > +#define PNV9_OCC_PSTATE_NOM              0xe2004
> > +#define PNV9_OCC_PSTATE_TURBO            0xe2005
> > +#define PNV9_OCC_PSTATE_ULTRA_TURBO      0xe2818
> > +#define PNV9_OCC_MAX_PSTATE_ULTRA_TURBO  0xe2006
> > +#define PNV9_OCC_PSTATE_MAJOR_VERSION    0xe2001
> > +#define PNV9_OCC_OPAL_RUNTIME_DATA       0xe2b85
> > +#define PNV9_CHIP_HOMER_IMAGE_POINTER    0x200008
> > +#define PNV9_CHIP_HOMER_BASE             0x0
> > +#define PNV9_OCC_PSTATE_ZERO_FREQUENCY   0xe201c
> > +#define PNV9_OCC_PSTATE_ONE_FREQUENCY    0xe2024
> > +#define PNV9_OCC_PSTATE_TWO_FREQUENCY    0xe202c
> > +#define PNV9_OCC_ROLE_MASTER_OR_SLAVE    0xe2002
> > +#define PNV9_CORE_MAX_BASE               0xe2819
> > +
> > +
> > +static uint64_t pnv_power9_homer_read(void *opaque, hwaddr addr,
> > +                                      unsigned size)
> > +{
> > +    switch (addr) {
> > +    case PNV9_OCC_MAX_PSTATE_ULTRA_TURBO:
> > +    case PNV9_OCC_PSTATE_ID_ZERO:
> > +        return 0;
> > +    case PNV9_OCC_PSTATE_DATA:
> > +    case PNV9_OCC_ROLE_MASTER_OR_SLAVE:
> > +    case PNV9_OCC_PSTATE_NOM:
> > +    case PNV9_OCC_PSTATE_TURBO:
> > +    case PNV9_OCC_PSTATE_ID_ONE:
> > +    case PNV9_OCC_PSTATE_ULTRA_TURBO:
> > +    case PNV9_OCC_OPAL_RUNTIME_DATA:
> > +        return 1;
> > +    case PNV9_OCC_PSTATE_MIN:
> > +    case PNV9_OCC_PSTATE_ID_TWO:
> > +        return 2;
> > +
> > +    /* 3000 khz frequency for 0, 1, and 2 pstates */
> > +    case PNV9_OCC_PSTATE_ZERO_FREQUENCY:
> > +    case PNV9_OCC_PSTATE_ONE_FREQUENCY:
> > +    case PNV9_OCC_PSTATE_TWO_FREQUENCY:
> > +        return 3000;
> > +    case PNV9_OCC_PSTATE_MAJOR_VERSION:
> > +        return 0x90;
> > +    case PNV9_CHIP_HOMER_BASE:
> > +    case PNV9_OCC_PSTATE_DATA_AREA:
> > +    case PNV9_CHIP_HOMER_IMAGE_POINTER:
> > +        return 0x1000000000000000;
> > +    }
> > +    /* pstate table core max array */
> > +    if (core_max_array(opaque, addr)) {
> > +        return 1;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static void pnv_power9_homer_write(void *opaque, hwaddr addr,
> > +                                   uint64_t val, unsigned size)
> > +{
> > +    /* callback function defined to homer write */
> > +    return;
> > +}
> > +
> > +static const MemoryRegionOps pnv_power9_homer_ops = {
> > +    .read = pnv_power9_homer_read,
> > +    .write = pnv_power9_homer_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_homer_power9_class_init(ObjectClass *klass, void *data)
> > +{
> > +    PnvHOMERClass *homer = PNV_HOMER_CLASS(klass);
> > +
> > +    homer->homer_size = PNV9_HOMER_SIZE;
> > +    homer->homer_ops = &pnv_power9_homer_ops;
> > +    homer->core_max_base = PNV9_CORE_MAX_BASE;
> > +}
> > +
> > +static const TypeInfo pnv_homer_power9_type_info = {
> > +    .name          = TYPE_PNV9_HOMER,
> > +    .parent        = TYPE_PNV_HOMER,
> > +    .instance_size = sizeof(PnvHOMER),
> > +    .class_init    = pnv_homer_power9_class_init,
> > +};
> > +
> > +static void pnv_homer_realize(DeviceState *dev, Error **errp)
> > +{
> > +    PnvHOMER *homer = PNV_HOMER(dev);
> > +    PnvHOMERClass *homer_class = PNV_HOMER_GET_CLASS(homer)
> hmrc would be shorter.

sure, will fix it.

> 
> > +
> > +    /* homer region */
> > +    memory_region_init_io(&homer->homer_regs, OBJECT(dev),
> > +                          homer_class->homer_ops, homer, "homer-main-memory",
> > +                          homer_class->homer_size);
> > +}
> > +
> > +static void pnv_homer_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->realize = pnv_homer_realize;
> > +    dc->desc = "PowerNV HOMER Memory";
> > +}
> > +
> > +static const TypeInfo pnv_homer_type_info = {
> > +    .name          = TYPE_PNV_HOMER,
> > +    .parent        = TYPE_DEVICE,
> > +    .instance_size = sizeof(PnvHOMER),
> > +    .class_init    = pnv_homer_class_init,
> > +    .class_size    = sizeof(PnvHOMERClass),
> > +    .abstract      = true,
> > +};
> > +
> > +static void pnv_homer_register_types(void)
> > +{
> > +    type_register_static(&pnv_homer_type_info);
> > +    type_register_static(&pnv_homer_power8_type_info);
> > +    type_register_static(&pnv_homer_power9_type_info);
> > +}
> > +
> > +type_init(pnv_homer_register_types);
> > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> > index 63a4b7b6a7..321d4ca967 100644
> > --- a/include/hw/ppc/pnv.h
> > +++ b/include/hw/ppc/pnv.h
> > @@ -26,6 +26,7 @@
> >  #include "hw/ppc/pnv_lpc.h"
> >  #include "hw/ppc/pnv_psi.h"
> >  #include "hw/ppc/pnv_occ.h"
> > +#include "hw/ppc/pnv_homer.h"
> >  #include "hw/ppc/pnv_xive.h"
> >  #include "hw/ppc/pnv_core.h"
> >  
> > @@ -76,6 +77,7 @@ typedef struct Pnv8Chip {
> >      PnvLpcController lpc;
> >      Pnv8Psi      psi;
> >      PnvOCC       occ;
> > +    PnvHOMER     homer;
> >  } Pnv8Chip;
> >  
> >  #define TYPE_PNV9_CHIP "pnv9-chip"
> > @@ -90,6 +92,7 @@ typedef struct Pnv9Chip {
> >      Pnv9Psi      psi;
> >      PnvLpcController lpc;
> >      PnvOCC       occ;
> > +    PnvHOMER     homer;
> >  
> >      uint32_t     nr_quads;
> >      PnvQuad      *quads;
> > diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h
> > new file mode 100644
> > index 0000000000..3b666a6245
> > --- /dev/null
> > +++ b/include/hw/ppc/pnv_homer.h
> > @@ -0,0 +1,52 @@
> > +/*
> > + * QEMU PowerPC PowerNV Emulation of a few HOMER related registers
> > + *
> > + * Copyright (c) 2019, 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_HOMER_H
> > +#define PPC_PNV_HOMER_H
> > +
> > +#define TYPE_PNV_HOMER "pnv-homer"
> > +#define PNV_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV_HOMER)
> > +#define TYPE_PNV8_HOMER TYPE_PNV_HOMER "-POWER8"
> > +#define PNV8_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV8_HOMER)
> > +#define TYPE_PNV9_HOMER TYPE_PNV_HOMER "-POWER9"
> > +#define PNV9_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV9_HOMER)
> > +
> > +typedef struct PnvHOMER {
> > +    DeviceState parent;
> > +
> > +    MemoryRegion homer_regs;
> 
> the homer_ prefix is not useful.

okay, I will change it to `hregs`.

> 
> > +} PnvHOMER;
> > +
> > +#define PNV_HOMER_CLASS(klass)   \
> > +     OBJECT_CLASS_CHECK(PnvHOMERClass, (klass), TYPE_PNV_HOMER)
> > +#define PNV_HOMER_GET_CLASS(obj) \
> > +     OBJECT_GET_CLASS(PnvHOMERClass, (obj), TYPE_PNV_HOMER)
> > +
> > +typedef struct PnvHOMERClass {
> > +    DeviceClass parent_class;
> > +
> > +    int homer_size;
> > +    const MemoryRegionOps *homer_ops;
> 
> 
> the homer_ prefix is not useful.

I will change it to `hregs`

> 
> > +
> > +    hwaddr core_max_base;
> > +} PnvHOMERClass;
> > +
> > +
> > +
> > +#endif /* PPC_PNV_HOMER_H */
> > 
> 

Re: [Qemu-devel] [PATCH v1 3/3] hw/ppc/pnv_homer: add PowerNV homer device model

[Cédric Le Goater]

>>> +
>>> +    object_initialize_child(obj, "homer",  &chip9->homer, sizeof(chip9->homer),
>>> +                            TYPE_PNV9_HOMER, &error_abort, NULL);
>>> +    object_property_add_const_link(OBJECT(&chip9->homer), "chip", obj,
>>> +                                   &error_abort);
>>
>> Does HOMER need the chip ? It is not used but we might want to in the 
>> core_max_array() ? 
> 
> sorry, no it is not required, I will remove it.

It seems you will need the chip in core_max_array(). I would keep it. 
See below,

[ ... ] 

>>> +static bool core_max_array(void *opaque, hwaddr addr)

Please change the 'void *opaque' function parameter in 'PnvHOMER *homer'

>>> +{
>>> +    PnvHOMER *homer = PNV_HOMER(opaque);
>>> +    PnvHOMERClass *homer_class = PNV_HOMER_GET_CLASS(homer);
>>> +
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>
>> Do you need the whole machine or only the chip ?  
> 
> yes, I see it is for active cores in the chip, I can use `nr_cores`
> defined in PnvChip.


If you keep the QOM link above, you can grab it in the realize handler of
the HOMER model with :  

    Object *obj;
    Error *local_err = NULL;

    obj = object_property_get_link(OBJECT(dev), "chip", &local_err);
    if (!obj) {
        error_propagate(errp, local_err);
        error_prepend(errp, "required link 'chip' not found: ");
        return;
    }

    homer->chip = PNV_CHIP(obj);

[ ... ] 

>>> +
>>> +/* P9 Pstate table */
>>> +
>>
>> no version ? 
> 
> PNV9_OCC_PSTATE_MAJOR_VERSION is the P9 pstate version.

why isn't it in the switch statement below ? 

[ ... ] 
 
>>> +typedef struct PnvHOMER {
>>> +    DeviceState parent;
>>> +
>>> +    MemoryRegion homer_regs;
>>
>> the homer_ prefix is not useful.
> 
> okay, I will change it to `hregs`.

I would just remove the prefix

Thanks,

C. 

Re: [Qemu-devel] [PATCH v1 3/3] hw/ppc/pnv_homer: add PowerNV homer device model

[Balamuruhan S]

On Tue, Sep 10, 2019 at 01:00:54PM +0200, Cédric Le Goater wrote:
> >>> +
> >>> +    object_initialize_child(obj, "homer",  &chip9->homer, sizeof(chip9->homer),
> >>> +                            TYPE_PNV9_HOMER, &error_abort, NULL);
> >>> +    object_property_add_const_link(OBJECT(&chip9->homer), "chip", obj,
> >>> +                                   &error_abort);
> >>
> >> Does HOMER need the chip ? It is not used but we might want to in the 
> >> core_max_array() ? 
> > 
> > sorry, no it is not required, I will remove it.
> 
> It seems you will need the chip in core_max_array(). I would keep it. 
> See below,

Thanks Cedric, I was not aware of how to use it,

> 
> [ ... ] 
> 
> >>> +static bool core_max_array(void *opaque, hwaddr addr)
> 
> Please change the 'void *opaque' function parameter in 'PnvHOMER *homer'
> 
> >>> +{
> >>> +    PnvHOMER *homer = PNV_HOMER(opaque);
> >>> +    PnvHOMERClass *homer_class = PNV_HOMER_GET_CLASS(homer);
> >>> +
> >>> +    MachineState *ms = MACHINE(qdev_get_machine());
> >>
> >> Do you need the whole machine or only the chip ?  
> > 
> > yes, I see it is for active cores in the chip, I can use `nr_cores`
> > defined in PnvChip.
> 
> 
> If you keep the QOM link above, you can grab it in the realize handler of
> the HOMER model with :  
> 
>     Object *obj;
>     Error *local_err = NULL;
> 
>     obj = object_property_get_link(OBJECT(dev), "chip", &local_err);
>     if (!obj) {
>         error_propagate(errp, local_err);
>         error_prepend(errp, "required link 'chip' not found: ");
>         return;
>     }
> 
>     homer->chip = PNV_CHIP(obj);

sure, :+1:

> 
> [ ... ] 
> 
> >>> +
> >>> +/* P9 Pstate table */
> >>> +
> >>
> >> no version ? 
> > 
> > PNV9_OCC_PSTATE_MAJOR_VERSION is the P9 pstate version.
> 
> why isn't it in the switch statement below ? 

it is there in the switch statement,

::
+    case PNV9_OCC_PSTATE_MAJOR_VERSION:
+        return 0x90;
::

> 
> [ ... ] 
>  
> >>> +typedef struct PnvHOMER {
> >>> +    DeviceState parent;
> >>> +
> >>> +    MemoryRegion homer_regs;
> >>
> >> the homer_ prefix is not useful.
> > 
> > okay, I will change it to `hregs`.
> 
> I would just remove the prefix

will make the change as suggested by removing the prefix.

-- Bala

> 
> Thanks,
> 
> C. 
> 

Re: [Qemu-devel] [PATCH v1 3/3] hw/ppc/pnv_homer: add PowerNV homer device model

[David Gibson]

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

On Tue, Sep 10, 2019 at 12:40:19PM +0530, Balamuruhan S wrote:
> add PnvHOMER device model to emulate homer memory access

Please use PnvHomer as the identifier instead.  I'm guessing that
messes with the usual capitalization of HOMER, but my experience with
spapr is that matching qemu capitalization style is more important for
code readability (hence fairly recent changes of sPAPRPHBState to
SpaprPhbState and so so forth).

> for pstate table, occ-sensors, slw, occ static and dynamic
> values for Power8 and Power9 chips. Fix few coding style
> warnings given by checkpatch.pl.
> 
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> ---
>  hw/ppc/Makefile.objs       |   1 +
>  hw/ppc/pnv.c               |  79 +++++++++++---
>  hw/ppc/pnv_homer.c         | 258 +++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/pnv.h       |   3 +
>  include/hw/ppc/pnv_homer.h |  52 +++++++++
>  5 files changed, 376 insertions(+), 17 deletions(-)
>  create mode 100644 hw/ppc/pnv_homer.c
>  create mode 100644 include/hw/ppc/pnv_homer.h
> 
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index 2c4e1c8de0..580bb4f0dd 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -9,6 +9,7 @@ obj-$(CONFIG_PSERIES) += spapr_tpm_proxy.o
>  obj-$(CONFIG_SPAPR_RNG) +=  spapr_rng.o
>  # IBM PowerNV
>  obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
> +obj-$(CONFIG_POWERNV) += pnv_homer.o
>  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
>  obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o
>  endif
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 80338ffe87..ddddcc9bb6 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -187,7 +187,8 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>  
>      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
>      _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
> -    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
> +    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size",
> +                           cpu->hash64_opts->slb_size)));
>      _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
>      _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
>  
> @@ -200,19 +201,23 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>                             segs, sizeof(segs))));
>      }
>  
> -    /* Advertise VMX/VSX (vector extensions) if available
> +    /*
> +     * Advertise VMX/VSX (vector extensions) if available
>       *   0 / no property == no vector extensions
>       *   1               == VMX / Altivec available
> -     *   2               == VSX available */
> +     *   2               == VSX available
> +     */
>      if (env->insns_flags & PPC_ALTIVEC) {
>          uint32_t vmx = (env->insns_flags2 & PPC2_VSX) ? 2 : 1;
>  
>          _FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", vmx)));
>      }
>  
> -    /* Advertise DFP (Decimal Floating Point) if available
> +    /*
> +     * Advertise DFP (Decimal Floating Point) if available
>       *   0 / no property == no DFP
> -     *   1               == DFP available */
> +     *   1               == DFP available
> +     */
>      if (env->insns_flags2 & PPC2_DFP) {
>          _FDT((fdt_setprop_cell(fdt, offset, "ibm,dfp", 1)));
>      }
> @@ -424,7 +429,8 @@ static int pnv_dt_isa_device(DeviceState *dev, void *opaque)
>      return 0;
>  }
>  
> -/* The default LPC bus of a multichip system is on chip 0. It's
> +/*
> + * The default LPC bus of a multichip system is on chip 0. It's
>   * recognized by the firmware (skiboot) using a "primary" property.
>   */
>  static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
> @@ -442,8 +448,10 @@ static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
>      assert(phandle > 0);
>      _FDT((fdt_setprop_cell(fdt, isa_offset, "phandle", phandle)));
>  
> -    /* ISA devices are not necessarily parented to the ISA bus so we
> -     * can not use object_child_foreach() */
> +    /*
> +     * ISA devices are not necessarily parented to the ISA bus so we
> +     * can not use object_child_foreach()
> +     */
>      qbus_walk_children(BUS(pnv->isa_bus), pnv_dt_isa_device, NULL, NULL, NULL,
>                         &args);
>  }
> @@ -545,7 +553,8 @@ static void pnv_reset(MachineState *machine)
>  
>      qemu_devices_reset();
>  
> -    /* OpenPOWER systems have a BMC, which can be defined on the
> +    /*
> +     * OpenPOWER systems have a BMC, which can be defined on the
>       * command line with:
>       *
>       *   -device ipmi-bmc-sim,id=bmc0
> @@ -705,7 +714,8 @@ static void pnv_init(MachineState *machine)
>  
>          pnv->chips[i] = PNV_CHIP(chip);
>  
> -        /* TODO: put all the memory in one node on chip 0 until we find a
> +        /*
> +         * TODO: put all the memory in one node on chip 0 until we find a
>           * way to specify different ranges for each chip
>           */
>          if (i == 0) {
> @@ -732,8 +742,10 @@ static void pnv_init(MachineState *machine)
>      /* Create an RTC ISA device too */
>      mc146818_rtc_init(pnv->isa_bus, 2000, NULL);
>  
> -    /* OpenPOWER systems use a IPMI SEL Event message to notify the
> -     * host to powerdown */
> +    /*
> +     * OpenPOWER systems use a IPMI SEL Event message to notify the
> +     * host to powerdown
> +     */
>      pnv->powerdown_notifier.notify = pnv_powerdown_notify;
>      qemu_register_powerdown_notifier(&pnv->powerdown_notifier);
>  }
> @@ -803,7 +815,8 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
>      pnv_cpu->intc = obj;
>  }
>  
> -/* Allowed core identifiers on a POWER8 Processor Chip :
> +/*
> + * Allowed core identifiers on a POWER8 Processor Chip :
>   *
>   * <EX0 reserved>
>   *  EX1  - Venice only
> @@ -847,6 +860,11 @@ static void pnv_chip_power8_instance_init(Object *obj)
>                              TYPE_PNV8_OCC, &error_abort, NULL);
>      object_property_add_const_link(OBJECT(&chip8->occ), "psi",
>                                     OBJECT(&chip8->psi), &error_abort);
> +
> +    object_initialize_child(obj, "homer",  &chip8->homer, sizeof(chip8->homer),
> +                            TYPE_PNV8_HOMER, &error_abort, NULL);
> +    object_property_add_const_link(OBJECT(&chip8->homer), "xics",
> +                                   OBJECT(qdev_get_machine()), &error_abort);
>  }
>  
>  static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
> @@ -923,8 +941,10 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>                                              (uint64_t) PNV_XSCOM_BASE(chip),
>                                              PNV_XSCOM_LPC_BASE);
>  
> -    /* Interrupt Management Area. This is the memory region holding
> -     * all the Interrupt Control Presenter (ICP) registers */
> +    /*
> +     * Interrupt Management Area. This is the memory region holding
> +     * all the Interrupt Control Presenter (ICP) registers
> +     */
>      pnv_chip_icp_realize(chip8, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> @@ -942,6 +962,16 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
>      /* OCC SRAM model */
>      memory_region_add_subregion(get_system_memory(), PNV_OCC_COMMON_AREA(chip),
>                                  &chip8->occ.sram_regs);
> +
> +    /* HOMER */
> +    object_property_set_bool(OBJECT(&chip8->homer), true, "realized",
> +                             &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +    memory_region_add_subregion(get_system_memory(), PNV_HOMER_BASE(chip),
> +                                &chip8->homer.homer_regs);
>  }
>  
>  static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
> @@ -1024,6 +1054,11 @@ static void pnv_chip_power9_instance_init(Object *obj)
>                              TYPE_PNV9_OCC, &error_abort, NULL);
>      object_property_add_const_link(OBJECT(&chip9->occ), "psi",
>                                     OBJECT(&chip9->psi), &error_abort);
> +
> +    object_initialize_child(obj, "homer",  &chip9->homer, sizeof(chip9->homer),
> +                            TYPE_PNV9_HOMER, &error_abort, NULL);
> +    object_property_add_const_link(OBJECT(&chip9->homer), "chip", obj,
> +                                   &error_abort);
>  }
>  
>  static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error **errp)
> @@ -1134,6 +1169,16 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>      /* OCC SRAM model */
>      memory_region_add_subregion(get_system_memory(), PNV9_OCC_COMMON_AREA(chip),
>                                  &chip9->occ.sram_regs);
> +
> +    /* HOMER */
> +    object_property_set_bool(OBJECT(&chip9->homer), true, "realized",
> +                             &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +    memory_region_add_subregion(get_system_memory(), PNV9_HOMER_BASE(chip),
> +                                &chip9->homer.homer_regs);
>  }
>  
>  static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
> @@ -1412,8 +1457,8 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
>      mc->init = pnv_init;
>      mc->reset = pnv_reset;
>      mc->max_cpus = MAX_CPUS;
> -    mc->block_default_type = IF_IDE; /* Pnv provides a AHCI device for
> -                                      * storage */
> +    /* Pnv provides a AHCI device for storage */
> +    mc->block_default_type = IF_IDE;
>      mc->no_parallel = 1;
>      mc->default_boot_order = NULL;
>      /*
> diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
> new file mode 100644
> index 0000000000..2886e27176
> --- /dev/null
> +++ b/hw/ppc/pnv_homer.c
> @@ -0,0 +1,258 @@
> +/*
> + * QEMU PowerPC PowerNV Emulation of a few HOMER related registers
> + *
> + * Copyright (c) 2019, IBM Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation.
> + *
> + * This program 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "sysemu/cpus.h"
> +#include "hw/ppc/pnv.h"
> +#include "hw/qdev-core.h"
> +#include "hw/ppc/pnv_homer.h"
> +
> +
> +static bool core_max_array(void *opaque, hwaddr addr)
> +{
> +    PnvHOMER *homer = PNV_HOMER(opaque);
> +    PnvHOMERClass *homer_class = PNV_HOMER_GET_CLASS(homer);
> +
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +
> +    for (int i = 0; i <= ms->smp.cores; i++) {
> +        if (addr == (homer_class->core_max_base + i)) {
> +            return true;
> +       }
> +    }
> +    return false;
> +}
> +
> +/* P8 Pstate table */
> +
> +#define PNV8_OCC_PSTATE_VERSION          0x1f8001
> +#define PNV8_OCC_PSTATE_MIN              0x1f8003
> +#define PNV8_OCC_PSTATE_VALID            0x1f8000
> +#define PNV8_OCC_PSTATE_THROTTLE         0x1f8002
> +#define PNV8_OCC_PSTATE_NOM              0x1f8004
> +#define PNV8_OCC_PSTATE_TURBO            0x1f8005
> +#define PNV8_OCC_PSTATE_ULTRA_TURBO      0x1f8006
> +#define PNV8_OCC_PSTATE_DATA             0x1f8008
> +#define PNV8_OCC_PSTATE_ID_ZERO          0x1f8010
> +#define PNV8_OCC_PSTATE_ID_ONE           0x1f8018
> +#define PNV8_OCC_PSTATE_ID_TWO           0x1f8020
> +#define PNV8_OCC_VDD_VOLTAGE_IDENTIFIER  0x1f8012
> +#define PNV8_OCC_VCS_VOLTAGE_IDENTIFIER  0x1f8013
> +#define PNV8_OCC_PSTATE_ZERO_FREQUENCY   0x1f8014
> +#define PNV8_OCC_PSTATE_ONE_FREQUENCY    0x1f801c
> +#define PNV8_OCC_PSTATE_TWO_FREQUENCY    0x1f8024
> +#define PNV8_CORE_MAX_BASE               0x1f8810
> +
> +
> +static uint64_t pnv_power8_homer_read(void *opaque, hwaddr addr,
> +                                      unsigned size)
> +{
> +    switch (addr) {
> +    case PNV8_OCC_PSTATE_VERSION:
> +    case PNV8_OCC_PSTATE_MIN:
> +    case PNV8_OCC_PSTATE_ID_ZERO:
> +        return 0;
> +    case PNV8_OCC_PSTATE_VALID:
> +    case PNV8_OCC_PSTATE_THROTTLE:
> +    case PNV8_OCC_PSTATE_NOM:
> +    case PNV8_OCC_PSTATE_TURBO:
> +    case PNV8_OCC_PSTATE_ID_ONE:
> +    case PNV8_OCC_VDD_VOLTAGE_IDENTIFIER:
> +    case PNV8_OCC_VCS_VOLTAGE_IDENTIFIER:
> +        return 1;
> +    case PNV8_OCC_PSTATE_ULTRA_TURBO:
> +    case PNV8_OCC_PSTATE_ID_TWO:
> +        return 2;
> +    case PNV8_OCC_PSTATE_DATA:
> +        return 0x1000000000000000;
> +    /* P8 frequency for 0, 1, and 2 pstates */
> +    case PNV8_OCC_PSTATE_ZERO_FREQUENCY:
> +    case PNV8_OCC_PSTATE_ONE_FREQUENCY:
> +    case PNV8_OCC_PSTATE_TWO_FREQUENCY:
> +        return 3000;
> +    }
> +    /* pstate table core max array */
> +    if (core_max_array(opaque, addr)) {
> +        return 1;
> +    }
> +    return 0;
> +}
> +
> +static void pnv_power8_homer_write(void *opaque, hwaddr addr,
> +                                   uint64_t val, unsigned size)
> +{
> +    /* callback function defined to homer write */
> +    return;
> +}
> +
> +static const MemoryRegionOps pnv_power8_homer_ops = {
> +    .read = pnv_power8_homer_read,
> +    .write = pnv_power8_homer_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_homer_power8_class_init(ObjectClass *klass, void *data)
> +{
> +    PnvHOMERClass *homer = PNV_HOMER_CLASS(klass);
> +
> +    homer->homer_size = PNV_HOMER_SIZE;
> +    homer->homer_ops = &pnv_power8_homer_ops;
> +    homer->core_max_base = PNV8_CORE_MAX_BASE;
> +}
> +
> +static const TypeInfo pnv_homer_power8_type_info = {
> +    .name          = TYPE_PNV8_HOMER,
> +    .parent        = TYPE_PNV_HOMER,
> +    .instance_size = sizeof(PnvHOMER),
> +    .class_init    = pnv_homer_power8_class_init,
> +};
> +
> +/* P9 Pstate table */
> +
> +#define PNV9_OCC_PSTATE_ID_ZERO          0xe2018
> +#define PNV9_OCC_PSTATE_ID_ONE           0xe2020
> +#define PNV9_OCC_PSTATE_ID_TWO           0xe2028
> +#define PNV9_OCC_PSTATE_DATA             0xe2000
> +#define PNV9_OCC_PSTATE_DATA_AREA        0xe2008
> +#define PNV9_OCC_PSTATE_MIN              0xe2003
> +#define PNV9_OCC_PSTATE_NOM              0xe2004
> +#define PNV9_OCC_PSTATE_TURBO            0xe2005
> +#define PNV9_OCC_PSTATE_ULTRA_TURBO      0xe2818
> +#define PNV9_OCC_MAX_PSTATE_ULTRA_TURBO  0xe2006
> +#define PNV9_OCC_PSTATE_MAJOR_VERSION    0xe2001
> +#define PNV9_OCC_OPAL_RUNTIME_DATA       0xe2b85
> +#define PNV9_CHIP_HOMER_IMAGE_POINTER    0x200008
> +#define PNV9_CHIP_HOMER_BASE             0x0
> +#define PNV9_OCC_PSTATE_ZERO_FREQUENCY   0xe201c
> +#define PNV9_OCC_PSTATE_ONE_FREQUENCY    0xe2024
> +#define PNV9_OCC_PSTATE_TWO_FREQUENCY    0xe202c
> +#define PNV9_OCC_ROLE_MASTER_OR_SLAVE    0xe2002
> +#define PNV9_CORE_MAX_BASE               0xe2819
> +
> +
> +static uint64_t pnv_power9_homer_read(void *opaque, hwaddr addr,
> +                                      unsigned size)
> +{
> +    switch (addr) {
> +    case PNV9_OCC_MAX_PSTATE_ULTRA_TURBO:
> +    case PNV9_OCC_PSTATE_ID_ZERO:
> +        return 0;
> +    case PNV9_OCC_PSTATE_DATA:
> +    case PNV9_OCC_ROLE_MASTER_OR_SLAVE:
> +    case PNV9_OCC_PSTATE_NOM:
> +    case PNV9_OCC_PSTATE_TURBO:
> +    case PNV9_OCC_PSTATE_ID_ONE:
> +    case PNV9_OCC_PSTATE_ULTRA_TURBO:
> +    case PNV9_OCC_OPAL_RUNTIME_DATA:
> +        return 1;
> +    case PNV9_OCC_PSTATE_MIN:
> +    case PNV9_OCC_PSTATE_ID_TWO:
> +        return 2;
> +
> +    /* 3000 khz frequency for 0, 1, and 2 pstates */
> +    case PNV9_OCC_PSTATE_ZERO_FREQUENCY:
> +    case PNV9_OCC_PSTATE_ONE_FREQUENCY:
> +    case PNV9_OCC_PSTATE_TWO_FREQUENCY:
> +        return 3000;
> +    case PNV9_OCC_PSTATE_MAJOR_VERSION:
> +        return 0x90;
> +    case PNV9_CHIP_HOMER_BASE:
> +    case PNV9_OCC_PSTATE_DATA_AREA:
> +    case PNV9_CHIP_HOMER_IMAGE_POINTER:
> +        return 0x1000000000000000;
> +    }
> +    /* pstate table core max array */
> +    if (core_max_array(opaque, addr)) {
> +        return 1;
> +    }
> +    return 0;
> +}
> +
> +static void pnv_power9_homer_write(void *opaque, hwaddr addr,
> +                                   uint64_t val, unsigned size)
> +{
> +    /* callback function defined to homer write */
> +    return;
> +}
> +
> +static const MemoryRegionOps pnv_power9_homer_ops = {
> +    .read = pnv_power9_homer_read,
> +    .write = pnv_power9_homer_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_homer_power9_class_init(ObjectClass *klass, void *data)
> +{
> +    PnvHOMERClass *homer = PNV_HOMER_CLASS(klass);
> +
> +    homer->homer_size = PNV9_HOMER_SIZE;
> +    homer->homer_ops = &pnv_power9_homer_ops;
> +    homer->core_max_base = PNV9_CORE_MAX_BASE;
> +}
> +
> +static const TypeInfo pnv_homer_power9_type_info = {
> +    .name          = TYPE_PNV9_HOMER,
> +    .parent        = TYPE_PNV_HOMER,
> +    .instance_size = sizeof(PnvHOMER),
> +    .class_init    = pnv_homer_power9_class_init,
> +};
> +
> +static void pnv_homer_realize(DeviceState *dev, Error **errp)
> +{
> +    PnvHOMER *homer = PNV_HOMER(dev);
> +    PnvHOMERClass *homer_class = PNV_HOMER_GET_CLASS(homer);
> +
> +    /* homer region */
> +    memory_region_init_io(&homer->homer_regs, OBJECT(dev),
> +                          homer_class->homer_ops, homer, "homer-main-memory",
> +                          homer_class->homer_size);
> +}
> +
> +static void pnv_homer_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = pnv_homer_realize;
> +    dc->desc = "PowerNV HOMER Memory";
> +}
> +
> +static const TypeInfo pnv_homer_type_info = {
> +    .name          = TYPE_PNV_HOMER,
> +    .parent        = TYPE_DEVICE,
> +    .instance_size = sizeof(PnvHOMER),
> +    .class_init    = pnv_homer_class_init,
> +    .class_size    = sizeof(PnvHOMERClass),
> +    .abstract      = true,
> +};
> +
> +static void pnv_homer_register_types(void)
> +{
> +    type_register_static(&pnv_homer_type_info);
> +    type_register_static(&pnv_homer_power8_type_info);
> +    type_register_static(&pnv_homer_power9_type_info);
> +}
> +
> +type_init(pnv_homer_register_types);
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 63a4b7b6a7..321d4ca967 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -26,6 +26,7 @@
>  #include "hw/ppc/pnv_lpc.h"
>  #include "hw/ppc/pnv_psi.h"
>  #include "hw/ppc/pnv_occ.h"
> +#include "hw/ppc/pnv_homer.h"
>  #include "hw/ppc/pnv_xive.h"
>  #include "hw/ppc/pnv_core.h"
>  
> @@ -76,6 +77,7 @@ typedef struct Pnv8Chip {
>      PnvLpcController lpc;
>      Pnv8Psi      psi;
>      PnvOCC       occ;
> +    PnvHOMER     homer;
>  } Pnv8Chip;
>  
>  #define TYPE_PNV9_CHIP "pnv9-chip"
> @@ -90,6 +92,7 @@ typedef struct Pnv9Chip {
>      Pnv9Psi      psi;
>      PnvLpcController lpc;
>      PnvOCC       occ;
> +    PnvHOMER     homer;
>  
>      uint32_t     nr_quads;
>      PnvQuad      *quads;
> diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h
> new file mode 100644
> index 0000000000..3b666a6245
> --- /dev/null
> +++ b/include/hw/ppc/pnv_homer.h
> @@ -0,0 +1,52 @@
> +/*
> + * QEMU PowerPC PowerNV Emulation of a few HOMER related registers
> + *
> + * Copyright (c) 2019, 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_HOMER_H
> +#define PPC_PNV_HOMER_H
> +
> +#define TYPE_PNV_HOMER "pnv-homer"
> +#define PNV_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV_HOMER)
> +#define TYPE_PNV8_HOMER TYPE_PNV_HOMER "-POWER8"
> +#define PNV8_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV8_HOMER)
> +#define TYPE_PNV9_HOMER TYPE_PNV_HOMER "-POWER9"
> +#define PNV9_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV9_HOMER)
> +
> +typedef struct PnvHOMER {
> +    DeviceState parent;
> +
> +    MemoryRegion homer_regs;
> +} PnvHOMER;
> +
> +#define PNV_HOMER_CLASS(klass)   \
> +     OBJECT_CLASS_CHECK(PnvHOMERClass, (klass), TYPE_PNV_HOMER)
> +#define PNV_HOMER_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(PnvHOMERClass, (obj), TYPE_PNV_HOMER)
> +
> +typedef struct PnvHOMERClass {
> +    DeviceClass parent_class;
> +
> +    int homer_size;
> +    const MemoryRegionOps *homer_ops;
> +
> +    hwaddr core_max_base;
> +} PnvHOMERClass;
> +
> +
> +
> +#endif /* PPC_PNV_HOMER_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: 833 bytes --]

Re: [Qemu-devel] [PATCH v1 3/3] hw/ppc/pnv_homer: add PowerNV homer device model

[Balamuruhan S]

On Wed, Sep 11, 2019 at 10:34:05AM +1000, David Gibson wrote:
> On Tue, Sep 10, 2019 at 12:40:19PM +0530, Balamuruhan S wrote:
> > add PnvHOMER device model to emulate homer memory access
> 
> Please use PnvHomer as the identifier instead.  I'm guessing that
> messes with the usual capitalization of HOMER, but my experience with
> spapr is that matching qemu capitalization style is more important for
> code readability (hence fairly recent changes of sPAPRPHBState to
> SpaprPhbState and so so forth).

Sure, I will make the change as suggested.

-- Bala

> 
> > for pstate table, occ-sensors, slw, occ static and dynamic
> > values for Power8 and Power9 chips. Fix few coding style
> > warnings given by checkpatch.pl.
> > 
> > Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> > ---
> >  hw/ppc/Makefile.objs       |   1 +
> >  hw/ppc/pnv.c               |  79 +++++++++++---
> >  hw/ppc/pnv_homer.c         | 258 +++++++++++++++++++++++++++++++++++++++++++++
> >  include/hw/ppc/pnv.h       |   3 +
> >  include/hw/ppc/pnv_homer.h |  52 +++++++++
> >  5 files changed, 376 insertions(+), 17 deletions(-)
> >  create mode 100644 hw/ppc/pnv_homer.c
> >  create mode 100644 include/hw/ppc/pnv_homer.h
> > 
> > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> > index 2c4e1c8de0..580bb4f0dd 100644
> > --- a/hw/ppc/Makefile.objs
> > +++ b/hw/ppc/Makefile.objs
> > @@ -9,6 +9,7 @@ obj-$(CONFIG_PSERIES) += spapr_tpm_proxy.o
> >  obj-$(CONFIG_SPAPR_RNG) +=  spapr_rng.o
> >  # IBM PowerNV
> >  obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
> > +obj-$(CONFIG_POWERNV) += pnv_homer.o
> >  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> >  obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o
> >  endif
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 80338ffe87..ddddcc9bb6 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -187,7 +187,8 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
> >  
> >      _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq)));
> >      _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq)));
> > -    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size)));
> > +    _FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size",
> > +                           cpu->hash64_opts->slb_size)));
> >      _FDT((fdt_setprop_string(fdt, offset, "status", "okay")));
> >      _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
> >  
> > @@ -200,19 +201,23 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
> >                             segs, sizeof(segs))));
> >      }
> >  
> > -    /* Advertise VMX/VSX (vector extensions) if available
> > +    /*
> > +     * Advertise VMX/VSX (vector extensions) if available
> >       *   0 / no property == no vector extensions
> >       *   1               == VMX / Altivec available
> > -     *   2               == VSX available */
> > +     *   2               == VSX available
> > +     */
> >      if (env->insns_flags & PPC_ALTIVEC) {
> >          uint32_t vmx = (env->insns_flags2 & PPC2_VSX) ? 2 : 1;
> >  
> >          _FDT((fdt_setprop_cell(fdt, offset, "ibm,vmx", vmx)));
> >      }
> >  
> > -    /* Advertise DFP (Decimal Floating Point) if available
> > +    /*
> > +     * Advertise DFP (Decimal Floating Point) if available
> >       *   0 / no property == no DFP
> > -     *   1               == DFP available */
> > +     *   1               == DFP available
> > +     */
> >      if (env->insns_flags2 & PPC2_DFP) {
> >          _FDT((fdt_setprop_cell(fdt, offset, "ibm,dfp", 1)));
> >      }
> > @@ -424,7 +429,8 @@ static int pnv_dt_isa_device(DeviceState *dev, void *opaque)
> >      return 0;
> >  }
> >  
> > -/* The default LPC bus of a multichip system is on chip 0. It's
> > +/*
> > + * The default LPC bus of a multichip system is on chip 0. It's
> >   * recognized by the firmware (skiboot) using a "primary" property.
> >   */
> >  static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
> > @@ -442,8 +448,10 @@ static void pnv_dt_isa(PnvMachineState *pnv, void *fdt)
> >      assert(phandle > 0);
> >      _FDT((fdt_setprop_cell(fdt, isa_offset, "phandle", phandle)));
> >  
> > -    /* ISA devices are not necessarily parented to the ISA bus so we
> > -     * can not use object_child_foreach() */
> > +    /*
> > +     * ISA devices are not necessarily parented to the ISA bus so we
> > +     * can not use object_child_foreach()
> > +     */
> >      qbus_walk_children(BUS(pnv->isa_bus), pnv_dt_isa_device, NULL, NULL, NULL,
> >                         &args);
> >  }
> > @@ -545,7 +553,8 @@ static void pnv_reset(MachineState *machine)
> >  
> >      qemu_devices_reset();
> >  
> > -    /* OpenPOWER systems have a BMC, which can be defined on the
> > +    /*
> > +     * OpenPOWER systems have a BMC, which can be defined on the
> >       * command line with:
> >       *
> >       *   -device ipmi-bmc-sim,id=bmc0
> > @@ -705,7 +714,8 @@ static void pnv_init(MachineState *machine)
> >  
> >          pnv->chips[i] = PNV_CHIP(chip);
> >  
> > -        /* TODO: put all the memory in one node on chip 0 until we find a
> > +        /*
> > +         * TODO: put all the memory in one node on chip 0 until we find a
> >           * way to specify different ranges for each chip
> >           */
> >          if (i == 0) {
> > @@ -732,8 +742,10 @@ static void pnv_init(MachineState *machine)
> >      /* Create an RTC ISA device too */
> >      mc146818_rtc_init(pnv->isa_bus, 2000, NULL);
> >  
> > -    /* OpenPOWER systems use a IPMI SEL Event message to notify the
> > -     * host to powerdown */
> > +    /*
> > +     * OpenPOWER systems use a IPMI SEL Event message to notify the
> > +     * host to powerdown
> > +     */
> >      pnv->powerdown_notifier.notify = pnv_powerdown_notify;
> >      qemu_register_powerdown_notifier(&pnv->powerdown_notifier);
> >  }
> > @@ -803,7 +815,8 @@ static void pnv_chip_power9_intc_create(PnvChip *chip, PowerPCCPU *cpu,
> >      pnv_cpu->intc = obj;
> >  }
> >  
> > -/* Allowed core identifiers on a POWER8 Processor Chip :
> > +/*
> > + * Allowed core identifiers on a POWER8 Processor Chip :
> >   *
> >   * <EX0 reserved>
> >   *  EX1  - Venice only
> > @@ -847,6 +860,11 @@ static void pnv_chip_power8_instance_init(Object *obj)
> >                              TYPE_PNV8_OCC, &error_abort, NULL);
> >      object_property_add_const_link(OBJECT(&chip8->occ), "psi",
> >                                     OBJECT(&chip8->psi), &error_abort);
> > +
> > +    object_initialize_child(obj, "homer",  &chip8->homer, sizeof(chip8->homer),
> > +                            TYPE_PNV8_HOMER, &error_abort, NULL);
> > +    object_property_add_const_link(OBJECT(&chip8->homer), "xics",
> > +                                   OBJECT(qdev_get_machine()), &error_abort);
> >  }
> >  
> >  static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error **errp)
> > @@ -923,8 +941,10 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
> >                                              (uint64_t) PNV_XSCOM_BASE(chip),
> >                                              PNV_XSCOM_LPC_BASE);
> >  
> > -    /* Interrupt Management Area. This is the memory region holding
> > -     * all the Interrupt Control Presenter (ICP) registers */
> > +    /*
> > +     * Interrupt Management Area. This is the memory region holding
> > +     * all the Interrupt Control Presenter (ICP) registers
> > +     */
> >      pnv_chip_icp_realize(chip8, &local_err);
> >      if (local_err) {
> >          error_propagate(errp, local_err);
> > @@ -942,6 +962,16 @@ static void pnv_chip_power8_realize(DeviceState *dev, Error **errp)
> >      /* OCC SRAM model */
> >      memory_region_add_subregion(get_system_memory(), PNV_OCC_COMMON_AREA(chip),
> >                                  &chip8->occ.sram_regs);
> > +
> > +    /* HOMER */
> > +    object_property_set_bool(OBJECT(&chip8->homer), true, "realized",
> > +                             &local_err);
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        return;
> > +    }
> > +    memory_region_add_subregion(get_system_memory(), PNV_HOMER_BASE(chip),
> > +                                &chip8->homer.homer_regs);
> >  }
> >  
> >  static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
> > @@ -1024,6 +1054,11 @@ static void pnv_chip_power9_instance_init(Object *obj)
> >                              TYPE_PNV9_OCC, &error_abort, NULL);
> >      object_property_add_const_link(OBJECT(&chip9->occ), "psi",
> >                                     OBJECT(&chip9->psi), &error_abort);
> > +
> > +    object_initialize_child(obj, "homer",  &chip9->homer, sizeof(chip9->homer),
> > +                            TYPE_PNV9_HOMER, &error_abort, NULL);
> > +    object_property_add_const_link(OBJECT(&chip9->homer), "chip", obj,
> > +                                   &error_abort);
> >  }
> >  
> >  static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error **errp)
> > @@ -1134,6 +1169,16 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> >      /* OCC SRAM model */
> >      memory_region_add_subregion(get_system_memory(), PNV9_OCC_COMMON_AREA(chip),
> >                                  &chip9->occ.sram_regs);
> > +
> > +    /* HOMER */
> > +    object_property_set_bool(OBJECT(&chip9->homer), true, "realized",
> > +                             &local_err);
> > +    if (local_err) {
> > +        error_propagate(errp, local_err);
> > +        return;
> > +    }
> > +    memory_region_add_subregion(get_system_memory(), PNV9_HOMER_BASE(chip),
> > +                                &chip9->homer.homer_regs);
> >  }
> >  
> >  static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
> > @@ -1412,8 +1457,8 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
> >      mc->init = pnv_init;
> >      mc->reset = pnv_reset;
> >      mc->max_cpus = MAX_CPUS;
> > -    mc->block_default_type = IF_IDE; /* Pnv provides a AHCI device for
> > -                                      * storage */
> > +    /* Pnv provides a AHCI device for storage */
> > +    mc->block_default_type = IF_IDE;
> >      mc->no_parallel = 1;
> >      mc->default_boot_order = NULL;
> >      /*
> > diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
> > new file mode 100644
> > index 0000000000..2886e27176
> > --- /dev/null
> > +++ b/hw/ppc/pnv_homer.c
> > @@ -0,0 +1,258 @@
> > +/*
> > + * QEMU PowerPC PowerNV Emulation of a few HOMER related registers
> > + *
> > + * Copyright (c) 2019, IBM Corporation.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License, version 2, as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program 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 General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "sysemu/cpus.h"
> > +#include "hw/ppc/pnv.h"
> > +#include "hw/qdev-core.h"
> > +#include "hw/ppc/pnv_homer.h"
> > +
> > +
> > +static bool core_max_array(void *opaque, hwaddr addr)
> > +{
> > +    PnvHOMER *homer = PNV_HOMER(opaque);
> > +    PnvHOMERClass *homer_class = PNV_HOMER_GET_CLASS(homer);
> > +
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> > +
> > +    for (int i = 0; i <= ms->smp.cores; i++) {
> > +        if (addr == (homer_class->core_max_base + i)) {
> > +            return true;
> > +       }
> > +    }
> > +    return false;
> > +}
> > +
> > +/* P8 Pstate table */
> > +
> > +#define PNV8_OCC_PSTATE_VERSION          0x1f8001
> > +#define PNV8_OCC_PSTATE_MIN              0x1f8003
> > +#define PNV8_OCC_PSTATE_VALID            0x1f8000
> > +#define PNV8_OCC_PSTATE_THROTTLE         0x1f8002
> > +#define PNV8_OCC_PSTATE_NOM              0x1f8004
> > +#define PNV8_OCC_PSTATE_TURBO            0x1f8005
> > +#define PNV8_OCC_PSTATE_ULTRA_TURBO      0x1f8006
> > +#define PNV8_OCC_PSTATE_DATA             0x1f8008
> > +#define PNV8_OCC_PSTATE_ID_ZERO          0x1f8010
> > +#define PNV8_OCC_PSTATE_ID_ONE           0x1f8018
> > +#define PNV8_OCC_PSTATE_ID_TWO           0x1f8020
> > +#define PNV8_OCC_VDD_VOLTAGE_IDENTIFIER  0x1f8012
> > +#define PNV8_OCC_VCS_VOLTAGE_IDENTIFIER  0x1f8013
> > +#define PNV8_OCC_PSTATE_ZERO_FREQUENCY   0x1f8014
> > +#define PNV8_OCC_PSTATE_ONE_FREQUENCY    0x1f801c
> > +#define PNV8_OCC_PSTATE_TWO_FREQUENCY    0x1f8024
> > +#define PNV8_CORE_MAX_BASE               0x1f8810
> > +
> > +
> > +static uint64_t pnv_power8_homer_read(void *opaque, hwaddr addr,
> > +                                      unsigned size)
> > +{
> > +    switch (addr) {
> > +    case PNV8_OCC_PSTATE_VERSION:
> > +    case PNV8_OCC_PSTATE_MIN:
> > +    case PNV8_OCC_PSTATE_ID_ZERO:
> > +        return 0;
> > +    case PNV8_OCC_PSTATE_VALID:
> > +    case PNV8_OCC_PSTATE_THROTTLE:
> > +    case PNV8_OCC_PSTATE_NOM:
> > +    case PNV8_OCC_PSTATE_TURBO:
> > +    case PNV8_OCC_PSTATE_ID_ONE:
> > +    case PNV8_OCC_VDD_VOLTAGE_IDENTIFIER:
> > +    case PNV8_OCC_VCS_VOLTAGE_IDENTIFIER:
> > +        return 1;
> > +    case PNV8_OCC_PSTATE_ULTRA_TURBO:
> > +    case PNV8_OCC_PSTATE_ID_TWO:
> > +        return 2;
> > +    case PNV8_OCC_PSTATE_DATA:
> > +        return 0x1000000000000000;
> > +    /* P8 frequency for 0, 1, and 2 pstates */
> > +    case PNV8_OCC_PSTATE_ZERO_FREQUENCY:
> > +    case PNV8_OCC_PSTATE_ONE_FREQUENCY:
> > +    case PNV8_OCC_PSTATE_TWO_FREQUENCY:
> > +        return 3000;
> > +    }
> > +    /* pstate table core max array */
> > +    if (core_max_array(opaque, addr)) {
> > +        return 1;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static void pnv_power8_homer_write(void *opaque, hwaddr addr,
> > +                                   uint64_t val, unsigned size)
> > +{
> > +    /* callback function defined to homer write */
> > +    return;
> > +}
> > +
> > +static const MemoryRegionOps pnv_power8_homer_ops = {
> > +    .read = pnv_power8_homer_read,
> > +    .write = pnv_power8_homer_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_homer_power8_class_init(ObjectClass *klass, void *data)
> > +{
> > +    PnvHOMERClass *homer = PNV_HOMER_CLASS(klass);
> > +
> > +    homer->homer_size = PNV_HOMER_SIZE;
> > +    homer->homer_ops = &pnv_power8_homer_ops;
> > +    homer->core_max_base = PNV8_CORE_MAX_BASE;
> > +}
> > +
> > +static const TypeInfo pnv_homer_power8_type_info = {
> > +    .name          = TYPE_PNV8_HOMER,
> > +    .parent        = TYPE_PNV_HOMER,
> > +    .instance_size = sizeof(PnvHOMER),
> > +    .class_init    = pnv_homer_power8_class_init,
> > +};
> > +
> > +/* P9 Pstate table */
> > +
> > +#define PNV9_OCC_PSTATE_ID_ZERO          0xe2018
> > +#define PNV9_OCC_PSTATE_ID_ONE           0xe2020
> > +#define PNV9_OCC_PSTATE_ID_TWO           0xe2028
> > +#define PNV9_OCC_PSTATE_DATA             0xe2000
> > +#define PNV9_OCC_PSTATE_DATA_AREA        0xe2008
> > +#define PNV9_OCC_PSTATE_MIN              0xe2003
> > +#define PNV9_OCC_PSTATE_NOM              0xe2004
> > +#define PNV9_OCC_PSTATE_TURBO            0xe2005
> > +#define PNV9_OCC_PSTATE_ULTRA_TURBO      0xe2818
> > +#define PNV9_OCC_MAX_PSTATE_ULTRA_TURBO  0xe2006
> > +#define PNV9_OCC_PSTATE_MAJOR_VERSION    0xe2001
> > +#define PNV9_OCC_OPAL_RUNTIME_DATA       0xe2b85
> > +#define PNV9_CHIP_HOMER_IMAGE_POINTER    0x200008
> > +#define PNV9_CHIP_HOMER_BASE             0x0
> > +#define PNV9_OCC_PSTATE_ZERO_FREQUENCY   0xe201c
> > +#define PNV9_OCC_PSTATE_ONE_FREQUENCY    0xe2024
> > +#define PNV9_OCC_PSTATE_TWO_FREQUENCY    0xe202c
> > +#define PNV9_OCC_ROLE_MASTER_OR_SLAVE    0xe2002
> > +#define PNV9_CORE_MAX_BASE               0xe2819
> > +
> > +
> > +static uint64_t pnv_power9_homer_read(void *opaque, hwaddr addr,
> > +                                      unsigned size)
> > +{
> > +    switch (addr) {
> > +    case PNV9_OCC_MAX_PSTATE_ULTRA_TURBO:
> > +    case PNV9_OCC_PSTATE_ID_ZERO:
> > +        return 0;
> > +    case PNV9_OCC_PSTATE_DATA:
> > +    case PNV9_OCC_ROLE_MASTER_OR_SLAVE:
> > +    case PNV9_OCC_PSTATE_NOM:
> > +    case PNV9_OCC_PSTATE_TURBO:
> > +    case PNV9_OCC_PSTATE_ID_ONE:
> > +    case PNV9_OCC_PSTATE_ULTRA_TURBO:
> > +    case PNV9_OCC_OPAL_RUNTIME_DATA:
> > +        return 1;
> > +    case PNV9_OCC_PSTATE_MIN:
> > +    case PNV9_OCC_PSTATE_ID_TWO:
> > +        return 2;
> > +
> > +    /* 3000 khz frequency for 0, 1, and 2 pstates */
> > +    case PNV9_OCC_PSTATE_ZERO_FREQUENCY:
> > +    case PNV9_OCC_PSTATE_ONE_FREQUENCY:
> > +    case PNV9_OCC_PSTATE_TWO_FREQUENCY:
> > +        return 3000;
> > +    case PNV9_OCC_PSTATE_MAJOR_VERSION:
> > +        return 0x90;
> > +    case PNV9_CHIP_HOMER_BASE:
> > +    case PNV9_OCC_PSTATE_DATA_AREA:
> > +    case PNV9_CHIP_HOMER_IMAGE_POINTER:
> > +        return 0x1000000000000000;
> > +    }
> > +    /* pstate table core max array */
> > +    if (core_max_array(opaque, addr)) {
> > +        return 1;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static void pnv_power9_homer_write(void *opaque, hwaddr addr,
> > +                                   uint64_t val, unsigned size)
> > +{
> > +    /* callback function defined to homer write */
> > +    return;
> > +}
> > +
> > +static const MemoryRegionOps pnv_power9_homer_ops = {
> > +    .read = pnv_power9_homer_read,
> > +    .write = pnv_power9_homer_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_homer_power9_class_init(ObjectClass *klass, void *data)
> > +{
> > +    PnvHOMERClass *homer = PNV_HOMER_CLASS(klass);
> > +
> > +    homer->homer_size = PNV9_HOMER_SIZE;
> > +    homer->homer_ops = &pnv_power9_homer_ops;
> > +    homer->core_max_base = PNV9_CORE_MAX_BASE;
> > +}
> > +
> > +static const TypeInfo pnv_homer_power9_type_info = {
> > +    .name          = TYPE_PNV9_HOMER,
> > +    .parent        = TYPE_PNV_HOMER,
> > +    .instance_size = sizeof(PnvHOMER),
> > +    .class_init    = pnv_homer_power9_class_init,
> > +};
> > +
> > +static void pnv_homer_realize(DeviceState *dev, Error **errp)
> > +{
> > +    PnvHOMER *homer = PNV_HOMER(dev);
> > +    PnvHOMERClass *homer_class = PNV_HOMER_GET_CLASS(homer);
> > +
> > +    /* homer region */
> > +    memory_region_init_io(&homer->homer_regs, OBJECT(dev),
> > +                          homer_class->homer_ops, homer, "homer-main-memory",
> > +                          homer_class->homer_size);
> > +}
> > +
> > +static void pnv_homer_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->realize = pnv_homer_realize;
> > +    dc->desc = "PowerNV HOMER Memory";
> > +}
> > +
> > +static const TypeInfo pnv_homer_type_info = {
> > +    .name          = TYPE_PNV_HOMER,
> > +    .parent        = TYPE_DEVICE,
> > +    .instance_size = sizeof(PnvHOMER),
> > +    .class_init    = pnv_homer_class_init,
> > +    .class_size    = sizeof(PnvHOMERClass),
> > +    .abstract      = true,
> > +};
> > +
> > +static void pnv_homer_register_types(void)
> > +{
> > +    type_register_static(&pnv_homer_type_info);
> > +    type_register_static(&pnv_homer_power8_type_info);
> > +    type_register_static(&pnv_homer_power9_type_info);
> > +}
> > +
> > +type_init(pnv_homer_register_types);
> > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> > index 63a4b7b6a7..321d4ca967 100644
> > --- a/include/hw/ppc/pnv.h
> > +++ b/include/hw/ppc/pnv.h
> > @@ -26,6 +26,7 @@
> >  #include "hw/ppc/pnv_lpc.h"
> >  #include "hw/ppc/pnv_psi.h"
> >  #include "hw/ppc/pnv_occ.h"
> > +#include "hw/ppc/pnv_homer.h"
> >  #include "hw/ppc/pnv_xive.h"
> >  #include "hw/ppc/pnv_core.h"
> >  
> > @@ -76,6 +77,7 @@ typedef struct Pnv8Chip {
> >      PnvLpcController lpc;
> >      Pnv8Psi      psi;
> >      PnvOCC       occ;
> > +    PnvHOMER     homer;
> >  } Pnv8Chip;
> >  
> >  #define TYPE_PNV9_CHIP "pnv9-chip"
> > @@ -90,6 +92,7 @@ typedef struct Pnv9Chip {
> >      Pnv9Psi      psi;
> >      PnvLpcController lpc;
> >      PnvOCC       occ;
> > +    PnvHOMER     homer;
> >  
> >      uint32_t     nr_quads;
> >      PnvQuad      *quads;
> > diff --git a/include/hw/ppc/pnv_homer.h b/include/hw/ppc/pnv_homer.h
> > new file mode 100644
> > index 0000000000..3b666a6245
> > --- /dev/null
> > +++ b/include/hw/ppc/pnv_homer.h
> > @@ -0,0 +1,52 @@
> > +/*
> > + * QEMU PowerPC PowerNV Emulation of a few HOMER related registers
> > + *
> > + * Copyright (c) 2019, 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_HOMER_H
> > +#define PPC_PNV_HOMER_H
> > +
> > +#define TYPE_PNV_HOMER "pnv-homer"
> > +#define PNV_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV_HOMER)
> > +#define TYPE_PNV8_HOMER TYPE_PNV_HOMER "-POWER8"
> > +#define PNV8_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV8_HOMER)
> > +#define TYPE_PNV9_HOMER TYPE_PNV_HOMER "-POWER9"
> > +#define PNV9_HOMER(obj) OBJECT_CHECK(PnvHOMER, (obj), TYPE_PNV9_HOMER)
> > +
> > +typedef struct PnvHOMER {
> > +    DeviceState parent;
> > +
> > +    MemoryRegion homer_regs;
> > +} PnvHOMER;
> > +
> > +#define PNV_HOMER_CLASS(klass)   \
> > +     OBJECT_CLASS_CHECK(PnvHOMERClass, (klass), TYPE_PNV_HOMER)
> > +#define PNV_HOMER_GET_CLASS(obj) \
> > +     OBJECT_GET_CLASS(PnvHOMERClass, (obj), TYPE_PNV_HOMER)
> > +
> > +typedef struct PnvHOMERClass {
> > +    DeviceClass parent_class;
> > +
> > +    int homer_size;
> > +    const MemoryRegionOps *homer_ops;
> > +
> > +    hwaddr core_max_base;
> > +} PnvHOMERClass;
> > +
> > +
> > +
> > +#endif /* PPC_PNV_HOMER_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

Re: [Qemu-devel] [PATCH v1 0/3] add Homer/OCC common area emulation for PowerNV

[Cédric Le Goater]

On 10/09/2019 09:10, Balamuruhan S wrote:
> Hi All,
> 
> This is follow-up patch that implements HOMER and OCC SRAM device
> models to emulate homer memory and occ common area access for pstate
> table, occ sensors, runtime data and slw.
> 
> This version addresses review comments in previous patchset and
> breaks it to have separate patch series for Homer and OCC emulation,
> 
> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg00979.html
> 
> currently skiboot disables the homer/occ code path with `QUIRK_NO_PBA`,
> this quirk have to be removed in skiboot for it to use HOMER and OCC
> SRAM device models along with few bug fixes,
> 
> https://github.com/balamuruhans/skiboot/commit/a655514d2a730e0372a2faee277d1cf01f71a524
> https://github.com/balamuruhans/skiboot/commit/fd3d93d92ec66a7494346d6d24ced7b48264c9a0

Can't we generate the sensors in QEMU ? I am not sure what this
patch does. Is the Header Block invalid ? 

It would be good to generate properties to control their values 
on the monitor line, like Rashmica did for GPIO model in the 
Aspeed machine.

> https://github.com/balamuruhans/skiboot/commit/165b3829a93bc177c18133945a8cca3a2d701173

This one is weird .

C. 

> 
> changes from v1:
>     * reuse PnvOCC device model to implement SRAM device.
>     * implement PnvHomer as separate device model.
>     * have core max base address as part of PnvHOMERClass.
>     * reuse PNV_CHIP_INDEX() instead of introducing new `chip_num`.
>     * define all the memory ops access address as macros.
>     * few coding style warnings given by checkpatch.pl.
> 
> I request for review, comments and suggestions for the changes.
> 
> Balamuruhan S (3):
>   hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs
>   hw/ppc/pnv_occ: add sram device model for occ common area
>   hw/ppc/pnv_homer: add PowerNV homer device model
> 
>  hw/ppc/Makefile.objs       |   1 +
>  hw/ppc/pnv.c               |  87 ++++++++++++---
>  hw/ppc/pnv_homer.c         | 258 +++++++++++++++++++++++++++++++++++++++++++++
>  hw/ppc/pnv_occ.c           |  78 ++++++++++++++
>  hw/ppc/pnv_xscom.c         |  34 +++++-
>  include/hw/ppc/pnv.h       |  21 ++++
>  include/hw/ppc/pnv_homer.h |  52 +++++++++
>  include/hw/ppc/pnv_occ.h   |   3 +
>  8 files changed, 513 insertions(+), 21 deletions(-)
>  create mode 100644 hw/ppc/pnv_homer.c
>  create mode 100644 include/hw/ppc/pnv_homer.h
> 

Re: [Qemu-devel] [PATCH v1 0/3] add Homer/OCC common area emulation for PowerNV

[Balamuruhan S]

On Tue, Sep 10, 2019 at 01:45:55PM +0200, Cédric Le Goater wrote:
> On 10/09/2019 09:10, Balamuruhan S wrote:
> > Hi All,
> > 
> > This is follow-up patch that implements HOMER and OCC SRAM device
> > models to emulate homer memory and occ common area access for pstate
> > table, occ sensors, runtime data and slw.
> > 
> > This version addresses review comments in previous patchset and
> > breaks it to have separate patch series for Homer and OCC emulation,
> > 
> > https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg00979.html
> > 
> > currently skiboot disables the homer/occ code path with `QUIRK_NO_PBA`,
> > this quirk have to be removed in skiboot for it to use HOMER and OCC
> > SRAM device models along with few bug fixes,
> > 
> > https://github.com/balamuruhans/skiboot/commit/a655514d2a730e0372a2faee277d1cf01f71a524
> > https://github.com/balamuruhans/skiboot/commit/fd3d93d92ec66a7494346d6d24ced7b48264c9a0
> 
> Can't we generate the sensors in QEMU ? I am not sure what this
> patch does. Is the Header Block invalid ? 

This doesn't directly affect Qemu, this is skiboot bug where it
creates device tree node for sensor-groups and does sensor
sanity check to initialize, but in negative scenario where there
is no sensors like in Qemu the sanity check fails but still device
tree populates the sensor-groups node wrongly. The cleanup is not
handled in skiboot and this patch does that.

> 
> It would be good to generate properties to control their values 
> on the monitor line, like Rashmica did for GPIO model in the 
> Aspeed machine.

> 
> > https://github.com/balamuruhans/skiboot/commit/165b3829a93bc177c18133945a8cca3a2d701173
> 
> This one is weird .

I did a miss here, in skiboot there is check whether parsed pstate id
for pmin and pmax is valid or not. In this check, pmax to pmin for P8
it is 0 to -N and for P9 0 to N. But in Qemu for the MemoryRegionOps
structure read() callback function can have only uint64_t as return
type, so for P8 I got error from skiboot as we return postive value
and misunderstood to make this skiboot change. Cedric how can we handle
this from Qemu ?

Thanks for review,

-- Bala

> 
> C. 
> 
> > 
> > changes from v1:
> >     * reuse PnvOCC device model to implement SRAM device.
> >     * implement PnvHomer as separate device model.
> >     * have core max base address as part of PnvHOMERClass.
> >     * reuse PNV_CHIP_INDEX() instead of introducing new `chip_num`.
> >     * define all the memory ops access address as macros.
> >     * few coding style warnings given by checkpatch.pl.
> > 
> > I request for review, comments and suggestions for the changes.
> > 
> > Balamuruhan S (3):
> >   hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs
> >   hw/ppc/pnv_occ: add sram device model for occ common area
> >   hw/ppc/pnv_homer: add PowerNV homer device model
> > 
> >  hw/ppc/Makefile.objs       |   1 +
> >  hw/ppc/pnv.c               |  87 ++++++++++++---
> >  hw/ppc/pnv_homer.c         | 258 +++++++++++++++++++++++++++++++++++++++++++++
> >  hw/ppc/pnv_occ.c           |  78 ++++++++++++++
> >  hw/ppc/pnv_xscom.c         |  34 +++++-
> >  include/hw/ppc/pnv.h       |  21 ++++
> >  include/hw/ppc/pnv_homer.h |  52 +++++++++
> >  include/hw/ppc/pnv_occ.h   |   3 +
> >  8 files changed, 513 insertions(+), 21 deletions(-)
> >  create mode 100644 hw/ppc/pnv_homer.c
> >  create mode 100644 include/hw/ppc/pnv_homer.h
> > 
>