[PATCH 0/8] ppc: cleanups

[PATCH 0/8] ppc: cleanups

[Cédric Le Goater]

Hello,

This is a small series taking into account the comments made by David
and Greg while reviewing the pnv/p10 series.

It also adds a POWER10 DD2 CPU to the pseries machine.

Thanks,

C.

Cédric Le Goater (8):
  docs/system: ppc: Update the URL for OpenPOWER firmware images
  ppc/spapr: Add a POWER10 DD2 CPU
  ppc/pnv: Add a comment on the "primary-topology-index" property
  ppc/pnv: Remove useless variable
  ppc/pnv: Add an assert when calculating the RAM distribution on chips
  ppc/xive: Export priority_to_ipb() helper
  ppc/xive: Export xive_tctx_word2() helper
  ppc/pnv: Rename "id" to "quad-id" in PnvQuad

 docs/system/ppc/powernv.rst |  3 +--
 include/hw/ppc/pnv_core.h   |  2 +-
 include/hw/ppc/xive.h       | 16 ++++++++++++++++
 hw/intc/xive.c              | 26 ++++++--------------------
 hw/ppc/pnv.c                | 13 +++++++------
 hw/ppc/pnv_core.c           |  4 ++--
 hw/ppc/pnv_xscom.c          |  4 ++++
 hw/ppc/spapr_cpu_core.c     |  1 +
 8 files changed, 38 insertions(+), 31 deletions(-)

-- 
2.31.1

[PATCH 1/8] docs/system: ppc: Update the URL for OpenPOWER firmware images

[Cédric Le Goater]

This also fixes a small skiboot/skiroot typo and removes the links to
the specific POWER8 and POWER9 images since the firmware images can be
used to run all machines.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 docs/system/ppc/powernv.rst | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/docs/system/ppc/powernv.rst b/docs/system/ppc/powernv.rst
index 4c4cdea527e2..4ccfb2258622 100644
--- a/docs/system/ppc/powernv.rst
+++ b/docs/system/ppc/powernv.rst
@@ -53,8 +53,7 @@ initramfs ``skiroot``. Source code can be found on GitHub:
 
   https://github.com/open-power.
 
-Prebuilt images of ``skiboot`` and ``skiboot`` are made available on the `OpenPOWER <https://openpower.xyz/job/openpower/job/openpower-op-build/>`__ site. To boot a POWER9 machine, use the `witherspoon <https://openpower.xyz/job/openpower/job/openpower-op-build/label=slave,target=witherspoon/lastSuccessfulBuild/>`__ images. For POWER8, use
-the `palmetto <https://openpower.xyz/job/openpower/job/openpower-op-build/label=slave,target=palmetto/lastSuccessfulBuild/>`__ images.
+Prebuilt images of ``skiboot`` and ``skiroot`` are made available on the `OpenPOWER <https://github.com/open-power/op-build/releases/download/v2.7/>`__ site.
 
 QEMU includes a prebuilt image of ``skiboot`` which is updated when a
 more recent version is required by the models.
-- 
2.31.1

[PATCH 2/8] ppc/spapr: Add a POWER10 DD2 CPU

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/spapr_cpu_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 4f316a6f9d31..58e7341cb784 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -382,6 +382,7 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
     DEFINE_SPAPR_CPU_CORE_TYPE("power9_v1.0"),
     DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.0"),
     DEFINE_SPAPR_CPU_CORE_TYPE("power10_v1.0"),
+    DEFINE_SPAPR_CPU_CORE_TYPE("power10_v2.0"),
 #ifdef CONFIG_KVM
     DEFINE_SPAPR_CPU_CORE_TYPE("host"),
 #endif
-- 
2.31.1

[PATCH 3/8] ppc/pnv: Add a comment on the "primary-topology-index" property

[Cédric Le Goater]

On P10, the chip id is calculated from the "Primary topology table
index". See skiboot commits for more information [1].

This information is extracted from the hdata on real systems which
QEMU needs to emulate. Add this property for all machines even if it
is only used on POWER10.

