All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models
@ 2023-06-03 23:36 Nicholas Piggin
  2023-06-03 23:36 ` [PATCH 1/4] pnv/chiptod: Add POWER9/10 chiptod model Nicholas Piggin
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Nicholas Piggin @ 2023-06-03 23:36 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Daniel Henrique Barboza,
	Cédric Le Goater

This adds support for chiptod and core timebase state machine models in
the powernv POWER9 and POWER10 models.

This does not actually change the time or the value in TB registers
(because they are alrady synced in QEMU), but it does go through the
motions. It is enough to be able to run skiboot's chiptod initialisation
code that synchronises core timebases (after a patch to prevent skiboot
skipping chiptod for QEMU, posted to skiboot mailing list).

Sorry there was some delay since the last posting. There is a bit more
interest in this recently but feedback and comments from RFC was not
forgotten and is much appreciated.

https://lists.gnu.org/archive/html/qemu-ppc/2022-08/msg00324.html

I think I accounted for everything except moving register defines to the
.h file. I'm on the fence about that but if they are only used in the .c
file I think it's okay to keep them there for now. I cut out a lot of
unused ones so it's not so cluttered now.

Lots of other changes and fixes since that RFC. Notably:
- Register names changed to match the workbook names instead of skiboot.
- TFMR moved to timebase_helper.c from misc_helper.c
- More comprehensive model and error checking, particularly of TFMR.
- POWER10 with multi-chip support.
- chiptod and core timebase linked via specific state instead of TFMR.

There is still a vast amount that is not modeled, but most of it related
to error handling, injection, failover, etc that is very complicated and
not required for normal operation.

Thanks,
Nick

Nicholas Piggin (4):
  pnv/chiptod: Add POWER9/10 chiptod model
  target/ppc: Tidy POWER book4 SPR registration
  target/ppc: add TFMR SPR implementation with read and write helpers
  target/ppc: Implement core timebase state machine and TFMR

 hw/ppc/meson.build           |   1 +
 hw/ppc/pnv.c                 |  38 +++
 hw/ppc/pnv_chiptod.c         | 488 +++++++++++++++++++++++++++++++++++
 hw/ppc/pnv_xscom.c           |   2 +
 hw/ppc/trace-events          |   4 +
 include/hw/ppc/pnv_chip.h    |   3 +
 include/hw/ppc/pnv_chiptod.h |  64 +++++
 include/hw/ppc/pnv_core.h    |   3 +
 include/hw/ppc/pnv_xscom.h   |   9 +
 target/ppc/cpu.h             |  40 +++
 target/ppc/cpu_init.c        |  92 ++++---
 target/ppc/helper.h          |   2 +
 target/ppc/spr_common.h      |   2 +
 target/ppc/timebase_helper.c | 156 +++++++++++
 target/ppc/translate.c       |  10 +
 15 files changed, 882 insertions(+), 32 deletions(-)
 create mode 100644 hw/ppc/pnv_chiptod.c
 create mode 100644 include/hw/ppc/pnv_chiptod.h

-- 
2.40.1



^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/4] pnv/chiptod: Add POWER9/10 chiptod model
  2023-06-03 23:36 [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models Nicholas Piggin
@ 2023-06-03 23:36 ` Nicholas Piggin
  2023-06-05 14:57   ` Cédric Le Goater
  2023-06-03 23:36 ` [PATCH 2/4] target/ppc: Tidy POWER book4 SPR registration Nicholas Piggin
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Nicholas Piggin @ 2023-06-03 23:36 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Daniel Henrique Barboza,
	Cédric Le Goater

The chiptod is a pervasive facility which can keep TOD (time-of-day),
synchronise it across multiple chips, and can move that TOD to or from
the core timebase units.

This driver implements basic emulation of chiptod registers sufficient
to successfully run the skiboot chiptod synchronisation procedure
(with the following TFMR and timebase state-machine implementation).

The main way chiptod affects the rest of the system (relevant to the
powernv model) is to interact with the timebase facility in the cores,
influencing the timebase state machine and registers.

The way this chiptod driver implements that interaction is with two
new flags in the CPUPPCState env, one is use for the core timebase to
indicate it is ready to receive a TOD from chiptod, the other used
by chiptod to indicate that it has sent TOD to the core timebase. The
core timebase will be implemented in later changes.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/meson.build           |   1 +
 hw/ppc/pnv.c                 |  38 +++
 hw/ppc/pnv_chiptod.c         | 488 +++++++++++++++++++++++++++++++++++
 hw/ppc/pnv_xscom.c           |   2 +
 hw/ppc/trace-events          |   4 +
 include/hw/ppc/pnv_chip.h    |   3 +
 include/hw/ppc/pnv_chiptod.h |  64 +++++
 include/hw/ppc/pnv_core.h    |   3 +
 include/hw/ppc/pnv_xscom.h   |   9 +
 target/ppc/cpu.h             |   6 +
 10 files changed, 618 insertions(+)
 create mode 100644 hw/ppc/pnv_chiptod.c
 create mode 100644 include/hw/ppc/pnv_chiptod.h

diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index c927337da0..afbf90e6da 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -45,6 +45,7 @@ ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
   'pnv_core.c',
   'pnv_lpc.c',
   'pnv_psi.c',
+  'pnv_chiptod.c',
   'pnv_occ.c',
   'pnv_sbe.c',
   'pnv_bmc.c',
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index dbdeba6c31..ce5e7d7582 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1414,6 +1414,8 @@ static void pnv_chip_power9_instance_init(Object *obj)
 
     object_initialize_child(obj, "lpc", &chip9->lpc, TYPE_PNV9_LPC);
 
+    object_initialize_child(obj, "chiptod", &chip9->chiptod, TYPE_PNV9_CHIPTOD);
+
     object_initialize_child(obj, "occ", &chip9->occ, TYPE_PNV9_OCC);
 
     object_initialize_child(obj, "sbe", &chip9->sbe, TYPE_PNV9_SBE);
@@ -1558,6 +1560,15 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
     chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
                                             (uint64_t) PNV9_LPCM_BASE(chip));
 
+    /* ChipTOD */
+    object_property_set_link(OBJECT(&chip9->chiptod), "chip", OBJECT(chip),
+                             &error_abort);
+    if (!qdev_realize(DEVICE(&chip9->chiptod), NULL, errp)) {
+        return;
+    }
+    pnv_xscom_add_subregion(chip, PNV9_XSCOM_CHIPTOD_BASE,
+                            &chip9->chiptod.xscom_regs);
+
     /* Create the simplified OCC model */
     if (!qdev_realize(DEVICE(&chip9->occ), NULL, errp)) {
         return;
@@ -1644,6 +1655,7 @@ static void pnv_chip_power10_instance_init(Object *obj)
                               "xive-fabric");
     object_initialize_child(obj, "psi", &chip10->psi, TYPE_PNV10_PSI);
     object_initialize_child(obj, "lpc", &chip10->lpc, TYPE_PNV10_LPC);
+    object_initialize_child(obj, "chiptod", &chip10->chiptod, TYPE_PNV10_CHIPTOD);
     object_initialize_child(obj, "occ",  &chip10->occ, TYPE_PNV10_OCC);
     object_initialize_child(obj, "sbe",  &chip10->sbe, TYPE_PNV10_SBE);
     object_initialize_child(obj, "homer", &chip10->homer, TYPE_PNV10_HOMER);
@@ -1773,6 +1785,15 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
     chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
                                             (uint64_t) PNV10_LPCM_BASE(chip));
 
+    /* ChipTOD */
+    object_property_set_link(OBJECT(&chip10->chiptod), "chip", OBJECT(chip),
+                             &error_abort);
+    if (!qdev_realize(DEVICE(&chip10->chiptod), NULL, errp)) {
+        return;
+    }
+    pnv_xscom_add_subregion(chip, PNV10_XSCOM_CHIPTOD_BASE,
+                            &chip10->chiptod.xscom_regs);
+
     /* Create the simplified OCC model */
     if (!qdev_realize(DEVICE(&chip10->occ), NULL, errp)) {
         return;
@@ -1938,6 +1959,23 @@ static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
     }
 }
 