[1] https://github.com/open-power/skiboot/commit/2ce3f083f399
    https://github.com/open-power/skiboot/commit/a2d4d7f9e14a

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/pnv_xscom.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
index faa488e3117a..9ce018dbc279 100644
--- a/hw/ppc/pnv_xscom.c
+++ b/hw/ppc/pnv_xscom.c
@@ -284,6 +284,10 @@ int pnv_dt_xscom(PnvChip *chip, void *fdt, int root_offset,
     _FDT(xscom_offset);
     g_free(name);
     _FDT((fdt_setprop_cell(fdt, xscom_offset, "ibm,chip-id", chip->chip_id)));
+    /*
+     * On P10, the xscom bus id has been deprecated and the chip id is
+     * calculated from the "Primary topology table index". See skiboot.
+     */
     _FDT((fdt_setprop_cell(fdt, xscom_offset, "ibm,primary-topology-index",
                            chip->chip_id)));
     _FDT((fdt_setprop_cell(fdt, xscom_offset, "#address-cells", 1)));
-- 
2.31.1

[PATCH 4/8] ppc/pnv: Remove useless variable

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/pnv.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 2f5358b70c95..a62e90b15e27 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -838,8 +838,7 @@ static void pnv_init(MachineState *machine)
     for (i = 0; i < pnv->num_chips; i++) {
         char chip_name[32];
         Object *chip = OBJECT(qdev_new(chip_typename));
-        int chip_id = i;
-        uint64_t chip_ram_size =  pnv_chip_get_ram_size(pnv, chip_id);
+        uint64_t chip_ram_size =  pnv_chip_get_ram_size(pnv, i);
 
         pnv->chips[i] = PNV_CHIP(chip);
 
@@ -850,9 +849,9 @@ static void pnv_init(MachineState *machine)
                                 &error_fatal);
         chip_ram_start += chip_ram_size;
 
-        snprintf(chip_name, sizeof(chip_name), "chip[%d]", chip_id);
+        snprintf(chip_name, sizeof(chip_name), "chip[%d]", i);
         object_property_add_child(OBJECT(pnv), chip_name, chip);
-        object_property_set_int(chip, "chip-id", chip_id, &error_fatal);
+        object_property_set_int(chip, "chip-id", i, &error_fatal);
         object_property_set_int(chip, "nr-cores", machine->smp.cores,
                                 &error_fatal);
         object_property_set_int(chip, "nr-threads", machine->smp.threads,
-- 
2.31.1

[PATCH 5/8] ppc/pnv: Add an assert when calculating the RAM distribution on chips

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/pnv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index a62e90b15e27..761b82be7401 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -723,6 +723,8 @@ static uint64_t pnv_chip_get_ram_size(PnvMachineState *pnv, int chip_id)
         return QEMU_ALIGN_DOWN(ram_per_chip, 1 * MiB);
     }
 
+    assert(pnv->num_chips < 2);
+
     ram_per_chip = (machine->ram_size - 1 * GiB) / (pnv->num_chips - 1);
     return chip_id == 0 ? 1 * GiB : QEMU_ALIGN_DOWN(ram_per_chip, 1 * MiB);
 }
-- 
2.31.1