+PnvCore *pnv_get_vcpu_by_xscom_base(PnvChip *chip, uint32_t xscom_base)
+{
+    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
+    int i;
+
+    for (i = 0; i < chip->nr_cores; i++) {
+        PnvCore *pc = chip->cores[i];
+        CPUCore *cc = CPU_CORE(pc);
+        int core_hwid = cc->core_id;
+
+        if (pcc->xscom_core_base(chip, core_hwid) == xscom_base) {
+            return pc;
+        }
+    }
+    return NULL;
+}
+
 static void pnv_chip_realize(DeviceState *dev, Error **errp)
 {
     PnvChip *chip = PNV_CHIP(dev);
diff --git a/hw/ppc/pnv_chiptod.c b/hw/ppc/pnv_chiptod.c
new file mode 100644
index 0000000000..04ef703e0f
--- /dev/null
+++ b/hw/ppc/pnv_chiptod.c
@@ -0,0 +1,488 @@
+/*
+ * QEMU PowerPC PowerNV Emulation of some CHIPTOD behaviour
+ *
+ * Copyright (c) 2022-2023, 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 "target/ppc/cpu.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/ppc/fdt.h"
+#include "hw/ppc/ppc.h"
+#include "hw/ppc/pnv.h"
+#include "hw/ppc/pnv_chip.h"
+#include "hw/ppc/pnv_core.h"
+#include "hw/ppc/pnv_xscom.h"
+#include "hw/ppc/pnv_chiptod.h"
+#include "trace.h"
+
+#include <libfdt.h>
+
+/* TOD chip XSCOM addresses */
+#define TOD_M_PATH_CTRL_REG             0x00000000 /* Master Path ctrl reg */
+#define TOD_PRI_PORT_0_CTRL_REG         0x00000001 /* Primary port0 ctrl reg */
+#define TOD_PRI_PORT_1_CTRL_REG         0x00000002 /* Primary port1 ctrl reg */
+#define TOD_SEC_PORT_0_CTRL_REG         0x00000003 /* Secondary p0 ctrl reg */
+#define TOD_SEC_PORT_1_CTRL_REG         0x00000004 /* Secondary p1 ctrl reg */
+#define TOD_S_PATH_CTRL_REG             0x00000005 /* Slave Path ctrl reg */
+#define TOD_I_PATH_CTRL_REG             0x00000006 /* Internal Path ctrl reg */
+
+/* -- TOD primary/secondary master/slave control register -- */
+#define TOD_PSS_MSS_CTRL_REG            0x00000007
+
+/* -- TOD primary/secondary master/slave status register -- */
+#define TOD_PSS_MSS_STATUS_REG          0x00000008
+
+/* TOD chip XSCOM addresses */
+#define TOD_CHIP_CTRL_REG               0x00000010 /* Chip control reg */
+
+#define TOD_TX_TTYPE_0_REG              0x00000011
+#define TOD_TX_TTYPE_1_REG              0x00000012 /* PSS switch reg */
+#define TOD_TX_TTYPE_2_REG              0x00000013 /* Enable step checkers */
+#define TOD_TX_TTYPE_3_REG              0x00000014 /* Request TOD reg */
+#define TOD_TX_TTYPE_4_REG              0x00000015 /* Send TOD reg */
+#define TOD_TX_TTYPE_5_REG              0x00000016 /* Invalidate TOD reg */
+
+#define TOD_MOVE_TOD_TO_TB_REG          0x00000017
+#define TOD_LOAD_TOD_MOD_REG            0x00000018
+#define TOD_LOAD_TOD_REG                0x00000021
+#define TOD_FSM_REG                     0x00000024
+
+#define TOD_TX_TTYPE_CTRL_REG           0x00000027 /* TX TTYPE Control reg */
+#define   TOD_TX_TTYPE_PIB_SLAVE_ADDR      PPC_BITMASK(26, 31)
+
+/* -- TOD Error interrupt register -- */
+#define TOD_ERROR_REG                   0x00000030
+
+/* PC unit PIB address which recieves the timebase transfer from TOD */
+#define   PC_TOD                        0x4A3
+
+static uint64_t pnv_chiptod_xscom_read(void *opaque, hwaddr addr,
+                                          unsigned size)
+{
+    PnvChipTOD *chiptod = PNV_CHIPTOD(opaque);
+    uint32_t offset = addr >> 3;
+    uint64_t val = 0;
+
+    switch (offset) {
+    case TOD_PSS_MSS_STATUS_REG:
+        /*
+         * ChipTOD does not support configurations other than primary
+         * master, does not support errors, etc.
+         */
+        val |= PPC_BITMASK(6,10); /* STEP checker validity */
+        val |= PPC_BIT(12); /* Primary config master path select */
+        val |= PPC_BIT(20); /* Is running */
+        val |= PPC_BIT(21); /* Is using primary config */
+        val |= PPC_BIT(26); /* Is using master path select */
+
+        if (chiptod->primary) {
+            val |= PPC_BIT(23); /* Is active master */
+        } else if (chiptod->secondary) {
+            val |= PPC_BIT(24); /* Is backup master */
+        }
+        break;
+    case TOD_PSS_MSS_CTRL_REG:
+        val = chiptod->pss_mss_ctrl_reg;
+        break;
+    case TOD_TX_TTYPE_CTRL_REG:
+        val = 0;
+	break;
+    case TOD_ERROR_REG:
+        val = chiptod->tod_error;
+        break;
+    case TOD_FSM_REG:
+        if (chiptod->tod_state == tod_running) {
+            val |= PPC_BIT(4);
+        }
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP, "pnv_chiptod: unimplemented register: Ox%"
+                      HWADDR_PRIx "\n", addr >> 3);
+    }
+
+    trace_pnv_chiptod_xscom_read(addr >> 3, val);
+
+    return val;
+}
+
+static void chiptod_power9_send_remotes(PnvChipTOD *chiptod)
+{
+    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
+    int i;
+
+    for (i = 0; i < pnv->num_chips; i++) {
+        Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
+        if (&chip9->chiptod != chiptod) {
+            chip9->chiptod.tod_state = tod_running;
+        }
+    }
+}
+
+static void chiptod_power10_send_remotes(PnvChipTOD *chiptod)
+{
+    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
+    int i;
+
+    for (i = 0; i < pnv->num_chips; i++) {
+        Pnv10Chip *chip10 = PNV10_CHIP(pnv->chips[i]);
+        if (&chip10->chiptod != chiptod) {
+            chip10->chiptod.tod_state = tod_running;
+        }
+    }
+}
+
+static void chiptod_power9_invalidate_remotes(PnvChipTOD *chiptod)
+{
+    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
+    int i;
+
+    for (i = 0; i < pnv->num_chips; i++) {
+        Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
+        if (&chip9->chiptod != chiptod) {
+            chip9->chiptod.tod_state = tod_not_set;
+        }
+    }
+}
+
+static void chiptod_power10_invalidate_remotes(PnvChipTOD *chiptod)
+{
+    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
+    int i;
+
+    for (i = 0; i < pnv->num_chips; i++) {
+        Pnv10Chip *chip10 = PNV10_CHIP(pnv->chips[i]);
+        if (&chip10->chiptod != chiptod) {
+            chip10->chiptod.tod_state = tod_not_set;
+        }
+    }
+}
+
+static void pnv_chiptod_xscom_write(void *opaque, hwaddr addr,
+                                    uint64_t val, unsigned size,
+                                    bool is_power9)
+{
+    PnvChipTOD *chiptod = PNV_CHIPTOD(opaque);
+    uint32_t offset = addr >> 3;
+
+    trace_pnv_chiptod_xscom_write(addr >> 3, val);
+
+    switch (offset) {
+    case TOD_PSS_MSS_CTRL_REG:
+        /* Is this correct? */
+        if (chiptod->primary) {
+            val |= PPC_BIT(1); /* TOD is master */
+        } else {
+            val &= ~PPC_BIT(1);
+        }
+        val |= PPC_BIT(2); /* Drawer is master (don't simulate multi-drawer) */
+        chiptod->pss_mss_ctrl_reg = val & PPC_BITMASK(0, 31);
+        break;
+
+    case TOD_TX_TTYPE_CTRL_REG:
+        if (val & PPC_BIT(35)) { /* SCOM addressing */
+            uint32_t addr = val >> 32;
+            uint32_t reg = addr & 0xfff;
+            PnvCore *pc;
+
+            if (reg != PC_TOD) {
+                qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: SCOM addressing: "
+                              "unimplemented slave register 0x%" PRIx32 "\n",
+                              reg);
+                return;
+            }
+
+            /*
+             * This may not deal with P10 big-core addressing at the moment.
+             * The big-core code in skiboot syncs small cores, but it targets
+             * the even PIR (first small-core) when syncing second small-core.
+             */
+            pc = pnv_get_vcpu_by_xscom_base(chiptod->chip, addr & ~0xfff);
+            if (pc) {
+                /*
+                 * If TCG implements SMT, TFMR is a per-core SPR and should
+                 * be updated such that it is reflected in all threads.
+                 * Same for TB if the chiptod model ever actually adjusted it.
+                 */
+                chiptod->slave_cpu_target = pc->threads[0];
+            } else {
+                chiptod->slave_cpu_target = NULL;
+                qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
+                              " TOD_TX_TTYPE_CTRL_REG val 0x%" PRIx64
+                              " invalid slave PIR\n", val);
+            }
+
+        } else { /* PIR addressing */
+            uint32_t pir;
+
+            if (!is_power9) {
+                qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: PIR addressing"
+                              " is only implemented for POWER9\n");
+                return;
+            }
+
+            pir = (GETFIELD(TOD_TX_TTYPE_PIB_SLAVE_ADDR, val) & 0x1f) << 2;
+            chiptod->slave_cpu_target = ppc_get_vcpu_by_pir(pir);
+            if (chiptod->slave_cpu_target == NULL) {
+                qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
+                              " TOD_TX_TTYPE_CTRL_REG val 0x%" PRIx64
+                              " invalid slave PIR 0x%" PRIx32 "\n", val, pir);
+            }
+        }
+        break;
+    case TOD_ERROR_REG:
+        chiptod->tod_error &= ~val;
+        break;
+    case TOD_LOAD_TOD_MOD_REG:
+        if (!(val & PPC_BIT(0))) {
+            qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
+                          " TOD_LOAD_TOD_MOD_REG with bad val 0x%016lx\n", val);
+        } else {
+            chiptod->tod_state = tod_not_set;
+        }
+        break;
+    case TOD_LOAD_TOD_REG:
+        chiptod->tod_state = tod_running;
+        break;
+    case TOD_MOVE_TOD_TO_TB_REG:
+        if (!(val & PPC_BIT(0))) {
+            qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
+                          " TOD_MOVE_TOD_TO_TB_REG with bad val 0x%016lx\n",
+                          val);
+        } else if (chiptod->slave_cpu_target == NULL) {
+            qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
+                          " TOD_MOVE_TOD_TO_TB_REG with no slave target\n");
+        } else {
+            PowerPCCPU *cpu = chiptod->slave_cpu_target;
+            CPUPPCState *env = &cpu->env;
+
+            if (env->tb_ready_for_tod) {
+                env->tod_sent_to_tb = 1;
+            } else {
+                qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
+                              " TOD_MOVE_TOD_TO_TB_REG with TB not ready to"
+                              " receive TOD\n");
+            }
+        }
+        break;
+    case TOD_TX_TTYPE_4_REG:
+        if (is_power9) {
+            chiptod_power9_send_remotes(chiptod);
+        } else {
+            chiptod_power10_send_remotes(chiptod);
+        }
+        break;
+    case TOD_TX_TTYPE_5_REG:
+        if (is_power9) {
+            chiptod_power9_invalidate_remotes(chiptod);
+        } else {
+            chiptod_power10_invalidate_remotes(chiptod);
+        }
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP, "pnv_chiptod: unimplemented register: Ox%"
+                      HWADDR_PRIx "\n", addr >> 3);
+    }
+}
+
+static void pnv_chiptod_power9_xscom_write(void *opaque, hwaddr addr,
+                                           uint64_t val, unsigned size)
+{
+    pnv_chiptod_xscom_write(opaque, addr, val, size, true);
+}
+
+static const MemoryRegionOps pnv_chiptod_power9_xscom_ops = {
+    .read = pnv_chiptod_xscom_read,
+    .write = pnv_chiptod_power9_xscom_write,
+    .valid.min_access_size = 8,
+    .valid.max_access_size = 8,
+    .impl.min_access_size = 8,
+    .impl.max_access_size = 8,
+    .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static int pnv_chiptod_dt_xscom(PnvXScomInterface *dev, void *fdt,
+                                int xscom_offset,
+                                const char compat[], size_t compat_size)
+{
+    PnvChipTOD *chiptod = PNV_CHIPTOD(dev);
+    g_autofree char *name = NULL;
+    int offset;
+    uint32_t lpc_pcba = PNV9_XSCOM_CHIPTOD_BASE;
+    uint32_t reg[] = {
+        cpu_to_be32(lpc_pcba),
+        cpu_to_be32(PNV9_XSCOM_CHIPTOD_SIZE)
+    };
+
+    name = g_strdup_printf("chiptod@%x", lpc_pcba);
+    offset = fdt_add_subnode(fdt, xscom_offset, name);
+    _FDT(offset);
+
+    if (chiptod->primary) {
+        _FDT((fdt_setprop(fdt, offset, "primary", NULL, 0)));
+    } else if (chiptod->secondary) {
+        _FDT((fdt_setprop(fdt, offset, "secondary", NULL, 0)));
+    }
+
+    _FDT((fdt_setprop(fdt, offset, "reg", reg, sizeof(reg))));
+    _FDT((fdt_setprop(fdt, offset, "compatible", compat, compat_size)));
+    return 0;
+}
+
+static int pnv_chiptod_power9_dt_xscom(PnvXScomInterface *dev, void *fdt,
+                             int xscom_offset)
+{
+    const char compat[] = "ibm,power-chiptod\0ibm,power9-chiptod";
+
+    return pnv_chiptod_dt_xscom(dev, fdt, xscom_offset, compat, sizeof(compat));
+}
+
+static Property pnv_chiptod_properties[] = {
+    DEFINE_PROP_LINK("chip", PnvChipTOD , chip, TYPE_PNV_CHIP, PnvChip *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void pnv_chiptod_power9_class_init(ObjectClass *klass, void *data)
+{
+    PnvChipTODClass *pctc = PNV_CHIPTOD_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PnvXScomInterfaceClass *xdc = PNV_XSCOM_INTERFACE_CLASS(klass);
+
+    dc->desc = "PowerNV ChipTOD Controller (POWER9)";
+    device_class_set_props(dc, pnv_chiptod_properties);
+
+    xdc->dt_xscom = pnv_chiptod_power9_dt_xscom;
+
+    pctc->xscom_size = PNV_XSCOM_CHIPTOD_SIZE;
+    pctc->xscom_ops = &pnv_chiptod_power9_xscom_ops;
+}
+
+static const TypeInfo pnv_chiptod_power9_type_info = {
+    .name          = TYPE_PNV9_CHIPTOD,
+    .parent        = TYPE_PNV_CHIPTOD,
+    .instance_size = sizeof(PnvChipTOD),
+    .class_init    = pnv_chiptod_power9_class_init,
+    .interfaces    = (InterfaceInfo[]) {
+        { TYPE_PNV_XSCOM_INTERFACE },
+        { }
+    }
+};
+
+static void pnv_chiptod_power10_xscom_write(void *opaque, hwaddr addr,
+                                           uint64_t val, unsigned size)
+{
+    pnv_chiptod_xscom_write(opaque, addr, val, size, false);
+}
+
+static const MemoryRegionOps pnv_chiptod_power10_xscom_ops = {
+    .read = pnv_chiptod_xscom_read,
+    .write = pnv_chiptod_power10_xscom_write,
+    .valid.min_access_size = 8,
+    .valid.max_access_size = 8,
+    .impl.min_access_size = 8,
+    .impl.max_access_size = 8,
+    .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static int pnv_chiptod_power10_dt_xscom(PnvXScomInterface *dev, void *fdt,
+                             int xscom_offset)
+{
+    const char compat[] = "ibm,power-chiptod\0ibm,power10-chiptod";
+
+    return pnv_chiptod_dt_xscom(dev, fdt, xscom_offset, compat, sizeof(compat));
+}
+
+static void pnv_chiptod_power10_class_init(ObjectClass *klass, void *data)
+{
+    PnvChipTODClass *pctc = PNV_CHIPTOD_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PnvXScomInterfaceClass *xdc = PNV_XSCOM_INTERFACE_CLASS(klass);
+
+    dc->desc = "PowerNV ChipTOD Controller (POWER10)";
+    device_class_set_props(dc, pnv_chiptod_properties);
+
+    xdc->dt_xscom = pnv_chiptod_power10_dt_xscom;
+
+    pctc->xscom_size = PNV_XSCOM_CHIPTOD_SIZE;
+    pctc->xscom_ops = &pnv_chiptod_power10_xscom_ops;
+}
+
+static const TypeInfo pnv_chiptod_power10_type_info = {
+    .name          = TYPE_PNV10_CHIPTOD,
+    .parent        = TYPE_PNV_CHIPTOD,
+    .instance_size = sizeof(PnvChipTOD),
+    .class_init    = pnv_chiptod_power10_class_init,
+    .interfaces    = (InterfaceInfo[]) {
+        { TYPE_PNV_XSCOM_INTERFACE },
+        { }
+    }
+};
+
+static void pnv_chiptod_realize(DeviceState *dev, Error **errp)
+{
+    static bool got_primary = false;
+    static bool got_secondary = false;
+
+    PnvChipTOD *chiptod = PNV_CHIPTOD(dev);
+    PnvChipTODClass *pctc = PNV_CHIPTOD_GET_CLASS(chiptod);
+
+    if (!got_primary) {
+        got_primary = true;
+        chiptod->primary = true;
+        chiptod->pss_mss_ctrl_reg |= PPC_BIT(1); /* TOD is master */
+    } else if (!got_secondary) {
+        got_secondary = true;
+        chiptod->secondary = true;
+    }
+    /* Drawer is master (we do not simulate multi-drawer) */
+    chiptod->pss_mss_ctrl_reg |= PPC_BIT(2);
+    chiptod->tod_state = tod_running;
+
+    /* XScom regions for ChipTOD registers */
+    pnv_xscom_region_init(&chiptod->xscom_regs, OBJECT(dev),
+                          pctc->xscom_ops, chiptod, "xscom-chiptod",
+                          pctc->xscom_size);
+}
+
+static void pnv_chiptod_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = pnv_chiptod_realize;
+    dc->desc = "PowerNV ChipTOD Controller";
+    dc->user_creatable = false;
+}
+
+static const TypeInfo pnv_chiptod_type_info = {
+    .name          = TYPE_PNV_CHIPTOD,
+    .parent        = TYPE_DEVICE,
+    .instance_size = sizeof(PnvChipTOD),
+    .class_init    = pnv_chiptod_class_init,
+    .class_size    = sizeof(PnvChipTODClass),
+    .abstract      = true,
+};
+
+static void pnv_chiptod_register_types(void)
+{
+    type_register_static(&pnv_chiptod_type_info);
+    type_register_static(&pnv_chiptod_power9_type_info);
+    type_register_static(&pnv_chiptod_power10_type_info);
+}
+
+type_init(pnv_chiptod_register_types);
diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
index d820e05e40..5bbbd3a7a9 100644
--- a/hw/ppc/pnv_xscom.c
+++ b/hw/ppc/pnv_xscom.c
@@ -298,6 +298,8 @@ int pnv_dt_xscom(PnvChip *chip, void *fdt, int root_offset,
     _FDT((fdt_setprop(fdt, xscom_offset, "scom-controller", NULL, 0)));
     if (chip->chip_id == 0) {
         _FDT((fdt_setprop(fdt, xscom_offset, "primary", NULL, 0)));
+    } else if (chip->chip_id == 1) {
+        _FDT((fdt_setprop(fdt, xscom_offset, "secondary", NULL, 0)));
     }
 
     args.fdt = fdt;
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index f670e8906c..57c4f265ef 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -95,6 +95,10 @@ vof_write(uint32_t ih, unsigned cb, const char *msg) "ih=0x%x [%u] \"%s\""
 vof_avail(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
 vof_claimed(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
 
+# pnv_chiptod.c
+pnv_chiptod_xscom_read(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
+pnv_chiptod_xscom_write(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
+
 # pnv_sbe.c
 pnv_sbe_xscom_ctrl_read(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
 pnv_sbe_xscom_ctrl_write(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
index 53e1d921d7..d22c013e7d 100644
--- a/include/hw/ppc/pnv_chip.h
+++ b/include/hw/ppc/pnv_chip.h
@@ -2,6 +2,7 @@
 #define PPC_PNV_CHIP_H
 
 #include "hw/pci-host/pnv_phb4.h"
+#include "hw/ppc/pnv_chiptod.h"
 #include "hw/ppc/pnv_core.h"
 #include "hw/ppc/pnv_homer.h"
 #include "hw/ppc/pnv_lpc.h"
@@ -77,6 +78,7 @@ struct Pnv9Chip {
     PnvXive      xive;
     Pnv9Psi      psi;
     PnvLpcController lpc;
+    PnvChipTOD   chiptod;
     PnvOCC       occ;
     PnvSBE       sbe;
     PnvHomer     homer;
@@ -106,6 +108,7 @@ struct Pnv10Chip {
     PnvXive2     xive;
     Pnv9Psi      psi;
     PnvLpcController lpc;
+    PnvChipTOD   chiptod;
     PnvOCC       occ;
     PnvSBE       sbe;
     PnvHomer     homer;
diff --git a/include/hw/ppc/pnv_chiptod.h b/include/hw/ppc/pnv_chiptod.h
new file mode 100644
index 0000000000..6723b07d93
--- /dev/null
+++ b/include/hw/ppc/pnv_chiptod.h
@@ -0,0 +1,64 @@
+/*
+ * QEMU PowerPC PowerNV Emulation of some CHIPTOD behaviour
+ *
+ * Copyright (c) 2022-2023, 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.1 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_CHIPTOD_H
+#define PPC_PNV_CHIPTOD_H
+
+#include "qom/object.h"
+
+#define TYPE_PNV_CHIPTOD "pnv-chiptod"
+OBJECT_DECLARE_TYPE(PnvChipTOD, PnvChipTODClass, PNV_CHIPTOD)
+#define TYPE_PNV9_CHIPTOD TYPE_PNV_CHIPTOD "-POWER9"
+DECLARE_INSTANCE_CHECKER(PnvChipTOD, PNV9_CHIPTOD, TYPE_PNV9_CHIPTOD)
+#define TYPE_PNV10_CHIPTOD TYPE_PNV_CHIPTOD "-POWER10"
+DECLARE_INSTANCE_CHECKER(PnvChipTOD, PNV10_CHIPTOD, TYPE_PNV10_CHIPTOD)
+
+enum tod_state {
+    tod_error = 0,
+    tod_not_set = 7,
+    tod_not_set_step = 11,
+    tod_running = 2,
+    tod_running_step = 10,
+    tod_running_sync = 14,
+    tod_wait_for_sync = 13,
+    tod_stopped = 1,
+};
+
+struct PnvChipTOD {
+    DeviceState xd;
+
+    PnvChip *chip;
+    MemoryRegion xscom_regs;
+
+    bool primary;
+    bool secondary;
+    enum tod_state tod_state;
+    uint64_t tod_error;
+    uint64_t pss_mss_ctrl_reg;
+    PowerPCCPU *slave_cpu_target;
+};
+
+struct PnvChipTODClass {
+    DeviceClass parent_class;
+
+    int xscom_size;
+    const MemoryRegionOps *xscom_ops;
+};
+
+#endif /* PPC_PNV_CHIPTOD_H */
diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
index 3d75706e95..832b339ed6 100644
--- a/include/hw/ppc/pnv_core.h
+++ b/include/hw/ppc/pnv_core.h
@@ -69,4 +69,7 @@ struct PnvQuad {
     uint32_t quad_id;
     MemoryRegion xscom_regs;
 };
+
+PnvCore *pnv_get_vcpu_by_xscom_base(PnvChip *chip, uint32_t xscom_base);
+
 #endif /* PPC_PNV_CORE_H */
diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
index cbe848d27b..530f89af55 100644
--- a/include/hw/ppc/pnv_xscom.h
+++ b/include/hw/ppc/pnv_xscom.h
@@ -64,6 +64,9 @@ struct PnvXScomInterfaceClass {
 #define PNV_XSCOM_PSIHB_BASE      0x2010900
 #define PNV_XSCOM_PSIHB_SIZE      0x20
 
+#define PNV_XSCOM_CHIPTOD_BASE    0x0040000
+#define PNV_XSCOM_CHIPTOD_SIZE    0x31
+
 #define PNV_XSCOM_OCC_BASE        0x0066000
 #define PNV_XSCOM_OCC_SIZE        0x6000
 
@@ -90,6 +93,9 @@ struct PnvXScomInterfaceClass {
     ((uint64_t)(((core) & 0x1C) + 0x40) << 22)
 #define PNV9_XSCOM_EQ_SIZE        0x100000
 
+#define PNV9_XSCOM_CHIPTOD_BASE   PNV_XSCOM_CHIPTOD_BASE
+#define PNV9_XSCOM_CHIPTOD_SIZE   PNV_XSCOM_CHIPTOD_SIZE
+
 #define PNV9_XSCOM_OCC_BASE       PNV_XSCOM_OCC_BASE
 #define PNV9_XSCOM_OCC_SIZE       0x8000
 
@@ -138,6 +144,9 @@ struct PnvXScomInterfaceClass {
 #define PNV10_XSCOM_PSIHB_BASE     0x3011D00
 #define PNV10_XSCOM_PSIHB_SIZE     0x100
 
+#define PNV10_XSCOM_CHIPTOD_BASE   PNV9_XSCOM_CHIPTOD_BASE
+#define PNV10_XSCOM_CHIPTOD_SIZE   PNV9_XSCOM_CHIPTOD_SIZE
+
 #define PNV10_XSCOM_OCC_BASE       PNV9_XSCOM_OCC_BASE
 #define PNV10_XSCOM_OCC_SIZE       PNV9_XSCOM_OCC_SIZE
 
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 8c30c59a56..d73cce8474 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1172,6 +1172,12 @@ struct CPUArchState {
     uint32_t tlb_need_flush; /* Delayed flush needed */
 #define TLB_NEED_LOCAL_FLUSH   0x1
 #define TLB_NEED_GLOBAL_FLUSH  0x2
+
+#if defined(TARGET_PPC64)
+    /* PowerNV chiptod / timebase facility state. */
+    int tb_ready_for_tod; /* core TB ready to receive TOD from chiptod */
+    int tod_sent_to_tb;   /* chiptod sent TOD to the core TB */
+#endif
 #endif
 
     /* Other registers */
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/4] target/ppc: Tidy POWER book4 SPR registration
  2023-06-03 23:36 [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models Nicholas Piggin
  2023-06-03 23:36 ` [PATCH 1/4] pnv/chiptod: Add POWER9/10 chiptod model Nicholas Piggin
@ 2023-06-03 23:36 ` Nicholas Piggin
  2023-06-05 14:58   ` Cédric Le Goater
  2023-06-03 23:36 ` [PATCH 3/4] target/ppc: add TFMR SPR implementation with read and write helpers Nicholas Piggin
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Nicholas Piggin @ 2023-06-03 23:36 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Daniel Henrique Barboza,
	Cédric Le Goater

POWER book4 (implementation-specific) SPRs are sometimes in their own
functions, but in other cases are mixed with architected SPRs. Do some
spring cleaning on these.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu_init.c | 92 ++++++++++++++++++++++++++++---------------
 1 file changed, 60 insertions(+), 32 deletions(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index e9717b8169..da0f7a7159 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5374,31 +5374,6 @@ static void register_book3s_ids_sprs(CPUPPCState *env)
                  &spr_read_generic, SPR_NOACCESS,
                  &spr_read_generic, NULL,
                  0x00000000);
-    spr_register_hv(env, SPR_HID0, "HID0",
-                 SPR_NOACCESS, SPR_NOACCESS,
-                 SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_core_write_generic,
-                 0x00000000);
-    spr_register_hv(env, SPR_TSCR, "TSCR",
-                 SPR_NOACCESS, SPR_NOACCESS,
-                 SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic32,
-                 0x00000000);
-    spr_register_hv(env, SPR_HMER, "HMER",
-                 SPR_NOACCESS, SPR_NOACCESS,
-                 SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_hmer,
-                 0x00000000);
-    spr_register_hv(env, SPR_HMEER, "HMEER",
-                 SPR_NOACCESS, SPR_NOACCESS,
-                 SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic,
-                 0x00000000);
-    spr_register_hv(env, SPR_TFMR, "TFMR",
-                 SPR_NOACCESS, SPR_NOACCESS,
-                 SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic,
-                 0x00000000);
     spr_register_hv(env, SPR_LPIDR, "LPIDR",
                  SPR_NOACCESS, SPR_NOACCESS,
                  SPR_NOACCESS, SPR_NOACCESS,
@@ -5454,11 +5429,6 @@ static void register_book3s_ids_sprs(CPUPPCState *env)
                  SPR_NOACCESS, SPR_NOACCESS,
                  &spr_read_generic, &spr_write_generic,
                  0x00000000);
-    spr_register_hv(env, SPR_LDBAR, "LDBAR",
-                 SPR_NOACCESS, SPR_NOACCESS,
-                 SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic,
-                 0x00000000);
 }
 
 static void register_rmor_sprs(CPUPPCState *env)
@@ -5665,14 +5635,65 @@ static void register_power8_ic_sprs(CPUPPCState *env)
 #endif
 }
 
+/* SPRs specific to IBM POWER CPUs */
+static void register_power_common_book4_sprs(CPUPPCState *env)
+{
+#if !defined(CONFIG_USER_ONLY)
+    spr_register_hv(env, SPR_HID0, "HID0",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_core_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_TSCR, "TSCR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic32,
+                 0x00000000);
+    spr_register_hv(env, SPR_HMER, "HMER",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_hmer,
+                 0x00000000);
+    spr_register_hv(env, SPR_HMEER, "HMEER",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_TFMR, "TFMR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_LDBAR, "LDBAR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+#endif
+}
+
+static void register_power9_book4_sprs(CPUPPCState *env)
+{
+    /* Add a number of P9 book4 registers */
+    register_power_common_book4_sprs(env);
+#if !defined(CONFIG_USER_ONLY)
+    spr_register_kvm(env, SPR_WORT, "WORT",
+                     SPR_NOACCESS, SPR_NOACCESS,
+                     &spr_read_generic, &spr_write_generic,
+                     KVM_REG_PPC_WORT, 0);
+#endif
+}
+
 static void register_power8_book4_sprs(CPUPPCState *env)
 {
     /* Add a number of P8 book4 registers */
+    register_power_common_book4_sprs(env);
 #if !defined(CONFIG_USER_ONLY)
     spr_register_kvm(env, SPR_ACOP, "ACOP",
                      SPR_NOACCESS, SPR_NOACCESS,
                      &spr_read_generic, &spr_write_generic,
                      KVM_REG_PPC_ACOP, 0);
+    /* PID is only in BookE in ISA v2.07 */
     spr_register_kvm(env, SPR_BOOKS_PID, "PID",
                      SPR_NOACCESS, SPR_NOACCESS,
                      &spr_read_generic, &spr_write_pidr,
@@ -5688,10 +5709,12 @@ static void register_power7_book4_sprs(CPUPPCState *env)
 {
     /* Add a number of P7 book4 registers */
 #if !defined(CONFIG_USER_ONLY)
+    register_power_common_book4_sprs(env);
     spr_register_kvm(env, SPR_ACOP, "ACOP",
                      SPR_NOACCESS, SPR_NOACCESS,
                      &spr_read_generic, &spr_write_generic,
                      KVM_REG_PPC_ACOP, 0);
+    /* PID is only in BookE in ISA v2.06 */
     spr_register_kvm(env, SPR_BOOKS_PID, "PID",
                      SPR_NOACCESS, SPR_NOACCESS,
                      &spr_read_generic, &spr_write_generic32,
@@ -5725,6 +5748,11 @@ static void register_power9_mmu_sprs(CPUPPCState *env)
                     SPR_NOACCESS, SPR_NOACCESS,
                     &spr_read_generic, &spr_write_generic,
                     0x0000000000000000);
+    /* PID is part of the BookS ISA from v3.0 */
+    spr_register_kvm(env, SPR_BOOKS_PID, "PID",
+                     SPR_NOACCESS, SPR_NOACCESS,
+                     &spr_read_generic, &spr_write_pidr,
+                     KVM_REG_PPC_PID, 0);
 #endif
 }
 
@@ -6278,7 +6306,7 @@ static void init_proc_POWER9(CPUPPCState *env)
     register_power8_dpdes_sprs(env);
     register_vtb_sprs(env);
     register_power8_ic_sprs(env);
-    register_power8_book4_sprs(env);
+    register_power9_book4_sprs(env);
     register_power8_rpr_sprs(env);
     register_power9_mmu_sprs(env);
 
@@ -6471,7 +6499,7 @@ static void init_proc_POWER10(CPUPPCState *env)
     register_power8_dpdes_sprs(env);
     register_vtb_sprs(env);
     register_power8_ic_sprs(env);
-    register_power8_book4_sprs(env);
+    register_power9_book4_sprs(env);
     register_power8_rpr_sprs(env);
     register_power9_mmu_sprs(env);
     register_power10_hash_sprs(env);
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/4] target/ppc: add TFMR SPR implementation with read and write helpers
  2023-06-03 23:36 [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models Nicholas Piggin
  2023-06-03 23:36 ` [PATCH 1/4] pnv/chiptod: Add POWER9/10 chiptod model Nicholas Piggin
  2023-06-03 23:36 ` [PATCH 2/4] target/ppc: Tidy POWER book4 SPR registration Nicholas Piggin
@ 2023-06-03 23:36 ` Nicholas Piggin
  2023-06-14  8:38   ` Cédric Le Goater
  2023-06-03 23:36 ` [PATCH 4/4] target/ppc: Implement core timebase state machine and TFMR Nicholas Piggin
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Nicholas Piggin @ 2023-06-03 23:36 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Daniel Henrique Barboza,
	Cédric Le Goater

TFMR is the Time Facility Management Register which is specific to POWER
CPUs, and used for the purpose of timebase management (generally by
firmware, not the OS).

This adds an initial simple TFMR register, which will form part of the
core timebase facility model in the next patch.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu_init.c        |  2 +-
 target/ppc/helper.h          |  2 ++
 target/ppc/spr_common.h      |  2 ++
 target/ppc/timebase_helper.c | 13 +++++++++++++
 target/ppc/translate.c       | 10 ++++++++++
 5 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index da0f7a7159..37088021d2 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5662,7 +5662,7 @@ static void register_power_common_book4_sprs(CPUPPCState *env)
     spr_register_hv(env, SPR_TFMR, "TFMR",
                  SPR_NOACCESS, SPR_NOACCESS,
                  SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic,
+                 &spr_read_tfmr, &spr_write_tfmr,
                  0x00000000);
     spr_register_hv(env, SPR_LDBAR, "LDBAR",
                  SPR_NOACCESS, SPR_NOACCESS,
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 16bb485c1a..166cacb3f9 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -746,6 +746,8 @@ DEF_HELPER_2(store_40x_dbcr0, void, env, tl)
 DEF_HELPER_2(store_40x_sler, void, env, tl)
 DEF_HELPER_FLAGS_2(store_booke_tcr, TCG_CALL_NO_RWG, void, env, tl)
 DEF_HELPER_FLAGS_2(store_booke_tsr, TCG_CALL_NO_RWG, void, env, tl)
+DEF_HELPER_1(load_tfmr, tl, env)
+DEF_HELPER_2(store_tfmr, void, env, tl)
 DEF_HELPER_3(store_ibatl, void, env, i32, tl)
 DEF_HELPER_3(store_ibatu, void, env, i32, tl)
 DEF_HELPER_3(store_dbatl, void, env, i32, tl)
diff --git a/target/ppc/spr_common.h b/target/ppc/spr_common.h
index d6c679cd99..8ab17123a4 100644
--- a/target/ppc/spr_common.h
+++ b/target/ppc/spr_common.h
@@ -196,6 +196,8 @@ void spr_write_ebb(DisasContext *ctx, int sprn, int gprn);
 void spr_read_ebb_upper32(DisasContext *ctx, int gprn, int sprn);
 void spr_write_ebb_upper32(DisasContext *ctx, int sprn, int gprn);
 void spr_write_hmer(DisasContext *ctx, int sprn, int gprn);
+void spr_read_tfmr(DisasContext *ctx, int gprn, int sprn);
+void spr_write_tfmr(DisasContext *ctx, int sprn, int gprn);
 void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn);
 void spr_read_dexcr_ureg(DisasContext *ctx, int gprn, int sprn);
 #endif
diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
index de1ee85e0b..34b1d5ad05 100644
--- a/target/ppc/timebase_helper.c
+++ b/target/ppc/timebase_helper.c
@@ -270,6 +270,19 @@ void helper_store_booke_tsr(CPUPPCState *env, target_ulong val)
     store_booke_tsr(env, val);
 }
 
+#if defined(TARGET_PPC64)
+/* POWER processor Timebase Facility */
+target_ulong helper_load_tfmr(CPUPPCState *env)
+{
+    return env->spr[SPR_TFMR];
+}
+
+void helper_store_tfmr(CPUPPCState *env, target_ulong val)
+{
+    env->spr[SPR_TFMR] = val;
+}
+#endif
+
 /*****************************************************************************/
 /* Embedded PowerPC specific helpers */
 
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 8b312b46e0..9dcd66eac8 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1255,6 +1255,16 @@ void spr_write_hmer(DisasContext *ctx, int sprn, int gprn)
     spr_store_dump_spr(sprn);
 }
 
+void spr_read_tfmr(DisasContext *ctx, int gprn, int sprn)
+{
+    gen_helper_load_tfmr(cpu_gpr[gprn], cpu_env);
+}
+
+void spr_write_tfmr(DisasContext *ctx, int sprn, int gprn)
+{
+    gen_helper_store_tfmr(cpu_env, cpu_gpr[gprn]);
+}
+
 void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn)
 {
     gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/4] target/ppc: Implement core timebase state machine and TFMR
  2023-06-03 23:36 [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models Nicholas Piggin
                   ` (2 preceding siblings ...)
  2023-06-03 23:36 ` [PATCH 3/4] target/ppc: add TFMR SPR implementation with read and write helpers Nicholas Piggin
@ 2023-06-03 23:36 ` Nicholas Piggin
  2023-06-14  8:55   ` Cédric Le Goater
  2023-06-06 13:59 ` [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models Cédric Le Goater
  2023-06-22  7:30 ` Cédric Le Goater
  5 siblings, 1 reply; 18+ messages in thread
From: Nicholas Piggin @ 2023-06-03 23:36 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Daniel Henrique Barboza,
	Cédric Le Goater

This implements the core timebase state machine, which is the core side
of the time-of-day system in POWER processors. This facility is operated
by control fields in the TFMR register, which also contains status
fields.

The core timebase interacts with the chiptod hardware, primarily to
receive TOD updates, to synchronise timebase with other cores. This
model does not actually update TB values with TOD or updates received
from the chiptod, as timebases are always synchronised. It does step
through the states required to perform the update.

There are several asynchronous state transitions. These are modelled
using using mfTFMR to drive state changes, because it is expected that
firmware poll the register to wait for those states. This is good enough
to test basic firmware behaviour without adding real timers. The values
chosen are arbitrary.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu.h             |  34 ++++++++
 target/ppc/timebase_helper.c | 147 ++++++++++++++++++++++++++++++++++-
 2 files changed, 179 insertions(+), 2 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index d73cce8474..b1520ea4db 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1177,6 +1177,13 @@ struct CPUArchState {
     /* PowerNV chiptod / timebase facility state. */
     int tb_ready_for_tod; /* core TB ready to receive TOD from chiptod */
     int tod_sent_to_tb;   /* chiptod sent TOD to the core TB */
+
+    /*
+     * Timers for async events are simulated by mfTFAC because TFAC is to be
+     * polled for event.
+     */
+    int tb_state_timer;
+    int tb_sync_pulse_timer;
 #endif
 #endif
 
@@ -2527,6 +2534,33 @@ enum {
     HMER_XSCOM_STATUS_MASK      = PPC_BITMASK(21, 23),
 };
 
+/* TFMR */
+enum {
+    TFMR_CONTROL_MASK           = PPC_BITMASK(0, 24),
+    TFMR_MASK_HMI               = PPC_BIT(10),
+    TFMR_TB_ECLIPZ              = PPC_BIT(14),
+    TFMR_LOAD_TOD_MOD           = PPC_BIT(16),
+    TFMR_MOVE_CHIP_TOD_TO_TB    = PPC_BIT(18),
+    TFMR_CLEAR_TB_ERRORS        = PPC_BIT(24),
+    TFMR_STATUS_MASK            = PPC_BITMASK(25, 63),
+    TFMR_TBST_ENCODED           = PPC_BITMASK(28, 31), /* TBST = TB State */
+    TFMR_TBST_LAST              = PPC_BITMASK(32, 35), /* Previous TBST */
+    TFMR_TB_ENABLED             = PPC_BIT(40),
+    TFMR_TB_VALID               = PPC_BIT(41),
+    TFMR_TB_SYNC_OCCURED        = PPC_BIT(42),
+};
+
+/* TFMR TBST */
+enum {
+    TBST_RESET                  = 0x0,
+    TBST_SEND_TOD_MOD           = 0x1,
+    TBST_NOT_SET                = 0x2,
+    TBST_SYNC_WAIT              = 0x6,
+    TBST_GET_TOD                = 0x7,
+    TBST_TB_RUNNING             = 0x8,
+    TBST_TB_ERROR               = 0x9,
+};
+
 /*****************************************************************************/
 
 #define is_isa300(ctx) (!!(ctx->insns_flags2 & PPC2_ISA300))
diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
index 34b1d5ad05..11a06fafe6 100644
--- a/target/ppc/timebase_helper.c
+++ b/target/ppc/timebase_helper.c
@@ -272,14 +272,157 @@ void helper_store_booke_tsr(CPUPPCState *env, target_ulong val)
 
 #if defined(TARGET_PPC64)
 /* POWER processor Timebase Facility */
+static unsigned int tfmr_get_tb_state(uint64_t tfmr)
+{
+    return (tfmr & TFMR_TBST_ENCODED) >> (63 - 31);
+}
+
+static uint64_t tfmr_new_tb_state(uint64_t tfmr, unsigned int tbst)
+{
+    tfmr &= ~TFMR_TBST_LAST;
+    tfmr |= (tfmr & TFMR_TBST_ENCODED) >> 4; /* move state to last state */
+    tfmr &= ~TFMR_TBST_ENCODED;
+    tfmr |= (uint64_t)tbst << (63 - 31); /* move new state to state */
+
+    if (tbst == TBST_TB_RUNNING) {
+        tfmr |= TFMR_TB_VALID;
+    } else {
+        tfmr &= ~TFMR_TB_VALID;
+    }
+
+    return tfmr;
+}
+
+static void tb_state_machine_step(CPUPPCState *env)
+{
+    uint64_t tfmr = env->spr[SPR_TFMR];
+    unsigned int tbst = tfmr_get_tb_state(tfmr);
+
+    if (!(tfmr & TFMR_TB_ECLIPZ) || tbst == TBST_TB_ERROR) {
+        return;
+    }
+
+    if (env->tb_sync_pulse_timer) {
+        env->tb_sync_pulse_timer--;
+    } else {
+        tfmr |= TFMR_TB_SYNC_OCCURED;
+        env->spr[SPR_TFMR] = tfmr;
+    }
+
+    if (env->tb_state_timer) {
+        env->tb_state_timer--;
+        return;
+    }
+
+    if (tfmr & TFMR_LOAD_TOD_MOD) {
+        tfmr &= ~TFMR_LOAD_TOD_MOD;
+        if (tbst == TBST_GET_TOD) {
+            tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR);
+        } else {
+            tfmr = tfmr_new_tb_state(tfmr, TBST_SEND_TOD_MOD);
+            /* State seems to transition immediately */
+            tfmr = tfmr_new_tb_state(tfmr, TBST_NOT_SET);
+        }
+    } else if (tfmr & TFMR_MOVE_CHIP_TOD_TO_TB) {
+        if (tbst == TBST_SYNC_WAIT) {
+            tfmr = tfmr_new_tb_state(tfmr, TBST_GET_TOD);
+            env->tb_state_timer = 3;
+        } else if (tbst == TBST_GET_TOD) {
+            if (env->tod_sent_to_tb) {
+                tfmr = tfmr_new_tb_state(tfmr, TBST_TB_RUNNING);
+                tfmr &= ~TFMR_MOVE_CHIP_TOD_TO_TB;
+                env->tb_ready_for_tod = 0;
+                env->tod_sent_to_tb = 0;
+            }
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: MOVE_CHIP_TOD_TO_TB "
+                          "state machine in invalid state 0x%x\n", tbst);
+            tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR);
+            env->tb_ready_for_tod = 0;
+        }
+    }
+
+    env->spr[SPR_TFMR] = tfmr;
+}
+
 target_ulong helper_load_tfmr(CPUPPCState *env)
 {
-    return env->spr[SPR_TFMR];
+    tb_state_machine_step(env);
+
+    return env->spr[SPR_TFMR] | TFMR_TB_ECLIPZ;
 }
 
 void helper_store_tfmr(CPUPPCState *env, target_ulong val)
 {
-    env->spr[SPR_TFMR] = val;
+    uint64_t tfmr = env->spr[SPR_TFMR];
+    unsigned int tbst = tfmr_get_tb_state(tfmr);
+
+    if (!(val & TFMR_TB_ECLIPZ)) {
+        qemu_log_mask(LOG_UNIMP, "TFMR non-ECLIPZ mode not implemented\n");
+        tfmr &= ~TFMR_TBST_ENCODED;
+        tfmr &= ~TFMR_TBST_LAST;
+        goto out;
+    }
+
+    /* Update control bits */
+    tfmr = (tfmr & ~TFMR_CONTROL_MASK) | (val & TFMR_CONTROL_MASK);
+
+    /* mtspr always clears this */
+    tfmr &= ~TFMR_TB_SYNC_OCCURED;
+    env->tb_sync_pulse_timer = 1;
+
+    /*
+     * We don't implement any of the error status bits that can be
+     * cleared by writing 1 to them. TB error injection / simulation
+     * would have to implement some.
+     *
+     * Also don't implement mfTB failing when the TB state machine is
+     * not running.
+     */
+
+    if (((tfmr | val) & (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB)) ==
+                        (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: LOAD_TOD_MOD and "
+                                       "MOVE_CHIP_TOD_TO_TB both set\n");
+        tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR);
+        env->tb_ready_for_tod = 0;
+        goto out;
+    }
+
+    if (tfmr & TFMR_CLEAR_TB_ERRORS) {
+        tfmr = tfmr_new_tb_state(tfmr, TBST_RESET);
+        tfmr &= ~TFMR_CLEAR_TB_ERRORS;
+        tfmr &= ~TFMR_LOAD_TOD_MOD;
+        tfmr &= ~TFMR_MOVE_CHIP_TOD_TO_TB;
+        env->tb_ready_for_tod = 0;
+        env->tod_sent_to_tb = 0;
+        goto out;
+    }
+
+    if (tbst == TBST_TB_ERROR) {
+        qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: mtspr TFMR in TB_ERROR"
+                                       " state\n");
+        return;
+    }
+
+    if (tfmr & TFMR_LOAD_TOD_MOD) {
+        env->tb_state_timer = 3;
+    } else if (tfmr & TFMR_MOVE_CHIP_TOD_TO_TB) {
+        if (tbst == TBST_NOT_SET) {
+            tfmr = tfmr_new_tb_state(tfmr, TBST_SYNC_WAIT);
+            env->tb_ready_for_tod = 1;
+            env->tb_state_timer = 3;
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: MOVE_CHIP_TOD_TO_TB "
+                                           "not in TB not set state 0x%x\n",
+                                           tbst);
+            tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR);
+            env->tb_ready_for_tod = 0;
+        }
+    }
+
+out:
+    env->spr[SPR_TFMR] = tfmr;
 }
 #endif
 
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] pnv/chiptod: Add POWER9/10 chiptod model
  2023-06-03 23:36 ` [PATCH 1/4] pnv/chiptod: Add POWER9/10 chiptod model Nicholas Piggin
@ 2023-06-05 14:57   ` Cédric Le Goater
  2023-06-14  5:30     ` Nicholas Piggin
  0 siblings, 1 reply; 18+ messages in thread
From: Cédric Le Goater @ 2023-06-05 14:57 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: qemu-devel, Daniel Henrique Barboza, Frederic Barrat

On 6/4/23 01:36, Nicholas Piggin wrote:
> The chiptod is a pervasive facility which can keep TOD (time-of-day),
> synchronise it across multiple chips, and can move that TOD to or from
> the core timebase units.
> 
> This driver implements basic emulation of chiptod registers sufficient
> to successfully run the skiboot chiptod synchronisation procedure
> (with the following TFMR and timebase state-machine implementation).
> 
> The main way chiptod affects the rest of the system (relevant to the
> powernv model) is to interact with the timebase facility in the cores,
> influencing the timebase state machine and registers.
> 
> The way this chiptod driver implements that interaction is with two
> new flags in the CPUPPCState env, one is use for the core timebase to
> indicate it is ready to receive a TOD from chiptod, the other used
> by chiptod to indicate that it has sent TOD to the core timebase. The
> core timebase will be implemented in later changes.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   hw/ppc/meson.build           |   1 +
>   hw/ppc/pnv.c                 |  38 +++
>   hw/ppc/pnv_chiptod.c         | 488 +++++++++++++++++++++++++++++++++++
>   hw/ppc/pnv_xscom.c           |   2 +
>   hw/ppc/trace-events          |   4 +
>   include/hw/ppc/pnv_chip.h    |   3 +
>   include/hw/ppc/pnv_chiptod.h |  64 +++++
>   include/hw/ppc/pnv_core.h    |   3 +
>   include/hw/ppc/pnv_xscom.h   |   9 +
>   target/ppc/cpu.h             |   6 +
>   10 files changed, 618 insertions(+)
>   create mode 100644 hw/ppc/pnv_chiptod.c
>   create mode 100644 include/hw/ppc/pnv_chiptod.h
> 
> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> index c927337da0..afbf90e6da 100644
> --- a/hw/ppc/meson.build
> +++ b/hw/ppc/meson.build
> @@ -45,6 +45,7 @@ ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
>     'pnv_core.c',
>     'pnv_lpc.c',
>     'pnv_psi.c',
> +  'pnv_chiptod.c',
>     'pnv_occ.c',
>     'pnv_sbe.c',
>     'pnv_bmc.c',
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index dbdeba6c31..ce5e7d7582 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1414,6 +1414,8 @@ static void pnv_chip_power9_instance_init(Object *obj)
>   
>       object_initialize_child(obj, "lpc", &chip9->lpc, TYPE_PNV9_LPC);
>   
> +    object_initialize_child(obj, "chiptod", &chip9->chiptod, TYPE_PNV9_CHIPTOD);
> +
>       object_initialize_child(obj, "occ", &chip9->occ, TYPE_PNV9_OCC);
>   
>       object_initialize_child(obj, "sbe", &chip9->sbe, TYPE_PNV9_SBE);
> @@ -1558,6 +1560,15 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>       chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
>                                               (uint64_t) PNV9_LPCM_BASE(chip));
>   
> +    /* ChipTOD */
> +    object_property_set_link(OBJECT(&chip9->chiptod), "chip", OBJECT(chip),
> +                             &error_abort);
> +    if (!qdev_realize(DEVICE(&chip9->chiptod), NULL, errp)) {
> +        return;
> +    }
> +    pnv_xscom_add_subregion(chip, PNV9_XSCOM_CHIPTOD_BASE,
> +                            &chip9->chiptod.xscom_regs);
> +
>       /* Create the simplified OCC model */
>       if (!qdev_realize(DEVICE(&chip9->occ), NULL, errp)) {
>           return;
> @@ -1644,6 +1655,7 @@ static void pnv_chip_power10_instance_init(Object *obj)
>                                 "xive-fabric");
>       object_initialize_child(obj, "psi", &chip10->psi, TYPE_PNV10_PSI);
>       object_initialize_child(obj, "lpc", &chip10->lpc, TYPE_PNV10_LPC);
> +    object_initialize_child(obj, "chiptod", &chip10->chiptod, TYPE_PNV10_CHIPTOD);
>       object_initialize_child(obj, "occ",  &chip10->occ, TYPE_PNV10_OCC);
>       object_initialize_child(obj, "sbe",  &chip10->sbe, TYPE_PNV10_SBE);
>       object_initialize_child(obj, "homer", &chip10->homer, TYPE_PNV10_HOMER);
> @@ -1773,6 +1785,15 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>       chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
>                                               (uint64_t) PNV10_LPCM_BASE(chip));
>   
> +    /* ChipTOD */
> +    object_property_set_link(OBJECT(&chip10->chiptod), "chip", OBJECT(chip),
> +                             &error_abort);
> +    if (!qdev_realize(DEVICE(&chip10->chiptod), NULL, errp)) {
> +        return;
> +    }
> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_CHIPTOD_BASE,
> +                            &chip10->chiptod.xscom_regs);
> +
>       /* Create the simplified OCC model */
>       if (!qdev_realize(DEVICE(&chip10->occ), NULL, errp)) {
>           return;
> @@ -1938,6 +1959,23 @@ static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
>       }
>   }
>   
> +PnvCore *pnv_get_vcpu_by_xscom_base(PnvChip *chip, uint32_t xscom_base)
> +{
> +    PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> +    int i;
> +
> +    for (i = 0; i < chip->nr_cores; i++) {
> +        PnvCore *pc = chip->cores[i];
> +        CPUCore *cc = CPU_CORE(pc);
> +        int core_hwid = cc->core_id;
> +
> +        if (pcc->xscom_core_base(chip, core_hwid) == xscom_base) {
> +            return pc;
> +        }
> +    }
> +    return NULL;
> +}
> +
>   static void pnv_chip_realize(DeviceState *dev, Error **errp)
>   {
>       PnvChip *chip = PNV_CHIP(dev);
> diff --git a/hw/ppc/pnv_chiptod.c b/hw/ppc/pnv_chiptod.c
> new file mode 100644
> index 0000000000..04ef703e0f
> --- /dev/null
> +++ b/hw/ppc/pnv_chiptod.c
> @@ -0,0 +1,488 @@
> +/*
> + * QEMU PowerPC PowerNV Emulation of some CHIPTOD behaviour
> + *
> + * Copyright (c) 2022-2023, 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/>.

You can simplify the header with

  * SPDX-License-Identifier: GPL-2.0-or-later


> + */
> +
> +#include "qemu/osdep.h"
> +#include "target/ppc/cpu.h"
> +#include "qapi/error.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +#include "hw/irq.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/ppc/fdt.h"
> +#include "hw/ppc/ppc.h"
> +#include "hw/ppc/pnv.h"
> +#include "hw/ppc/pnv_chip.h"
> +#include "hw/ppc/pnv_core.h"
> +#include "hw/ppc/pnv_xscom.h"
> +#include "hw/ppc/pnv_chiptod.h"
> +#include "trace.h"
> +
> +#include <libfdt.h>
> +
> +/* TOD chip XSCOM addresses */
> +#define TOD_M_PATH_CTRL_REG             0x00000000 /* Master Path ctrl reg */
> +#define TOD_PRI_PORT_0_CTRL_REG         0x00000001 /* Primary port0 ctrl reg */
> +#define TOD_PRI_PORT_1_CTRL_REG         0x00000002 /* Primary port1 ctrl reg */
> +#define TOD_SEC_PORT_0_CTRL_REG         0x00000003 /* Secondary p0 ctrl reg */
> +#define TOD_SEC_PORT_1_CTRL_REG         0x00000004 /* Secondary p1 ctrl reg */
> +#define TOD_S_PATH_CTRL_REG             0x00000005 /* Slave Path ctrl reg */
> +#define TOD_I_PATH_CTRL_REG             0x00000006 /* Internal Path ctrl reg */
> +
> +/* -- TOD primary/secondary master/slave control register -- */
> +#define TOD_PSS_MSS_CTRL_REG            0x00000007
> +
> +/* -- TOD primary/secondary master/slave status register -- */
> +#define TOD_PSS_MSS_STATUS_REG          0x00000008
> +
> +/* TOD chip XSCOM addresses */
> +#define TOD_CHIP_CTRL_REG               0x00000010 /* Chip control reg */
> +
> +#define TOD_TX_TTYPE_0_REG              0x00000011
> +#define TOD_TX_TTYPE_1_REG              0x00000012 /* PSS switch reg */
> +#define TOD_TX_TTYPE_2_REG              0x00000013 /* Enable step checkers */
> +#define TOD_TX_TTYPE_3_REG              0x00000014 /* Request TOD reg */
> +#define TOD_TX_TTYPE_4_REG              0x00000015 /* Send TOD reg */
> +#define TOD_TX_TTYPE_5_REG              0x00000016 /* Invalidate TOD reg */
> +
> +#define TOD_MOVE_TOD_TO_TB_REG          0x00000017
> +#define TOD_LOAD_TOD_MOD_REG            0x00000018
> +#define TOD_LOAD_TOD_REG                0x00000021
> +#define TOD_FSM_REG                     0x00000024
> +
> +#define TOD_TX_TTYPE_CTRL_REG           0x00000027 /* TX TTYPE Control reg */
> +#define   TOD_TX_TTYPE_PIB_SLAVE_ADDR      PPC_BITMASK(26, 31)
> +
> +/* -- TOD Error interrupt register -- */
> +#define TOD_ERROR_REG                   0x00000030
> +
> +/* PC unit PIB address which recieves the timebase transfer from TOD */
> +#define   PC_TOD                        0x4A3
> +
> +static uint64_t pnv_chiptod_xscom_read(void *opaque, hwaddr addr,
> +                                          unsigned size)
> +{
> +    PnvChipTOD *chiptod = PNV_CHIPTOD(opaque);
> +    uint32_t offset = addr >> 3;
> +    uint64_t val = 0;
> +
> +    switch (offset) {
> +    case TOD_PSS_MSS_STATUS_REG:
> +        /*
> +         * ChipTOD does not support configurations other than primary
> +         * master, does not support errors, etc.
> +         */
> +        val |= PPC_BITMASK(6,10); /* STEP checker validity */
> +        val |= PPC_BIT(12); /* Primary config master path select */
> +        val |= PPC_BIT(20); /* Is running */
> +        val |= PPC_BIT(21); /* Is using primary config */
> +        val |= PPC_BIT(26); /* Is using master path select */
> +
> +        if (chiptod->primary) {
> +            val |= PPC_BIT(23); /* Is active master */
> +        } else if (chiptod->secondary) {
> +            val |= PPC_BIT(24); /* Is backup master */
> +        }
> +        break;
> +    case TOD_PSS_MSS_CTRL_REG:
> +        val = chiptod->pss_mss_ctrl_reg;
> +        break;
> +    case TOD_TX_TTYPE_CTRL_REG:
> +        val = 0;
> +	break;
> +    case TOD_ERROR_REG:
> +        val = chiptod->tod_error;
> +        break;
> +    case TOD_FSM_REG:
> +        if (chiptod->tod_state == tod_running) {
> +            val |= PPC_BIT(4);
> +        }
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "pnv_chiptod: unimplemented register: Ox%"
> +                      HWADDR_PRIx "\n", addr >> 3);
> +    }
> +
> +    trace_pnv_chiptod_xscom_read(addr >> 3, val);
> +
> +    return val;
> +}
> +
> +static void chiptod_power9_send_remotes(PnvChipTOD *chiptod)

Adding a class handler could be an alternative implementation.

> +{
> +    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> +    int i;
> +
> +    for (i = 0; i < pnv->num_chips; i++) {

There are a few other models (XIVE, XIVE2) which loop on the chips,
is it time to introduce a pnv_foreach_chip(fn, data) routine ?


> +        Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
> +        if (&chip9->chiptod != chiptod) {
> +            chip9->chiptod.tod_state = tod_running;
> +        }
> +    }
> +}
> +
> +static void chiptod_power10_send_remotes(PnvChipTOD *chiptod)
> +{
> +    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> +    int i;
> +
> +    for (i = 0; i < pnv->num_chips; i++) {
> +        Pnv10Chip *chip10 = PNV10_CHIP(pnv->chips[i]);
> +        if (&chip10->chiptod != chiptod) {
> +            chip10->chiptod.tod_state = tod_running;
> +        }
> +    }
> +}
> +
> +static void chiptod_power9_invalidate_remotes(PnvChipTOD *chiptod)
> +{
> +    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> +    int i;
> +
> +    for (i = 0; i < pnv->num_chips; i++) {
> +        Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
> +        if (&chip9->chiptod != chiptod) {
> +            chip9->chiptod.tod_state = tod_not_set;
> +        }
> +    }
> +}
> +
> +static void chiptod_power10_invalidate_remotes(PnvChipTOD *chiptod)
> +{
> +    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> +    int i;
> +
> +    for (i = 0; i < pnv->num_chips; i++) {
> +        Pnv10Chip *chip10 = PNV10_CHIP(pnv->chips[i]);
> +        if (&chip10->chiptod != chiptod) {
> +            chip10->chiptod.tod_state = tod_not_set;
> +        }
> +    }
> +}
> +
> +static void pnv_chiptod_xscom_write(void *opaque, hwaddr addr,
> +                                    uint64_t val, unsigned size,
> +                                    bool is_power9)
> +{
> +    PnvChipTOD *chiptod = PNV_CHIPTOD(opaque);
> +    uint32_t offset = addr >> 3;
> +
> +    trace_pnv_chiptod_xscom_write(addr >> 3, val);
> +
> +    switch (offset) {
> +    case TOD_PSS_MSS_CTRL_REG:
> +        /* Is this correct? */
> +        if (chiptod->primary) {
> +            val |= PPC_BIT(1); /* TOD is master */
> +        } else {
> +            val &= ~PPC_BIT(1);
> +        }
> +        val |= PPC_BIT(2); /* Drawer is master (don't simulate multi-drawer) */
> +        chiptod->pss_mss_ctrl_reg = val & PPC_BITMASK(0, 31);
> +        break;
> +
> +    case TOD_TX_TTYPE_CTRL_REG:
> +        if (val & PPC_BIT(35)) { /* SCOM addressing */
> +            uint32_t addr = val >> 32;
> +            uint32_t reg = addr & 0xfff;
> +            PnvCore *pc;
> +
> +            if (reg != PC_TOD) {
> +                qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: SCOM addressing: "
> +                              "unimplemented slave register 0x%" PRIx32 "\n",
> +                              reg);
> +                return;
> +            }
> +
> +            /*
> +             * This may not deal with P10 big-core addressing at the moment.
> +             * The big-core code in skiboot syncs small cores, but it targets
> +             * the even PIR (first small-core) when syncing second small-core.
> +             */
> +            pc = pnv_get_vcpu_by_xscom_base(chiptod->chip, addr & ~0xfff);

hmm, couldn't we map xscom subregions, one for each thread instead ?

> +            if (pc) {
> +                /*
> +                 * If TCG implements SMT, TFMR is a per-core SPR and should
> +                 * be updated such that it is reflected in all threads.
> +                 * Same for TB if the chiptod model ever actually adjusted it.
> +                 */
> +                chiptod->slave_cpu_target = pc->threads[0];

ah ! SMT is implemented :) The xscom subregions would help to get the
CPU pointer.


> +            } else {
> +                chiptod->slave_cpu_target = NULL;
> +                qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
> +                              " TOD_TX_TTYPE_CTRL_REG val 0x%" PRIx64
> +                              " invalid slave PIR\n", val);
> +            }
> +
> +        } else { /* PIR addressing */
> +            uint32_t pir;
> +
> +            if (!is_power9) {
> +                qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: PIR addressing"
> +                              " is only implemented for POWER9\n");
> +                return;
> +            }
> +
> +            pir = (GETFIELD(TOD_TX_TTYPE_PIB_SLAVE_ADDR, val) & 0x1f) << 2;
> +            chiptod->slave_cpu_target = ppc_get_vcpu_by_pir(pir);
> +            if (chiptod->slave_cpu_target == NULL) {
> +                qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
> +                              " TOD_TX_TTYPE_CTRL_REG val 0x%" PRIx64
> +                              " invalid slave PIR 0x%" PRIx32 "\n", val, pir);
> +            }
> +        }
> +        break;
> +    case TOD_ERROR_REG:
> +        chiptod->tod_error &= ~val;
> +        break;
> +    case TOD_LOAD_TOD_MOD_REG:
> +        if (!(val & PPC_BIT(0))) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
> +                          " TOD_LOAD_TOD_MOD_REG with bad val 0x%016lx\n", val);
> +        } else {
> +            chiptod->tod_state = tod_not_set;
> +        }
> +        break;
> +    case TOD_LOAD_TOD_REG:
> +        chiptod->tod_state = tod_running;
> +        break;
> +    case TOD_MOVE_TOD_TO_TB_REG:
> +        if (!(val & PPC_BIT(0))) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
> +                          " TOD_MOVE_TOD_TO_TB_REG with bad val 0x%016lx\n",
> +                          val);
> +        } else if (chiptod->slave_cpu_target == NULL) {
> +            qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
> +                          " TOD_MOVE_TOD_TO_TB_REG with no slave target\n");
> +        } else {
> +            PowerPCCPU *cpu = chiptod->slave_cpu_target;
> +            CPUPPCState *env = &cpu->env;
> +
> +            if (env->tb_ready_for_tod) {
> +                env->tod_sent_to_tb = 1;
> +            } else {
> +                qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: xscom write reg"
> +                              " TOD_MOVE_TOD_TO_TB_REG with TB not ready to"
> +                              " receive TOD\n");
> +            }
> +        }
> +        break;
> +    case TOD_TX_TTYPE_4_REG:
> +        if (is_power9) {
> +            chiptod_power9_send_remotes(chiptod);
> +        } else {
> +            chiptod_power10_send_remotes(chiptod);
> +        }
> +        break;
> +    case TOD_TX_TTYPE_5_REG:
> +        if (is_power9) {
> +            chiptod_power9_invalidate_remotes(chiptod);
> +        } else {
> +            chiptod_power10_invalidate_remotes(chiptod);
> +        }
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "pnv_chiptod: unimplemented register: Ox%"
> +                      HWADDR_PRIx "\n", addr >> 3);
> +    }
> +}
> +
> +static void pnv_chiptod_power9_xscom_write(void *opaque, hwaddr addr,
> +                                           uint64_t val, unsigned size)
> +{
> +    pnv_chiptod_xscom_write(opaque, addr, val, size, true);
> +}
> +
> +static const MemoryRegionOps pnv_chiptod_power9_xscom_ops = {
> +    .read = pnv_chiptod_xscom_read,
> +    .write = pnv_chiptod_power9_xscom_write,
> +    .valid.min_access_size = 8,
> +    .valid.max_access_size = 8,
> +    .impl.min_access_size = 8,
> +    .impl.max_access_size = 8,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
> +static int pnv_chiptod_dt_xscom(PnvXScomInterface *dev, void *fdt,
> +                                int xscom_offset,
> +                                const char compat[], size_t compat_size)
> +{
> +    PnvChipTOD *chiptod = PNV_CHIPTOD(dev);
> +    g_autofree char *name = NULL;
> +    int offset;
> +    uint32_t lpc_pcba = PNV9_XSCOM_CHIPTOD_BASE;

lpc ?


> +    uint32_t reg[] = {
> +        cpu_to_be32(lpc_pcba),
> +        cpu_to_be32(PNV9_XSCOM_CHIPTOD_SIZE)
> +    };
> +
> +    name = g_strdup_printf("chiptod@%x", lpc_pcba);
> +    offset = fdt_add_subnode(fdt, xscom_offset, name);
> +    _FDT(offset);
> +
> +    if (chiptod->primary) {
> +        _FDT((fdt_setprop(fdt, offset, "primary", NULL, 0)));
> +    } else if (chiptod->secondary) {
> +        _FDT((fdt_setprop(fdt, offset, "secondary", NULL, 0)));
> +    }
> +
> +    _FDT((fdt_setprop(fdt, offset, "reg", reg, sizeof(reg))));
> +    _FDT((fdt_setprop(fdt, offset, "compatible", compat, compat_size)));
> +    return 0;
> +}
> +
> +static int pnv_chiptod_power9_dt_xscom(PnvXScomInterface *dev, void *fdt,
> +                             int xscom_offset)
> +{
> +    const char compat[] = "ibm,power-chiptod\0ibm,power9-chiptod";
> +
> +    return pnv_chiptod_dt_xscom(dev, fdt, xscom_offset, compat, sizeof(compat));
> +}
> +
> +static Property pnv_chiptod_properties[] = {
> +    DEFINE_PROP_LINK("chip", PnvChipTOD , chip, TYPE_PNV_CHIP, PnvChip *),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void pnv_chiptod_power9_class_init(ObjectClass *klass, void *data)
> +{
> +    PnvChipTODClass *pctc = PNV_CHIPTOD_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PnvXScomInterfaceClass *xdc = PNV_XSCOM_INTERFACE_CLASS(klass);
> +
> +    dc->desc = "PowerNV ChipTOD Controller (POWER9)";
> +    device_class_set_props(dc, pnv_chiptod_properties);
> +
> +    xdc->dt_xscom = pnv_chiptod_power9_dt_xscom;
> +
> +    pctc->xscom_size = PNV_XSCOM_CHIPTOD_SIZE;
> +    pctc->xscom_ops = &pnv_chiptod_power9_xscom_ops;
> +}
> +
> +static const TypeInfo pnv_chiptod_power9_type_info = {
> +    .name          = TYPE_PNV9_CHIPTOD,
> +    .parent        = TYPE_PNV_CHIPTOD,
> +    .instance_size = sizeof(PnvChipTOD),
> +    .class_init    = pnv_chiptod_power9_class_init,
> +    .interfaces    = (InterfaceInfo[]) {
> +        { TYPE_PNV_XSCOM_INTERFACE },
> +        { }
> +    }
> +};
> +
> +static void pnv_chiptod_power10_xscom_write(void *opaque, hwaddr addr,
> +                                           uint64_t val, unsigned size)
> +{
> +    pnv_chiptod_xscom_write(opaque, addr, val, size, false);
> +}
> +
> +static const MemoryRegionOps pnv_chiptod_power10_xscom_ops = {
> +    .read = pnv_chiptod_xscom_read,
> +    .write = pnv_chiptod_power10_xscom_write,
> +    .valid.min_access_size = 8,
> +    .valid.max_access_size = 8,
> +    .impl.min_access_size = 8,
> +    .impl.max_access_size = 8,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
> +static int pnv_chiptod_power10_dt_xscom(PnvXScomInterface *dev, void *fdt,
> +                             int xscom_offset)
> +{
> +    const char compat[] = "ibm,power-chiptod\0ibm,power10-chiptod";
> +
> +    return pnv_chiptod_dt_xscom(dev, fdt, xscom_offset, compat, sizeof(compat));
> +}
> +
> +static void pnv_chiptod_power10_class_init(ObjectClass *klass, void *data)
> +{
> +    PnvChipTODClass *pctc = PNV_CHIPTOD_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    PnvXScomInterfaceClass *xdc = PNV_XSCOM_INTERFACE_CLASS(klass);
> +
> +    dc->desc = "PowerNV ChipTOD Controller (POWER10)";
> +    device_class_set_props(dc, pnv_chiptod_properties);
> +
> +    xdc->dt_xscom = pnv_chiptod_power10_dt_xscom;
> +
> +    pctc->xscom_size = PNV_XSCOM_CHIPTOD_SIZE;
> +    pctc->xscom_ops = &pnv_chiptod_power10_xscom_ops;
> +}
> +
> +static const TypeInfo pnv_chiptod_power10_type_info = {
> +    .name          = TYPE_PNV10_CHIPTOD,
> +    .parent        = TYPE_PNV_CHIPTOD,
> +    .instance_size = sizeof(PnvChipTOD),
> +    .class_init    = pnv_chiptod_power10_class_init,
> +    .interfaces    = (InterfaceInfo[]) {
> +        { TYPE_PNV_XSCOM_INTERFACE },
> +        { }
> +    }
> +};
> +
> +static void pnv_chiptod_realize(DeviceState *dev, Error **errp)
> +{
> +    static bool got_primary = false;
> +    static bool got_secondary = false;
> +
> +    PnvChipTOD *chiptod = PNV_CHIPTOD(dev);
> +    PnvChipTODClass *pctc = PNV_CHIPTOD_GET_CLASS(chiptod);
> +
> +    if (!got_primary) {
> +        got_primary = true;
> +        chiptod->primary = true;
> +        chiptod->pss_mss_ctrl_reg |= PPC_BIT(1); /* TOD is master */
> +    } else if (!got_secondary) {
> +        got_secondary = true;
> +        chiptod->secondary = true;
> +    }

It would be cleaner to introduce "primary" and "secondary" properties
defined by the model realizing the PnvChipTOD objects.

> +    /* Drawer is master (we do not simulate multi-drawer) */
> +    chiptod->pss_mss_ctrl_reg |= PPC_BIT(2);
> +    chiptod->tod_state = tod_running;
> +
> +    /* XScom regions for ChipTOD registers */
> +    pnv_xscom_region_init(&chiptod->xscom_regs, OBJECT(dev),
> +                          pctc->xscom_ops, chiptod, "xscom-chiptod",
> +                          pctc->xscom_size);
> +}
> +
> +static void pnv_chiptod_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = pnv_chiptod_realize;
> +    dc->desc = "PowerNV ChipTOD Controller";
> +    dc->user_creatable = false;
> +}
> +
> +static const TypeInfo pnv_chiptod_type_info = {
> +    .name          = TYPE_PNV_CHIPTOD,
> +    .parent        = TYPE_DEVICE,
> +    .instance_size = sizeof(PnvChipTOD),
> +    .class_init    = pnv_chiptod_class_init,
> +    .class_size    = sizeof(PnvChipTODClass),
> +    .abstract      = true,
> +};
> +
> +static void pnv_chiptod_register_types(void)
> +{
> +    type_register_static(&pnv_chiptod_type_info);
> +    type_register_static(&pnv_chiptod_power9_type_info);
> +    type_register_static(&pnv_chiptod_power10_type_info);
> +}
> +
> +type_init(pnv_chiptod_register_types);
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index d820e05e40..5bbbd3a7a9 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -298,6 +298,8 @@ int pnv_dt_xscom(PnvChip *chip, void *fdt, int root_offset,
>       _FDT((fdt_setprop(fdt, xscom_offset, "scom-controller", NULL, 0)));
>       if (chip->chip_id == 0) {
>           _FDT((fdt_setprop(fdt, xscom_offset, "primary", NULL, 0)));
> +    } else if (chip->chip_id == 1) {
> +        _FDT((fdt_setprop(fdt, xscom_offset, "secondary", NULL, 0)));

This is a more-or-less related change. May be we should have "primary" and
"secondary" property in PnvChip also.

>       }
>   
>       args.fdt = fdt;
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index f670e8906c..57c4f265ef 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -95,6 +95,10 @@ vof_write(uint32_t ih, unsigned cb, const char *msg) "ih=0x%x [%u] \"%s\""
>   vof_avail(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
>   vof_claimed(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
>   
> +# pnv_chiptod.c
> +pnv_chiptod_xscom_read(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
> +pnv_chiptod_xscom_write(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
> +
>   # pnv_sbe.c
>   pnv_sbe_xscom_ctrl_read(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
>   pnv_sbe_xscom_ctrl_write(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
> diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
> index 53e1d921d7..d22c013e7d 100644
> --- a/include/hw/ppc/pnv_chip.h
> +++ b/include/hw/ppc/pnv_chip.h
> @@ -2,6 +2,7 @@
>   #define PPC_PNV_CHIP_H
>   
>   #include "hw/pci-host/pnv_phb4.h"
> +#include "hw/ppc/pnv_chiptod.h"
>   #include "hw/ppc/pnv_core.h"
>   #include "hw/ppc/pnv_homer.h"
>   #include "hw/ppc/pnv_lpc.h"
> @@ -77,6 +78,7 @@ struct Pnv9Chip {
>       PnvXive      xive;
>       Pnv9Psi      psi;
>       PnvLpcController lpc;
> +    PnvChipTOD   chiptod;
>       PnvOCC       occ;
>       PnvSBE       sbe;
>       PnvHomer     homer;
> @@ -106,6 +108,7 @@ struct Pnv10Chip {
>       PnvXive2     xive;
>       Pnv9Psi      psi;
>       PnvLpcController lpc;
> +    PnvChipTOD   chiptod;
>       PnvOCC       occ;
>       PnvSBE       sbe;
>       PnvHomer     homer;
> diff --git a/include/hw/ppc/pnv_chiptod.h b/include/hw/ppc/pnv_chiptod.h
> new file mode 100644
> index 0000000000..6723b07d93
> --- /dev/null
> +++ b/include/hw/ppc/pnv_chiptod.h
> @@ -0,0 +1,64 @@
> +/*
> + * QEMU PowerPC PowerNV Emulation of some CHIPTOD behaviour
> + *
> + * Copyright (c) 2022-2023, 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.1 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_CHIPTOD_H
> +#define PPC_PNV_CHIPTOD_H
> +
> +#include "qom/object.h"
> +
> +#define TYPE_PNV_CHIPTOD "pnv-chiptod"
> +OBJECT_DECLARE_TYPE(PnvChipTOD, PnvChipTODClass, PNV_CHIPTOD)
> +#define TYPE_PNV9_CHIPTOD TYPE_PNV_CHIPTOD "-POWER9"
> +DECLARE_INSTANCE_CHECKER(PnvChipTOD, PNV9_CHIPTOD, TYPE_PNV9_CHIPTOD)
> +#define TYPE_PNV10_CHIPTOD TYPE_PNV_CHIPTOD "-POWER10"
> +DECLARE_INSTANCE_CHECKER(PnvChipTOD, PNV10_CHIPTOD, TYPE_PNV10_CHIPTOD)
> +
> +enum tod_state {

PnvChipTODState would be better naming.

Thanks,

C.

> +    tod_error = 0,
> +    tod_not_set = 7,
> +    tod_not_set_step = 11,
> +    tod_running = 2,
> +    tod_running_step = 10,
> +    tod_running_sync = 14,
> +    tod_wait_for_sync = 13,
> +    tod_stopped = 1,
> +};
>
> +struct PnvChipTOD {
> +    DeviceState xd;
> +
> +    PnvChip *chip;
> +    MemoryRegion xscom_regs;
> +
> +    bool primary;
> +    bool secondary;
> +    enum tod_state tod_state;
> +    uint64_t tod_error;
> +    uint64_t pss_mss_ctrl_reg;
> +    PowerPCCPU *slave_cpu_target;
> +};
> +
> +struct PnvChipTODClass {
> +    DeviceClass parent_class;
> +
> +    int xscom_size;
> +    const MemoryRegionOps *xscom_ops;
> +};
> +
> +#endif /* PPC_PNV_CHIPTOD_H */
> diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
> index 3d75706e95..832b339ed6 100644
> --- a/include/hw/ppc/pnv_core.h
> +++ b/include/hw/ppc/pnv_core.h
> @@ -69,4 +69,7 @@ struct PnvQuad {
>       uint32_t quad_id;
>       MemoryRegion xscom_regs;
>   };
> +
> +PnvCore *pnv_get_vcpu_by_xscom_base(PnvChip *chip, uint32_t xscom_base);
> +
>   #endif /* PPC_PNV_CORE_H */
> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
> index cbe848d27b..530f89af55 100644
> --- a/include/hw/ppc/pnv_xscom.h
> +++ b/include/hw/ppc/pnv_xscom.h
> @@ -64,6 +64,9 @@ struct PnvXScomInterfaceClass {
>   #define PNV_XSCOM_PSIHB_BASE      0x2010900
>   #define PNV_XSCOM_PSIHB_SIZE      0x20
>   
> +#define PNV_XSCOM_CHIPTOD_BASE    0x0040000
> +#define PNV_XSCOM_CHIPTOD_SIZE    0x31
> +
>   #define PNV_XSCOM_OCC_BASE        0x0066000
>   #define PNV_XSCOM_OCC_SIZE        0x6000
>   
> @@ -90,6 +93,9 @@ struct PnvXScomInterfaceClass {
>       ((uint64_t)(((core) & 0x1C) + 0x40) << 22)
>   #define PNV9_XSCOM_EQ_SIZE        0x100000
>   
> +#define PNV9_XSCOM_CHIPTOD_BASE   PNV_XSCOM_CHIPTOD_BASE
> +#define PNV9_XSCOM_CHIPTOD_SIZE   PNV_XSCOM_CHIPTOD_SIZE
> +
>   #define PNV9_XSCOM_OCC_BASE       PNV_XSCOM_OCC_BASE
>   #define PNV9_XSCOM_OCC_SIZE       0x8000
>   
> @@ -138,6 +144,9 @@ struct PnvXScomInterfaceClass {
>   #define PNV10_XSCOM_PSIHB_BASE     0x3011D00
>   #define PNV10_XSCOM_PSIHB_SIZE     0x100
>   
> +#define PNV10_XSCOM_CHIPTOD_BASE   PNV9_XSCOM_CHIPTOD_BASE
> +#define PNV10_XSCOM_CHIPTOD_SIZE   PNV9_XSCOM_CHIPTOD_SIZE
> +
>   #define PNV10_XSCOM_OCC_BASE       PNV9_XSCOM_OCC_BASE
>   #define PNV10_XSCOM_OCC_SIZE       PNV9_XSCOM_OCC_SIZE
>   
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 8c30c59a56..d73cce8474 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1172,6 +1172,12 @@ struct CPUArchState {
>       uint32_t tlb_need_flush; /* Delayed flush needed */
>   #define TLB_NEED_LOCAL_FLUSH   0x1
>   #define TLB_NEED_GLOBAL_FLUSH  0x2
> +
> +#if defined(TARGET_PPC64)
> +    /* PowerNV chiptod / timebase facility state. */
> +    int tb_ready_for_tod; /* core TB ready to receive TOD from chiptod */
> +    int tod_sent_to_tb;   /* chiptod sent TOD to the core TB */
> +#endif
>   #endif
>   
>       /* Other registers */



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] target/ppc: Tidy POWER book4 SPR registration
  2023-06-03 23:36 ` [PATCH 2/4] target/ppc: Tidy POWER book4 SPR registration Nicholas Piggin
@ 2023-06-05 14:58   ` Cédric Le Goater
  0 siblings, 0 replies; 18+ messages in thread
From: Cédric Le Goater @ 2023-06-05 14:58 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc; +Cc: qemu-devel, Daniel Henrique Barboza

On 6/4/23 01:36, Nicholas Piggin wrote:
> POWER book4 (implementation-specific) SPRs are sometimes in their own
> functions, but in other cases are mixed with architected SPRs. Do some
> spring cleaning on these.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

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

Thanks,

C.


> ---
>   target/ppc/cpu_init.c | 92 ++++++++++++++++++++++++++++---------------
>   1 file changed, 60 insertions(+), 32 deletions(-)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index e9717b8169..da0f7a7159 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -5374,31 +5374,6 @@ static void register_book3s_ids_sprs(CPUPPCState *env)
>                    &spr_read_generic, SPR_NOACCESS,
>                    &spr_read_generic, NULL,
>                    0x00000000);
> -    spr_register_hv(env, SPR_HID0, "HID0",
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_core_write_generic,
> -                 0x00000000);
> -    spr_register_hv(env, SPR_TSCR, "TSCR",
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_write_generic32,
> -                 0x00000000);
> -    spr_register_hv(env, SPR_HMER, "HMER",
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_write_hmer,
> -                 0x00000000);
> -    spr_register_hv(env, SPR_HMEER, "HMEER",
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_write_generic,
> -                 0x00000000);
> -    spr_register_hv(env, SPR_TFMR, "TFMR",
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_write_generic,
> -                 0x00000000);
>       spr_register_hv(env, SPR_LPIDR, "LPIDR",
>                    SPR_NOACCESS, SPR_NOACCESS,
>                    SPR_NOACCESS, SPR_NOACCESS,
> @@ -5454,11 +5429,6 @@ static void register_book3s_ids_sprs(CPUPPCState *env)
>                    SPR_NOACCESS, SPR_NOACCESS,
>                    &spr_read_generic, &spr_write_generic,
>                    0x00000000);
> -    spr_register_hv(env, SPR_LDBAR, "LDBAR",
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_write_generic,
> -                 0x00000000);
>   }
>   
>   static void register_rmor_sprs(CPUPPCState *env)
> @@ -5665,14 +5635,65 @@ static void register_power8_ic_sprs(CPUPPCState *env)
>   #endif
>   }
>   
> +/* SPRs specific to IBM POWER CPUs */
> +static void register_power_common_book4_sprs(CPUPPCState *env)
> +{
> +#if !defined(CONFIG_USER_ONLY)
> +    spr_register_hv(env, SPR_HID0, "HID0",
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 &spr_read_generic, &spr_core_write_generic,
> +                 0x00000000);
> +    spr_register_hv(env, SPR_TSCR, "TSCR",
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 &spr_read_generic, &spr_write_generic32,
> +                 0x00000000);
> +    spr_register_hv(env, SPR_HMER, "HMER",
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 &spr_read_generic, &spr_write_hmer,
> +                 0x00000000);
> +    spr_register_hv(env, SPR_HMEER, "HMEER",
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 &spr_read_generic, &spr_write_generic,
> +                 0x00000000);
> +    spr_register_hv(env, SPR_TFMR, "TFMR",
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 &spr_read_generic, &spr_write_generic,
> +                 0x00000000);
> +    spr_register_hv(env, SPR_LDBAR, "LDBAR",
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 &spr_read_generic, &spr_write_generic,
> +                 0x00000000);
> +#endif
> +}
> +
> +static void register_power9_book4_sprs(CPUPPCState *env)
> +{
> +    /* Add a number of P9 book4 registers */
> +    register_power_common_book4_sprs(env);
> +#if !defined(CONFIG_USER_ONLY)
> +    spr_register_kvm(env, SPR_WORT, "WORT",
> +                     SPR_NOACCESS, SPR_NOACCESS,
> +                     &spr_read_generic, &spr_write_generic,
> +                     KVM_REG_PPC_WORT, 0);
> +#endif
> +}
> +
>   static void register_power8_book4_sprs(CPUPPCState *env)
>   {
>       /* Add a number of P8 book4 registers */
> +    register_power_common_book4_sprs(env);
>   #if !defined(CONFIG_USER_ONLY)
>       spr_register_kvm(env, SPR_ACOP, "ACOP",
>                        SPR_NOACCESS, SPR_NOACCESS,
>                        &spr_read_generic, &spr_write_generic,
>                        KVM_REG_PPC_ACOP, 0);
> +    /* PID is only in BookE in ISA v2.07 */
>       spr_register_kvm(env, SPR_BOOKS_PID, "PID",
>                        SPR_NOACCESS, SPR_NOACCESS,
>                        &spr_read_generic, &spr_write_pidr,
> @@ -5688,10 +5709,12 @@ static void register_power7_book4_sprs(CPUPPCState *env)
>   {
>       /* Add a number of P7 book4 registers */
>   #if !defined(CONFIG_USER_ONLY)
> +    register_power_common_book4_sprs(env);
>       spr_register_kvm(env, SPR_ACOP, "ACOP",
>                        SPR_NOACCESS, SPR_NOACCESS,
>                        &spr_read_generic, &spr_write_generic,
>                        KVM_REG_PPC_ACOP, 0);
> +    /* PID is only in BookE in ISA v2.06 */
>       spr_register_kvm(env, SPR_BOOKS_PID, "PID",
>                        SPR_NOACCESS, SPR_NOACCESS,
>                        &spr_read_generic, &spr_write_generic32,
> @@ -5725,6 +5748,11 @@ static void register_power9_mmu_sprs(CPUPPCState *env)
>                       SPR_NOACCESS, SPR_NOACCESS,
>                       &spr_read_generic, &spr_write_generic,
>                       0x0000000000000000);
> +    /* PID is part of the BookS ISA from v3.0 */
> +    spr_register_kvm(env, SPR_BOOKS_PID, "PID",
> +                     SPR_NOACCESS, SPR_NOACCESS,
> +                     &spr_read_generic, &spr_write_pidr,
> +                     KVM_REG_PPC_PID, 0);
>   #endif
>   }
>   
> @@ -6278,7 +6306,7 @@ static void init_proc_POWER9(CPUPPCState *env)
>       register_power8_dpdes_sprs(env);
>       register_vtb_sprs(env);
>       register_power8_ic_sprs(env);
> -    register_power8_book4_sprs(env);
> +    register_power9_book4_sprs(env);
>       register_power8_rpr_sprs(env);
>       register_power9_mmu_sprs(env);
>   
> @@ -6471,7 +6499,7 @@ static void init_proc_POWER10(CPUPPCState *env)
>       register_power8_dpdes_sprs(env);
>       register_vtb_sprs(env);
>       register_power8_ic_sprs(env);
> -    register_power8_book4_sprs(env);
> +    register_power9_book4_sprs(env);
>       register_power8_rpr_sprs(env);
>       register_power9_mmu_sprs(env);
>       register_power10_hash_sprs(env);



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models
  2023-06-03 23:36 [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models Nicholas Piggin
                   ` (3 preceding siblings ...)
  2023-06-03 23:36 ` [PATCH 4/4] target/ppc: Implement core timebase state machine and TFMR Nicholas Piggin
@ 2023-06-06 13:59 ` Cédric Le Goater
  2023-06-14  5:14   ` Nicholas Piggin
  2023-06-22  7:30 ` Cédric Le Goater
  5 siblings, 1 reply; 18+ messages in thread
From: Cédric Le Goater @ 2023-06-06 13:59 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: qemu-devel, Daniel Henrique Barboza, Frederic Barrat, Michael Neuling

On 6/4/23 01:36, Nicholas Piggin wrote:
> This adds support for chiptod and core timebase state machine models in
> the powernv POWER9 and POWER10 models.
> 
> This does not actually change the time or the value in TB registers
> (because they are alrady synced in QEMU), but it does go through the
> motions. It is enough to be able to run skiboot's chiptod initialisation
> code that synchronises core timebases (after a patch to prevent skiboot
> skipping chiptod for QEMU, posted to skiboot mailing list).
> 
> Sorry there was some delay since the last posting. There is a bit more
> interest in this recently but feedback and comments from RFC was not
> forgotten and is much appreciated.
> 
> https://lists.gnu.org/archive/html/qemu-ppc/2022-08/msg00324.html
> 
> I think I accounted for everything except moving register defines to the
> .h file. I'm on the fence about that but if they are only used in the .c
> file I think it's okay to keep them there for now. I cut out a lot of
> unused ones so it's not so cluttered now.
> 
> Lots of other changes and fixes since that RFC. Notably:
> - Register names changed to match the workbook names instead of skiboot.
> - TFMR moved to timebase_helper.c from misc_helper.c
> - More comprehensive model and error checking, particularly of TFMR.
> - POWER10 with multi-chip support.
> - chiptod and core timebase linked via specific state instead of TFMR.


The chiptod units are not exposed to the OS, it is all handled at FW
level AFAIK. Could the OPAL people provide some feedback on the low level
models ?

Thanks,

C.

> There is still a vast amount that is not modeled, but most of it related
> to error handling, injection, failover, etc that is very complicated and
> not required for normal operation.
> 
> Thanks,
> Nick
> 
> Nicholas Piggin (4):
>    pnv/chiptod: Add POWER9/10 chiptod model
>    target/ppc: Tidy POWER book4 SPR registration
>    target/ppc: add TFMR SPR implementation with read and write helpers
>    target/ppc: Implement core timebase state machine and TFMR
> 
>   hw/ppc/meson.build           |   1 +
>   hw/ppc/pnv.c                 |  38 +++
>   hw/ppc/pnv_chiptod.c         | 488 +++++++++++++++++++++++++++++++++++
>   hw/ppc/pnv_xscom.c           |   2 +
>   hw/ppc/trace-events          |   4 +
>   include/hw/ppc/pnv_chip.h    |   3 +
>   include/hw/ppc/pnv_chiptod.h |  64 +++++
>   include/hw/ppc/pnv_core.h    |   3 +
>   include/hw/ppc/pnv_xscom.h   |   9 +
>   target/ppc/cpu.h             |  40 +++
>   target/ppc/cpu_init.c        |  92 ++++---
>   target/ppc/helper.h          |   2 +
>   target/ppc/spr_common.h      |   2 +
>   target/ppc/timebase_helper.c | 156 +++++++++++
>   target/ppc/translate.c       |  10 +
>   15 files changed, 882 insertions(+), 32 deletions(-)
>   create mode 100644 hw/ppc/pnv_chiptod.c
>   create mode 100644 include/hw/ppc/pnv_chiptod.h
> 



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models
  2023-06-06 13:59 ` [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models Cédric Le Goater
@ 2023-06-14  5:14   ` Nicholas Piggin
  2023-06-14  8:54     ` Cédric Le Goater
  0 siblings, 1 reply; 18+ messages in thread
From: Nicholas Piggin @ 2023-06-14  5:14 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc
  Cc: qemu-devel, Daniel Henrique Barboza, Frederic Barrat, Michael Neuling

On Tue Jun 6, 2023 at 11:59 PM AEST, Cédric Le Goater wrote:
> On 6/4/23 01:36, Nicholas Piggin wrote:
> > This adds support for chiptod and core timebase state machine models in
> > the powernv POWER9 and POWER10 models.
> > 
> > This does not actually change the time or the value in TB registers
> > (because they are alrady synced in QEMU), but it does go through the
> > motions. It is enough to be able to run skiboot's chiptod initialisation
> > code that synchronises core timebases (after a patch to prevent skiboot
> > skipping chiptod for QEMU, posted to skiboot mailing list).
> > 
> > Sorry there was some delay since the last posting. There is a bit more
> > interest in this recently but feedback and comments from RFC was not
> > forgotten and is much appreciated.
> > 
> > https://lists.gnu.org/archive/html/qemu-ppc/2022-08/msg00324.html
> > 
> > I think I accounted for everything except moving register defines to the
> > .h file. I'm on the fence about that but if they are only used in the .c
> > file I think it's okay to keep them there for now. I cut out a lot of
> > unused ones so it's not so cluttered now.
> > 
> > Lots of other changes and fixes since that RFC. Notably:
> > - Register names changed to match the workbook names instead of skiboot.
> > - TFMR moved to timebase_helper.c from misc_helper.c
> > - More comprehensive model and error checking, particularly of TFMR.
> > - POWER10 with multi-chip support.
> > - chiptod and core timebase linked via specific state instead of TFMR.
>
>
> The chiptod units are not exposed to the OS, it is all handled at FW
> level AFAIK. Could the OPAL people provide some feedback on the low level
> models ?

Well, no takers so far. I guess I'm a OPAL people :)

I did some of the P10 chiptod addressing in skiboot, at least. This
patch does work with the skiboot chiptod driver at least.

I would eventually like to add in the ability to actually change the
TB with it, and inject errors and generate HMIs because that's an area
that seem to be a bit lacking (most of such testing seemed to be done
on real hardware using special time facility corruption injection).

But yes for now it is a bit difficult to verify it does much useful
aside from booting skiboot (+ patch to enable chiptod on QEMU I posted
recently).

Thanks,
Nick


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] pnv/chiptod: Add POWER9/10 chiptod model
  2023-06-05 14:57   ` Cédric Le Goater
@ 2023-06-14  5:30     ` Nicholas Piggin
  2023-06-14  7:01       ` Cédric Le Goater
  0 siblings, 1 reply; 18+ messages in thread
From: Nicholas Piggin @ 2023-06-14  5:30 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc
  Cc: qemu-devel, Daniel Henrique Barboza, Frederic Barrat

On Tue Jun 6, 2023 at 12:57 AM AEST, Cédric Le Goater wrote:
> On 6/4/23 01:36, Nicholas Piggin wrote:

> > diff --git a/hw/ppc/pnv_chiptod.c b/hw/ppc/pnv_chiptod.c
> > new file mode 100644
> > index 0000000000..04ef703e0f
> > --- /dev/null
> > +++ b/hw/ppc/pnv_chiptod.c
> > @@ -0,0 +1,488 @@
> > +/*
> > + * QEMU PowerPC PowerNV Emulation of some CHIPTOD behaviour
> > + *
> > + * Copyright (c) 2022-2023, 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/>.
>
> You can simplify the header with
>
>   * SPDX-License-Identifier: GPL-2.0-or-later

Sure.

> > +
> > +static void chiptod_power9_send_remotes(PnvChipTOD *chiptod)
>
> Adding a class handler could be an alternative implementation.

Might be a good idea, I'll see how it looks.

> > +{
> > +    PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
> > +    int i;
> > +
> > +    for (i = 0; i < pnv->num_chips; i++) {
>
> There are a few other models (XIVE, XIVE2) which loop on the chips,
> is it time to introduce a pnv_foreach_chip(fn, data) routine ?
>
>
> > +        Pnv9Chip *chip9 = PNV9_CHIP(pnv->chips[i]);
> > +        if (&chip9->chiptod != chiptod) {
> > +            chip9->chiptod.tod_state = tod_running;
> > +        }
> > +    }
> > +}

[snip]

> > +    case TOD_TX_TTYPE_CTRL_REG:
> > +        if (val & PPC_BIT(35)) { /* SCOM addressing */
> > +            uint32_t addr = val >> 32;
> > +            uint32_t reg = addr & 0xfff;
> > +            PnvCore *pc;
> > +
> > +            if (reg != PC_TOD) {
> > +                qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: SCOM addressing: "
> > +                              "unimplemented slave register 0x%" PRIx32 "\n",
> > +                              reg);
> > +                return;
> > +            }
> > +
> > +            /*
> > +             * This may not deal with P10 big-core addressing at the moment.
> > +             * The big-core code in skiboot syncs small cores, but it targets
> > +             * the even PIR (first small-core) when syncing second small-core.
> > +             */
> > +            pc = pnv_get_vcpu_by_xscom_base(chiptod->chip, addr & ~0xfff);
>
> hmm, couldn't we map xscom subregions, one for each thread instead ?

I'm not sure what you mean. This scom-addressing uses the xscom
address of the core's PC unit (where its time facilities are) to
point nest chiptod transfers to cores.

>
> > +            if (pc) {
> > +                /*
> > +                 * If TCG implements SMT, TFMR is a per-core SPR and should
> > +                 * be updated such that it is reflected in all threads.
> > +                 * Same for TB if the chiptod model ever actually adjusted it.
> > +                 */
> > +                chiptod->slave_cpu_target = pc->threads[0];
>
> ah ! SMT is implemented :) The xscom subregions would help to get the
> CPU pointer.

I think I may be mistaken at least in the "get_vcpu" part. I think
it should be get core, I don't know if chiptod is capable of addressing
threads individually.

I could test it with SMT patches and see what skiboot does.

> > +static int pnv_chiptod_dt_xscom(PnvXScomInterface *dev, void *fdt,
> > +                                int xscom_offset,
> > +                                const char compat[], size_t compat_size)
> > +{
> > +    PnvChipTOD *chiptod = PNV_CHIPTOD(dev);
> > +    g_autofree char *name = NULL;
> > +    int offset;
> > +    uint32_t lpc_pcba = PNV9_XSCOM_CHIPTOD_BASE;
>
> lpc ?

Shh don't tell anybody I mostly code via copy and paste :P

[snip]

> > +static void pnv_chiptod_realize(DeviceState *dev, Error **errp)
> > +{
> > +    static bool got_primary = false;
> > +    static bool got_secondary = false;
> > +
> > +    PnvChipTOD *chiptod = PNV_CHIPTOD(dev);
> > +    PnvChipTODClass *pctc = PNV_CHIPTOD_GET_CLASS(chiptod);
> > +
> > +    if (!got_primary) {
> > +        got_primary = true;
> > +        chiptod->primary = true;
> > +        chiptod->pss_mss_ctrl_reg |= PPC_BIT(1); /* TOD is master */
> > +    } else if (!got_secondary) {
> > +        got_secondary = true;
> > +        chiptod->secondary = true;
> > +    }
>
> It would be cleaner to introduce "primary" and "secondary" properties
> defined by the model realizing the PnvChipTOD objects.

Hum. There's a few hard-coded primaries on chip_id == 0 already
in the tree. I don't know how closely related they are, chiptod
can set other chips as primary AFAIK but maybe it just makes
sense to make primary a chip property.

I might dig a bit more into what we (and other IBM firmware) want
to test or expect with these primaries. Maybe another pass to
tidy all that up can be done.

[...]

> > +#ifndef PPC_PNV_CHIPTOD_H
> > +#define PPC_PNV_CHIPTOD_H
> > +
> > +#include "qom/object.h"
> > +
> > +#define TYPE_PNV_CHIPTOD "pnv-chiptod"
> > +OBJECT_DECLARE_TYPE(PnvChipTOD, PnvChipTODClass, PNV_CHIPTOD)
> > +#define TYPE_PNV9_CHIPTOD TYPE_PNV_CHIPTOD "-POWER9"
> > +DECLARE_INSTANCE_CHECKER(PnvChipTOD, PNV9_CHIPTOD, TYPE_PNV9_CHIPTOD)
> > +#define TYPE_PNV10_CHIPTOD TYPE_PNV_CHIPTOD "-POWER10"
> > +DECLARE_INSTANCE_CHECKER(PnvChipTOD, PNV10_CHIPTOD, TYPE_PNV10_CHIPTOD)
> > +
> > +enum tod_state {
>
> PnvChipTODState would be better naming.

Agree.

Thanks,
Nick


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] pnv/chiptod: Add POWER9/10 chiptod model
  2023-06-14  5:30     ` Nicholas Piggin
@ 2023-06-14  7:01       ` Cédric Le Goater
  0 siblings, 0 replies; 18+ messages in thread
From: Cédric Le Goater @ 2023-06-14  7:01 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: qemu-devel, Daniel Henrique Barboza, Frederic Barrat

Hello Nick,

[ ... ]

>>> +    case TOD_TX_TTYPE_CTRL_REG:
>>> +        if (val & PPC_BIT(35)) { /* SCOM addressing */
>>> +            uint32_t addr = val >> 32;
>>> +            uint32_t reg = addr & 0xfff;
>>> +            PnvCore *pc;
>>> +
>>> +            if (reg != PC_TOD) {
>>> +                qemu_log_mask(LOG_GUEST_ERROR, "pnv_chiptod: SCOM addressing: "
>>> +                              "unimplemented slave register 0x%" PRIx32 "\n",
>>> +                              reg);
>>> +                return;
>>> +            }
>>> +
>>> +            /*
>>> +             * This may not deal with P10 big-core addressing at the moment.
>>> +             * The big-core code in skiboot syncs small cores, but it targets
>>> +             * the even PIR (first small-core) when syncing second small-core.
>>> +             */
>>> +            pc = pnv_get_vcpu_by_xscom_base(chiptod->chip, addr & ~0xfff);
>>
>> hmm, couldn't we map xscom subregions, one for each thread instead ?
> 
> I'm not sure what you mean. This scom-addressing uses the xscom
> address of the core's PC unit (where its time facilities are) to
> point nest chiptod transfers to cores.

What I meant is that if you could map one xscom subregion per thread in
an overall container region, each region could have its own 'opaque' data
(the thread object you are interested in) and you wouldn't need to do the
CPU lookup. A bit like for the ICP on POWER8 :


     0003ffff80000000-0003ffff800fffff (prio 0, i/o): icp-0
       0003ffff80008000-0003ffff80008fff (prio 0, i/o): icp-thread
       0003ffff80010000-0003ffff80010fff (prio 0, i/o): icp-thread
       0003ffff80018000-0003ffff80018fff (prio 0, i/o): icp-thread
       0003ffff80020000-0003ffff80020fff (prio 0, i/o): icp-thread
       0003ffff80028000-0003ffff80028fff (prio 0, i/o): icp-thread
       0003ffff80030000-0003ffff80030fff (prio 0, i/o): icp-thread
       0003ffff80048000-0003ffff80048fff (prio 0, i/o): icp-thread
       0003ffff80050000-0003ffff80050fff (prio 0, i/o): icp-thread


But, I missed that part :

+        if (val & PPC_BIT(35)) { /* SCOM addressing */
+            uint32_t addr = val >> 32;
+            uint32_t reg = addr & 0xfff;

and pnv_get_vcpu_by_xscom_base() is a bit of a hack. That's why you need
a backpointer to the chip which is always a bit suspicious for a sub unit.
An address space would be cleaner since writing to this register generates
another transaction it seems.

Do you plan to support other registers than PC_TOD ?

The part handling "SCOM addressing" deserves its own patch IMO. It would
clarifies how the logic works and the modeling.

[ ... ]

>>> +static void pnv_chiptod_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    static bool got_primary = false;
>>> +    static bool got_secondary = false;
>>> +
>>> +    PnvChipTOD *chiptod = PNV_CHIPTOD(dev);
>>> +    PnvChipTODClass *pctc = PNV_CHIPTOD_GET_CLASS(chiptod);
>>> +
>>> +    if (!got_primary) {
>>> +        got_primary = true;
>>> +        chiptod->primary = true;
>>> +        chiptod->pss_mss_ctrl_reg |= PPC_BIT(1); /* TOD is master */
>>> +    } else if (!got_secondary) {
>>> +        got_secondary = true;
>>> +        chiptod->secondary = true;
>>> +    }
>>
>> It would be cleaner to introduce "primary" and "secondary" properties
>> defined by the model realizing the PnvChipTOD objects.
> 
> Hum. There's a few hard-coded primaries on chip_id == 0 already

XCSOM doesn't have a QOM model but it should be done that way. XIVE is
bit special because the TIMA is an overlapping mapping on all chips.

> in the tree. I don't know how closely related they are, chiptod
> can set other chips as primary AFAIK but maybe it just makes
> sense to make primary a chip property.

It really shouldn't be too much work, mostly setting properties in the
chip realize routine :

     object_property_set_bool(OBJECT(&chip10->chiptod), "primary",
                              chip->chip_id == 0, &error_abort);
     object_property_set_bool(OBJECT(&chip10->chiptod), "secondary",
                              chip->chip_id == 1, &error_abort);
     if (!qdev_realize(DEVICE(&chip10->chiptod), NULL, errp)) {
         return;
     }

  
> I might dig a bit more into what we (and other IBM firmware) want
> to test or expect with these primaries. Maybe another pass to
> tidy all that up can be done.


Thanks,

C.





^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] target/ppc: add TFMR SPR implementation with read and write helpers
  2023-06-03 23:36 ` [PATCH 3/4] target/ppc: add TFMR SPR implementation with read and write helpers Nicholas Piggin
@ 2023-06-14  8:38   ` Cédric Le Goater
  0 siblings, 0 replies; 18+ messages in thread
From: Cédric Le Goater @ 2023-06-14  8:38 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc; +Cc: qemu-devel, Daniel Henrique Barboza

On 6/4/23 01:36, Nicholas Piggin wrote:
> TFMR is the Time Facility Management Register which is specific to POWER
> CPUs, and used for the purpose of timebase management (generally by
> firmware, not the OS).
> 
> This adds an initial simple TFMR register, which will form part of the
> core timebase facility model in the next patch.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>


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

Thanks,

C.


> ---
>   target/ppc/cpu_init.c        |  2 +-
>   target/ppc/helper.h          |  2 ++
>   target/ppc/spr_common.h      |  2 ++
>   target/ppc/timebase_helper.c | 13 +++++++++++++
>   target/ppc/translate.c       | 10 ++++++++++
>   5 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index da0f7a7159..37088021d2 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -5662,7 +5662,7 @@ static void register_power_common_book4_sprs(CPUPPCState *env)
>       spr_register_hv(env, SPR_TFMR, "TFMR",
>                    SPR_NOACCESS, SPR_NOACCESS,
>                    SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_write_generic,
> +                 &spr_read_tfmr, &spr_write_tfmr,
>                    0x00000000);
>       spr_register_hv(env, SPR_LDBAR, "LDBAR",
>                    SPR_NOACCESS, SPR_NOACCESS,
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index 16bb485c1a..166cacb3f9 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -746,6 +746,8 @@ DEF_HELPER_2(store_40x_dbcr0, void, env, tl)
>   DEF_HELPER_2(store_40x_sler, void, env, tl)
>   DEF_HELPER_FLAGS_2(store_booke_tcr, TCG_CALL_NO_RWG, void, env, tl)
>   DEF_HELPER_FLAGS_2(store_booke_tsr, TCG_CALL_NO_RWG, void, env, tl)
> +DEF_HELPER_1(load_tfmr, tl, env)
> +DEF_HELPER_2(store_tfmr, void, env, tl)
>   DEF_HELPER_3(store_ibatl, void, env, i32, tl)
>   DEF_HELPER_3(store_ibatu, void, env, i32, tl)
>   DEF_HELPER_3(store_dbatl, void, env, i32, tl)
> diff --git a/target/ppc/spr_common.h b/target/ppc/spr_common.h
> index d6c679cd99..8ab17123a4 100644
> --- a/target/ppc/spr_common.h
> +++ b/target/ppc/spr_common.h
> @@ -196,6 +196,8 @@ void spr_write_ebb(DisasContext *ctx, int sprn, int gprn);
>   void spr_read_ebb_upper32(DisasContext *ctx, int gprn, int sprn);
>   void spr_write_ebb_upper32(DisasContext *ctx, int sprn, int gprn);
>   void spr_write_hmer(DisasContext *ctx, int sprn, int gprn);
> +void spr_read_tfmr(DisasContext *ctx, int gprn, int sprn);
> +void spr_write_tfmr(DisasContext *ctx, int sprn, int gprn);
>   void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn);
>   void spr_read_dexcr_ureg(DisasContext *ctx, int gprn, int sprn);
>   #endif
> diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
> index de1ee85e0b..34b1d5ad05 100644
> --- a/target/ppc/timebase_helper.c
> +++ b/target/ppc/timebase_helper.c
> @@ -270,6 +270,19 @@ void helper_store_booke_tsr(CPUPPCState *env, target_ulong val)
>       store_booke_tsr(env, val);
>   }
>   
> +#if defined(TARGET_PPC64)
> +/* POWER processor Timebase Facility */
> +target_ulong helper_load_tfmr(CPUPPCState *env)
> +{
> +    return env->spr[SPR_TFMR];
> +}
> +
> +void helper_store_tfmr(CPUPPCState *env, target_ulong val)
> +{
> +    env->spr[SPR_TFMR] = val;
> +}
> +#endif
> +
>   /*****************************************************************************/
>   /* Embedded PowerPC specific helpers */
>   
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 8b312b46e0..9dcd66eac8 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -1255,6 +1255,16 @@ void spr_write_hmer(DisasContext *ctx, int sprn, int gprn)
>       spr_store_dump_spr(sprn);
>   }
>   
> +void spr_read_tfmr(DisasContext *ctx, int gprn, int sprn)
> +{
> +    gen_helper_load_tfmr(cpu_gpr[gprn], cpu_env);
> +}
> +
> +void spr_write_tfmr(DisasContext *ctx, int sprn, int gprn)
> +{
> +    gen_helper_store_tfmr(cpu_env, cpu_gpr[gprn]);
> +}
> +
>   void spr_write_lpcr(DisasContext *ctx, int sprn, int gprn)
>   {
>       gen_helper_store_lpcr(cpu_env, cpu_gpr[gprn]);



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models
  2023-06-14  5:14   ` Nicholas Piggin
@ 2023-06-14  8:54     ` Cédric Le Goater
  2023-06-15  2:18       ` Nicholas Piggin
  0 siblings, 1 reply; 18+ messages in thread
From: Cédric Le Goater @ 2023-06-14  8:54 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: qemu-devel, Daniel Henrique Barboza, Frederic Barrat, Michael Neuling

On 6/14/23 07:14, Nicholas Piggin wrote:
> On Tue Jun 6, 2023 at 11:59 PM AEST, Cédric Le Goater wrote:
>> On 6/4/23 01:36, Nicholas Piggin wrote:
>>> This adds support for chiptod and core timebase state machine models in
>>> the powernv POWER9 and POWER10 models.
>>>
>>> This does not actually change the time or the value in TB registers
>>> (because they are alrady synced in QEMU), but it does go through the
>>> motions. It is enough to be able to run skiboot's chiptod initialisation
>>> code that synchronises core timebases (after a patch to prevent skiboot
>>> skipping chiptod for QEMU, posted to skiboot mailing list).
>>>
>>> Sorry there was some delay since the last posting. There is a bit more
>>> interest in this recently but feedback and comments from RFC was not
>>> forgotten and is much appreciated.
>>>
>>> https://lists.gnu.org/archive/html/qemu-ppc/2022-08/msg00324.html
>>>
>>> I think I accounted for everything except moving register defines to the
>>> .h file. I'm on the fence about that but if they are only used in the .c
>>> file I think it's okay to keep them there for now. I cut out a lot of
>>> unused ones so it's not so cluttered now.
>>>
>>> Lots of other changes and fixes since that RFC. Notably:
>>> - Register names changed to match the workbook names instead of skiboot.
>>> - TFMR moved to timebase_helper.c from misc_helper.c
>>> - More comprehensive model and error checking, particularly of TFMR.
>>> - POWER10 with multi-chip support.
>>> - chiptod and core timebase linked via specific state instead of TFMR.
>>
>>
>> The chiptod units are not exposed to the OS, it is all handled at FW
>> level AFAIK. Could the OPAL people provide some feedback on the low level
>> models ?
> 
> Well, no takers so far. I guess I'm a OPAL people :)
>> I did some of the P10 chiptod addressing in skiboot, at least. This
> patch does work with the skiboot chiptod driver at least.

cool, with 2 chips ?

> I would eventually like to add in the ability to actually change the
> TB with it, and inject errors and generate HMIs because that's an area
> that seem to be a bit lacking (most of such testing seemed to be done
> on real hardware using special time facility corruption injection).

The MCE injection was a nice addon

   https://lore.kernel.org/qemu-devel/20200325144147.221875-1-npiggin@gmail.com/
   https://lore.kernel.org/qemu-devel/20211013214042.618918-1-clg@kaod.org/

I hope it will get more attention if you resend.

> But yes for now it is a bit difficult to verify it does much useful
> aside from booting skiboot (+ patch to enable chiptod on QEMU I posted
> recently).

It's difficult to review PATCH 4 without some good knowledge of HW. I know
you do but you can not review your own patches ! That said, the impact is
limited to PowerNV machines, I guess we are fine.

Thanks,

C.






^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] target/ppc: Implement core timebase state machine and TFMR
  2023-06-03 23:36 ` [PATCH 4/4] target/ppc: Implement core timebase state machine and TFMR Nicholas Piggin
@ 2023-06-14  8:55   ` Cédric Le Goater
  0 siblings, 0 replies; 18+ messages in thread
From: Cédric Le Goater @ 2023-06-14  8:55 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc; +Cc: qemu-devel, Daniel Henrique Barboza

On 6/4/23 01:36, Nicholas Piggin wrote:
> This implements the core timebase state machine, which is the core side
> of the time-of-day system in POWER processors. This facility is operated
> by control fields in the TFMR register, which also contains status
> fields.
> 
> The core timebase interacts with the chiptod hardware, primarily to
> receive TOD updates, to synchronise timebase with other cores. This
> model does not actually update TB values with TOD or updates received
> from the chiptod, as timebases are always synchronised. It does step
> through the states required to perform the update.
> 
> There are several asynchronous state transitions. These are modelled
> using using mfTFMR to drive state changes, because it is expected that
> firmware poll the register to wait for those states. This is good enough
> to test basic firmware behaviour without adding real timers. The values
> chosen are arbitrary.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Looks correct,

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

Thanks,

C.

> ---
>   target/ppc/cpu.h             |  34 ++++++++
>   target/ppc/timebase_helper.c | 147 ++++++++++++++++++++++++++++++++++-
>   2 files changed, 179 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index d73cce8474..b1520ea4db 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1177,6 +1177,13 @@ struct CPUArchState {
>       /* PowerNV chiptod / timebase facility state. */
>       int tb_ready_for_tod; /* core TB ready to receive TOD from chiptod */
>       int tod_sent_to_tb;   /* chiptod sent TOD to the core TB */
> +
> +    /*
> +     * Timers for async events are simulated by mfTFAC because TFAC is to be
> +     * polled for event.
> +     */
> +    int tb_state_timer;
> +    int tb_sync_pulse_timer;
>   #endif
>   #endif
>   
> @@ -2527,6 +2534,33 @@ enum {
>       HMER_XSCOM_STATUS_MASK      = PPC_BITMASK(21, 23),
>   };
>   
> +/* TFMR */
> +enum {
> +    TFMR_CONTROL_MASK           = PPC_BITMASK(0, 24),
> +    TFMR_MASK_HMI               = PPC_BIT(10),
> +    TFMR_TB_ECLIPZ              = PPC_BIT(14),
> +    TFMR_LOAD_TOD_MOD           = PPC_BIT(16),
> +    TFMR_MOVE_CHIP_TOD_TO_TB    = PPC_BIT(18),
> +    TFMR_CLEAR_TB_ERRORS        = PPC_BIT(24),
> +    TFMR_STATUS_MASK            = PPC_BITMASK(25, 63),
> +    TFMR_TBST_ENCODED           = PPC_BITMASK(28, 31), /* TBST = TB State */
> +    TFMR_TBST_LAST              = PPC_BITMASK(32, 35), /* Previous TBST */
> +    TFMR_TB_ENABLED             = PPC_BIT(40),
> +    TFMR_TB_VALID               = PPC_BIT(41),
> +    TFMR_TB_SYNC_OCCURED        = PPC_BIT(42),
> +};
> +
> +/* TFMR TBST */
> +enum {
> +    TBST_RESET                  = 0x0,
> +    TBST_SEND_TOD_MOD           = 0x1,
> +    TBST_NOT_SET                = 0x2,
> +    TBST_SYNC_WAIT              = 0x6,
> +    TBST_GET_TOD                = 0x7,
> +    TBST_TB_RUNNING             = 0x8,
> +    TBST_TB_ERROR               = 0x9,
> +};
> +
>   /*****************************************************************************/
>   
>   #define is_isa300(ctx) (!!(ctx->insns_flags2 & PPC2_ISA300))
> diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
> index 34b1d5ad05..11a06fafe6 100644
> --- a/target/ppc/timebase_helper.c
> +++ b/target/ppc/timebase_helper.c
> @@ -272,14 +272,157 @@ void helper_store_booke_tsr(CPUPPCState *env, target_ulong val)
>   
>   #if defined(TARGET_PPC64)
>   /* POWER processor Timebase Facility */
> +static unsigned int tfmr_get_tb_state(uint64_t tfmr)
> +{
> +    return (tfmr & TFMR_TBST_ENCODED) >> (63 - 31);
> +}
> +
> +static uint64_t tfmr_new_tb_state(uint64_t tfmr, unsigned int tbst)
> +{
> +    tfmr &= ~TFMR_TBST_LAST;
> +    tfmr |= (tfmr & TFMR_TBST_ENCODED) >> 4; /* move state to last state */
> +    tfmr &= ~TFMR_TBST_ENCODED;
> +    tfmr |= (uint64_t)tbst << (63 - 31); /* move new state to state */
> +
> +    if (tbst == TBST_TB_RUNNING) {
> +        tfmr |= TFMR_TB_VALID;
> +    } else {
> +        tfmr &= ~TFMR_TB_VALID;
> +    }
> +
> +    return tfmr;
> +}
> +
> +static void tb_state_machine_step(CPUPPCState *env)
> +{
> +    uint64_t tfmr = env->spr[SPR_TFMR];
> +    unsigned int tbst = tfmr_get_tb_state(tfmr);
> +
> +    if (!(tfmr & TFMR_TB_ECLIPZ) || tbst == TBST_TB_ERROR) {
> +        return;
> +    }
> +
> +    if (env->tb_sync_pulse_timer) {
> +        env->tb_sync_pulse_timer--;
> +    } else {
> +        tfmr |= TFMR_TB_SYNC_OCCURED;
> +        env->spr[SPR_TFMR] = tfmr;
> +    }
> +
> +    if (env->tb_state_timer) {
> +        env->tb_state_timer--;
> +        return;
> +    }
> +
> +    if (tfmr & TFMR_LOAD_TOD_MOD) {
> +        tfmr &= ~TFMR_LOAD_TOD_MOD;
> +        if (tbst == TBST_GET_TOD) {
> +            tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR);
> +        } else {
> +            tfmr = tfmr_new_tb_state(tfmr, TBST_SEND_TOD_MOD);
> +            /* State seems to transition immediately */
> +            tfmr = tfmr_new_tb_state(tfmr, TBST_NOT_SET);
> +        }
> +    } else if (tfmr & TFMR_MOVE_CHIP_TOD_TO_TB) {
> +        if (tbst == TBST_SYNC_WAIT) {
> +            tfmr = tfmr_new_tb_state(tfmr, TBST_GET_TOD);
> +            env->tb_state_timer = 3;
> +        } else if (tbst == TBST_GET_TOD) {
> +            if (env->tod_sent_to_tb) {
> +                tfmr = tfmr_new_tb_state(tfmr, TBST_TB_RUNNING);
> +                tfmr &= ~TFMR_MOVE_CHIP_TOD_TO_TB;
> +                env->tb_ready_for_tod = 0;
> +                env->tod_sent_to_tb = 0;
> +            }
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: MOVE_CHIP_TOD_TO_TB "
> +                          "state machine in invalid state 0x%x\n", tbst);
> +            tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR);
> +            env->tb_ready_for_tod = 0;
> +        }
> +    }
> +
> +    env->spr[SPR_TFMR] = tfmr;
> +}
> +
>   target_ulong helper_load_tfmr(CPUPPCState *env)
>   {
> -    return env->spr[SPR_TFMR];
> +    tb_state_machine_step(env);
> +
> +    return env->spr[SPR_TFMR] | TFMR_TB_ECLIPZ;
>   }
>   
>   void helper_store_tfmr(CPUPPCState *env, target_ulong val)
>   {
> -    env->spr[SPR_TFMR] = val;
> +    uint64_t tfmr = env->spr[SPR_TFMR];
> +    unsigned int tbst = tfmr_get_tb_state(tfmr);
> +
> +    if (!(val & TFMR_TB_ECLIPZ)) {
> +        qemu_log_mask(LOG_UNIMP, "TFMR non-ECLIPZ mode not implemented\n");
> +        tfmr &= ~TFMR_TBST_ENCODED;
> +        tfmr &= ~TFMR_TBST_LAST;
> +        goto out;
> +    }
> +
> +    /* Update control bits */
> +    tfmr = (tfmr & ~TFMR_CONTROL_MASK) | (val & TFMR_CONTROL_MASK);
> +
> +    /* mtspr always clears this */
> +    tfmr &= ~TFMR_TB_SYNC_OCCURED;
> +    env->tb_sync_pulse_timer = 1;
> +
> +    /*
> +     * We don't implement any of the error status bits that can be
> +     * cleared by writing 1 to them. TB error injection / simulation
> +     * would have to implement some.
> +     *
> +     * Also don't implement mfTB failing when the TB state machine is
> +     * not running.
> +     */
> +
> +    if (((tfmr | val) & (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB)) ==
> +                        (TFMR_LOAD_TOD_MOD | TFMR_MOVE_CHIP_TOD_TO_TB)) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: LOAD_TOD_MOD and "
> +                                       "MOVE_CHIP_TOD_TO_TB both set\n");
> +        tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR);
> +        env->tb_ready_for_tod = 0;
> +        goto out;
> +    }
> +
> +    if (tfmr & TFMR_CLEAR_TB_ERRORS) {
> +        tfmr = tfmr_new_tb_state(tfmr, TBST_RESET);
> +        tfmr &= ~TFMR_CLEAR_TB_ERRORS;
> +        tfmr &= ~TFMR_LOAD_TOD_MOD;
> +        tfmr &= ~TFMR_MOVE_CHIP_TOD_TO_TB;
> +        env->tb_ready_for_tod = 0;
> +        env->tod_sent_to_tb = 0;
> +        goto out;
> +    }
> +
> +    if (tbst == TBST_TB_ERROR) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: mtspr TFMR in TB_ERROR"
> +                                       " state\n");
> +        return;
> +    }
> +
> +    if (tfmr & TFMR_LOAD_TOD_MOD) {
> +        env->tb_state_timer = 3;
> +    } else if (tfmr & TFMR_MOVE_CHIP_TOD_TO_TB) {
> +        if (tbst == TBST_NOT_SET) {
> +            tfmr = tfmr_new_tb_state(tfmr, TBST_SYNC_WAIT);
> +            env->tb_ready_for_tod = 1;
> +            env->tb_state_timer = 3;
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR, "TFMR error: MOVE_CHIP_TOD_TO_TB "
> +                                           "not in TB not set state 0x%x\n",
> +                                           tbst);
> +            tfmr = tfmr_new_tb_state(tfmr, TBST_TB_ERROR);
> +            env->tb_ready_for_tod = 0;
> +        }
> +    }
> +
> +out:
> +    env->spr[SPR_TFMR] = tfmr;
>   }
>   #endif
>   



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models
  2023-06-14  8:54     ` Cédric Le Goater
@ 2023-06-15  2:18       ` Nicholas Piggin
  2023-06-15  9:45         ` Cédric Le Goater
  0 siblings, 1 reply; 18+ messages in thread
From: Nicholas Piggin @ 2023-06-15  2:18 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc
  Cc: qemu-devel, Daniel Henrique Barboza, Frederic Barrat, Michael Neuling

On Wed Jun 14, 2023 at 6:54 PM AEST, Cédric Le Goater wrote:
> On 6/14/23 07:14, Nicholas Piggin wrote:
> > On Tue Jun 6, 2023 at 11:59 PM AEST, Cédric Le Goater wrote:
> >> On 6/4/23 01:36, Nicholas Piggin wrote:
> >>> This adds support for chiptod and core timebase state machine models in
> >>> the powernv POWER9 and POWER10 models.
> >>>
> >>> This does not actually change the time or the value in TB registers
> >>> (because they are alrady synced in QEMU), but it does go through the
> >>> motions. It is enough to be able to run skiboot's chiptod initialisation
> >>> code that synchronises core timebases (after a patch to prevent skiboot
> >>> skipping chiptod for QEMU, posted to skiboot mailing list).
> >>>
> >>> Sorry there was some delay since the last posting. There is a bit more
> >>> interest in this recently but feedback and comments from RFC was not
> >>> forgotten and is much appreciated.
> >>>
> >>> https://lists.gnu.org/archive/html/qemu-ppc/2022-08/msg00324.html
> >>>
> >>> I think I accounted for everything except moving register defines to the
> >>> .h file. I'm on the fence about that but if they are only used in the .c
> >>> file I think it's okay to keep them there for now. I cut out a lot of
> >>> unused ones so it's not so cluttered now.
> >>>
> >>> Lots of other changes and fixes since that RFC. Notably:
> >>> - Register names changed to match the workbook names instead of skiboot.
> >>> - TFMR moved to timebase_helper.c from misc_helper.c
> >>> - More comprehensive model and error checking, particularly of TFMR.
> >>> - POWER10 with multi-chip support.
> >>> - chiptod and core timebase linked via specific state instead of TFMR.
> >>
> >>
> >> The chiptod units are not exposed to the OS, it is all handled at FW
> >> level AFAIK. Could the OPAL people provide some feedback on the low level
> >> models ?
> > 
> > Well, no takers so far. I guess I'm a OPAL people :)
> >> I did some of the P10 chiptod addressing in skiboot, at least. This
> > patch does work with the skiboot chiptod driver at least.
>
> cool, with 2 chips ?

Yes it worked with 2 chips.

> > I would eventually like to add in the ability to actually change the
> > TB with it, and inject errors and generate HMIs because that's an area
> > that seem to be a bit lacking (most of such testing seemed to be done
> > on real hardware using special time facility corruption injection).
>
> The MCE injection was a nice addon
>
>    https://lore.kernel.org/qemu-devel/20200325144147.221875-1-npiggin@gmail.com/
>    https://lore.kernel.org/qemu-devel/20211013214042.618918-1-clg@kaod.org/
>
> I hope it will get more attention if you resend.

Will take another look.

> > But yes for now it is a bit difficult to verify it does much useful
> > aside from booting skiboot (+ patch to enable chiptod on QEMU I posted
> > recently).
>
> It's difficult to review PATCH 4 without some good knowledge of HW. I know
> you do but you can not review your own patches ! That said, the impact is
> limited to PowerNV machines, I guess we are fine.

Yeah. I appreciate all the review so far. It's pretty complicated even
with the workbook. I might be able to add a simpler and higher-level
description of the basic init sequence and states. You would still have
to trust me, but it might make it easier to see what's going on.

Thanks,
Nick


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models
  2023-06-15  2:18       ` Nicholas Piggin
@ 2023-06-15  9:45         ` Cédric Le Goater
  0 siblings, 0 replies; 18+ messages in thread
From: Cédric Le Goater @ 2023-06-15  9:45 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: qemu-devel, Daniel Henrique Barboza, Frederic Barrat, Michael Neuling

[ ... ]
> Yes it worked with 2 chips.

I will give the next series a try.

[ ... ]

>> It's difficult to review PATCH 4 without some good knowledge of HW. I know
>> you do but you can not review your own patches ! That said, the impact is
>> limited to PowerNV machines, I guess we are fine.
> 
> Yeah. I appreciate all the review so far. It's pretty complicated even
> with the workbook. I might be able to add a simpler and higher-level
> description of the basic init sequence and states. You would still have
> to trust me, but it might make it easier to see what's going on.

Sure. Patches 2-4 are acked. I am only looking at patch 1 now.

Thanks,

C.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models
  2023-06-03 23:36 [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models Nicholas Piggin
                   ` (4 preceding siblings ...)
  2023-06-06 13:59 ` [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models Cédric Le Goater
@ 2023-06-22  7:30 ` Cédric Le Goater
  2023-06-22  9:54   ` Nicholas Piggin
  5 siblings, 1 reply; 18+ messages in thread
From: Cédric Le Goater @ 2023-06-22  7:30 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc; +Cc: qemu-devel, Daniel Henrique Barboza

On 6/4/23 01:36, Nicholas Piggin wrote:
> This adds support for chiptod and core timebase state machine models in
> the powernv POWER9 and POWER10 models.
> 
> This does not actually change the time or the value in TB registers
> (because they are alrady synced in QEMU), but it does go through the
> motions. It is enough to be able to run skiboot's chiptod initialisation
> code that synchronises core timebases (after a patch to prevent skiboot
> skipping chiptod for QEMU, posted to skiboot mailing list).
> 
> Sorry there was some delay since the last posting. There is a bit more
> interest in this recently but feedback and comments from RFC was not
> forgotten and is much appreciated.
> 
> https://lists.gnu.org/archive/html/qemu-ppc/2022-08/msg00324.html
> 
> I think I accounted for everything except moving register defines to the
> .h file. I'm on the fence about that but if they are only used in the .c
> file I think it's okay to keep them there for now. I cut out a lot of
> unused ones so it's not so cluttered now.
> 
> Lots of other changes and fixes since that RFC. Notably:
> - Register names changed to match the workbook names instead of skiboot.
> - TFMR moved to timebase_helper.c from misc_helper.c
> - More comprehensive model and error checking, particularly of TFMR.
> - POWER10 with multi-chip support.
> - chiptod and core timebase linked via specific state instead of TFMR.
> 
> There is still a vast amount that is not modeled, but most of it related
> to error handling, injection, failover, etc that is very complicated and
> not required for normal operation.
> 
> Thanks,
> Nick
> 
> Nicholas Piggin (4):
>    pnv/chiptod: Add POWER9/10 chiptod model
>    target/ppc: Tidy POWER book4 SPR registration
>    target/ppc: add TFMR SPR implementation with read and write helpers
>    target/ppc: Implement core timebase state machine and TFMR

patch 2-4 could be merged in the next PR. Could you please rebase on
ppc-next and resend ?

Then we still have 2+ weeks to polish pnv/chiptod which would be a
nice addition to QEMU 8.1.

Thanks,

C.


> 
>   hw/ppc/meson.build           |   1 +
>   hw/ppc/pnv.c                 |  38 +++
>   hw/ppc/pnv_chiptod.c         | 488 +++++++++++++++++++++++++++++++++++
>   hw/ppc/pnv_xscom.c           |   2 +
>   hw/ppc/trace-events          |   4 +
>   include/hw/ppc/pnv_chip.h    |   3 +
>   include/hw/ppc/pnv_chiptod.h |  64 +++++
>   include/hw/ppc/pnv_core.h    |   3 +
>   include/hw/ppc/pnv_xscom.h   |   9 +
>   target/ppc/cpu.h             |  40 +++
>   target/ppc/cpu_init.c        |  92 ++++---
>   target/ppc/helper.h          |   2 +
>   target/ppc/spr_common.h      |   2 +
>   target/ppc/timebase_helper.c | 156 +++++++++++
>   target/ppc/translate.c       |  10 +
>   15 files changed, 882 insertions(+), 32 deletions(-)
>   create mode 100644 hw/ppc/pnv_chiptod.c
>   create mode 100644 include/hw/ppc/pnv_chiptod.h
> 



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models
  2023-06-22  7:30 ` Cédric Le Goater
@ 2023-06-22  9:54   ` Nicholas Piggin
  0 siblings, 0 replies; 18+ messages in thread
From: Nicholas Piggin @ 2023-06-22  9:54 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc; +Cc: qemu-devel, Daniel Henrique Barboza

On Thu Jun 22, 2023 at 5:30 PM AEST, Cédric Le Goater wrote:
> On 6/4/23 01:36, Nicholas Piggin wrote:
> > This adds support for chiptod and core timebase state machine models in
> > the powernv POWER9 and POWER10 models.
> > 
> > This does not actually change the time or the value in TB registers
> > (because they are alrady synced in QEMU), but it does go through the
> > motions. It is enough to be able to run skiboot's chiptod initialisation
> > code that synchronises core timebases (after a patch to prevent skiboot
> > skipping chiptod for QEMU, posted to skiboot mailing list).
> > 
> > Sorry there was some delay since the last posting. There is a bit more
> > interest in this recently but feedback and comments from RFC was not
> > forgotten and is much appreciated.
> > 
> > https://lists.gnu.org/archive/html/qemu-ppc/2022-08/msg00324.html
> > 
> > I think I accounted for everything except moving register defines to the
> > .h file. I'm on the fence about that but if they are only used in the .c
> > file I think it's okay to keep them there for now. I cut out a lot of
> > unused ones so it's not so cluttered now.
> > 
> > Lots of other changes and fixes since that RFC. Notably:
> > - Register names changed to match the workbook names instead of skiboot.
> > - TFMR moved to timebase_helper.c from misc_helper.c
> > - More comprehensive model and error checking, particularly of TFMR.
> > - POWER10 with multi-chip support.
> > - chiptod and core timebase linked via specific state instead of TFMR.
> > 
> > There is still a vast amount that is not modeled, but most of it related
> > to error handling, injection, failover, etc that is very complicated and
> > not required for normal operation.
> > 
> > Thanks,
> > Nick
> > 
> > Nicholas Piggin (4):
> >    pnv/chiptod: Add POWER9/10 chiptod model
> >    target/ppc: Tidy POWER book4 SPR registration
> >    target/ppc: add TFMR SPR implementation with read and write helpers
> >    target/ppc: Implement core timebase state machine and TFMR
>
> patch 2-4 could be merged in the next PR. Could you please rebase on
> ppc-next and resend ?

Good idea, I have a couple of other minor register additions that
depend on patch 1 too.

> Then we still have 2+ weeks to polish pnv/chiptod which would be a
> nice addition to QEMU 8.1.

Yeah. Been trying to get to it... Hopefully pseries SMT is close
now so I will have some more time.

Thanks,
Nick


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-06-22  9:55 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-03 23:36 [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models Nicholas Piggin
2023-06-03 23:36 ` [PATCH 1/4] pnv/chiptod: Add POWER9/10 chiptod model Nicholas Piggin
2023-06-05 14:57   ` Cédric Le Goater
2023-06-14  5:30     ` Nicholas Piggin
2023-06-14  7:01       ` Cédric Le Goater
2023-06-03 23:36 ` [PATCH 2/4] target/ppc: Tidy POWER book4 SPR registration Nicholas Piggin
2023-06-05 14:58   ` Cédric Le Goater
2023-06-03 23:36 ` [PATCH 3/4] target/ppc: add TFMR SPR implementation with read and write helpers Nicholas Piggin
2023-06-14  8:38   ` Cédric Le Goater
2023-06-03 23:36 ` [PATCH 4/4] target/ppc: Implement core timebase state machine and TFMR Nicholas Piggin
2023-06-14  8:55   ` Cédric Le Goater
2023-06-06 13:59 ` [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models Cédric Le Goater
2023-06-14  5:14   ` Nicholas Piggin
2023-06-14  8:54     ` Cédric Le Goater
2023-06-15  2:18       ` Nicholas Piggin
2023-06-15  9:45         ` Cédric Le Goater
2023-06-22  7:30 ` Cédric Le Goater
2023-06-22  9:54   ` Nicholas Piggin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.