[PATCH 6/8] ppc/xive: Export priority_to_ipb() helper

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h | 11 +++++++++++
 hw/intc/xive.c        | 21 ++++++---------------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index db7641165484..29b130eaea59 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -458,6 +458,17 @@ struct XiveENDSource {
  */
 #define XIVE_PRIORITY_MAX  7
 
+/*
+ * Convert a priority number to an Interrupt Pending Buffer (IPB)
+ * register, which indicates a pending interrupt at the priority
+ * corresponding to the bit number
+ */
+static inline uint8_t xive_priority_to_ipb(uint8_t priority)
+{
+    return priority > XIVE_PRIORITY_MAX ?
+        0 : 1 << (XIVE_PRIORITY_MAX - priority);
+}
+
 /*
  * XIVE Thread Interrupt Management Aera (TIMA)
  *
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index b817ee8e3704..b0c4f76b1d4b 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -27,17 +27,6 @@
  * XIVE Thread Interrupt Management context
  */
 
-/*
- * Convert a priority number to an Interrupt Pending Buffer (IPB)
- * register, which indicates a pending interrupt at the priority
- * corresponding to the bit number
- */
-static uint8_t priority_to_ipb(uint8_t priority)
-{
-    return priority > XIVE_PRIORITY_MAX ?
-        0 : 1 << (XIVE_PRIORITY_MAX - priority);
-}
-
 /*
  * Convert an Interrupt Pending Buffer (IPB) register to a Pending
  * Interrupt Priority Register (PIPR), which contains the priority of
@@ -89,7 +78,7 @@ static uint64_t xive_tctx_accept(XiveTCTX *tctx, uint8_t ring)
         regs[TM_CPPR] = cppr;
 
         /* Reset the pending buffer bit */
-        regs[TM_IPB] &= ~priority_to_ipb(cppr);
+        regs[TM_IPB] &= ~xive_priority_to_ipb(cppr);
         regs[TM_PIPR] = ipb_to_pipr(regs[TM_IPB]);
 
         /* Drop Exception bit */
@@ -353,7 +342,7 @@ static void xive_tm_set_os_cppr(XivePresenter *xptr, XiveTCTX *tctx,
 static void xive_tm_set_os_pending(XivePresenter *xptr, XiveTCTX *tctx,
                                    hwaddr offset, uint64_t value, unsigned size)
 {
-    xive_tctx_ipb_update(tctx, TM_QW1_OS, priority_to_ipb(value & 0xff));
+    xive_tctx_ipb_update(tctx, TM_QW1_OS, xive_priority_to_ipb(value & 0xff));
 }
 
 static void xive_os_cam_decode(uint32_t cam, uint8_t *nvt_blk,
@@ -1535,7 +1524,8 @@ bool xive_presenter_notify(XiveFabric *xfb, uint8_t format,
     /* handle CPU exception delivery */
     if (count) {
         trace_xive_presenter_notify(nvt_blk, nvt_idx, match.ring);
-        xive_tctx_ipb_update(match.tctx, match.ring, priority_to_ipb(priority));
+        xive_tctx_ipb_update(match.tctx, match.ring,
+                             xive_priority_to_ipb(priority));
     }
 
     return !!count;
@@ -1682,7 +1672,8 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
          * use. The presenter will resend the interrupt when the vCPU
          * is dispatched again on a HW thread.
          */
-        ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority);
+        ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) |
+            xive_priority_to_ipb(priority);
         nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb);
         xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4);
 
-- 
2.31.1

[PATCH 7/8] ppc/xive: Export xive_tctx_word2() helper

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/xive.h | 5 +++++
 hw/intc/xive.c        | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 29b130eaea59..252c58a1d691 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -335,6 +335,11 @@ struct XiveTCTX {
     XivePresenter *xptr;
 };
 
+static inline uint32_t xive_tctx_word2(uint8_t *ring)
+{
+    return *((uint32_t *) &ring[TM_WORD2]);
+}
+
 /*
  * XIVE Router
  */
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index b0c4f76b1d4b..6c82326ec768 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -141,11 +141,6 @@ void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb)
     xive_tctx_notify(tctx, ring);
 }
 
-static inline uint32_t xive_tctx_word2(uint8_t *ring)
-{
-    return *((uint32_t *) &ring[TM_WORD2]);
-}
-
 /*
  * XIVE Thread Interrupt Management Area (TIMA)
  */
-- 
2.31.1

[PATCH 8/8] ppc/pnv: Rename "id" to "quad-id" in PnvQuad

[Cédric Le Goater]

This to avoid possible conflicts with the "id" property of QOM objects.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ppc/pnv_core.h | 2 +-
 hw/ppc/pnv.c              | 4 ++--
 hw/ppc/pnv_core.c         | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
index 6ecee98a76ed..c22eab2e1f69 100644
--- a/include/hw/ppc/pnv_core.h
+++ b/include/hw/ppc/pnv_core.h
@@ -67,7 +67,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PnvQuad, PNV_QUAD)
 struct PnvQuad {
     DeviceState parent_obj;
 
-    uint32_t id;
+    uint32_t quad_id;
     MemoryRegion xscom_regs;
 };
 #endif /* PPC_PNV_CORE_H */
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 761b82be7401..93f76738fc94 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1370,10 +1370,10 @@ static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error **errp)
                                            sizeof(*eq), TYPE_PNV_QUAD,
                                            &error_fatal, NULL);
 
-        object_property_set_int(OBJECT(eq), "id", core_id, &error_fatal);
+        object_property_set_int(OBJECT(eq), "quad-id", core_id, &error_fatal);
         qdev_realize(DEVICE(eq), NULL, &error_fatal);
 
-        pnv_xscom_add_subregion(chip, PNV9_XSCOM_EQ_BASE(eq->id),
+        pnv_xscom_add_subregion(chip, PNV9_XSCOM_EQ_BASE(eq->quad_id),
                                 &eq->xscom_regs);
     }
 }
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 4de8414df212..19e8eb885f71 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -407,13 +407,13 @@ static void pnv_quad_realize(DeviceState *dev, Error **errp)
     PnvQuad *eq = PNV_QUAD(dev);
     char name[32];
 
-    snprintf(name, sizeof(name), "xscom-quad.%d", eq->id);
+    snprintf(name, sizeof(name), "xscom-quad.%d", eq->quad_id);
     pnv_xscom_region_init(&eq->xscom_regs, OBJECT(dev), &pnv_quad_xscom_ops,
                           eq, name, PNV9_XSCOM_EQ_SIZE);
 }
 
 static Property pnv_quad_properties[] = {
-    DEFINE_PROP_UINT32("id", PnvQuad, id, 0),
+    DEFINE_PROP_UINT32("quad-id", PnvQuad, quad_id, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.31.1

Re: [PATCH 1/8] docs/system: ppc: Update the URL for OpenPOWER firmware images

[Greg Kurz]

On Wed, 1 Sep 2021 11:41:46 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> This also fixes a small skiboot/skiroot typo and removes the links to
> the specific POWER8 and POWER9 images since the firmware images can be
> used to run all machines.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  docs/system/ppc/powernv.rst | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/docs/system/ppc/powernv.rst b/docs/system/ppc/powernv.rst
> index 4c4cdea527e2..4ccfb2258622 100644
> --- a/docs/system/ppc/powernv.rst
> +++ b/docs/system/ppc/powernv.rst
> @@ -53,8 +53,7 @@ initramfs ``skiroot``. Source code can be found on GitHub:
>  
>    https://github.com/open-power.
>  
> -Prebuilt images of ``skiboot`` and ``skiboot`` are made available on the `OpenPOWER <https://openpower.xyz/job/openpower/job/openpower-op-build/>`__ site. To boot a POWER9 machine, use the `witherspoon <https://openpower.xyz/job/openpower/job/openpower-op-build/label=slave,target=witherspoon/lastSuccessfulBuild/>`__ images. For POWER8, use
> -the `palmetto <https://openpower.xyz/job/openpower/job/openpower-op-build/label=slave,target=palmetto/lastSuccessfulBuild/>`__ images.
> +Prebuilt images of ``skiboot`` and ``skiroot`` are made available on the `OpenPOWER <https://github.com/open-power/op-build/releases/download/v2.7/>`__ site.
>  

This URL ends up 404 for me.

I suggest you change it to:

    https://github.com/open-power/op-build/releases/tag/v2.7

or even

    https://github.com/open-power/op-build/releases

which are valid and display individual links for the image files.

>  QEMU includes a prebuilt image of ``skiboot`` which is updated when a
>  more recent version is required by the models.

Re: [PATCH 2/8] ppc/spapr: Add a POWER10 DD2 CPU

[Greg Kurz]

On Wed, 1 Sep 2021 11:41:47 +0200
Cédric Le Goater <clg@kaod.org> wrote:

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

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr_cpu_core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 4f316a6f9d31..58e7341cb784 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -382,6 +382,7 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
>      DEFINE_SPAPR_CPU_CORE_TYPE("power9_v1.0"),
>      DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.0"),
>      DEFINE_SPAPR_CPU_CORE_TYPE("power10_v1.0"),
> +    DEFINE_SPAPR_CPU_CORE_TYPE("power10_v2.0"),
>  #ifdef CONFIG_KVM
>      DEFINE_SPAPR_CPU_CORE_TYPE("host"),
>  #endif

Re: [PATCH 1/8] docs/system: ppc: Update the URL for OpenPOWER firmware images

[Cédric Le Goater]

On 9/1/21 12:10 PM, Greg Kurz wrote:
> On Wed, 1 Sep 2021 11:41:46 +0200
> Cédric Le Goater <clg@kaod.org> wrote:
> 
>> This also fixes a small skiboot/skiroot typo and removes the links to
>> the specific POWER8 and POWER9 images since the firmware images can be
>> used to run all machines.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  docs/system/ppc/powernv.rst | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/docs/system/ppc/powernv.rst b/docs/system/ppc/powernv.rst
>> index 4c4cdea527e2..4ccfb2258622 100644
>> --- a/docs/system/ppc/powernv.rst
>> +++ b/docs/system/ppc/powernv.rst
>> @@ -53,8 +53,7 @@ initramfs ``skiroot``. Source code can be found on GitHub:
>>  
>>    https://github.com/open-power.
>>  
>> -Prebuilt images of ``skiboot`` and ``skiboot`` are made available on the `OpenPOWER <https://openpower.xyz/job/openpower/job/openpower-op-build/>`__ site. To boot a POWER9 machine, use the `witherspoon <https://openpower.xyz/job/openpower/job/openpower-op-build/label=slave,target=witherspoon/lastSuccessfulBuild/>`__ images. For POWER8, use
>> -the `palmetto <https://openpower.xyz/job/openpower/job/openpower-op-build/label=slave,target=palmetto/lastSuccessfulBuild/>`__ images.
>> +Prebuilt images of ``skiboot`` and ``skiroot`` are made available on the `OpenPOWER <https://github.com/open-power/op-build/releases/download/v2.7/>`__ site.
>>  
> 
> This URL ends up 404 for me.
> 
> I suggest you change it to:
> 
>     https://github.com/open-power/op-build/releases/tag/v2.7
> 
> or even
> 
>     https://github.com/open-power/op-build/releases
> 
> which are valid and display individual links for the image files.

Yes. I will resend this one.

Thanks,

C.

Re: [PATCH 2/8] ppc/spapr: Add a POWER10 DD2 CPU

[David Gibson]

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

On Wed, Sep 01, 2021 at 11:41:47AM +0200, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-6.2, thanks.

> ---
>  hw/ppc/spapr_cpu_core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 4f316a6f9d31..58e7341cb784 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -382,6 +382,7 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
>      DEFINE_SPAPR_CPU_CORE_TYPE("power9_v1.0"),
>      DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.0"),
>      DEFINE_SPAPR_CPU_CORE_TYPE("power10_v1.0"),
> +    DEFINE_SPAPR_CPU_CORE_TYPE("power10_v2.0"),
>  #ifdef CONFIG_KVM
>      DEFINE_SPAPR_CPU_CORE_TYPE("host"),
>  #endif

-- 
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: [PATCH 3/8] ppc/pnv: Add a comment on the "primary-topology-index" property

[David Gibson]

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

On Wed, Sep 01, 2021 at 11:41:48AM +0200, Cédric Le Goater wrote:
> On P10, the chip id is calculated from the "Primary topology table
> index". See skiboot commits for more information [1].
> 
> This information is extracted from the hdata on real systems which
> QEMU needs to emulate. Add this property for all machines even if it
> is only used on POWER10.
> 
> [1] https://github.com/open-power/skiboot/commit/2ce3f083f399
>     https://github.com/open-power/skiboot/commit/a2d4d7f9e14a
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-6.2, thanks.

> ---
>  hw/ppc/pnv_xscom.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index faa488e3117a..9ce018dbc279 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -284,6 +284,10 @@ int pnv_dt_xscom(PnvChip *chip, void *fdt, int root_offset,
>      _FDT(xscom_offset);
>      g_free(name);
>      _FDT((fdt_setprop_cell(fdt, xscom_offset, "ibm,chip-id", chip->chip_id)));
> +    /*
> +     * On P10, the xscom bus id has been deprecated and the chip id is
> +     * calculated from the "Primary topology table index". See skiboot.
> +     */
>      _FDT((fdt_setprop_cell(fdt, xscom_offset, "ibm,primary-topology-index",
>                             chip->chip_id)));
>      _FDT((fdt_setprop_cell(fdt, xscom_offset, "#address-cells", 1)));

-- 
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: [PATCH 4/8] ppc/pnv: Remove useless variable

[David Gibson]

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

On Wed, Sep 01, 2021 at 11:41:49AM +0200, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-6.2.

> ---
>  hw/ppc/pnv.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 2f5358b70c95..a62e90b15e27 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -838,8 +838,7 @@ static void pnv_init(MachineState *machine)
>      for (i = 0; i < pnv->num_chips; i++) {
>          char chip_name[32];
>          Object *chip = OBJECT(qdev_new(chip_typename));
> -        int chip_id = i;
> -        uint64_t chip_ram_size =  pnv_chip_get_ram_size(pnv, chip_id);
> +        uint64_t chip_ram_size =  pnv_chip_get_ram_size(pnv, i);
>  
>          pnv->chips[i] = PNV_CHIP(chip);
>  
> @@ -850,9 +849,9 @@ static void pnv_init(MachineState *machine)
>                                  &error_fatal);
>          chip_ram_start += chip_ram_size;
>  
> -        snprintf(chip_name, sizeof(chip_name), "chip[%d]", chip_id);
> +        snprintf(chip_name, sizeof(chip_name), "chip[%d]", i);
>          object_property_add_child(OBJECT(pnv), chip_name, chip);
> -        object_property_set_int(chip, "chip-id", chip_id, &error_fatal);
> +        object_property_set_int(chip, "chip-id", i, &error_fatal);
>          object_property_set_int(chip, "nr-cores", machine->smp.cores,
>                                  &error_fatal);
>          object_property_set_int(chip, "nr-threads", machine->smp.threads,

-- 
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: [PATCH 5/8] ppc/pnv: Add an assert when calculating the RAM distribution on chips

[David Gibson]

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

On Wed, Sep 01, 2021 at 11:41:50AM +0200, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Uh.. I thought the proposed assert was about making it clear there
wouldn't be a divide by zero, which would want > 1, not < 2.

> ---
>  hw/ppc/pnv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index a62e90b15e27..761b82be7401 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -723,6 +723,8 @@ static uint64_t pnv_chip_get_ram_size(PnvMachineState *pnv, int chip_id)
>          return QEMU_ALIGN_DOWN(ram_per_chip, 1 * MiB);
>      }
>  
> +    assert(pnv->num_chips < 2);
> +
>      ram_per_chip = (machine->ram_size - 1 * GiB) / (pnv->num_chips - 1);
>      return chip_id == 0 ? 1 * GiB : QEMU_ALIGN_DOWN(ram_per_chip, 1 * MiB);
>  }

-- 
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: [PATCH 6/8] ppc/xive: Export priority_to_ipb() helper

[David Gibson]

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

On Wed, Sep 01, 2021 at 11:41:51AM +0200, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-6.2.

> ---
>  include/hw/ppc/xive.h | 11 +++++++++++
>  hw/intc/xive.c        | 21 ++++++---------------
>  2 files changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index db7641165484..29b130eaea59 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -458,6 +458,17 @@ struct XiveENDSource {
>   */
>  #define XIVE_PRIORITY_MAX  7
>  
> +/*
> + * Convert a priority number to an Interrupt Pending Buffer (IPB)
> + * register, which indicates a pending interrupt at the priority
> + * corresponding to the bit number
> + */
> +static inline uint8_t xive_priority_to_ipb(uint8_t priority)
> +{
> +    return priority > XIVE_PRIORITY_MAX ?
> +        0 : 1 << (XIVE_PRIORITY_MAX - priority);
> +}
> +
>  /*
>   * XIVE Thread Interrupt Management Aera (TIMA)
>   *
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index b817ee8e3704..b0c4f76b1d4b 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -27,17 +27,6 @@
>   * XIVE Thread Interrupt Management context
>   */
>  
> -/*
> - * Convert a priority number to an Interrupt Pending Buffer (IPB)
> - * register, which indicates a pending interrupt at the priority
> - * corresponding to the bit number
> - */
> -static uint8_t priority_to_ipb(uint8_t priority)
> -{
> -    return priority > XIVE_PRIORITY_MAX ?
> -        0 : 1 << (XIVE_PRIORITY_MAX - priority);
> -}
> -
>  /*
>   * Convert an Interrupt Pending Buffer (IPB) register to a Pending
>   * Interrupt Priority Register (PIPR), which contains the priority of
> @@ -89,7 +78,7 @@ static uint64_t xive_tctx_accept(XiveTCTX *tctx, uint8_t ring)
>          regs[TM_CPPR] = cppr;
>  
>          /* Reset the pending buffer bit */
> -        regs[TM_IPB] &= ~priority_to_ipb(cppr);
> +        regs[TM_IPB] &= ~xive_priority_to_ipb(cppr);
>          regs[TM_PIPR] = ipb_to_pipr(regs[TM_IPB]);
>  
>          /* Drop Exception bit */
> @@ -353,7 +342,7 @@ static void xive_tm_set_os_cppr(XivePresenter *xptr, XiveTCTX *tctx,
>  static void xive_tm_set_os_pending(XivePresenter *xptr, XiveTCTX *tctx,
>                                     hwaddr offset, uint64_t value, unsigned size)
>  {
> -    xive_tctx_ipb_update(tctx, TM_QW1_OS, priority_to_ipb(value & 0xff));
> +    xive_tctx_ipb_update(tctx, TM_QW1_OS, xive_priority_to_ipb(value & 0xff));
>  }
>  
>  static void xive_os_cam_decode(uint32_t cam, uint8_t *nvt_blk,
> @@ -1535,7 +1524,8 @@ bool xive_presenter_notify(XiveFabric *xfb, uint8_t format,
>      /* handle CPU exception delivery */
>      if (count) {
>          trace_xive_presenter_notify(nvt_blk, nvt_idx, match.ring);
> -        xive_tctx_ipb_update(match.tctx, match.ring, priority_to_ipb(priority));
> +        xive_tctx_ipb_update(match.tctx, match.ring,
> +                             xive_priority_to_ipb(priority));
>      }
>  
>      return !!count;
> @@ -1682,7 +1672,8 @@ static void xive_router_end_notify(XiveRouter *xrtr, uint8_t end_blk,
>           * use. The presenter will resend the interrupt when the vCPU
>           * is dispatched again on a HW thread.
>           */
> -        ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) | priority_to_ipb(priority);
> +        ipb = xive_get_field32(NVT_W4_IPB, nvt.w4) |
> +            xive_priority_to_ipb(priority);
>          nvt.w4 = xive_set_field32(NVT_W4_IPB, nvt.w4, ipb);
>          xive_router_write_nvt(xrtr, nvt_blk, nvt_idx, &nvt, 4);
>  

-- 
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: [PATCH 7/8] ppc/xive: Export xive_tctx_word2() helper

[David Gibson]

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

On Wed, Sep 01, 2021 at 11:41:52AM +0200, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-6.2

> ---
>  include/hw/ppc/xive.h | 5 +++++
>  hw/intc/xive.c        | 5 -----
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 29b130eaea59..252c58a1d691 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -335,6 +335,11 @@ struct XiveTCTX {
>      XivePresenter *xptr;
>  };
>  
> +static inline uint32_t xive_tctx_word2(uint8_t *ring)
> +{
> +    return *((uint32_t *) &ring[TM_WORD2]);
> +}
> +
>  /*
>   * XIVE Router
>   */
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index b0c4f76b1d4b..6c82326ec768 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -141,11 +141,6 @@ void xive_tctx_ipb_update(XiveTCTX *tctx, uint8_t ring, uint8_t ipb)
>      xive_tctx_notify(tctx, ring);
>  }
>  
> -static inline uint32_t xive_tctx_word2(uint8_t *ring)
> -{
> -    return *((uint32_t *) &ring[TM_WORD2]);
> -}
> -
>  /*
>   * XIVE Thread Interrupt Management Area (TIMA)
>   */

-- 
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: [PATCH 8/8] ppc/pnv: Rename "id" to "quad-id" in PnvQuad

[David Gibson]

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

On Wed, Sep 01, 2021 at 11:41:53AM +0200, Cédric Le Goater wrote:
> This to avoid possible conflicts with the "id" property of QOM objects.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-6.2, thanks.

> ---
>  include/hw/ppc/pnv_core.h | 2 +-
>  hw/ppc/pnv.c              | 4 ++--
>  hw/ppc/pnv_core.c         | 4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
> index 6ecee98a76ed..c22eab2e1f69 100644
> --- a/include/hw/ppc/pnv_core.h
> +++ b/include/hw/ppc/pnv_core.h
> @@ -67,7 +67,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PnvQuad, PNV_QUAD)
>  struct PnvQuad {
>      DeviceState parent_obj;
>  
> -    uint32_t id;
> +    uint32_t quad_id;
>      MemoryRegion xscom_regs;
>  };
>  #endif /* PPC_PNV_CORE_H */
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 761b82be7401..93f76738fc94 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1370,10 +1370,10 @@ static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error **errp)
>                                             sizeof(*eq), TYPE_PNV_QUAD,
>                                             &error_fatal, NULL);
>  
> -        object_property_set_int(OBJECT(eq), "id", core_id, &error_fatal);
> +        object_property_set_int(OBJECT(eq), "quad-id", core_id, &error_fatal);
>          qdev_realize(DEVICE(eq), NULL, &error_fatal);
>  
> -        pnv_xscom_add_subregion(chip, PNV9_XSCOM_EQ_BASE(eq->id),
> +        pnv_xscom_add_subregion(chip, PNV9_XSCOM_EQ_BASE(eq->quad_id),
>                                  &eq->xscom_regs);
>      }
>  }
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index 4de8414df212..19e8eb885f71 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -407,13 +407,13 @@ static void pnv_quad_realize(DeviceState *dev, Error **errp)
>      PnvQuad *eq = PNV_QUAD(dev);
>      char name[32];
>  
> -    snprintf(name, sizeof(name), "xscom-quad.%d", eq->id);
> +    snprintf(name, sizeof(name), "xscom-quad.%d", eq->quad_id);
>      pnv_xscom_region_init(&eq->xscom_regs, OBJECT(dev), &pnv_quad_xscom_ops,
>                            eq, name, PNV9_XSCOM_EQ_SIZE);
>  }
>  
>  static Property pnv_quad_properties[] = {
> -    DEFINE_PROP_UINT32("id", PnvQuad, id, 0),
> +    DEFINE_PROP_UINT32("quad-id", PnvQuad, quad_id, 0),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  

-- 
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: [PATCH 5/8] ppc/pnv: Add an assert when calculating the RAM distribution on chips

[Cédric Le Goater]

On 9/2/21 3:37 AM, David Gibson wrote:
> On Wed, Sep 01, 2021 at 11:41:50AM +0200, Cédric Le Goater wrote:
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> Uh.. I thought the proposed assert was about making it clear there
> wouldn't be a divide by zero, which would want > 1, not < 2.

This is very wrong :) How did I come with this ? 

I will keep it for the next round of powernv10. Are you ready for a v2 ? 


Thanks, 

C.

Re: [PATCH 5/8] ppc/pnv: Add an assert when calculating the RAM distribution on chips

[David Gibson]

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

On Thu, Sep 02, 2021 at 08:28:49AM +0200, Cédric Le Goater wrote:
> On 9/2/21 3:37 AM, David Gibson wrote:
> > On Wed, Sep 01, 2021 at 11:41:50AM +0200, Cédric Le Goater wrote:
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > 
> > Uh.. I thought the proposed assert was about making it clear there
> > wouldn't be a divide by zero, which would want > 1, not < 2.
> 
> This is very wrong :) How did I come with this ? 
> 
> I will keep it for the next round of powernv10. Are you ready for a
> v2 ?

Ready enough.

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